Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DitherFade material layer causes serious artifact with forward rendering / MSAA #1104

Open
kring opened this issue May 23, 2023 · 1 comment · May be fixed by #1416
Open

DitherFade material layer causes serious artifact with forward rendering / MSAA #1104

kring opened this issue May 23, 2023 · 1 comment · May be fixed by #1416
Labels
bug Something isn't working

Comments

@kring
Copy link
Member

kring commented May 23, 2023

  1. Enable forward shading by check the Forward Shading box under Edit -> Project Settings -> Engine - Rendering -> Forward Renderer.
  2. Change "Anti-Aliasing Method" to "Multisample Anti-Aliasing (MSAA)" under Edit -> Project Settings -> Engine - Rendering -> DEfault Settings)

You'll see serious dithering artifacts in all Cesium objects, while objects that user other materials look fine:

image

Deleting the DitherFade layer from our default material fixes the problem. We're using an engine-provided material node that is meant for temporal AA, but it's not clear why it fails so badly with MSAA.

Originally reported on the forum:
https://community.cesium.com/t/forward-rendering-msaa-black-squares-and-dithering-artifacts-on-terrain/24149

@j9liu
Copy link
Contributor

j9liu commented May 7, 2024

#1414 doesn't address this. I think that it has to do with the fact that the "DitherTemporal AA" node that we use, doesn't have good defaults when Temporal AA isn't enabled. So it's stuck on a permanent dither pattern. You can actually kind of see that pattern in the material function preview:
image

We could possibly account for the current AA mode by querying it, and passing a flag into the material indicating whether or not to use dithering. But because the user could possibly change it, we'd need to constantly check for updates to that setting. So something like this (taken from RendererScene.cpp):

static auto* CVarAntiAliasingMethod = IConsoleManager::Get().FindTConsoleVariableDataInt(TEXT("r.AntiAliasingMethod"));
static int32 CachedAntiAliasingMethod = CVarAntiAliasingMethod->GetValueOnGameThread();
const int32 AntiAliasingMethod = CVarAntiAliasingMethod->GetValueOnGameThread();
if (AntiAliasingMethod != CachedAntiAliasingMethod) {
     // ... 
}

But I'm concerned about doing it this way because it seems like things can commonly override the project's default AA method? From SceneView.cpp:

// see EAntiAliasingMethod
static TAutoConsoleVariable<int32> CVarDefaultAntiAliasing(
	TEXT("r.AntiAliasingMethod"),
	4,
	TEXT("Engine default (project setting) for AntiAliasingMethod is (postprocess volume/camera/game setting still can override)\n")
	TEXT(" 0: off (no anti-aliasing)\n")
	TEXT(" 1: Fast Approximate Anti-Aliasing (FXAA)\n")
	TEXT(" 2: Temporal Anti-Aliasing (TAA)\n")
	TEXT(" 3: Multisample Anti-Aliasing (MSAA, Only available on the desktop forward renderer)\n")
	TEXT(" 4: Temporal Super-Resolution (TSR, Default)"),
	ECVF_RenderThreadSafe);

And I can't really tell where a post process volume / camera / game setting override would make a difference, but it does make me nervous to try and make this change.

I think a better change instead would be a UseLODTransitions parameter, and set it to 1 for Enable LOD Transitions = true, 0 otherwise. So people who don't use LOD transitions at all won't even have to worry about this material layer. As long as people keep this setting disabled, changing the AA mode won't make a difference. If they try to enable LOD transitions for a non-temporal AA mode, then that's moreso user error.

@j9liu j9liu linked a pull request May 7, 2024 that will close this issue
@j9liu j9liu linked a pull request May 8, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants