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 the IRenderBackend interface as the first step in a planned renderer refactor, adding a DX8Backend adapter that forwards every virtual method to the existing DX8Wrapper static facade and wiring the lifecycle into Do_Onetime_Device_Dependent_Inits/Shutdowns. There is zero behavior change — the DX8 path remains the only renderer and all new methods are one-line trampolines or no-ops.

• IRenderBackend.h defines the abstract interface with forward-declared W3D types, a TransformKind enum, and RenderBackendViewport struct; Initialize/Shutdown are optional no-ops to allow the DX8 backend to remain device-agnostic.
• DX8Backend.{h,cpp} implements every pure-virtual via a single-line forwarding call to DX8Wrapper; a To_D3D_Transform helper maps the new enum to D3DTRANSFORMSTATETYPE.
• RenderBackend.{h,cpp} owns the global g_renderBackend pointer with guarded Init_Render_Backend() / Shutdown_Render_Backend() lifecycle functions; shutdown correctly null-checks before calling Shutdown() and then deletes.

Confidence Score: 5/5

Safe to merge — all new code is either pure forwarding trampolines or no-ops with no behavioral impact on the existing DX8 path.

Every DX8Backend method is a one-line call to the existing DX8Wrapper static function, Initialize/Shutdown are no-ops, the shutdown path has a correct null guard, and CI confirms the game runs identically.

The dx8wrapper.cpp wiring (lines 398-399) is worth revisiting when a real Initialize() implementation is introduced in a future backend.

Important Files Changed

Filename	Overview
Core/Libraries/Source/WWVegas/WW3D2/IRenderBackend.h	New abstract interface covering the W3D-facing subset of DX8Wrapper's public API; uses #pragma once, forward-declares all types cleanly, defines TransformKind enum and RenderBackendViewport struct, and leaves Initialize/Shutdown as optional no-ops.
Core/Libraries/Source/WWVegas/WW3D2/Backend/DX8Backend.h	Concrete adapter header fully overriding all pure-virtual IRenderBackend methods; uses #pragma once and correct TheSuperHackers copyright.
Core/Libraries/Source/WWVegas/WW3D2/Backend/DX8Backend.cpp	Pure forwarding trampolines to DX8Wrapper static API; To_D3D_Transform helper correctly maps TransformKind enum to D3D constants.
Core/Libraries/Source/WWVegas/WW3D2/Backend/RenderBackend.cpp	Global g_renderBackend owner with guarded Init/Shutdown lifecycle functions; null checks are correct and symmetric.
Core/Libraries/Source/WWVegas/WW3D2/Backend/RenderBackend.h	Declares g_renderBackend extern and the two lifecycle functions; clean and minimal.
Core/Libraries/Source/WWVegas/WW3D2/dx8wrapper.cpp	Wires Init_Render_Backend/Initialize into Do_Onetime_Device_Dependent_Inits and shutdown into Do_Onetime_Device_Dependent_Shutdowns; Initialize() is called unconditionally after the guarded Init_Render_Backend(), which is fine today but fragile for future backends.
Core/Libraries/Source/WWVegas/WW3D2/CMakeLists.txt	Adds Backend/ sources and IRenderBackend.h to WW3D2_SRC in alphabetical order; no issues.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant DX8W as DX8Wrapper
    participant RB as RenderBackend.cpp
    participant DX8B as DX8Backend
    participant IRB as IRenderBackend

    Note over DX8W: Do_Onetime_Device_Dependent_Inits()
    DX8W->>RB: Init_Render_Backend()
    RB->>DX8B: new DX8Backend()
    DX8B-->>RB: g_renderBackend set
    RB-->>DX8W: return
    DX8W->>IRB: "g_renderBackend->Initialize(hwnd, w, h)"
    Note over IRB: no-op in DX8Backend

    Note over DX8W: Per-frame rendering
    DX8W->>IRB: "g_renderBackend->Begin_Scene()"
    IRB->>DX8B: DX8Backend::Begin_Scene()
    DX8B->>DX8W: DX8Wrapper::Begin_Scene()

    Note over DX8W: Do_Onetime_Device_Dependent_Shutdowns()
    DX8W->>IRB: "g_renderBackend->Shutdown() [null-guarded]"
    Note over IRB: no-op in DX8Backend
    DX8W->>RB: Shutdown_Render_Backend()
    RB->>DX8B: delete g_renderBackend
    RB-->>DX8W: "g_renderBackend = nullptr"

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant DX8W as DX8Wrapper
    participant RB as RenderBackend.cpp
    participant DX8B as DX8Backend
    participant IRB as IRenderBackend

    Note over DX8W: Do_Onetime_Device_Dependent_Inits()
    DX8W->>RB: Init_Render_Backend()
    RB->>DX8B: new DX8Backend()
    DX8B-->>RB: g_renderBackend set
    RB-->>DX8W: return
    DX8W->>IRB: "g_renderBackend->Initialize(hwnd, w, h)"
    Note over IRB: no-op in DX8Backend

    Note over DX8W: Per-frame rendering
    DX8W->>IRB: "g_renderBackend->Begin_Scene()"
    IRB->>DX8B: DX8Backend::Begin_Scene()
    DX8B->>DX8W: DX8Wrapper::Begin_Scene()

    Note over DX8W: Do_Onetime_Device_Dependent_Shutdowns()
    DX8W->>IRB: "g_renderBackend->Shutdown() [null-guarded]"
    Note over IRB: no-op in DX8Backend
    DX8W->>RB: Shutdown_Render_Backend()
    RB->>DX8B: delete g_renderBackend
    RB-->>DX8W: "g_renderBackend = nullptr"

Loading

_{Reviews (5): Last reviewed commit: "chore(ww3d2): use qualified include path..." | 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]

This looks suspicious. Shouldn't this happen in the DX8Backend Constructor? Right now DX8Wrapper is always initialized even if a OpenGL Backend was used.