Add ability to display depth buffer in editor 3d preview. #67735

Joxno · 2022-10-22T00:39:34Z

This pull request implements the ability to quickly display the depth buffer in editor 3d preview and fixes a bug in the copy.glsl shader related to how linear depth is calculated.

clayjohn

Good idea! I think this will be nice addition and it makes sense seeing as how we already have a display mode for the normal buffer.

One thing I am unsure about is whether we actually need to invert the depth. I left some comments on implementation assuming we keep the inverted depth code, but to me it seems like it may make more sense for the debug mode to show users something closer to the actual value instead of inverting the range. Did you do it this way because non-inverted depth was too difficult to read?

clayjohn · 2022-10-22T00:44:08Z

servers/rendering/renderer_rd/shaders/effects/copy.glsl

-	depth = depth * 2.0 - 1.0;
-	depth = 2.0 * params.camera_z_near * params.camera_z_far / (params.camera_z_far + params.camera_z_near - depth * (params.camera_z_far - params.camera_z_near));
+
+	depth = params.camera_z_near * params.camera_z_far /
+			(params.camera_z_far + depth * (params.camera_z_near - params.camera_z_far));


Could you explain in more detail what is wrong with this way of linearizing depth? This is the same method used throughout the engine, so if it changes here it should change there as well

I think I was a bit quick in editing that shader, I've restored line 248 in a commit I'll be pushing.
In terms of line 247, if I have understood the maths correctly that sort of conversion should not be needed in Vulkan compared to OpenGL, is it correct that it is no longer needed?

No, it is still needed. We aren't dealing with NDC values here, we are dealing with the contents of the depth buffer which are still values in the 0-1 range regardless of using Vulkan or OpenGL.

If you look at other parts of the codebase, they all use the same function to convert depth buffer depth into linear depth. I would stick with using the same code here as elsewhere so that everything matches.

servers/rendering/renderer_rd/shaders/effects/copy.glsl

servers/rendering/renderer_rd/effects/copy_effects.cpp

RedMser · 2022-10-22T15:19:32Z

@Joxno You need to run godot --doctool to generate the XML documentation for the newly added enum entries.

Joxno · 2022-10-22T20:40:16Z

Good idea! I think this will be nice addition and it makes sense seeing as how we already have a display mode for the normal buffer.

One thing I am unsure about is whether we actually need to invert the depth. I left some comments on implementation assuming we keep the inverted depth code, but to me it seems like it may make more sense for the debug mode to show users something closer to the actual value instead of inverting the range. Did you do it this way because non-inverted depth was too difficult to read?

Thanks!
Yeah, I can understand the argument to display closer to the truth values for debugging purposes although I think without an inversion it makes the visualisation harder to read.

Here's an example if you do not invert the values:

Obviously this is a trivial scene with an infinite background but I think in a more complex scene objects that occupy the middle-ground would be washed out. Possibly making it difficult to debug depth issues.

Maybe having 2 alternatives to choose from, one that displays the values as is, and a second one that is inverted could be a possibility?
"Depth Buffer"
"Depth Buffer (Inv)"

It could also be that I am biased to dark mode haha.

clayjohn · 2022-11-11T19:25:55Z

doc/classes/Viewport.xml

@@ -436,54 +436,56 @@
 		</constant>
 		<constant name="DEBUG_DRAW_NORMAL_BUFFER" value="5" enum="DebugDraw">
 		</constant>
-		<constant name="DEBUG_DRAW_VOXEL_GI_ALBEDO" value="6" enum="DebugDraw">
+		<constant name="DEBUG_DRAW_DEPTH_BUFFER" value="6" enum="DebugDraw">


This should be filled in

clayjohn · 2022-11-11T19:27:05Z

servers/rendering/renderer_rd/effects/copy_effects.h

+		// Depth.
+		bool inverse_depth;
+		uint32_t pad3[3];


This should replace one of the pads above (I would replace pad) There is no reason to introduce 3 new pads when there is already so much padding

clayjohn · 2022-11-11T19:36:29Z

servers/rendering/renderer_rd/shaders/effects/copy.glsl

-	depth = depth * 2.0 - 1.0;
-	depth = 2.0 * params.camera_z_near * params.camera_z_far / (params.camera_z_far + params.camera_z_near - depth * (params.camera_z_far - params.camera_z_near));
+
+	depth = params.camera_z_near * params.camera_z_far /
+			(params.camera_z_far + depth * (params.camera_z_near - params.camera_z_far));


No, it is still needed. We aren't dealing with NDC values here, we are dealing with the contents of the depth buffer which are still values in the 0-1 range regardless of using Vulkan or OpenGL.

If you look at other parts of the codebase, they all use the same function to convert depth buffer depth into linear depth. I would stick with using the same code here as elsewhere so that everything matches.

clayjohn · 2022-11-24T16:51:55Z

Discussed in the PR meeting today. Consensus was that this looks mostly good, but both non-inverse and inverse result in far away objects being difficult to see. Lawnjelly helpfully suggested (and the other contributors agreed) that we should use the non-inverted linear depth, but we should set the far plane to a fixed color so that far objects contrast well against it.

I am thinking a dark blue colour from this page would be good https://davidmathlogic.com/colorblind/#%23D81B60-%231E88E5-%23FFC107-%23004D40. There are a few colours suggested here that have good visibility for many different types of colourblindness.

Calinou · 2023-08-01T16:53:06Z

@Joxno Could you look into applying clayjohn's suggestions above? The PR should be good to merge afterwards.

If you don't have time, let us know and we'll mark this as salvageable for another contributor to pick up.

…ersing depth values.

Changed mode to push constant instead after feedback. Changed method of separate copy function to boolean flag instead after feedback.

doc/classes/RenderingServer.xml

doc/classes/Viewport.xml

servers/rendering/renderer_rd/renderer_scene_render_rd.cpp

…alculations. Added background colour at max depth. Corrected and moved documentation.

Calinou · 2024-02-14T21:23:26Z

Tested locally (rebased on top of master 907db8e), there seems to be a mismatch between the UI in the Perspective menu and the actual debug draw mode you get:

simplescreenrecorder-2024-02-14_22.21.50.mp4

Testing project: test_shadowmask.zip

Notice how selecting Disable Mesh LOD shows directional shadow splits instead, for instance.

Joxno · 2024-02-14T23:26:42Z

Ah, I made a complete blunder when I reordered the debug_draw_modes array but forgot to take into account the order in display_options as well. Should be resolved in the latest commit.

Calinou

Tested locally, it works as expected in Forward+. I can't get this debug draw mode to show up when using the Mobile rendering method though, even though it should be able to work according to what I see in the diff.
It would be nice to get it working in Compatibility too, but this can be left for a future PR.

I noticed there's quite a lot of banding in the debug view:

Even with camera near set to 1.0 and far set to 25.0 in the editor view settings, I still get a fair amount of banding:

It appears the stepping is actually correct (there are no shades of gray "missed" between the bands), but since the depth is displayed in a non-linear manner, the banding in near-black shades gets pretty obvious. This may be less noticeable depending on viewing conditions or the display you're using (I'm using a LG C2 42" at 120 nits brightness in SDR).

PS: This PR will likely need changes if #88328 is merged before this one.

…ction values.

Joxno requested review from a team as code owners October 22, 2022 00:39

clayjohn added enhancement topic:rendering topic:editor labels Oct 22, 2022

clayjohn added this to the 4.0 milestone Oct 22, 2022

clayjohn reviewed Oct 22, 2022

View reviewed changes

Calinou added the topic:3d label Oct 22, 2022

Joxno requested a review from a team as a code owner October 22, 2022 23:15

clayjohn added the for pr meeting label Oct 22, 2022

clayjohn reviewed Nov 11, 2022

View reviewed changes

YuriSizov modified the milestones: 4.0, 4.1 Feb 10, 2023

clayjohn modified the milestones: 4.1, 4.2 Jun 9, 2023

clayjohn modified the milestones: 4.2, 4.x Oct 25, 2023

Joxno force-pushed the contribute branch from 56b1ab4 to fbe1d51 Compare February 14, 2024 10:02

Joxno added 3 commits February 14, 2024 10:28

Fixed bug with incorrect linear depth calculation. Added mode for rev…

9a6ea8b

…ersing depth values.

Implemented ability to display depth buffer in editor 3d preview.

fb21c6c

Restored linear depth calculation

4d8d87d

Changed mode to push constant instead after feedback. Changed method of separate copy function to boolean flag instead after feedback.

Joxno force-pushed the contribute branch from f299b90 to 4d8d87d Compare February 14, 2024 10:28

AThousandShips reviewed Feb 14, 2024

View reviewed changes

doc/classes/RenderingServer.xml Outdated Show resolved Hide resolved

AThousandShips reviewed Feb 14, 2024

View reviewed changes

doc/classes/Viewport.xml Show resolved Hide resolved

AThousandShips reviewed Feb 14, 2024

View reviewed changes

servers/rendering/renderer_rd/renderer_scene_render_rd.cpp Outdated Show resolved Hide resolved

Joxno added 3 commits February 14, 2024 16:01

Removed inverse depth functionality. Fixed issues with depth linear c…

32f0140

…alculations. Added background colour at max depth. Corrected and moved documentation.

Fixed style issue in copy shader.

e51929c

Fixed issue with debug depth viewport enum binding having wrong order.

3a87a76

Fixed issue with wrong order in display_options array.

7a7e162

Calinou reviewed Feb 14, 2024

View reviewed changes

Joxno added 2 commits February 15, 2024 08:50

Implemented stubbed mobile function for retrieving depth texture.

a0420f8

Changed the hardcoded zNear, zFar values to actual scene camera proje…

21f2441

…ction values.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ability to display depth buffer in editor 3d preview. #67735

Add ability to display depth buffer in editor 3d preview. #67735

Joxno commented Oct 22, 2022

clayjohn left a comment

clayjohn Oct 22, 2022

Joxno Oct 22, 2022

clayjohn Nov 11, 2022

RedMser commented Oct 22, 2022

Joxno commented Oct 22, 2022

clayjohn Nov 11, 2022

clayjohn Nov 11, 2022

clayjohn Nov 11, 2022

clayjohn commented Nov 24, 2022

Calinou commented Aug 1, 2023 •

edited

Loading

Calinou commented Feb 14, 2024 •

edited

Loading

Joxno commented Feb 14, 2024

Calinou left a comment •

edited

Loading

Add ability to display depth buffer in editor 3d preview. #67735

Are you sure you want to change the base?

Add ability to display depth buffer in editor 3d preview. #67735

Conversation

Joxno commented Oct 22, 2022

clayjohn left a comment

Choose a reason for hiding this comment

clayjohn Oct 22, 2022

Choose a reason for hiding this comment

Joxno Oct 22, 2022

Choose a reason for hiding this comment

clayjohn Nov 11, 2022

Choose a reason for hiding this comment

RedMser commented Oct 22, 2022

Joxno commented Oct 22, 2022

clayjohn Nov 11, 2022

Choose a reason for hiding this comment

clayjohn Nov 11, 2022

Choose a reason for hiding this comment

clayjohn Nov 11, 2022

Choose a reason for hiding this comment

clayjohn commented Nov 24, 2022

Calinou commented Aug 1, 2023 • edited Loading

Calinou commented Feb 14, 2024 • edited Loading

Joxno commented Feb 14, 2024

Calinou left a comment • edited Loading

Choose a reason for hiding this comment

Calinou commented Aug 1, 2023 •

edited

Loading

Calinou commented Feb 14, 2024 •

edited

Loading

Calinou left a comment •

edited

Loading