chore(ww3d2): add IRenderBackend Interface by bobtista · Pull Request #2613 · TheSuperHackers/GeneralsGameCode

bobtista · 2026-04-17T23:33:37Z

Summary

First PR in a planned multi-step refactor introducing an IRenderBackend interface in WW3D2. This PR adds only the scaffolding — interface, adapter, lifecycle wiring, and one isolated proof-of-concept call site. zero behavior change. The DX8 path remains the only renderer; g_renderBackend is constructed as a DX8Backend that forwards every method to the existing DX8Wrapper static facade.

What this PR adds

IRenderBackend.h — abstract interface covering the W3D-facing subset of DX8Wrapper's public API. Only high-level methods (those taking ShaderClass/TextureBaseClass/Matrix4x4/etc.) are virtualized. D3D8-typed low-level methods stay on DX8Wrapper as DX8Backend-specific escape hatches — see the doc for the rationale.
DX8Backend.{h,cpp} — concrete adapter that forwards every virtual method to the existing DX8Wrapper:: static functions. Pure forwarding, no new rendering logic.
RenderBackend.{h,cpp} — exposes the global IRenderBackend* g_renderBackend. Init_Render_Backend() constructs new DX8Backend() from DX8Wrapper::Do_Onetime_Device_Dependent_Inits(); Shutdown_Render_Backend() tears it down from DX8Wrapper::Do_Onetime_Device_Dependent_Shutdowns().

Test plan

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

greptile-apps · 2026-04-17T23:48:13Z

Greptile Summary

Introduces IRenderBackend, a virtual interface abstracting the W3D-facing subset of DX8Wrapper, along with DX8Backend (a pure forwarding adapter) and the g_renderBackend global lifecycle managed through Init_Render_Backend/Shutdown_Render_Backend. The DX8 rendering path is unchanged; this is scaffolding only.

IRenderBackend.h — new abstract interface with ~40 pure-virtual methods covering scene, draw, transform, texture, and light operations; also defines the D3D-independent RenderBackendViewport struct and TransformKind enum.
DX8Backend.{h,cpp} — concrete one-liner trampolines to every DX8Wrapper static; Initialize/Shutdown are no-ops as DX8Wrapper owns the device.
RenderBackend.{h,cpp} + dx8wrapper.cpp — lifecycle hooks wired into Do_Onetime_Device_Dependent_Inits/Shutdowns; the shutdown call site dereferences g_renderBackend before the null guard in Shutdown_Render_Backend.

Confidence Score: 4/5

Safe to merge after fixing the unguarded g_renderBackend dereference in the shutdown path.

The shutdown path in dx8wrapper.cpp calls g_renderBackend->Shutdown() directly before the null guard inside Shutdown_Render_Backend(), so any code path reaching Do_Onetime_Device_Dependent_Shutdowns() without a prior successful init will crash. The rest of the change is mechanical forwarding with no new logic.

Core/Libraries/Source/WWVegas/WW3D2/dx8wrapper.cpp — the shutdown block needs a null guard before dereferencing g_renderBackend.

Important Files Changed

Filename	Overview
Core/Libraries/Source/WWVegas/WW3D2/dx8wrapper.cpp	Wires Init/Shutdown into the DX8 device lifecycle; shutdown path calls g_renderBackend->Shutdown() without a null guard before the guarded Shutdown_Render_Backend(), creating a crash if reached before Init runs.
Core/Libraries/Source/WWVegas/WW3D2/IRenderBackend.h	New abstract interface covering the W3D-facing DX8Wrapper API; TransformKind enum pins values to D3DTS_* constants, leaking a D3D8 ABI detail into the backend-agnostic interface.
Core/Libraries/Source/WWVegas/WW3D2/Backend/DX8Backend.h	Concrete adapter header; all overrides declared with virtual but without the override specifier, missing compile-time mismatch detection.
Core/Libraries/Source/WWVegas/WW3D2/Backend/DX8Backend.cpp	Pure one-line trampolines to DX8Wrapper static methods; Set_Viewport and Set_Transform include correct struct/enum conversions. No issues found.
Core/Libraries/Source/WWVegas/WW3D2/Backend/RenderBackend.cpp	Owns the g_renderBackend global; Init/Shutdown functions have correct null guards internally.
Core/Libraries/Source/WWVegas/WW3D2/Backend/RenderBackend.h	Declares the g_renderBackend extern and the two lifecycle functions; clean header with #pragma once.
Core/Libraries/Source/WWVegas/WW3D2/CMakeLists.txt	Adds the four new Backend/ sources and IRenderBackend.h to the WW3D2 source list; straightforward and correct.

Class Diagram

%%{init: {'theme': 'neutral'}}%%
classDiagram
    class IRenderBackend {
        <<abstract>>
        +Initialize(window, width, height)
        +Shutdown()
        +Is_Device_Lost() bool
        +Begin_Scene()
        +End_Scene(flip_frame)
        +Set_Shader(shader)
        +Set_Texture(stage, texture)
        +Set_Transform(kind, matrix)
        +Draw_Triangles(...)
    }
    class DX8Backend {
        +Is_Device_Lost() bool
        +Begin_Scene()
        +End_Scene(flip_frame)
        +Set_Shader(shader)
        +Set_Texture(stage, texture)
        +Set_Transform(kind, matrix)
        +Draw_Triangles(...)
    }
    class DX8Wrapper {
        <<static facade>>
        +Is_Device_Lost()$
        +Begin_Scene()$
        +Set_Transform(type, matrix)$
        +Draw_Triangles(...)$
        +Do_Onetime_Device_Dependent_Inits()$
        +Do_Onetime_Device_Dependent_Shutdowns()$
    }
    class RenderBackend {
        <<global>>
        +g_renderBackend IRenderBackend*
        +Init_Render_Backend()
        +Shutdown_Render_Backend()
    }
    IRenderBackend <|-- DX8Backend : implements
    DX8Backend --> DX8Wrapper : forwards every call to
    DX8Wrapper --> RenderBackend : calls Init/Shutdown
    RenderBackend o-- IRenderBackend : owns g_renderBackend

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
Core/Libraries/Source/WWVegas/WW3D2/dx8wrapper.cpp:460-461
`g_renderBackend->Shutdown()` is called unconditionally before `Shutdown_Render_Backend()`, which contains the null guard. If `Do_Onetime_Device_Dependent_Shutdowns()` is ever reached without a paired `Do_Onetime_Device_Dependent_Inits()` — for example through an error-recovery path that resets the device partway through initialization — this is a null-pointer dereference crash. The guard should wrap both calls.

```suggestion
	if (g_renderBackend != nullptr)
	{
		g_renderBackend->Shutdown();
	}
	Shutdown_Render_Backend();
```

### Issue 2 of 3
Core/Libraries/Source/WWVegas/WW3D2/IRenderBackend.h:46-51
`TransformKind` values are pinned to the D3DTS_* integer constants — including the non-sequential `RB_TRANSFORM_WORLD = 256` — so `DX8Backend` can cast them directly. This leaks a D3D8 ABI detail into the abstract interface: any future non-D3D backend will receive magic numbers (2, 3, 256) it must silently understand to mean View/Projection/World. Consider sequential values (0, 1, 2) in the interface and letting `DX8Backend::Set_Transform` map them to the D3D8 constants locally.

### Issue 3 of 3
Core/Libraries/Source/WWVegas/WW3D2/Backend/DX8Backend.h:35-103
All overriding virtual methods use only `virtual` without `override`. The `override` specifier would catch any signature mismatch between `DX8Backend` and `IRenderBackend` at compile time. Prefer `virtual ... override` (or just `override` without `virtual`) for all overriding declarations.

_{Reviews (4): Last reviewed commit: "feat(ww3d2): add IRenderBackend Initiali..." | Re-trigger Greptile}

xezon

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

xezon · 2026-05-30T10:59:02Z

+	// TheSuperHackers @refactor bobtista 10/04/2026 Construct the global
+	// IRenderBackend instance now that the D3D device is ready. See
+	// Core/Libraries/Source/WWVegas/WW3D2/RENDER_BACKEND.md.
+	Init_Render_Backend();


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

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.

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

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.

xezon · 2026-05-30T11:03:32Z

+// Implementations (DX8Backend.cpp, future BgfxBackend.cpp, etc.) can use
+// whatever C++ features the project's main build allows.
+
+class IRenderBackend


How will we cover DX8Caps related stuff?

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.

xezon · 2026-05-30T11:04:41Z

+// Implementations (DX8Backend.cpp, future BgfxBackend.cpp, etc.) can use
+// whatever C++ features the project's main build allows.
+
+class IRenderBackend


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

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 · 2026-05-31T01:38:59Z

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.

greptile-apps · 2026-05-31T01:55:44Z

+	g_renderBackend->Shutdown();
+	Shutdown_Render_Backend();


g_renderBackend->Shutdown() is called unconditionally before Shutdown_Render_Backend(), which contains the null guard. If Do_Onetime_Device_Dependent_Shutdowns() is ever reached without a paired Do_Onetime_Device_Dependent_Inits() — for example through an error-recovery path that resets the device partway through initialization — this is a null-pointer dereference crash. The guard should wrap both calls.

Suggested change

g_renderBackend->Shutdown();

Shutdown_Render_Backend();

if (g_renderBackend != nullptr)

{

g_renderBackend->Shutdown();

}

Shutdown_Render_Backend();

Prompt To Fix With AI

This is a comment left during a code review. Path: Core/Libraries/Source/WWVegas/WW3D2/dx8wrapper.cpp Line: 460-461 Comment: `g_renderBackend->Shutdown()` is called unconditionally before `Shutdown_Render_Backend()`, which contains the null guard. If `Do_Onetime_Device_Dependent_Shutdowns()` is ever reached without a paired `Do_Onetime_Device_Dependent_Inits()` — for example through an error-recovery path that resets the device partway through initialization — this is a null-pointer dereference crash. The guard should wrap both calls. ```suggestion if (g_renderBackend != nullptr) { g_renderBackend->Shutdown(); } Shutdown_Render_Backend(); ``` How can I resolve this? If you propose a fix, please make it concise.

xezon · 2026-06-04T14:42:28Z

    distlod.cpp
    distlod.h
    dllist.h
+    Backend/DX8Backend.cpp


Can be sorted alphabetically

xezon · 2026-06-04T14:59:43Z

+// Method names intentionally match the existing DX8Wrapper names so migrating a
+// caller is a mechanical DX8Wrapper::X(...) -> g_renderBackend->X(...) rewrite.
+
+class IRenderBackend


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

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.

xezon · 2026-06-04T15:02:03Z

+// Method names intentionally match the existing DX8Wrapper names so migrating a
+// caller is a mechanical DX8Wrapper::X(...) -> g_renderBackend->X(...) rewrite.
+
+class IRenderBackend


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.

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

xezon · 2026-06-04T15:11:42Z

+	// TheSuperHackers @refactor bobtista 10/04/2026 Construct the global
+	// IRenderBackend instance now that the D3D device is ready. See
+	// Core/Libraries/Source/WWVegas/WW3D2/RENDER_BACKEND.md.
+	Init_Render_Backend();


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

xezon · 2026-06-04T15:16:43Z

+
+// The active rendering backend. Set by Init_Render_Backend() and cleared by
+// Shutdown_Render_Backend(); never null between those two calls.
+extern IRenderBackend * g_renderBackend;


Should this perhaps be part of WW3D class ? It seems like it fits there. Basically WW3D already is a high level entry point to and owner of the rendering ecosystem. See WW3D Static Globals in ww3d.cpp

I like this idea. Want it done in this PR or a separate one?

WW3D isn't in Core though, it's still in Generals/ and GeneralsMD/, while RenderBackend/IRenderBackend/DX8Wrapper are shared Core code. For layering and ordering this is a bit messier until we unify it. What do you think?

xezon · 2026-06-04T15:19:39Z

+// Method names intentionally match the existing DX8Wrapper names so migrating a
+// caller is a mechanical DX8Wrapper::X(...) -> g_renderBackend->X(...) rewrite.
+
+class IRenderBackend


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 changed the title ~~feat(ww3d2): add IRenderBackend skeleton (Phase 1, DX8 default)~~ feat(ww3d2): add IRenderBackend Interface Apr 17, 2026

bobtista added 3 commits April 17, 2026 19:40

feat(ww3d2): add IRenderBackend abstract interface

42f2aea

feat(ww3d2): add DX8Backend adapter forwarding to DX8Wrapper statics

97a23e4

feat(ww3d2): add global g_renderBackend pointer and lifecycle wiring

e648a3e

bobtista force-pushed the bobtista/feat/render-backend-interface-skeleton branch 2 times, most recently from 5e014bf to e648a3e Compare April 17, 2026 23:43

TheSuperHackers deleted a comment from greptile-apps Bot Apr 17, 2026

fix(ww3d2): Update copyright headers to TheSuperHackers for new files

1c0f02d

xezon reviewed May 30, 2026

View reviewed changes

bobtista added 3 commits May 31, 2026 11:46

refactor(ww3d2): trim redundant comments from render backend interface

a8f04fa

refactor(ww3d2): move backend implementations into Backend subfolder

987406a

feat(ww3d2): add IRenderBackend Initialize and Shutdown lifecycle hooks

a3d0d92

greptile-apps Bot reviewed May 31, 2026

View reviewed changes

xezon reviewed Jun 4, 2026

View reviewed changes

xezon added Refactor Edits the code with insignificant behavior changes, is never user facing Rendering Is Rendering related labels Jun 4, 2026

xezon changed the title ~~feat(ww3d2): add IRenderBackend Interface~~ chore(ww3d2): add IRenderBackend Interface Jun 4, 2026

Conversation

bobtista commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What this PR adds

Test plan

Uh oh!

greptile-apps Bot commented Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Class Diagram

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bobtista commented May 31, 2026

Uh oh!

greptile-apps Bot May 31, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bobtista commented Apr 17, 2026 •

edited

Loading

greptile-apps Bot commented Apr 17, 2026 •

edited

Loading