Rewrote how barriers work for faster rendering #45672

reduz · 2021-02-02T20:10:48Z

-Added more finegrained control in RenderingDevice API
-Optimized barriers (use less ones for thee same)
-General optimizations
-Shadows render all together unbarriered
-GI can render together with shadows.
-SDFGI can render together with depth-preoass.
-General fixes

Xartorx · 2021-02-03T00:40:35Z

Tried out Windows artifact from Github Actions, editor crashes when removing material from MeshInstance.

Stacktrace:

CrasERROR: FATAL: Index p_index = 3 is out of bounds (count = 3).
hHan at: LocalVector<struct RendererSceneRenderForward::GeometryInstanceSurfaceDataCache *,unsigned int,0>::operator [] (D:\a\godot\godot\core/templates/local_vector.h:164)
dlerException: Program crashed
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[0] oidnUnmapBuffer
[1] oidnUnmapBuffer
[2] oidnUnmapBuffer
[3] oidnUnmapBuffer
[4] oidnUnmapBuffer
[5] oidnUnmapBuffer
[6] oidnUnmapBuffer
[7] oidnUnmapBuffer
[8] oidnUnmapBuffer
[9] oidnUnmapBuffer
[10] oidnUnmapBuffer
[11] oidnUnmapBuffer
[12] oidnUnmapBuffer
[13] <couldn't map PC to fn name>
[14] <couldn't map PC to fn name>
[15] <couldn't map PC to fn name>
[16] <couldn't map PC to fn name>
[17] <couldn't map PC to fn name>
[18] oidnUnmapBuffer
[19] BaseThreadInitThunk
-- END OF BACKTRACE --

Specs: Windows 10 / Nvidia GeForce GTX 1080 Ti (457.30)

clayjohn

I did a brief review, but didn't get through renderer_scene_render_rd. I will take another look tomorrow night.

Do we still need to do a pass over the rest of the effects in effects_rd.cpp to optimize barrier usage?

drivers/vulkan/rendering_device_vulkan.cpp

servers/rendering/renderer_rd/shaders/shadow_reduce.glsl

JFonS · 2021-02-03T15:03:30Z

servers/rendering/renderer_rd/renderer_scene_render_forward.cpp

+	_fill_render_list(RENDER_LIST_SECONDARY, p_instances, pass_mode, p_projection, p_transform, false, false, p_camera_plane, p_lod_distance_multiplier, p_screen_lod_threshold, true);
+	uint32_t render_list_size = render_list[RENDER_LIST_SECONDARY].elements.size() - render_list_from;
+	render_list[RENDER_LIST_SECONDARY].sort_by_key_range(render_list_from, render_list_size);
+	_fill_instance_data(RENDER_LIST_SECONDARY, render_list_from, render_list_size, false);


While testing, I got a crash in RendererSceneRenderForward::_fill_instance_data() because it was being called from here with render_list_size = 0.

My understanding is that _fill_render_list() did not add any elements for some reason. I got the crash by changing the scale of a MeshInstance in the inspector, but I could only reproduce this once.

@reduz Do you think we should just guard against this possibility? Or we should look into why _fill_render_list() didn't add any elements to the render list?

It should not be a problem, I tested with zero elements and seems to work ok, if you can figure out how to reproduce it again that would be great, else I guess we will eventually rely on user reports to track this down.

To be more precise, it crashed at renderer_scene_render_forward.cpp line 1300.

total_elements was 10, the size of instance_data was also 10, and p_offset was 10 as well. This made me think this line:
uint32_t element_total = p_max_elements > 0 ? p_max_elements : rl->elements.size();
may be incorrect, because in this case there were 0 elements in total.

Maybe the check needs to be p_max_elements >= 0 and the default argument value should be -1?

Look like it was the correct fix, then? :)

qarmin · 2021-02-03T19:07:29Z

When running https://github.com/qarmin/RegressionTestProject/archive/4.0.zip
I have crash

Changed scene to res://MainScenes/Node3D.tscn
servers/rendering/renderer_rd/renderer_scene_render_forward.cpp:1830:118: runtime error: member access within null pointer of type 'struct RenderBufferDataForward'
handle_crash: Program crashed with signal 11
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[1] /mnt/Miecz/mojgodot/bin/godot.linuxbsd.tools.64s() [0x1e49860] (/mnt/Miecz/mojgodot/platform/linuxbsd/crash_handler_linuxbsd.cpp:54)
[2] /lib/x86_64-linux-gnu/libc.so.6(+0x46210) [0x7f9ad6162210] (??:0)
[3] RendererSceneRenderForward::_render_scene(RID, Transform const&, CameraMatrix const&, bool, PagedArray<RendererSceneRender::GeometryInstance*> const&, PagedArray<RID> const&, PagedArray<RID> const&, RID, RID, unsigned int, unsigned int, RID, RID, RID, RID, int, Color const&, float) (/mnt/Miecz/mojgodot/servers/rendering/renderer_rd/renderer_scene_render_forward.cpp:1830)
[4] RendererSceneRenderRD::render_scene(RID, Transform const&, CameraMatrix const&, bool, PagedArray<RendererSceneRender::GeometryInstance*> const&, PagedArray<RID> const&, PagedArray<RID> const&, PagedArray<RID> const&, PagedArray<RID> const&, PagedArray<RID> const&, RID, RID, RID, RID, RID, int, float, RendererSceneRender::RenderShadowData const*, int, RendererSceneRender::RenderSDFGIData const*, int, RendererSceneRender::RenderSDFGIUpdateData const*) (/mnt/Miecz/mojgodot/servers/rendering/renderer_rd/renderer_scene_render_rd.cpp:7628 (discriminator 3))
[5] RendererSceneCull::_render_scene(Transform, CameraMatrix const&, bool, bool, RID, RID, RID, unsigned int, RID, RID, RID, int, float, bool) (/mnt/Miecz/mojgodot/servers/rendering/renderer_scene_cull.cpp:2782 (discriminator 4))
[6] RendererSceneCull::_render_reflection_probe_step(RendererSceneCull::Instance*, int) (/mnt/Miecz/mojgodot/servers/rendering/renderer_scene_cull.cpp:2892 (discriminator 1))
[7] RendererSceneCull::render_probes() (/mnt/Miecz/mojgodot/servers/rendering/renderer_scene_cull.cpp:2920)
[8] RenderingServerDefault::draw(bool, double) (/mnt/Miecz/mojgodot/servers/rendering/rendering_server_default.cpp:114)
[9] RenderingServerWrapMT::draw(bool, double) (/mnt/Miecz/mojgodot/servers/rendering/rendering_server_wrap_mt.cpp:93)
[10] Main::iteration() (/mnt/Miecz/mojgodot/main/main.cpp:2500)
[11] OS_LinuxBSD::run() (/mnt/Miecz/mojgodot/platform/linuxbsd/os_linuxbsd.cpp:261)
[12] /mnt/Miecz/mojgodot/bin/godot.linuxbsd.tools.64s(main+0x40b) [0x1e47521] (/mnt/Miecz/mojgodot/platform/linuxbsd/godot_linuxbsd.cpp:60)
[13] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3) [0x7f9ad61430b3] (??:0)
[14] /mnt/Miecz/mojgodot/bin/godot.linuxbsd.tools.64s(_start+0x2e) [0x1e4705e] (??:?)
-- END OF BACKTRACE --
Aborted (core dumped)

Time can be adjusted by adding at the end argument with time in seconds

godot4 20 # Will run project 20 seconds

https://github.com/reduz/godot/blob/388b576b1f6e8399e57de6b628eb399060b93b1a/servers/rendering/renderer_rd/renderer_scene_render_forward.cpp#L1830

reduz · 2021-02-03T19:43:55Z

@qarmin you are my hero, as always

reduz · 2021-02-03T20:04:28Z

Ok, if no further crashes found, we can merge.

servers/rendering/renderer_rd/shaders/shadow_reduce.glsl

LiveTrower · 2021-02-03T23:46:36Z

I had a crash
Specs: Windows 10 pro Nvidia GTX 960

Sorry for not having it in text
I found a weird way to reproduce it:

First create a root node "Node3D".
Create a MeshInstance3D with any geometry.
Disable origin.
And finally create a directional light and activate the shadows.
This should cause the crash.

volzhs · 2021-02-04T01:01:44Z

I had a crash
Specs: Windows 10 pro Nvidia GTX 960

Sorry for not having it in text
I found a weird way to reproduce it:

First create a root node "Node3D".

Create a MeshInstance3D with any geometry.

Disable origin.

And finally create a directional light and activate the shadows.
This should cause the crash.

I got text extracted from image.

CrashHandlerException: Program crashed
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[0] LocalVector<RendererScene RenderForward::GeometryInstanceSurfaceDataCache *,unsigned int,0)::operator[] (core\templates\local_vector.h:164)
[1] RendererScene RenderForward::_fill_instance_data (servers\rendering\renderer_rd\renderer_scene_render_forward.cpp:1300)
[2] RendererScene RenderForward::render-shadow_append (servers rendering\renderer_rd renderer_scene_render_forward.cpp:2007)
[3] RendererScene RenderRD::_render_shadow_pass (servers\rendering\renderer_rd\renderer_scene_render_rd.cpp:7850)
[4] RendererScene RenderRD::_pre_opaque_render (servers\rendering\renderer_rd\renderer_scene_render_rd.cpp:7487)
[5] RendererScene RenderForward::_render_scene (servers\rendering\renderer_rd\renderer_scene_render_forward.cpp:1836)
[6] RendererScene RenderRD::render_scene (servers\rendering\renderer_rd\renderer-scene_render_rd.cpp:7651)
[7] RendererSceneCull::_render_scene (servers\rendering\renderer_scene_cull.cpp:2784)
[8] RendererScene Cull::render_camera (servers\rendering\renderer_scene_cull.cpp:2164)
[9] RendererViewport::_draw_3d (servers\rendering\renderer_viewport.cpp:88)
[10] RendererViewport::_draw_viewport (servers\rendering\renderer_viewport.cpp:136)
[11] RendererViewport::draw_viewports (servers\rendering\renderer_viewport.cpp:562)
[12] RenderingServer Default::draw (servers\rendering\rendering_server_default.cpp:115)
[13] RenderingServerWrapMT::draw (servers\rendering\rendering_server_wrap_mt.cpp:93)
[14] Main::iteration (main main.cpp:2496)
[15] Os_Windows::run (platform\windows\os_windows.cpp:623)
[16] widechar_main (platform\Windows\godot_windows.cpp:163)
[17] _main (platform windows\godot_windows.cpp:185)
[18] main (platform windows\godot_windows.cpp:199)
[19] _scrt_common_main_seh (d:\agent\_work\63\s\src\uctools\crt\vcstartup\src\startup\exe_common.in1:288)
[20] Base Thread Init Thunk
END OF BACKTRACE

clayjohn · 2021-02-04T04:47:40Z

Rendering in Sponza is completely broken. It appears to be related to the auto instancing code:

How it looks:

After adding another mesh what is visible changes:

clayjohn · 2021-02-04T04:50:04Z

I had a crash
Specs: Windows 10 pro Nvidia GTX 960

Sorry for not having it in text
I found a weird way to reproduce it:

First create a root node "Node3D".

Create a MeshInstance3D with any geometry.

Disable origin.

And finally create a directional light and activate the shadows.
This should cause the crash.

I was able to reproduce this

clayjohn · 2021-02-04T04:51:25Z

drivers/vulkan/rendering_device_vulkan.cpp

+		dst_stage_mask = VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT;
+	}
+
+	_buffer_memory_barrier(buffer->buffer, p_offset, p_size, VK_PIPELINE_STAGE_TRANSFER_BIT, dst_stage_mask, VK_ACCESS_TRANSFER_WRITE_BIT, dst_access, dst_stage_mask);


Shouldn't there be a check if p_post_barrier != RD::BARRIER_MASK_NO_BARRIER

reduz · 2021-02-04T11:47:04Z

@clayjohn I really can't reproduce the crash, so any help with the debugger I would greatly appreciate

-Added more finegrained control in RenderingDevice API -Optimized barriers (use less ones for thee same) -General optimizations -Shadows render all together unbarriered -GI can render together with shadows. -SDFGI can render together with depth-preoass. -General fixes -Added GPU detection

Xartorx · 2021-02-04T12:43:25Z

Tried out Windows artifact from Github Actions, editor crashes when removing material from MeshInstance.
Stacktrace:

Specs: Windows 10 / Nvidia GeForce GTX 1080 Ti (457.30)

Here's project to test

Video of crash:

0001-0671.mp4

reduz · 2021-02-04T12:48:01Z

OK, crash and broken Sponza should be both fixed.

Xartorx · 2021-02-04T13:13:58Z

OK, crash and broken Sponza should be both fixed.

Can confirm - crashes are gone 👍

clayjohn

Lets merge now that crashes are gone. I can confirm Sponza is fixed. I'm sure there are other bugs that we haven't uncovered yet. But I think it is ready to merge so you can move on.

akien-mga · 2021-02-04T16:19:38Z

Thanks!

Calinou added enhancement topic:core topic:rendering labels Feb 2, 2021

Calinou added this to the 4.0 milestone Feb 2, 2021

reduz force-pushed the barrier-optimization branch 2 times, most recently from 8d25277 to 846a049 Compare February 3, 2021 00:00

clayjohn reviewed Feb 3, 2021

View reviewed changes

reduz force-pushed the barrier-optimization branch from 846a049 to 388b576 Compare February 3, 2021 13:14

JFonS reviewed Feb 3, 2021

View reviewed changes

reduz force-pushed the barrier-optimization branch from 388b576 to 0b30f19 Compare February 3, 2021 19:45

clayjohn reviewed Feb 3, 2021

View reviewed changes

servers/rendering/renderer_rd/shaders/shadow_reduce.glsl Show resolved Hide resolved

reduz force-pushed the barrier-optimization branch from 0b30f19 to af2dbc5 Compare February 3, 2021 22:36

clayjohn reviewed Feb 4, 2021

View reviewed changes

reduz force-pushed the barrier-optimization branch from af2dbc5 to f20999f Compare February 4, 2021 12:43

clayjohn approved these changes Feb 4, 2021

View reviewed changes

akien-mga merged commit 2ba66c1 into godotengine:master Feb 4, 2021

madmiraal mentioned this pull request Feb 6, 2021

Make precision of time_step consistently float. #41853

Closed

This was referenced Apr 18, 2021

Test Godot with Vulkan in CI #47414

Merged

Revert "Test Godot with Vulkan in CI" #48024

Closed

follower mentioned this pull request Oct 5, 2021

Crash (segfault) when opening/creating project due to Vulkan debug utils [Mesa Haswell] #51955

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrote how barriers work for faster rendering #45672

Rewrote how barriers work for faster rendering #45672

reduz commented Feb 2, 2021

Xartorx commented Feb 3, 2021

clayjohn left a comment

JFonS Feb 3, 2021

reduz Feb 3, 2021

JFonS Feb 3, 2021

JFonS Feb 4, 2021

qarmin commented Feb 3, 2021

reduz commented Feb 3, 2021

reduz commented Feb 3, 2021

LiveTrower commented Feb 3, 2021

volzhs commented Feb 4, 2021

clayjohn commented Feb 4, 2021

clayjohn commented Feb 4, 2021

clayjohn Feb 4, 2021

reduz commented Feb 4, 2021

Xartorx commented Feb 4, 2021

reduz commented Feb 4, 2021

Xartorx commented Feb 4, 2021

clayjohn left a comment

akien-mga commented Feb 4, 2021

Rewrote how barriers work for faster rendering #45672

Rewrote how barriers work for faster rendering #45672

Conversation

reduz commented Feb 2, 2021

Xartorx commented Feb 3, 2021

clayjohn left a comment

Choose a reason for hiding this comment

JFonS Feb 3, 2021

Choose a reason for hiding this comment

reduz Feb 3, 2021

Choose a reason for hiding this comment

JFonS Feb 3, 2021

Choose a reason for hiding this comment

JFonS Feb 4, 2021

Choose a reason for hiding this comment

qarmin commented Feb 3, 2021

reduz commented Feb 3, 2021

reduz commented Feb 3, 2021

LiveTrower commented Feb 3, 2021

volzhs commented Feb 4, 2021

clayjohn commented Feb 4, 2021

clayjohn commented Feb 4, 2021

clayjohn Feb 4, 2021

Choose a reason for hiding this comment

reduz commented Feb 4, 2021

Xartorx commented Feb 4, 2021

reduz commented Feb 4, 2021

Xartorx commented Feb 4, 2021

clayjohn left a comment

Choose a reason for hiding this comment

akien-mga commented Feb 4, 2021