Fixing artifacts in SSR

[BastiaanOlij]

This PR fixes :

• issues with self reflection
• issues with changing Roughness quality

Note that in the comments below I'm highlighting some of the different fixes, and highlight a few other issues I found that need further discussion and may need to be solved in further PRs.

[BastiaanOlij]

So first issue I've fixed, we get bits in the reflection where we're stopped because we're reflecting our object itself:

(note, these screenshots are made with renderdoc looking at the SSR buffers)

Added logic to check if our first step results in remaining on our starting pixel (can happen with rounding issue), seems to fix this issues:

Fix I applied here is:

	if (ivec2(pos + line_advance - 0.5) == ssC) {
		// It is possible for rounding to cause our first pixel to check to be the pixel we're reflecting.
		// Make sure we skip it
		pos += line_advance;
		z += z_advance;
		w += w_advance;
	}

Though I am wondering if the - 0.5 makes sense in depth = imageLoad(source_depth, ivec2(pos - 0.5)).r;, seems to increase the likelyhood of getting the first pixel to overlap.

[BastiaanOlij]

Second issue is related to the first as we can see in this image:

Its especially clear on the green square, we're still getting self reflections. It's happening on the sphere as well but due to the color of the sphere it's harder to see.

The fix I'm applying here is to check the ray direction against the surface we're reflecting, we should be hitting the "front" of a surface, not the "back", if we're hitting the back it's 99% likely we're self reflecting. The result is proper reflections:

There is a cost to this solution as we're reading the normal of the surface we're reflecting. If we're hitting the proper surface the cost is just one extra read, but if we're self reflecting we may have 2 or 3 reads.

Our fix here is:

		if (depth > z_to) {
			// Test if our ray is hitting the "right" side of the surface, if not we're likely self reflecting and should skip.
			vec4 test_normal_roughness = imageLoad(source_normal_roughness, test_pos);
			vec3 test_normal = test_normal_roughness.xyz * 2.0 - 1.0;
			test_normal = normalize(test_normal);
			test_normal.y = -test_normal.y; //because this code reads flipped

			if (dot(ray_dir, test_normal) < 0.001) {
				// if depth was surpassed
				if (depth <= max(z_to, z_from) + params.depth_tolerance && -depth < params.camera_z_far * 0.95) {
					// check the depth tolerance and far clip
					// check that normal is valid
					found = true;
				}
				break;
			}
		}

Also there might be a case where we actually hit another object on the back side, we may need to apply this extra test only for the first 2 or 3 pixels we're testing, so change it to something like if (steps_taken > 2.0 || dot(ray_dir, test_normal) < 0.001) {.

Most artifacts in the test scene are now gone:

[BastiaanOlij]

When looking at the back of our cube (shadow side) we can see our reflection is missing:

Looks like the reflections render fine so its an issue in how its merged in:

Turns out that due to the surface being in shadow, it's dark, and unless you crank up metallic on the floor there is just very little color being mixed in:

[BastiaanOlij]

When we look at the base of the cube we see a dark line where the cube meets the ground:

This is caused by the fact that we are blurring our reflection texture. Where the cube hits the floor we either get no reflection, or we are reflecting the ground back into the cube (even though that data is not used).

Notice before blur:

And after blur:

We need to improve the blur logic to at the very least ignore where we don't have reflections, but also find a way to isolate surfaces. Not sure yet how we're going to fix this (or if it can even be fixed).

[unfa]

Maybe it'd be possible to dilate edges of the reflection buffer before blurring to avoid bleeding in black?

Or alpha blend the unblurred version underneath the blurred one - that could be relatively cheap. It would not fix the issue completely but it'd patch the black edges with unblurred reflection. I guess it would be less visible?

Of course this assumes the SSR buffer has alpha information that can be used for correctly blending the blurred edge without affecting anything else.

[jcostello]

Can AA be applied to SSR?

[Calinou]

When we look at the base of the cube we see a dark line where the cube meets the ground:

#56844 should have fixed this when there's no blur, but maybe it's not done correctly or I didn't remove enough steps.

Can AA be applied to SSR?

No, but SSR can be jittered every frame when TAA is enabled to smooth it out over time. This is what AAA games do 🙂

[jcostello]

No, but SSR can be jittered every frame when TAA is enabled to smooth it out over time. This is what AAA games do slightly_smiling_face

That would be nice to have

[BastiaanOlij]

#56844 should have fixed this when there's no blur, but maybe it's not done correctly or I didn't remove enough steps.

It's the blur step that causes the lines because it blurs with either black from neighboring pixels where there is no reflection, or with the reflection on adjacent faces.

[BastiaanOlij]

Noticed that when you turn off the roughness setting, SSR stopped working as it was using the wrong buffers. Fixed that. This is what it looks like without roughness:

Note that the black dithering is not related to SSR and what I believe causes the rough reflections. Also there is a small bit of separation which I think is a result of rendering SSR at half resolution

Also when changing the SSR settings things tended to crash, dealt with that.

[clayjohn]

Changes look good to me so far!

[BastiaanOlij]

Here is another issue I noticed that we probably need to handle in a separate PR, if its even solvable without a huge performance impact.

Our max steps translates to how many pixels we end up tracing. This means that reflections change by distance.

Camera up close:

Camera moved back:

Notice that up close we have a very short reflection, while with the camera moved back the entire cube is reflected on the floor.
This seems unnatural and especially when the player is moving around is very visible.

This issue becomes even more apparent when looking past a long object, the further away, the longer the reflection:

edit One solution here would be to not have a max steps setting but a max distance, reflections on surface up close will require more steps while reflections further away will require less steps to reach the max distance. Maybe even have both so max steps still becomes a limiter to ensure we don't loose too much performance.

[fracteed]

@BastiaanOlij good to see this getting some attention, thanks for working on it. Is there any way of adding an option to isolate the SSR to just the ground plane or y up normals? In so many cases you only want reflections on floors/water/puddles etc, but end up getting lots of SSR artifacts on walls, small objects etc. where they are not desired. If such an option were possible, it may also be more performant?

[BastiaanOlij]

@fracteed that should be possible as a setting and indeed it would limit the pixels on which we're raymarching the reflection. Should be something for a separate PR though.

[clayjohn]

Here is another issue I noticed that we probably need to handle in a separate PR, if its even solvable without a huge performance impact.

Our max steps translates to how many pixels we end up tracing. This means that reflections change by distance.

The best way to solve this will be to move to Hi-Z tracing. It essentially allows us to trace much further with the same number of samples. Here is a detailed blog post explaining the method https://sugulee.wordpress.com/2021/01/19/screen-space-reflections-implementation-and-optimization-part-2-hi-z-tracing-method/. The original paper is very short and doesn't do a good job outlining the benefits over linear tracing.

AFAIK Hi-Z tracing is the current popular method

[Calinou]

@BastiaanOlij good to see this getting some attention, thanks for working on it. Is there any way of adding an option to isolate the SSR to just the ground plane or y up normals? In so many cases you only want reflections on floors/water/puddles etc, but end up getting lots of SSR artifacts on walls, small objects etc. where they are not desired. If such an option were possible, it may also be more performant?

Using a roughness cutoff can help in this case: #56804

I suppose this is still relevant with Hi-Z tracing too, as it can be used to further improve performance in this case.

[clayjohn]

I suppose this is still relevant with Hi-Z tracing too, as it can be used to further improve performance in this case.

Ya, Hi-z tracing just changes the ray-marching loop. It doesn't impact the other parts. So it is a drop-in replacement for what we are doing now.

[BastiaanOlij]

@clayjohn it looks like its a little more then that as we need to create mipmaps of the depth buffer. But it does look like a really sweet solution.

[clayjohn]

@clayjohn it looks like its a little more then that as we need to create mipmaps of the depth buffer. But it does look like a really sweet solution.

Yes, but that won't be too challenging as we already generate mipmaps of the depth buffer for SSAO and SSIL and we can share them with SSR.

[unfa]

@clayjohn it looks like its a little more then that as we need to create mipmaps of the depth buffer. But it does look like a really sweet solution.

Yes, but that won't be too challenging as we already generate mipmaps of the depth buffer for SSAO and SSIL and we can share them with SSR.

Oh, so we could pretty much get the Hi-Z tracing optimization nearly for free?

[clayjohn]

@clayjohn it looks like its a little more then that as we need to create mipmaps of the depth buffer. But it does look like a really sweet solution.

Yes, but that won't be too challenging as we already generate mipmaps of the depth buffer for SSAO and SSIL and we can share them with SSR.

Oh, so we could pretty much get the Hi-Z tracing optimization nearly for free?

I don't know if I would say that. But there should be very little additional cost, so the performance benefit should far outweigh the cost.

[akien-mga]

Thanks!