Reconstruct psdisp helper hierarchy - #2986
Open
MarkMcCaskey wants to merge 122 commits into
Open
Conversation
Split psDispParticles along helper boundaries preserved in the historical HSD header and the GNT4 implementation. This is an experimental structural baseline. psDispSubMakePolygon still does not inline under the current translation-unit flags, and psDispParticles is not yet matched.
Report for GALE01 (c638972 - 20aaf75)📈 Matched data: 87.52% (+0.05%, +616 bytes) ✅ 6 new matches
📈 2 improvements in unmatched items
|
Restore the single historically declared polygon helper and inline the reconstructed helper hierarchy with a narrowly scoped depth pragma. Recover AppSRT trail-form rendering and align matrix, channel, TEV, Z, and fog control flow with the target. The resulting psDispParticles has the target's exact ordered external-call inventory and improves from 52.60% to 64.64%.
Match the target control flow by ending the pass when the edge-only path reaches a non-edge particle, while routing undersized particles through the common particle-advance tail. This improves psDispParticles from 64.64% to 64.80% without changing its external call sequence.
Read AppSRT state through each particle in both AppSRT rendering paths. This recovers the target's repeated field-load shape and raises psDispParticles from 64.80% to 65.68%.
Compose perspective and orthographic projection rows through small inline helpers, then update billboard axes from shared inverse-view loads. Wrap the operation in psUpdateProjectionCache so psDispParticles retains a readable high-level shape. The helper hierarchy inlines completely and improves psDispParticles from 65.68% to 66.41%.
Model HSD_PSDisp_803B9628 and its stack destination as aggregate matrix values. The previous twelve-s32 reconstruction converted floats to integers and did not materialize the full matrix passed to GXLoadPosMtxImm. The aggregate assignment emits the target's exact twelve-word copy and improves psDispParticles from 66.41% to 66.49%.
Model the lighting color path as the target does: initialize only RGB for PrimEnv particles, otherwise fetch the primitive color before applying ambient modulation, and copy cached GXColor values as aggregates. This removes redundant control flow and raises psDispParticles from 66.4870% to 67.6060%.
Replace artificial getColorMatAmb wrapper inlines with a semantic render-state hierarchy. The outer helper coordinates TEV and channel setup while the nested color helper preserves the original inline depth naturally, leaving the emitted getColorMatAmb calls and psDispParticles output unchanged. Also type the initialization temporaries as GXColor instead of casting integer storage.
Fill cached GXColor values through a compile-time RGBA helper and pass typed color locals directly to GX, avoiding five redundant ABI aggregate copies. Replace fourteen dead stack-shaping locals with a documented pad while the remaining inline-local source shape is unresolved. This raises psDispParticles from 67.6060% to 67.9169%.
Express projection-row composition as one inline selected by a constant perspective mode, and describe each GX vertex format through constant color and texture flags. MWCC prunes every static branch, producing byte-identical psDispParticles output while consolidating the repeated setup logic.
Share the AppSRT matrix update and transformed-position calculations between point and polygon rendering. Isolate the perspective direction calculation without changing its emitted code, and use the repository-standard HSD_ASSERT form for the texture-format guard.
Scale the local AppSRT axes before direction projection and calculate the previous particle position before assembling perspective coefficients, matching the target control-flow order. The recovered locals replace 0x40 bytes of synthetic stack padding while preserving the path-specific renderer.
ribbanya
previously requested changes
Jul 26, 2026
ribbanya
reviewed
Jul 26, 2026
ribbanya
requested changes
Jul 26, 2026
ribbanya
requested changes
Jul 26, 2026
Review feedback: Vec4 already exists in dolphin/mtx.h, and Point3d should not be used.
Collaborator
|
@MarkMcCaskey Sorry if my reviews are annoying, I can wait until you undraft the PR if you prefer. |
The target stack band places prim_color below amb_color below mat_color, proving the original declaration order. The 0xFF white and zero ambient updates store b, g, r, identifying chained assignments. Byte-neutral against the current match; makes the channel-register color zone structurally exact relative to the band.
MarkMcCaskey
marked this pull request as ready for review
July 29, 2026 15:39
Collaborator
Author
|
I’ve hit my codex usage limit as well. When reviewing the diff I think it’s clear that there’s more structure here that I wasn’t able to make work. Like assigning colors or any other commonly repeated code block. This code needs more iteration for both matching progress and to clean it up more. But I think given that I likely won’t be able to make significant progress on this soon, I’ll open this up for review now. |
PsiLupan
reviewed
Jul 29, 2026
PsiLupan
reviewed
Jul 29, 2026
PsiLupan
reviewed
Jul 29, 2026
PsiLupan
reviewed
Jul 29, 2026
PsiLupan
reviewed
Jul 29, 2026
PsiLupan
reviewed
Jul 29, 2026
PsiLupan
reviewed
Jul 29, 2026
PsiLupan
reviewed
Jul 29, 2026
PsiLupan
reviewed
Jul 29, 2026
PsiLupan
reviewed
Jul 29, 2026
PsiLupan
reviewed
Jul 29, 2026
PsiLupan identifies the frame counter and the point-size, line-width, and channel-color render-state caches.
PsiLupan identifies the raw 3 as GX_PNMTX1 and the AppSRT status values as PS_APPSTATUS_ONCE and PS_APPSTATUS_STILL.
The size cutoff is FLT_EPSILON; the angle cutoffs are the FLT_MIN already declared in util.h. MSL/math.h's FLT_EPSILON is not the IEEE value, so redefine it locally.
The nine index writes in the point, point-trail, and AppSRT point emitters are byte-identical through the GX inline. The polygon, AppSRT quad, and custom-form sites still need the direct FIFO spelling to preserve the surrounding allocation.
PsiLupan supplies the original spelling; MWCC's b, g, r store order is what the target emits.
PsiLupan reports that later HSD emits this block through an AppSRT-aware psDispSubMakePolygon. Melee's target has none of those instructions, so record the provenance rather than reshaping the emission.
All five color writes in psDispSubAppSRT convert together; each inline call site bills band space, so the reconstruction pad shrinks from 0x44 to 0x20 to keep the target 0x9A0 frame. That replaces 36 bytes of unexplained padding with real source structure and raises psDispParticles from 98.63694% to 98.63810%. Four-site subsets are only neutral, and the same conversion in the point, point-trail, and polygon emitters stays negative at their best pad.
Every colour write in the file now uses GXColor4u8, and the point, point-trail, and polygon position writes use GXPosition3f32 with their coordinates as arguments, which deletes twelve blocks of staged f32 temporaries. Objdiff confirms those temporaries emitted the same instruction stream as the direct arguments, so they were reconstruction scaffolding rather than recovered structure. Each inline call site bills stack band space, so the root placeholder drops from 0x20 to 0x14 to hold the target 0x9A0 frame. psDispParticles goes from 98.63810% to 98.63363%; the difference is entirely r1 displacements, with the instruction stream and the three-instruction size deficit unchanged. The eight AppSRT quad position writes keep the direct FIFO spelling. Routing them through GXPosition3f32 costs 0.038% whether the staged temporaries are kept or dropped, and keeping them while calling the helper overflows the band budget outright. The staged vx, vz, vy order recovered in 0461bbf is now measurably redundant: converting those two vertices or leaving them alone scores identically.
The file previously mixed GXColor4u8/GXPosition3f32/GXTexCoord1x8 with raw GXWGFifo union stores, sometimes three lines apart. Convert the remaining 37 groups so no raw FIFO write is left: 17 GXPosition3f32, 16 GXTexCoord1x8, and 4 GXTexCoord2f32. The two point loops staged z/y/x purely to force the reverse read order that GXPosition3f32's argument evaluation already provides, so they now read p->x/y/z directly and advance afterwards. This costs match: each GX inline homes its f32 parameters on the stack, which raises the frame from the target 0x9A0 to 0xA98 and shifts every displacement (98.6336% -> 98.5624%). No instruction is added or removed -- 3620 both sides -- and only 30 of them differ, all a single f22/f23/f24 rotation inside a region that already does not match. The reconstruction PAD_STACK is dropped because it can no longer reach the target frame.
Every GX emission site wrapped its call in a block scope that staged the components into temps first (u8 a/b/g/r in reverse store order, or f32 vx/vy). That ordering existed only to drive the raw GXWGFifo store sequence; now that the inlines take arguments, their own evaluation supplies it. Drop all 22 staging blocks and pass the fields directly. Removes 143 lines for 27 added, recovers 152 bytes of stack band (frame 0xA98 -> 0xA00), and improves the match 98.5624% -> 98.5657% with the instruction count unchanged at 3620. Two GXColor4u8 sites keep r/g/b/alpha: those are texform-decode loop variables and a clamped integer alpha, not staging temps.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WIP PR to try to match this giant function but first it needs to be split up into helpers because 🙈.
It turns out this is the third largest function in the entire decomp with:
hsd_8039930Calso clearly wants lots of inlines based on the huge stack padding but the code doesn't seem shaped that way. None of these 3 are matched yet 👀Chibi Robo and GNT4 were used as references for some of these helper functions (and for symbols) which only exist as inlines in melee but are emitted in those games. These helpers were matched against the emitted the functions in those games using their compilers + settings.