refactor(ww3d2): Introduce IRenderBackend interface

[bobtista]

Summary

First PR in a planned multi-step refactor introducing an IRenderBackend interface in WW3D2.

DX8 remains the only renderer. DX8Backend forwards each interface call to the existing DX8Wrapper - there is no rendering or behavioral change.

What this PR adds

• IRenderBackend.h — a minimal backend-neutral interface containing only methods with migrated callers.
• Backend/DX8Backend.{h,cpp} — the DX8 adapter. Its methods are forwarding calls to the existing DX8Wrapper implementation.
• Backend/RenderBackend.h — the backend-selection seam. The build links one implementation of Create_Render_Backend().
• WW3D owns the active backend from WW3D::Init() through WW3D::Shutdown() and exposes it through Get_Render_Backend().
• Existing high-level WW3D rendering calls now use the active backend.

Interface methods will be added alongside the caller migrations that require them in later changes

Test plan

• Windows CI builds pass
• Game launches and runs a Skirmish round identically (no visual or behavioral difference)

[greptile-apps]

Greptile Summary

This PR introduces an IRenderBackend abstract interface as the first step in a planned multi-step render-backend refactor. DX8Backend wraps the existing DX8Wrapper static API with one-line trampolines, and WW3D now owns the backend lifetime from Init through Shutdown. There is no behavioral change: DX8Wrapper remains the sole renderer and all initialization/shutdown ordering is preserved.

• IRenderBackend.h defines the new interface; Backend/DX8Backend.{h,cpp} provide the DX8 adapter; Backend/RenderBackend.h declares the Create_Render_Backend factory that links the build to one implementation.
• WW3D::Init constructs the backend via Create_Render_Backend (which internally calls Init_D3D_To_WW3_Conversion and DX8Wrapper::Init), WW3D::Shutdown deletes it, and DX8Backend::~DX8Backend conditionally calls DX8Wrapper::Shutdown (preserving the original !Lite guard).
• All former DX8Wrapper:: call sites in ww3d.cpp are mechanically replaced with Get_Render_Backend()-> equivalents; formconv.h is removed from ww3d.cpp now that its call has moved into DX8Backend::Create.

Confidence Score: 5/5

• This PR is safe to merge; it is a mechanical refactoring with no behavioral change to the rendering path.
• All changed logic is a direct forwarding layer: every DX8Wrapper call that moved to the backend interface produces identical runtime behavior. Lifecycle ordering (texture manager shutdown before DX8Wrapper shutdown, !Lite guard) is preserved exactly. The only new code that could fail at runtime is the null return from Create_Render_Backend, which is correctly checked before proceeding in WW3D::Init.
• No files require special attention.

Important Files Changed

Filename	Overview
Core/Libraries/Source/WWVegas/WW3D2/IRenderBackend.h	New abstract backend interface; clean design with forward declarations, #pragma once, default parameters matching DX8Wrapper, and a virtual destructor.
Core/Libraries/Source/WWVegas/WW3D2/Backend/DX8Backend.h	DX8 adapter header; uses #pragma once, private constructor enforces factory creation, virtual override on destructor is correct.
Core/Libraries/Source/WWVegas/WW3D2/Backend/DX8Backend.cpp	All methods are one-line DX8Wrapper trampolines; DX8Wrapper::Shutdown is guarded by !Lite in destructor, matching original WW3D::Shutdown behavior; Init_D3D_To_WW3_Conversion moved here correctly.
Core/Libraries/Source/WWVegas/WW3D2/Backend/RenderBackend.h	Minimal seam header; forward declaration of IRenderBackend and declaration of Create_Render_Backend factory; clean and correct.
Core/Libraries/Source/WWVegas/WW3D2/ww3d.cpp	Backend lifecycle correctly transferred to WW3D::Init/Shutdown; DX8TextureManagerClass::Shutdown ordering before delete RenderBackend is preserved; all call-site migrations look correct.
Core/Libraries/Source/WWVegas/WW3D2/ww3d.h	Adds forward declaration for IRenderBackend and exposes Get_Render_Backend(); RenderBackend static member placed alongside the other static pointer members.
Core/Libraries/Source/WWVegas/WW3D2/CMakeLists.txt	New Backend/ sources and IRenderBackend.h added in alphabetical order; no other changes.

Sequence Diagram

sequenceDiagram
    participant WW3D
    participant IRenderBackend
    participant DX8Backend
    participant DX8Wrapper

    Note over WW3D: WW3D::Init()
    WW3D->>DX8Backend: Create_Render_Backend(hwnd, lite)
    DX8Backend->>DX8Wrapper: Init_D3D_To_WW3_Conversion()
    DX8Backend->>DX8Wrapper: DX8Wrapper::Init(window, lite)
    DX8Wrapper-->>DX8Backend: success/failure
    DX8Backend-->>WW3D: "DX8Backend* (or nullptr on failure)"
    WW3D->>WW3D: "RenderBackend = result"

    Note over WW3D: WW3D::Begin_Render()
    WW3D->>IRenderBackend: "Get_Render_Backend()->Set_Viewport(vp)"
    IRenderBackend->>DX8Wrapper: "DX8Wrapper::Set_Viewport(&d3dvp)"
    WW3D->>IRenderBackend: "Get_Render_Backend()->Clear(...)"
    IRenderBackend->>DX8Wrapper: DX8Wrapper::Clear(...)
    WW3D->>IRenderBackend: "Get_Render_Backend()->Begin_Scene()"
    IRenderBackend->>DX8Wrapper: DX8Wrapper::Begin_Scene()

    Note over WW3D: WW3D::End_Render()
    WW3D->>IRenderBackend: "Get_Render_Backend()->End_Scene(flip)"
    IRenderBackend->>DX8Wrapper: DX8Wrapper::End_Scene(flip)
    WW3D->>IRenderBackend: "Get_Render_Backend()->Invalidate_Cached_Render_States()"
    IRenderBackend->>DX8Wrapper: DX8Wrapper::Invalidate_Cached_Render_States()

    Note over WW3D: WW3D::Shutdown()
    WW3D->>WW3D: DX8TextureManagerClass::Shutdown()
    WW3D->>DX8Backend: delete RenderBackend
    DX8Backend->>DX8Wrapper: DX8Wrapper::Shutdown() [if !Lite]

Loading

_{Reviews (7): Last reviewed commit: "refactor(ww3d2): Restore remaining backe..." | Re-trigger Greptile}

[xezon]

Logical approach in an effort to start introducing new render backends. Some open questions.

[xezon]

This looks suspicious. Shouldn't Init_Render_Backend call DX8Wrapper::Do_Onetime_Device_Dependent_Inits instead?

[bobtista]

I don't think so. The contract is: DX8Wrapper owns when the renderer comes up (it owns the D3D device), and the backend owns how it brings up its own device. To make that explicit rather than implied, I'll add a lifecycle pair to the interface eg Initialize(hwnd, w, h) / Shutdown() with empty default bodies, called after Init_Render_Backend() and before Shutdown_Render_Backend(). DX8Backend treats them as no-ops since DX8Wrapper still owns the real device, and non-DX8 backends use Initialize() to create their own device/swapchain against the game window. Device-lost/reset stays DX8-internal, it's a D3D8 artifact, and modern backends recover via swapchain reset without surfacing it.

[xezon]

How about it calls g_renderBackend = new DX8Backend(); directly then? Is Init_Render_Backend needed?

[bobtista]

Let's keep the named Init_Render_Backend() / Shutdown_Render_Backend() pair. It's the single place backend selection lives, so dx8wrapper.cpp includes only RenderBackend.h and not DX8Backend.h. Inlining new DX8Backend() would couple DX8Wrapper to the concrete adapter and push backend-selection into the call site. The pair also gives a symmetric construct/teardown that nulls the pointer on the way out.

[bobtista]

WW3D owns the backend object now. DX8Wrapper only brings up its device-dependent state through Initialize/Shutdown. Init_Render_Backend is gone

[xezon]

How will we cover DX8Caps related stuff?

[bobtista]

Capability queries become a set of narrow virtuals with safe defaults eg Supports_Texture_Format(WW3DFormat), Supports_Compressed_Textures(), Get_Texture_Limits(), Supports_Texture_Op(...), etc.

DX8Backend forwards each to DX8Wrapper::Get_Current_Caps(), so DX8Caps stays the reference implementation and is never exposed directly. The few sites that read raw D3DCAPS8 bitfields today (COLORWRITEENABLE in W3DScene/W3DVolumetricShadow, TextureOpCaps in shader.cpp) and the Voodoo3 vendor check get promoted to neutral predicates (Supports_Color_Write_Mask(), Supports_Texture_Op(), Is_Legacy_Voodoo3()) so shared engine code carries no D3D types or #ifdefs. I've kept these out of this scaffolding PR deliberately, each lands with the call-site it unblocks, but the shape is good.

[bobtista]

Nothing queries caps through the backend yet, so nothing is on the interface. When a caller needs it, it comes across as narrow predicates forwarding to DX8Caps, not as exposed D3DCAPS8.

[xezon]

What will we do with direct calls to DX8, for example _Get_D3D_Device8 or Get_DX8_Texture_Stage_State_Value_Name ?

[bobtista]

This took a lot of doing, but has been worth it. In my run ahead branches for bgfx (still local), I have it so there are two classes, handled differently. The high-frequency render/texture-stage-state calls aren't re-exposed as raw D3DRS_/D3DTSS_; they route through a backend-neutral fixed-function state cache (keyed by the same ordinals) plus typed semantic setters, so a non-DX8 backend reads intent rather than D3D enums. The genuinely DX8-only entry points (_Get_D3D_Device8, Create_DX8*, raw SetRenderTarget) migrate to named high-level methods (Set_Render_Target_With_Z, a view-capture primitive, …); the irreducible cases e.g. hand-written water pixel-shader bytecode go behind a named-enum hatch (Create_Legacy_Pixel_Shader(kind)) rather than a raw device pointer. The end state has no _Get_D3D_Device8 left in the engine subsystems; the raw device stays inside DX8Backend. Pure DX8 diagnostics eg Get_DX8_Texture_Stage_State_Value_Name simply stay on DX8Wrapper, as they're debug-only and not part of the abstraction.

[bobtista]

They stay on DX8Wrapper. DX8-only entry points aren't interface candidates.

[bobtista]

I've already got the bgfx backend working on my Mac and Windows machines, a lot was figured along the way, and some stuff like the lifecycle I'm happy to add here and inherit from. I'm still ironing out some quirks, will share more when things feel polished enough.

[xezon]

What about the following functions? How will they be dealt with if not as part of the IRenderBackend? Does the IRenderBackend already claim to be complete or are the chosen function just the simple ones that can be abstracted so far?

SetCleanupHook
Is_Initted
Get_Format_Name
Get_Render_State
Set_Render_State
Release_Render_State
Get_Free_Texture_RAM
Begin_Statistics
End_Statistics
Get_Last_Frame_Statistics
Get_FrameCount
Get_Fog_Color
Convert_Color (looks like utility function to be moved elsewhere)
Clamp_Color (looks like utility function to be moved elsewhere)
Set_Alpha (looks like utility function to be moved elsewhere)
Create_Additional_Swap_Chain
Set_Render_Target
Apply_Default_State
Get_Vertex_Processing_Behavior
getBackBufferFormat
Reset_Device
Registry_Save_Render_Device
Registry_Load_Render_Device
Set_Draw_Polygon_Low_Bound_Limit

[bobtista]

I left it minimalistic here on purpose, the idea is that in future PRs we add each method as needed for a given backend eg bgfx. I'm also happy to add them here if that's better. Could split them into buckets like so:

• Will be promoted when a backend needs them: render-state, statistics, Reset_Device, Set_Render_Target (skeleton already has Set_Render_Target_With_Z + Create_Render_Target).
• Stay DX8-specific (escape hatches on DX8Wrapper): _Get_D3D_Device8, format-name/state-name debug helpers, registry device save/load, Get_Vertex_Processing_Behavior.
• Don't belong on a backend at all (utilities): Convert_Color, Clamp_Color, Set_Alpha — agree with him, separate cleanup.

[bobtista]

I trimmed to the 11 methods WW3D and DX8Wrapper actually call. We can add methods once a caller routes through it. So for now, everything on your list stays on DX8Wrapper until something migrates it.

[xezon]

Will there be a change to cleanup DX8Wrapper? It looks like it contains a number of things that do not directly belong there, such as Convert_Color, Clamp_Color.

[bobtista]

Yes, would you rather that happen in this PR vs a separate one?

[bobtista]

I'll do that as a separate PR, keeps this one reviewable

[xezon]

How about it calls g_renderBackend = new DX8Backend(); directly then? Is Init_Render_Backend needed?

[xezon]

Will IRenderBackend then also be served with static functions through WW3D class? It already does so for a number of 1 to 1 DX8Wrapper function calls, such as

void WW3D::Flip_To_Primary()
{
	DX8Wrapper::Flip_To_Primary();
}

void WW3D::Set_Gamma(float gamma,float bright,float contrast,bool calibrate)
{
	DX8Wrapper::Set_Gamma(gamma,bright,contrast,calibrate);
}

[bobtista]

Yes for the existing high-level WW3D API. Methods such as Flip_To_Primary and Set_Gamma should remain public WW3D entry points and delegate to the active backend once migrated. I don’t intend to mirror every IRenderBackend method on WW3D - lower-level WW3D2 code can use the backend interface directly. I’m leaving that rewiring out of this skeleton PR because WW3D still exists separately under Generals and GeneralsMD rather than in Core.

[bobtista]

Rebased onto main and updated, addressed comments. Since the last review, I've been using a complete bgfx backend running behind this interface on both Windows and macOS, so the open questions above have practical answers now. The interface had state setters promoted as callers needed them, DX8-only entry points stayed on DX8Wrapper, the color utilities never belonged on a backend.

[eydotan]

I built a native D3D11 backend for ZH (W3DNext) whose interface deliberately mirrors this PR's shape, so I ran a detailed comparative review of this PR, the bgfx branch, and my backend — full write-up here. Four concrete suggestions for this slice, from scars both codebases have already earned:

1. Widen Draw_Triangles' unsigned short params to unsigned int — DX8-era limits in a backend-neutral interface; your own branch's newer Submit_*_Packet methods already take unsigned int. Cheap now, painful after N backends implement it.
2. Reconsider Get_Back_Buffer(...) -> SurfaceClass* — it leaks a D3D8-wrapping type; your branch already superseded it with Get_Back_Buffer_Description/Capture_Back_Buffer_Image. Worth landing the newer shape directly.
3. Guard g_renderBackend->Initialize(...) like Init_Render_Backend() — a repeated Do_Onetime_Device_Dependent_Inits double-initializes any backend with a non-trivial Initialize (may be unreachable today; cheap to fence).
4. Consider landing your audit_bgfx_dx8_dependencies.py alongside, as a CI ratchet on the escape-hatch count — both your branch and mine got bitten by raw-device bypasses (my worst open bug is a raw IDirect3DVertexBuffer8::Lock invisible to my backend); a shrinking hatch inventory is the mechanical defense.

Happy to align my interface to whatever lands here — the review doc has the both-directions detail, including what my backend gets wrong.

Disclosure: review produced with AI assistance (Claude), findings human-curated; file:line evidence in the linked doc.

[bobtista]

I built a native D3D11 backend for ZH (W3DNext) whose interface deliberately mirrors this PR's shape, so I ran a detailed comparative review of this PR, the bgfx branch, and my backend — full write-up here. Four concrete suggestions for this slice, from scars both codebases have already earned:

1. Widen Draw_Triangles' unsigned short params to unsigned int — DX8-era limits in a backend-neutral interface; your own branch's newer Submit_*_Packet methods already take unsigned int. Cheap now, painful after N backends implement it.
2. Reconsider Get_Back_Buffer(...) -> SurfaceClass* — it leaks a D3D8-wrapping type; your branch already superseded it with Get_Back_Buffer_Description/Capture_Back_Buffer_Image. Worth landing the newer shape directly.
3. Guard g_renderBackend->Initialize(...) like Init_Render_Backend() — a repeated Do_Onetime_Device_Dependent_Inits double-initializes any backend with a non-trivial Initialize (may be unreachable today; cheap to fence).
4. Consider landing your audit_bgfx_dx8_dependencies.py alongside, as a CI ratchet on the escape-hatch count — both your branch and mine got bitten by raw-device bypasses (my worst open bug is a raw IDirect3DVertexBuffer8::Lock invisible to my backend); a shrinking hatch inventory is the mechanical defense.

Happy to align my interface to whatever lands here — the review doc has the both-directions detail, including what my backend gets wrong.

Disclosure: review produced with AI assistance (Claude), findings human-curated; file:line evidence in the linked doc.

Thank you for this! It's a genuinely helpful review, and the file references made it easy to verify. Rebased main and four commits pushed.

Widths. Taken, and widened across the whole boundary rather than just Draw_Triangles: both overloads, Draw_Strip, and both Set_Index_Buffer base offsets. DX8Backend narrows where the legacy wrapper still requires 16 bits, so it stays behavior-neutral — nothing calls the interface yet, which is exactly what makes now the cheap moment.

Auditing that turned up a related portability bug too: Set_Vertex_Shader/Set_Pixel_Shader translate the wrapper's DWORD into unsigned long, which is 32-bit on MSVC but 64-bit wherever long is 64-bit - a silent width change introduced by the very translation this header exists to perform. Both are now unsigned int, and the "opaque" comment is replaced with an honest note that the value is a legacy FVF-or-handle code.

Get_Back_Buffer. Removed rather than replaced. SurfaceClass wraps IDirect3DSurface8 either way, and no caller reaches it through the interface - the existing users still call DX8Wrapper::_Get_DX8_Back_Buffer directly and are unaffected. I'd rather the screenshot and smudge migrations introduce a neutral capture API designed around their real ownership needs than backport an owning-buffer type into a scaffold, where it would also drag in the header's language-baseline question.

Lifecycle. I don't think the double-initialization path is reachable — Do_Onetime_Device_Dependent_Inits runs only from Create_Device, the DX8 path asserts D3DDevice == nullptr first, and the paired teardown lives in Release_Device. But you're right that idempotent construction followed by unconditional initialization is muddled, and a guard would conceal that rather than resolve it.

Rather than fencing it, I've separated backend-object lifetime from device lifetime: the backend is constructed in WW3D::Init and destroyed in WW3D::Shutdown, while Initialize/Shutdown pair with device creation and release. The object now survives a full device release/create cycle instead of being rebuilt with it, which is what a backend holding real device-independent state will need. Construction is also asserted single-shot, so unbalanced use stays visible.

The auditor. Good idea, not yet a ratchet though. It always exits successfully and has no committed baseline. Turning it into a real fail-on-increase gate needs that baseline plus stable categorized output, and it needs something meaningful to count, which arrives with the first caller-migration slice. Happy to do it then.

The broader write-up is useful too, particularly the interface-bloat criticism, which I'm not going to argue. I'd just keep those decomposition questions off this scaffold's merge bar and take them up as the migration slices land. Happy to keep comparing notes.

[eydotan]

Thanks for the thorough pass — you went further than each suggestion, and in the right direction all three times.

The unsigned long → unsigned int find on the shader handles is the better catch of the two: my header carries the same translation and I hadn't looked at it as a portability issue at all.

Removing Get_Back_Buffer rather than replacing it is the right call, and it's a delta on my side — mine still has SurfaceClass * Get_Back_Buffer(unsigned int), which exists purely because DX8Wrapper had it. Letting the screenshot and smudge migrations design a neutral capture API around their real ownership needs is cleaner than backporting the owning type.

On lifecycle: separating backend-object lifetime from device lifetime is better than the fence I suggested, and it matters more for my backend than for the DX8 reference one — mine holds device-independent state (fixed-function shader permutations, state translation tables) that has no business being rebuilt on a device reset. Agreed a guard would have concealed that question rather than answered it.

Aligning my side, from diffing my header against yours just now:

• Set_Index_Buffer base offsets are still unsigned short in mine — widening to match.
• Get_Back_Buffer — dropping it.
• Draw_Triangles / Draw_Strip are already unsigned int, and the Initialize/Shutdown contract already reads the same, so those two are in sync.

Fair on the auditor — it isn't a ratchet until it has a committed baseline and something real to count. Happy to wire the fail-on-increase version when the first caller-migration slice lands.

And no argument on keeping the decomposition questions off this scaffold's merge bar.

GitHub's the most reliable way to reach me, by the way — here, or on the W3DNext issues.

[eydotan]

Following this closely — I've been building a D3D11 backend against this interface in a fork, so I've got a second implementer's view of it that might be useful.

Two of your recent commits are things I'd flagged as needed and hadn't done yet: 8acd5f0d widening the draw/index ranges to unsigned int and bdd6d938 dropping the SurfaceClass back-buffer accessor. Both are right; I'll pick them up rather than carry my own variants.

On 93ae331e — trimming to the methods callers currently use — I want to flag one thing while the stack is still small, because it's the only place my experience diverges from the rule. Caller-driven growth works cleanly when there's one implementation behind the interface. With a second backend, the methods that aren't yet routed are exactly the ones that determine whether the abstraction holds: my fork implements ~44 beyond the current 12, and most of the design pressure came from the drawing surface — Draw_Triangles/Draw_Strip, buffer binding and dynamic staging, shader constants, render targets, stencil. Several of those needed their signatures reconsidered once a non-DX8 backend had to satisfy them, and one of those reconsiderations is 8acd5f0d.

I'm not suggesting you take 44 methods on spec — the trim is the right call for a reviewable PR. But if it'd help, I can share which ones a real second backend ends up needing and where the DX8-shaped signatures caused friction, so the ones that do land arrive in a shape that won't need widening later. Happy to do that as a list here, or as issues, whichever is less noise.

[bobtista]

Following this closely — I've been building a D3D11 backend against this interface in a fork, so I've got a second implementer's view of it that might be useful.

Two of your recent commits are things I'd flagged as needed and hadn't done yet: 8acd5f0d widening the draw/index ranges to unsigned int and bdd6d938 dropping the SurfaceClass back-buffer accessor. Both are right; I'll pick them up rather than carry my own variants.

On 93ae331e — trimming to the methods callers currently use — I want to flag one thing while the stack is still small, because it's the only place my experience diverges from the rule. Caller-driven growth works cleanly when there's one implementation behind the interface. With a second backend, the methods that aren't yet routed are exactly the ones that determine whether the abstraction holds: my fork implements ~44 beyond the current 12, and most of the design pressure came from the drawing surface — Draw_Triangles/Draw_Strip, buffer binding and dynamic staging, shader constants, render targets, stencil. Several of those needed their signatures reconsidered once a non-DX8 backend had to satisfy them, and one of those reconsiderations is 8acd5f0d.

I'm not suggesting you take 44 methods on spec — the trim is the right call for a reviewable PR. But if it'd help, I can share which ones a real second backend ends up needing and where the DX8-shaped signatures caused friction, so the ones that do land arrive in a shape that won't need widening later. Happy to do that as a list here, or as issues, whichever is less noise.

Absolutely please do share what's worked for you. And my bgfx work is here if you want to see how it's played out there.

[bobtista]

Pushed three changes.

Ownership moved to WW3D since #3012 landed. So WW3D is in Core and the backend pointer lives there now, behind WW3D::Get_Render_Backend(). Created in WW3D::Init, destroyed in WW3D::Shutdown. Backend/RenderBackend.cpp and the g_renderBackend global are gone; RenderBackend.h is now just the Create_Render_Backend() declaration.

WW3D now routes through the backend. Every DX8Backend method is a 1:1 forward, so behavior is unchanged.

Interface trimmed to what callers use. We can add more as implementations are added.

[eydotan]

Concrete example of the thing I mentioned above, from 77b2768d — worth a look before it settles.

Dropping uselimit from Set_Gamma is safe in this branch because WW3D::Set_Gamma is the only routed caller and it always passes true. But it isn't the only caller in the engine — W3DDisplay passes false:

GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp:582
    Set_Gamma(gamma, bright, contrast, calibrate, false);
Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp:501
    Set_Gamma(gamma, bright, contrast, calibrate, false);

Those still go through DX8Wrapper::Set_Gamma statically today, so nothing breaks right now. But when W3DDisplay is the caller that gets migrated, the four-arg interface method has no way to express what it's asking for, and the limiting silently switches on — a behavior change that compiles clean and won't show up in a diff review of the migration commit.

I hit this adopting your commits into my fork: I took 4d2aeeb0, 8acd5f0d and bdd6d938 as-is (all three are improvements — thanks), and had to skip this one for exactly that reason.

Not arguing for keeping the parameter as-is; uselimit is an ugly boolean and the DX8-specific clamping arguably shouldn't be in the interface at all. The narrow point is that "no routed caller passes anything but the default" and "no caller needs anything but the default" come apart, and the second one is what determines whether a parameter can be dropped. Same shape as the general note above — worth a grep across the unmigrated callers before trimming a parameter, since that's the population the current interface can't see.

[bobtista]

Concrete example of the thing I mentioned above, from 77b2768d — worth a look before it settles.

Dropping uselimit from Set_Gamma is safe in this branch because WW3D::Set_Gamma is the only routed caller and it always passes true. But it isn't the only caller in the engine — W3DDisplay passes false:

GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp:582
    Set_Gamma(gamma, bright, contrast, calibrate, false);
Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp:501
    Set_Gamma(gamma, bright, contrast, calibrate, false);

Those still go through DX8Wrapper::Set_Gamma statically today, so nothing breaks right now. But when W3DDisplay is the caller that gets migrated, the four-arg interface method has no way to express what it's asking for, and the limiting silently switches on — a behavior change that compiles clean and won't show up in a diff review of the migration commit.

I hit this adopting your commits into my fork: I took 4d2aeeb0, 8acd5f0d and bdd6d938 as-is (all three are improvements — thanks), and had to skip this one for exactly that reason.

Not arguing for keeping the parameter as-is; uselimit is an ugly boolean and the DX8-specific clamping arguably shouldn't be in the interface at all. The narrow point is that "no routed caller passes anything but the default" and "no caller needs anything but the default" come apart, and the second one is what determines whether a parameter can be dropped. Same shape as the general note above — worth a grep across the unmigrated callers before trimming a parameter, since that's the population the current interface can't see.

Good call - restored uselimit to IRenderBackend::Set_Gamma in the latest push

[eydotan]

Nice — thanks for turning that around so quickly. I'll pick up the successor commit in my fork and drop the local exception I was carrying for it.

[xezon]

Looks clean to me.

[xezon]

Is any further polishing coming for it?

[bobtista]

Just pushed the remaining polish: restored the Clear defaults on IRenderBackend, updated the migrated calls, and moved Create_Render_Backend to the top of DX8Backend.cpp. That’s all the polishing I have planned for this PR.