Add outline rendering system ported from C++ to Zig

Port the model outline hook system (CM2SceneRenderDraw, ManageRenderListNode, DrawBatchProjected) from the C++ idris DLL to pure Zig. Includes D3D9 vtable hooks for stencil-based outline rendering with per-category colors and thickness. Fix GetObjectByGUID calling convention: was using fastcall (ECX/EDX) but Ghidra confirms it's __stdcall with stack params and RET 8. Fix lua_pushcclosure address from 0x6F3B80 (mid-body of another function) to 0x6F3920. Guard resolveModelOwner in DrawBatchProj with hasTargets() check and cache unit model status during ManageRenderListNode to avoid raw pointer chasing at render time. Add IsBadReadPtr validation in resolveModelOwner to match C++ IsValidReadPtr pattern for page-level memory safety.
2026-02-24 00:51:33 -08:00
parent c0d9a20943
commit 25ed0b3780
13 changed files with 2216 additions and 1 deletions
@@ -0,0 +1,108 @@
+# WoW 1.12.1 Calling Conventions — Ghidra Verified
+
+All conventions verified against WoW.exe 1.12.1 build 5875 via Ghidra decompilation and raw byte analysis.
+
+## Model Pipeline Hooks (outline/model_hook.zig)
+
+| # | Address | Function | Convention | Params | Prologue | RET | Status |
+|---|---------|----------|------------|--------|----------|-----|--------|
+| 1 | `0x0070b360` | CM2SceneRenderDraw | `__thiscall` | ECX=this, stack: viewMatrix, batchData, batchIndices, batchCount | `55 8B EC 81 EC 80 00 00 00` (9B) | — | CORRECT |
+| 2 | `0x00710b90` | CM2Model_ManageRenderListNode | `__thiscall` | ECX=model, stack: addToList | `55 8B EC 8B 45 08` (6B) | — | CORRECT |
+| 3 | `0x0070cb30` | CM2Scene_DrawBatchProjected | `__fastcall` | ECX=renderContext | `55 8B EC 83 EC 10` (6B) | — | CORRECT |
+
+## Game Function Wrappers (outline/wow.zig)
+
+| # | Address | Function | Convention | Params | Prologue | RET | Status |
+|---|---------|----------|------------|--------|----------|-----|--------|
+| 4 | `0x00515970` | Script_UnitGUID | `__fastcall` | ECX=unitIdStr → EAX:EDX (64-bit) | `55 8B EC 51 56 68 90 00 00 00` | — | CORRECT |
+| 5 | `0x00464870` | GetObjectByGUID | **`__stdcall`** | **stack: guidLow, guidHigh → EAX** | `55 8B EC 8B 45 08 8B 4D 0C` | **RET 8** | **FIXED** — was incorrectly using `hook.fastcall` |
+| 6 | `0x006061E0` | CGUnit_C::UnitReaction | `__thiscall` | ECX=localPlayer, stack: unit → EAX (reaction int) | `53 8B DC 83 EC 08 83 E4 F8` | — | CORRECT |
+
+### GetObjectByGUID Detail
+
+Disassembly at `0x464870`:
+```
+55          PUSH EBP
+8B EC       MOV EBP, ESP
+8B 45 08    MOV EAX, [EBP+8]     ; guidLow from STACK (not ECX!)
+8B 4D 0C    MOV ECX, [EBP+C]     ; guidHigh from STACK (not EDX!)
+8B D0       MOV EDX, EAX
+0B D1       OR  EDX, ECX          ; test if guid == 0
+74 0B       JZ  return_zero
+51          PUSH ECX              ; push guidHigh for inner call
+50          PUSH EAX              ; push guidLow for inner call
+E8 ...      CALL FindObjectByGUID
+5D          POP EBP
+C2 08 00    RET 8                 ; callee cleans 8 bytes
+```
+
+The C++ reference declared this as `__fastcall(uint64_t)`. Under MSVC, `uint64_t` (8 bytes) is too large for a single 32-bit register, so `__fastcall` passes it on the stack — making it behave like `__stdcall`. The Zig code split it into two `u32` args and passed them in ECX/EDX via `hook.fastcall`, which was wrong.
+
+The transmog addon (`transmogfix/src/main.zig:134`) and interact module (`weirdutils/src/interact.zig:50`) already had the correct push-to-stack implementation.
+
+## Dead Overlay Functions (not yet ported — for future reference)
+
+| # | Address | Function | Convention | Params | Status |
+|---|---------|----------|------------|--------|--------|
+| 7 | `0x00483EE0` | WorldProjection_WorldToScreenCoords | `__thiscall` | ECX=WorldFrame, stack: float* worldXYZ, float* screenXYZ → uint (bool) | VERIFIED |
+| 8 | `0x0041ADE0` | DDCToNDC | `__fastcall` | ECX=float* outX, EDX=float* outY, stack: float inX, float inY | VERIFIED |
+| 9 | `0x00609210` | CGUnit_C::GetUnitName | `__thiscall` | ECX=unit, stack: uint** param → char* | VERIFIED |
+| 10 | `0x006264E0` | GetObjectName | `__fastcall` | ECX=uint64_t* guidPtr → char* | VERIFIED |
+
+## Lua API (main.zig)
+
+All WoW 1.12.1 Lua C API functions use `__fastcall` with L (lua_State*) in ECX.
+
+| # | Address | Function | Convention | Params | RET | Status |
+|---|---------|----------|------------|--------|-----|--------|
+| 11 | `0x00704120` | FrameScript::Register | `__fastcall` | ECX=name, EDX=funcAddr | — | CORRECT |
+| 12 | `0x006F3070` | lua_gettop | `__fastcall` | ECX=L → int | RET | CORRECT |
+| 13 | `0x006F3080` | lua_settop | `__fastcall` | ECX=L, EDX=index | — | CORRECT |
+| 14 | `0x006F3350` | lua_pushvalue | `__fastcall` | ECX=L, EDX=index | — | CORRECT |
+| 15 | `0x006F3400` | lua_type | `__fastcall` | ECX=L, EDX=index → int | RET | CORRECT |
+| 16 | `0x006F3510` | lua_isstring | `__fastcall` | ECX=L, EDX=index → int | RET | CORRECT |
+| 17 | `0x006F3690` | lua_tostring | `__fastcall` | ECX=L, EDX=index → char* | — | CORRECT |
+| 18 | `0x006F39F0` | lua_pushboolean | `__fastcall` | ECX=L, EDX=bool | — | CORRECT |
+| 19 | `0x006F3890` | lua_pushstring | `__fastcall` | ECX=L, EDX=string | — | CORRECT |
+| 20 | `0x006F3810` | lua_pushnumber | `__fastcall` | ECX=L, stack: f64 (8 bytes) | RET 8 | CORRECT |
+| 21 | `0x006F3920` | lua_pushcclosure | `__fastcall` | ECX=L, EDX=func, stack: nupvalues | — | **FIXED** — was 0x6F3B80 (wrong addr) |
+| 22 | `0x006F4940` | luaL_error | `__cdecl` | stack: L, fmt, ... | — | CORRECT |
+| 23 | `0x006F4DC0` | luaL_openlib | `__fastcall` | ECX=L, EDX=libname, stack: funcs, nup | — | CORRECT |
+
+### lua_pushcclosure Detail
+
+Ghidra search found `lua_pushcclosure @ 006f3920`. No function exists at the old address `0x6F3B80` — it falls mid-body of another function. The wrapper was unused (never called from current code) so no crash occurred.
+
+### lua_pushnumber Detail
+
+Takes a `double` (8 bytes) which is too large for EDX, so it goes on the stack per `__fastcall` rules. Callee cleans with `RET 8`. The inline asm workaround in `weirdUtilsVersion` correctly does `SUB ESP,8; FSTPL (ESP); CALL` and relies on `RET 8` to rebalance.
+
+## File/Addon Hooks (main.zig)
+
+| # | Address | Function | Convention | Params | Prologue | RET | Status |
+|---|---------|----------|------------|--------|----------|-----|--------|
+| 24 | `0x0042a320` | ValidateFunctionPointer | `__fastcall` | ECX=addr | `55 8B EC 83 EC 40` (6B) | — | CORRECT (empty detour) |
+| 25 | `0x00648620` | LoadFileWithTextureResourceFallback | `__stdcall` | 7 stack params | `55 8B EC 8B 4D 1C` (6B) | RET 0x1C | CORRECT |
+| 26 | `0x00490250` | FrameScript_RegisterAllSystemCommands | `void(void)` | none | `56 E8 ...` (6B) | — | CORRECT (fixup at offset 1) |
+| 27 | `0x0051F600` | LoadAddonsRecursively | `__fastcall` | ECX=error_handler | `53 8B 1D ...` (7B) | — | CORRECT |
+| 28 | `0x006EDB90` | loadFileListWithIncludes | `__fastcall` | ECX=path, EDX=md5ctx, stack: error_handler | `55 8B EC 6A FF ...` | RET 4 | CORRECT |
+| 29 | `0x004B6F70` | LoadUIBindingsFromFile | `__thiscall` | ECX=binding_mgr, stack: path, md5ctx, callback | `55 8B EC 81 EC 1C 04 00 00` | RET 0x0C | CORRECT |
+| 30 | `0x0046a400` | GameEngine_MainInitialize | `void(void)` | none | `55 8B EC 83 EC 28` (6B) | — | CORRECT |
+| 31 | `0x00490BD0` | World_HandlePlayerLogin | `void(void)` | none | `56 E8 ...` (6B) | — | CORRECT (fixup at offset 1) |
+
+### Note on #31
+
+Ghidra names this `World_HandlePlayerLogin`, not `CGGameUI_Shutdown`. The Zig hook's detour just calls the original with no extra logic, so the naming discrepancy has no functional impact.
+
+## Utility Functions
+
+| # | Address | Function | Convention | Params | RET | Status |
+|---|---------|----------|------------|--------|-----|--------|
+| 32 | `0x006462E0` | M2_AllocateModelBuffer | `__stdcall` | stack: size, source_file, line, flags | RET 0x10 | CORRECT |
+| 33 | `0x007040D0` | FrameScript::GetContext | `void(void)` | none → lua_State* in EAX | RET | CORRECT (trivial: `MOV EAX,[global]; RET`) |
+
+## Bugs Fixed (2026-02-24)
+
+1. **GetObjectByGUID** (`outline/wow.zig`): Changed from `hook.fastcall(u32, 0x464870, lo, hi)` to inline asm `push hi; push lo; call`. The function reads params from stack `[EBP+8]`/`[EBP+C]` and does `RET 8`.
+
+2. **lua_pushcclosure** (`main.zig`): Changed address from `0x6F3B80` to `0x6F3920`. The old address pointed into the middle of another function's body.
@@ -0,0 +1,83 @@
+# Outline Port Notes
+
+Pure Zig reimplementation of the WoW 1.12.1 unit outline system, ported from the Idris C/C++ reference at `/media/storage/projects/idris/dlls`.
+
+## Hooked Functions
+
+### Model pipeline (inline hooks via `libs/hook`)
+
+| Address      | Function                          | Convention   | Prologue | Fixups | Notes |
+|-------------|-----------------------------------|-------------|----------|--------|-------|
+| `0x0070b360` | `CM2SceneRenderDraw`              | `__thiscall` | 9 bytes  | none   | Batch reordering for outline priority |
+| `0x00710b90` | `CM2Model_ManageRenderListNode`   | `__thiscall` | 6 bytes  | none   | Classifies models on render-list add |
+| `0x0070cb30` | `CM2Scene_DrawBatchProjected`     | `__fastcall` | 6 bytes  | none   | Flags DIP hook for outline rendering |
+
+**Prologue verification** (Ghidra, WoW.exe 1.12.1 build 5875):
+
+- `0x0070b360`: `55 8B EC 81 EC 80 00 00 00` — `PUSH EBP; MOV EBP,ESP; SUB ESP,0x80`. Boundaries at +1, +3, +9. The `SUB ESP,0x80` is a 6-byte instruction (81 EC + imm32) spanning offset +3..+9, so 6-byte overwrite is **unsafe** — changed to 9.
+- `0x00710b90`: `55 8B EC 8B 45 08` — `PUSH EBP; MOV EBP,ESP; MOV EAX,[EBP+8]`. Boundaries at +1, +3, +6. Clean 6-byte boundary.
+- `0x0070cb30`: `55 8B EC 83 EC 10` — `PUSH EBP; MOV EBP,ESP; SUB ESP,0x10`. Boundaries at +1, +3, +6. Clean 6-byte boundary.
+
+All three use `buildFastcallToCdeclThunk` to bridge to `callconv(.c)` detour functions (since `__thiscall` is `__fastcall` with unused EDX).
+
+### D3D9 (vtable patching via dummy device)
+
+| VTable Index | Method                    | Purpose |
+|-------------|---------------------------|---------|
+| 42           | `EndScene`                | Per-frame object scan, stencil clear |
+| 82           | `DrawIndexedPrimitive`    | Three-pass stencil outline rendering |
+| 16           | `Reset`                   | Force D24S8 depth/stencil format |
+
+Vtable obtained by creating a temporary `IDirect3DDevice9` via `Direct3DCreate9` → `CreateDevice` with a hidden window.  All D3D9 devices share the same vtable, so patching affects the game's device.
+
+## Outline Rendering (DIP hook)
+
+Three-pass stencil approach per outline model:
+
+1. **Pass 1 — Mark body**: Draw original geometry to stencil buffer (bit 0), no colour write.
+2. **Pass 2 — Draw outline**: Screen-space vertex shader expands vertices along normals. Stencil test rejects body pixels. Write outline bit 1. Dead players disable depth test (through-wall); targets/raid marks respect depth.
+3. **Pass 3 — Normal draw**: Restore all state, draw model normally on top.
+
+## Vertex Shader
+
+Compiled at runtime via `D3DXAssembleShader` from `d3dx9_43.dll` (loaded dynamically).  Falls back to no outlines if the DLL isn't present.
+
+Format: `vs_2_0`, uses WoW's bone matrix constants (`c[idx+31..33]`), view-projection at `c2-c5`, custom constants at `c251` (bone scale) and `c252` (pixel thickness).
+
+Pixel shader: `ps_3_0`, outputs solid colour from `c0`.
+
+## Key Offsets
+
+| Address/Offset | Purpose |
+|---------------|---------|
+| `0x00B41414`  | Object Manager pointer |
+| `+0xAC`       | First object in linked list |
+| `+0xA4`       | Base for next-object traversal |
+| `+0xC0`       | Local player GUID (from ObjMgr) |
+| `0x00B71368`  | Raid target GUID array (8 × 8 bytes) |
+| `0x515970`    | `UnitGUID(__fastcall, string_ECX→EAX:EDX)` |
+| `0x464870`    | `GetObjectByGUID(__stdcall, lo_stack, hi_stack→EAX)` — NOT fastcall! |
+| `0x6061E0`    | `UnitReaction(__thiscall, player_ECX, unit_stack→int)` |
+| model+`0x28`  | Direct owner object pointer |
+| model+`0x3C0` | Callback owner object pointer |
+| ctx+`0x3310`  | Model pointer in render context |
+
+## Category Priority
+
+1. **Target** (golden amber `#FFC800`) — current target, 2.25px outline
+2. **Raid-marked** (per-icon colour) — units with raid icons 1-8, 1.5px
+3. **Dead player** (cyan `#00FFFF`) — deceased friendly players, 2.5px, through walls
+
+## Per-frame Flow
+
+1. `EndScene` fires → `tracker.scanObjects()` rebuilds GUID tracking → stencil cleared
+2. Next frame: `ManageRenderListNode` classifies models → `DrawBatchProj` sets flags → `DIP` does three-pass rendering
+3. One-frame latency for tracking updates (imperceptible)
+
+## Differences from Reference
+
+- No `std::unordered_set`/`std::unordered_map` — fixed arrays with linear search (max 64 dead GUIDs, 8 raid marks, 256 outline models).
+- No `CriticalSection` — all hooks run on the main WoW thread; no synchronisation needed.
+- No MinHook — uses the project's existing `libs/hook` inline hook library.
+- Object scanning is frame-based (EndScene), not event-driven (no separate Idris runtime thread).
+- Shader loading uses dynamic `d3dx9_43.dll` lookup; gracefully disabled if absent.
@@ -0,0 +1,84 @@
+# Outline Validation Checklist
+
+## Build
+
+- [ ] `zig build` succeeds with no errors (x86 windows-msvc target)
+- [ ] Output DLL present at `zig-out/lib/weirdutils.dll`
+
+## Existing Feature Regression
+
+- [ ] DLL loads without crash (inject into WoW 1.12.1)
+- [ ] `/wu version` responds correctly
+- [ ] `/wu test` calls C function and returns string
+- [ ] Screenshot hook works (`/wu ss`, PNG output)
+- [ ] Interact/loot bindings work (InteractNearest, LootAllCorpses)
+- [ ] Addon loads on login (green "WeirdUtils loaded" message)
+
+## Outline Subsystem
+
+### Commands
+- [ ] `/wu outline` shows ON/OFF status
+- [ ] `/wu outline on` enables outlines
+- [ ] `/wu outline off` disables outlines
+- [ ] `/wu` help text includes outline commands
+
+### Dead Player Outlines
+- [ ] Dead friendly players in party/raid show cyan outline
+- [ ] Outline is visible through walls (no depth test)
+- [ ] Outline disappears when player is resurrected
+- [ ] Skeleton corpses are NOT outlined
+
+### Raid Mark Outlines
+- [ ] Units with raid icons (Star through Skull) show coloured outlines
+- [ ] Each icon has a distinct colour
+- [ ] Outlines update when raid marks change
+- [ ] Outlines respect terrain occlusion (depth test enabled)
+
+### Target Outline
+- [ ] Current target shows golden amber outline
+- [ ] Outline changes when target changes
+- [ ] Outline clears when target is deselected
+- [ ] Outline respects terrain occlusion
+
+### Rendering Quality
+- [ ] Outlines are clean (no jagged edges or flicker)
+- [ ] Outline thickness is consistent across distances
+- [ ] Normal model renders correctly on top of outline
+- [ ] No visible render state leakage to other models
+- [ ] Non-outlined units don't cover outline pixels (stencil test)
+
+### Performance
+- [ ] No noticeable FPS drop in normal gameplay
+- [ ] No FPS drop in 40-player raid with multiple outlines
+- [ ] Frame rate stable when toggling outlines on/off
+
+### Edge Cases
+- [ ] D3DX DLL not present → outlines gracefully disabled (no crash)
+- [ ] No targets tracked → zero overhead (fast path)
+- [ ] `/wu outline off` → DIP hook passes through immediately
+- [ ] Device Reset (resolution change) → shaders recreated properly
+
+## Hook Prologue Verification (Ghidra)
+
+Verified 2026-02-23 using Ghidra MCP against WoW.exe 1.12.1 (build 5875, 4,907,008 bytes).
+
+### Method
+
+Raw bytes read from each hook target address via `get_bytes`. Instruction boundaries decoded to confirm the `prologue_size` parameter passed to `hook.prepare()` lands on a clean instruction boundary. A mid-instruction cut would corrupt the trampoline — the copied bytes would decode as a different instruction when followed by the trampoline's JMP.
+
+### Results
+
+| Address | Function | Prologue bytes | Decoded | Boundary | Safe | Fixups |
+|---------|----------|---------------|---------|----------|------|--------|
+| `0x0070b360` | `CM2SceneRenderDraw` | `55 8B EC 81 EC 80 00 00 00` | `PUSH EBP; MOV EBP,ESP; SUB ESP,0x80` | +1,+3,**+9** | 9B | none |
+| `0x00710b90` | `CM2Model_ManageRenderListNode` | `55 8B EC 8B 45 08` | `PUSH EBP; MOV EBP,ESP; MOV EAX,[EBP+8]` | +1,+3,**+6** | 6B | none |
+| `0x0070cb30` | `CM2Scene_DrawBatchProjected` | `55 8B EC 83 EC 10` | `PUSH EBP; MOV EBP,ESP; SUB ESP,0x10` | +1,+3,**+6** | 6B | none |
+
+### Fix applied
+
+`CM2SceneRenderDraw` prologue changed from 6 to **9** bytes in `src/outline/model_hook.zig`. The original 6-byte value would have split the `SUB ESP, 0x80` instruction (opcode `81 EC` + 4-byte immediate, spanning offset +3 to +9). The trampoline would have executed `81 EC 80 E9 xx xx` as `SUB ESP, <corrupted>`, leading to stack corruption and a crash.
+
+### No other changes needed
+
+- All three prologues contain only register/memory instructions (no `E8 CALL` or `E9 JMP`), so `rel32_fixups` remains `&.{}`.
+- The 4-byte NOP padding (bytes 5-8) at `0x0070b360` after the 5-byte `E9 JMP` is harmless — it is never executed (execution jumps to the detour thunk).