Bugfix: Exclude PostFx from UI by default (Issue #1154) by yuechen-li-dev · Pull Request #3128 · stride3d/stride

yuechen-li-dev · 2026-04-10T08:24:29Z

PR Details

The blurry, washed out UI stemming from the PostFx being applied on UI elements has been a bug since 2021, and the RenderGroup 31 bypass is well known, and I've even made a script that is specifically designed to work around it.

So, I've decided to fix it today.

Details:

ForwardRenderer.cs
Added a PostEffectsUIRenderStage property. When set, this stage is drawn onto the final resolved output target after PostEffects has already run, meaning UI geometry assigned to it is never touched by tone-mapping, bloom, or any other post-processing effect.

DefaultGraphicsCompositorLevel10/9.sdgfxcomp
Added UIRenderStage as a new render stage and pointed the UIRenderFeature selector at it instead of Transparent. Wired PostEffectsUIRenderStage on the ForwardRenderer to this new stage. The result is that all UI renders after post-processing by default, with no configuration required from the user.

Added a separate unit test in new group: Stride.Rendering.Tests to guard against regression.

Related Issue

Fixes #1154

Types of changes

Docs change / refactoring / dependency upgrade
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist

My change requires a change to the documentation.
I have added tests to cover my changes.
All new and existing tests passed.
I have built and run the editor to try this change out.

Ethereal77

Looks good overall, but a few comments:

If this PR is a proper fix that improves the defaults to not need the post-FX Clean UI "hack", why mention it all the time in comments and tests? Just do your thing and verify it is working!
Naming is ambiguous in my opinion. Look at the comment I left.
Some of the comments and logs seem like a WIP. Maybe a little cleanup is needed.

I'll massage the tests a bit to check the rendering is working and the render stage is being invoked in the correct order, excluding any details or specific behavior that may come from the Clean UI patch. However were it implemented it were a "hacky" solution. If this PR is the proper fix, forget about that solution.

Ethereal77 · 2026-04-10T08:30:44Z

This PR should not touch the solution file

A new xunit test project was added to the solution, which is why the .sln file was changed.

My bad, I missed that. Sorry

Ethereal77 · 2026-04-10T08:34:42Z

+// ForwardRendererCompositorTests.cs
+// Place at: sources/engine/Stride.Rendering.Tests/Compositing/ForwardRendererCompositorTests.cs
+//
+// Structural tests confirming the UI-before-PostFx ordering bug.
+// No GPU required — pure object-graph assertions.


What are these comments?

Ethereal77 · 2026-04-10T08:36:04Z

+        // ----------------------------------------------------------------
+        // 2. REGRESSION GUARD (currently expected to fail — uncomment after fix):
+        //    Once the fix is in, ForwardRenderer should expose a
+        //    PostEffectsUIRenderStage property that is drawn AFTER PostEffects.
+        // ----------------------------------------------------------------


Seems like in-progress comments. Can be removed?

Ethereal77 · 2026-04-10T08:37:01Z

+        // ----------------------------------------------------------------
+        // 3. CONFIRM: UIRenderFeature inherits RenderStageSelectors from
+        //    RootRenderFeature — the collection used by CleanUiPostFxPatch
+        //    to route Group31 to a separate stage.
+        // ----------------------------------------------------------------


Same as above

Ethereal77 · 2026-04-10T08:40:09Z

+
+            Assert.NotNull(feature.RenderStageSelectors);
+
+            _output.WriteLine($"UIRenderFeature.RenderStageSelectors is present (type: {feature.RenderStageSelectors.GetType().Name}).");


It is a bit weird to write to the console inside a test. Imho tests should not talk, just verify things work as intended

Removed. However, I'd like to note that writing console output inside unit tests does significantly reduce tendency for LLMs to hallucinate while generating code, in case Stride does decide to use AI coding more extensively in the future.

I would prefer to add comments inside the tests detailing what they are trying to test than to write the results to the console.

However, it hadn’t occurred to me that LLMs could understand better the context from console output. Makes sense, I have to try it 😁

Ethereal77 · 2026-04-10T08:50:47Z

+
+            Assert.True(hasStage);
+            Assert.True(hasSelector);
+            _output.WriteLine("IsAlreadyPatched: correctly returns true when stage and selector both exist.");


Same as above: logging in tests

Ethereal77 · 2026-04-10T08:51:03Z

+        // ----------------------------------------------------------------
+        // 7. DOCUMENT: The null-conditional assignment bug in the original
+        //    CleanUiPostFxPatch.MoveUiRecursive.
+        //    `ui?.RenderGroup = value` is a compile error in C# — it cannot
+        //    assign through a null-conditional. The correct form is guarded
+        //    with an explicit null check.
+        // ----------------------------------------------------------------


Same as above

Ethereal77 · 2026-04-10T08:53:56Z

+            // WRONG (compile error if used on real UIComponent):
+            //   fakeUi?.RenderGroup = RenderGroup.Group31;
+            //
+            // CORRECT:
+            if (fakeUi != null)
+                fakeUi.RenderGroup = RenderGroup.Group31;


Why is this wrong? In theory, C# 14 allows null-conditional assignments, so both should be equivalent, no?

Ethereal77 · 2026-04-10T08:54:54Z

+            _output.WriteLine("Null-conditional assignment bug documented.");
+            _output.WriteLine("  Correct pattern: if (ui != null) ui.RenderGroup = RenderGroup.Group31;");


Same as above: logging in tests

Ethereal77 · 2026-04-10T08:58:54Z

+        /// making it suitable for HUD and screen-space UI.
+        /// Leave null (the default) when no post-FX-immune UI is required.
+        /// </summary>
+        public RenderStage PostEffectsUIRenderStage { get; set; }


PostEffectsUIRenderStage naming is a bit ambiguous imho. It is a render stage that draws UI after post-FX, but he naming does not implies it occurs after post-FX, but in post-FX.

I changed it to "AfterPostEffectUIRenderStage" locally, before I commit it, would that be a good name?

I suppose it is. I'm bad myself at naming things, but sounds right.

Is there something in your logic that makes this render-stage UI-specific? I ask because I'm thinking about other effects that may benefit from no-post-fx (for example, debug rendering, outlines, etc). Nevermind if this is specific for UI rendering.

Is there something in your logic that makes this render-stage UI-specific?
Nothing in particular, I just wanted to close this bug. I've done some research on Stride's graphic compositor in the past, and I think it could be better if there is more of a code-first approach to writing a custom graphics compositor, maybe with an interface or JSON profile or something, as it is a bit difficult to tinker with it in GameStudio the last time I've tried.

Yes, I know. I think the graphics compositor editor is/was a half-way solution to a more complete version Silicon had planned that never actually ended happening. There are several places and features like that in the codebase. A pity.

Anyway, my question was that if there is no UI-specific limitation to the use of this render stage, maybe just calling it AfterPostEffectRenderStage is good enough. That way it can be used for other things without implying it is for UI.

yuechen-li-dev · 2026-04-10T10:00:43Z

Looks good overall, but a few comments:

If this PR is a proper fix that improves the defaults to not need the post-FX Clean UI "hack", why mention it all the time in comments and tests? Just do your thing and verify it is working!

Naming is ambiguous in my opinion. Look at the comment I left.

Some of the comments and logs seem like a WIP. Maybe a little cleanup is needed.

I'll massage the tests a bit to check the rendering is working and the render stage is being invoked in the correct order, excluding any details or specific behavior that may come from the Clean UI patch. However were it implemented it were a "hacky" solution. If this PR is the proper fix, forget about that solution.

Unit tests were generated by Claude. It's very good at debugging if you use it properly, but it gets weirdly hung up about random stuff and it has a tendency to be confidently wrong, so you always have to check its answers. I built GameStudio on my own VS locally and confirmed that it works though.

My own CleanUI patch pretty much automated the RenderGroup 31 bypass documented by others which was the "hacky" solution. This PR should be the more proper fix of the root cause and does not use the same method as the Clean UI patch.

Ethereal77 · 2026-04-10T11:30:45Z

it has a tendency to be confidently wrong

Yeah, I can relate. I find it both funny and infuriating how stubborn LLMs can get about things sometimes 😁

yuechen-li-dev · 2026-04-13T00:26:15Z

Yeah, I think this is pretty much done. It's a simple patch, so I think it's ready if there are no other concerns.

Ethereal77

It is in good shape overall. I left a few comments

Ethereal77 · 2026-04-13T09:22:54Z

            87ff1d9cdd52418daf76385176a0e316: ref!! 555e84b4-b68a-4f38-ac3a-f0f563028ef0
            5e059d4cc2db4ee8a1f28a40f4ac3ae8: ref!! b03a45c6-7a56-417c-8a80-69cc608671f1
        GBufferRenderStage: ref!! ecab139e-5f55-42b5-a324-310c195a9c89
+        PostEffectsUIRenderStage: ref!! 5b5e4f7e-e8b5-497f-9d3b-418d06823b14


Here it still has the old name. It should be AfterPostEffectsUIRenderStage.

Changed to “AfterPostEffectsRenderStage" now.

Ethereal77 · 2026-04-13T09:24:33Z

        ShadowMapRenderStages:
            fc4d1e0de5c2b0bbc27bcf96e9a848fd: ref!! c0524e55-4061-464d-84dd-7c4c70f70e0e
        GBufferRenderStage: null
+        PostEffectsUIRenderStage: ref!! 62af1128-cc9c-49a0-9d81-3f7f598a4b16


Here it still has the old name. It should be AfterPostEffectsUIRenderStage.

Changed to “AfterPostEffectsRenderStage" now.

Ethereal77 · 2026-04-13T09:26:55Z

+        /// <summary>
+        /// A UI render stage drawn directly onto <see cref="viewOutputTarget"/> after
+        /// <see cref="PostEffects"/> have resolved. Geometry assigned to this stage is
+        /// never tone-mapped, bloomed, or otherwise affected by post-processing effects,
+        /// making it suitable for HUD and screen-space UI.
+        /// Leave null (the default) when no post-FX-immune UI is required.
+        /// </summary>
+        public RenderStage AfterPostEffectsUIRenderStage { get; set; }
+


Just my opinion, but I'd remove UI from the name. The more I rhink of it, the more I believe it would be useful for more than just UI (gizmos, debug rendering, outlines, maybe even some custom material unaffected by post-FX).

Done. Changed to “AfterPostEffectsRenderStage" now.

Ethereal77 · 2026-04-13T09:29:44Z

Nit: some double blank lines between methods and blank lines before closing braces

Should be corrected in latest PR as well.

Ethereal77 · 2026-04-13T09:36:45Z

Btw, Stride.Graphics.Tests projects (for GraphicsProfile 10 and 11) and I believe also Stride.Engine.Tests already include some tests that cover the rendering logic (i.e., materials, layering, lighting and shadowing, etc.)

Your new Stride.Rendering.Tests is a bit alone currently with just the few tests for post-FX render stage. Maybe these should be moved to those other test projects, or the pure rendering tests should be moved here (a bigger change).

@xen2 You have been refactoring tests lately. What do you think?

…rStage", fix spacing inconsistencies in unit test.

yuechen-li-dev · 2026-04-13T09:53:21Z

Btw, Stride.Graphics.Tests projects (for GraphicsProfile 10 and 11) and I believe also Stride.Engine.Tests already include some tests that cover the rendering logic (i.e., materials, layering, lighting and shadowing, etc.)

Your new Stride.Rendering.Tests is a bit alone currently with just the few tests for post-FX render stage. Maybe these should be moved to those other test projects, or the pure rendering tests should be moved here (a bigger change).

@xen2 You have been refactoring tests lately. What do you think?

Strides.Graphics.Test project is very heavy to run, since it has what I assumed to be image generation checks via DX11. So, that's why I would like to keep a separate project to keep things that doesn't require those image checks separate and maybe move some other tests over as well, if you guys would like, for separation of test concerns.

Ethereal77 · 2026-04-13T21:25:13Z

LGTM. Thanks for your work. A much needed feature imho (and one that took a lot longer than needed).

Strides.Graphics.Test project is very heavy to run, since it has what I assumed to be image generation checks via DX11. So, that's why I would like to keep a separate project to keep things that doesn't require those image checks separate and maybe move some other tests over as well, if you guys would like, for separation of test concerns.

Wrt to that I'll wait for other mantainers' take on this. If it eases the workflow of testing the rendering without drawbacks, why not?
(anyway, that would go in a different PR, I suppose)

yuechen-li-dev added 4 commits April 10, 2026 00:23

Add Rendering Test

c84af8e

Diagnose post effect affecting UI bug. Confirmed.

184a0c0

Fix ForwardRenderer with unit test guard.

ac2d1cb

Fix graphic compositor.

5056184

Ethereal77 suggested changes Apr 10, 2026

View reviewed changes

yuechen-li-dev added 2 commits April 10, 2026 02:36

Removed comments and extra unit test that tests CleanUI bypass.

7ee4d83

Removed console output from unit tests.

81c596b

Rename "PostEffectRenderStage" to "AfterPostEffectRenderStage"

42b09b1

Ethereal77 suggested changes Apr 13, 2026

View reviewed changes

Changed newly introduced render stange name to “AfterPostEffectsRende…

9617148

…rStage", fix spacing inconsistencies in unit test.

xen2 force-pushed the master branch 2 times, most recently from ab329f3 to 482bd28 Compare April 16, 2026 07:56


		Assert.NotNull(feature.RenderStageSelectors);

		_output.WriteLine($"UIRenderFeature.RenderStageSelectors is present (type: {feature.RenderStageSelectors.GetType().Name}).");

		_output.WriteLine("Null-conditional assignment bug documented.");
		_output.WriteLine(" Correct pattern: if (ui != null) ui.RenderGroup = RenderGroup.Group31;");

Uh oh!

Conversation

yuechen-li-dev commented Apr 10, 2026

PR Details

Related Issue

Types of changes

Checklist

Uh oh!

Ethereal77 left a comment

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!

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!

yuechen-li-dev commented Apr 10, 2026

Uh oh!

Ethereal77 commented Apr 10, 2026

Uh oh!

yuechen-li-dev commented Apr 13, 2026

Uh oh!

Ethereal77 left a comment

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!

Ethereal77 commented Apr 13, 2026

Uh oh!

yuechen-li-dev commented Apr 13, 2026

Uh oh!

Ethereal77 commented Apr 13, 2026 •

edited

Loading