Skip to content

Port DX9 rendering foundation and independent SceneView system - #59

Open
borsuczyna wants to merge 1 commit into
Dryxio:masterfrom
borsuczyna:codex/dx9-rendering-foundation
Open

borsuczyna wants to merge 1 commit into
Dryxio:masterfrom
borsuczyna:codex/dx9-rendering-foundation

Conversation

@borsuczyna

Copy link
Copy Markdown
Contributor

Squashed port of the codex/dx9-rendering-foundation branch (17 commits) from the upstream mtasa-blue repository.

Summary

  • Safe DX9 rendering API foundations
  • Independently-rendered RenderWare scene views (matrix/orthographic camera control)
  • Cubemap and depth-stencil render targets, MRT sets
  • Isolated shaders and shadow queues for scene views
  • PostFx colour filter reproduction for independently-rendered SceneViews
  • Sun direction API and sampleable depth targets

Reconciled with Neon-specific additions during the port (native hit/bike-jack handlers, renderer stats, script camera and near-clip API, gang tag spray, etc.) so both feature sets coexist without ABI breaks.

🤖 Generated with Claude Code

…tasa-blue

Squashed port of the codex/dx9-rendering-foundation branch (17 commits) from
the upstream mtasa-blue repository: safe DX9 rendering API foundations,
independently-rendered RenderWare scene views (matrix/orthographic camera
control, cubemap and depth-stencil render targets, isolated shaders and
shadow queues, MRT sets, PostFx colour filter reproduction, sun direction
API), reconciled with Neon-specific additions (native hit/bike-jack
handlers, renderer stats, script camera and near-clip API).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@Dryxio

Dryxio commented Aug 19, 2026

Copy link
Copy Markdown
Owner

Hi, please check those ones (codex pr review) :

Merge blockers

1. ABI / vtable compatibility is currently broken

The PR description explicitly mentions preserving Neon compatibility without ABI breaks, but several new virtual methods are inserted in the middle of existing SDK interfaces instead of being appended.

Examples include:

  • CCamera
    • CopyCameraMatrixToRWCam
    • CalculateDerivedValues
  • CRenderWare
    • GetRenderingEntityType
    • GetRenderingClientEntity
    • OnShaderReplacementResolved
  • CMultiplayer
    • GetSunDirection
    • SetPreConstructRenderListHandler
    • SetSceneViewProjection
    • SetSceneViewSquarePerspective
    • RenderSecondaryScene
    • ReleaseSecondarySceneResources
  • CRenderItemManagerInterface
    • the new depth target / cubemap / MRT / render-pass / SceneView APIs

Inserting virtual methods shifts the vtable index of every existing method that follows them. This can break cross-DLL compatibility even if everything builds from the same source tree.

The PR already correctly appends GetLastSecondarySceneRenderError() with a comment explicitly stating that this preserves existing ABI indices. The same rule should be applied to every new virtual method.

I would also review the new enum entries that are inserted in the middle of existing entity/class enums, because those additions change the numeric value of later entries.

Required before merge: move all new cross-module virtual methods to the end of their interfaces, or otherwise prove that these interfaces are not ABI-sensitive.


2. Perspective SceneViews do not currently respect the render-target aspect ratio

Cubemap rendering explicitly corrects the RenderWare view window to a square 1:1 projection, but regular perspective SceneViews still inherit GTA's primary-camera projection/aspect behavior.

The progress document itself lists:

Perspective SceneView target-aspect correction ... pending

This means a SceneView such as 1024x1024 or 512x1024 can render with a projection derived from the primary display aspect ratio rather than from its own target dimensions.

Since dxCreateSceneView(width, height, ...) exposes arbitrary target sizes publicly, this is a correctness issue rather than only a future enhancement.

Required before merge: derive the perspective RenderWare view window from the SceneView target aspect ratio, with a well-defined FOV convention.


3. dxSetShaderDepthTextureValue is currently internally contradictory

dxCreateSceneView(..., sampleableDepth = true) explicitly rejects sampleableDepth = true.

However, dxSetShaderDepthTextureValue(shader, name, sceneView) expects the SceneView to contain a sampleable depth target and even reports:

create it with sampleableDepth = true

That state is impossible to create through the public SceneView API.

The standalone DxDepthStencilTarget sampleable path still makes sense for direct render passes / MRT.

Required before merge: either remove this SceneView-specific function, or change it to operate on a standalone DxDepthStencilTarget (or another actually reachable sampleable depth object).


4. Failed cubemap face renders can be removed from the retry queue

The cubemap scheduler is documented to keep failed faces queued so they can retry next frame.

However, uiRenderedMask is updated once BeginCubemapFaceRender() succeeds, even if RenderSecondaryScene() subsequently returns false.

That causes the face bit to be cleared from the pending request mask even though the actual world render failed.

Required before merge: only add a face to uiRenderedMask after bRendered == true.


High-priority issues

5. Multi-resolution SceneViews can cause repeated RenderWare resource destruction/recreation

There is one shared secondary RenderWare camera/color/depth raster set.

Whenever the requested SceneView dimensions differ from the currently cached dimensions, ReleaseSecondarySceneResources() is called and the camera/rasters are recreated.

With two always SceneViews at different resolutions, this can result in destroy/recreate churn every frame. The same problem can happen when alternating between a SceneView and a differently-sized cubemap.

For a system that intentionally allows multiple SceneViews without a fixed per-frame cap, this can become a serious performance and stability problem.

I would strongly recommend either:

  • caching secondary resources by dimensions,
  • owning them per SceneView/cubemap,
  • or otherwise avoiding per-frame destruction/recreation.

At minimum, this needs a stress test before merge.


6. dxCreateShader changes its existing return contract

Successful dxCreateShader calls previously returned two values and now return a third diagnostics table.

The failure path also changes its returned values.

Extra Lua return values are often harmless when callers explicitly assign a fixed number of variables, but they are not universally transparent. Forwarding, table construction, wrapper functions, and generic multi-return handling can observe the difference.

Since dxGetShaderDiagnostics() already exists, I would prefer to keep dxCreateShader's historical return contract unchanged unless this is deliberately versioned/documented.


7. SceneView shader target assignments retain raw entity pointers

SSceneViewShaderAssignment stores pTargetEntity as a raw pointer and intentionally does not remove it when the entity is destroyed.

While the pointer is only compared and not dereferenced, address reuse can still cause a newly allocated entity at the same address to accidentally match an old shader assignment.

It would be safer to mirror the global shader system's entity-reference cleanup behavior or otherwise use a lifetime-safe identity.


8. Render-pass ownership is global rather than resource-scoped

The progress document already notes that one resource can theoretically call dxEndRenderPass() and close another resource's currently-open pass.

This is similar to the historical global dxSetRenderTarget model, but nesting makes the failure mode more significant.

I would not necessarily block the entire architecture on redesigning this immediately, but it should be explicitly covered by a multi-resource test harness before merge.

Suggested merge gate

Before merging, I would require at minimum:

  1. Fix all ABI/vtable ordering issues.
  2. Fix perspective SceneView target-aspect handling.
  3. Resolve/remove the unreachable SceneView sampleable-depth API.
  4. Fix cubemap failed-face retry behavior.
  5. Address or demonstrate acceptable behavior for multi-resolution SceneView/cubemap resource churn.
  6. Get Build + CodeQL green.
  7. Run a dedicated runtime harness covering:
    • existing DX API regressions
    • 1:1, 4:3 and 16:9 SceneViews on a 16:9 display
    • multiple SceneViews with different resolutions
    • SceneView + cubemap with different resolutions
    • nested render passes
    • cross-resource render-pass behavior
    • resource stop while a pass is open
    • repeated device reset / alt-tab
    • forced cubemap render failure + retry
    • output shaders and SceneView shader isolation
    • dxCreateShader compatibility behavior

Once those are clean, this should be much easier to approve with confidence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants