D3D12: Use the right state for resources in certain heap types

[RandomShaper]

When enhanced resource barriers are not available, and thus the legacy barriers are used, buffers in an upload heap need to have the D3D12_RESOURCE_STATE_GENERIC_READ status (otherwise, the creation fails and also the debug layer complains about it).

2024-07-01: Also using D3D12_RESOURCE_STATE_COPY_DEST for readback heaps.

Fixes #93670.

[clayjohn]

I am still getting a crash with this PR, but the crash happens later. It seems it crashes when trying to read back data from a texture. I was able to reproduce this just by opening a project in the editor. I'll do more testing to see if I can find more info for you

edit: Crashing when opening https://github.com/RPicster/godot4-demo-desert-light in the editor and the MRP from #93249 (but with a slightly different backtrace

ERROR: Can't create buffer of size: 3904, error 0x80070057.
   at: (drivers\d3d12\rendering_device_driver_d3d12.cpp:922)
ERROR: Condition "!tmp_buffer" is true. Returning: Vector<uint8_t>()
   at: RenderingDevice::texture_get_data (servers\rendering\rendering_device.cpp:1615)
ERROR: Condition "data.is_empty()" is true. Returning: Ref<Image>()
   at: RendererRD::TextureStorage::texture_2d_get (servers\rendering\renderer_rd\storage_rd\texture_storage.cpp:1272)
ERROR: Can't create buffer of size: 3904, error 0x80070057.
   at: (drivers\d3d12\rendering_device_driver_d3d12.cpp:922)
ERROR: Condition "!tmp_buffer" is true. Returning: Vector<uint8_t>()
   at: RenderingDevice::texture_get_data (servers\rendering\rendering_device.cpp:1615)
ERROR: Condition "data.is_empty()" is true. Returning: Ref<Image>()
   at: RendererRD::TextureStorage::texture_2d_get (servers\rendering\renderer_rd\storage_rd\texture_storage.cpp:1272)

================================================================
CrashHandlerException: Program crashed
Engine version: Godot Engine v4.3.beta.custom_build (181e9e1ede994a265de73136b9ae0bcb1b05b66c)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[0] Image::get_size (F:\dev\godot\core\io\image.cpp:453)
[1] Image::get_size (F:\dev\godot\core\io\image.cpp:453)
[2] AnimationPlayerEditor::_notification (F:\dev\godot\editor\plugins\animation_player_editor_plugin.cpp:163)
[3] AnimationPlayerEditor::_notificationv (F:\dev\godot\editor\plugins\animation_player_editor_plugin.h:48)
[4] Object::notification (F:\dev\godot\core\object\object.cpp:873)
[5] Node::_notification (F:\dev\godot\scene\main\node.cpp:126)
[6] Node::_notificationv (F:\dev\godot\scene\main\node.h:50)
[7] CanvasItem::_notificationv (F:\dev\godot\scene\main\canvas_item.h:45)
[8] Control::_notificationv (F:\dev\godot\scene\gui\control.h:48)
[9] Container::_notificationv (F:\dev\godot\scene\gui\container.h:37)
[10] BoxContainer::_notificationv (F:\dev\godot\scene\gui\box_container.h:37)
[11] VBoxContainer::_notificationv (F:\dev\godot\scene\gui\box_container.h:90)
[12] AnimationPlayerEditor::_notificationv (F:\dev\godot\editor\plugins\animation_player_editor_plugin.h:48)
[13] Object::notification (F:\dev\godot\core\object\object.cpp:873)
[14] Node::_propagate_enter_tree (F:\dev\godot\scene\main\node.cpp:294)
[15] Node::_propagate_enter_tree (F:\dev\godot\scene\main\node.cpp:313)
[16] Node::_propagate_enter_tree (F:\dev\godot\scene\main\node.cpp:313)
[17] Node::_propagate_enter_tree (F:\dev\godot\scene\main\node.cpp:313)
[18] Node::_propagate_enter_tree (F:\dev\godot\scene\main\node.cpp:313)
[19] Node::_propagate_enter_tree (F:\dev\godot\scene\main\node.cpp:313)
[20] Node::_propagate_enter_tree (F:\dev\godot\scene\main\node.cpp:313)
[21] Node::_propagate_enter_tree (F:\dev\godot\scene\main\node.cpp:313)
[22] Node::_propagate_enter_tree (F:\dev\godot\scene\main\node.cpp:313)
[23] Node::_propagate_enter_tree (F:\dev\godot\scene\main\node.cpp:313)
[24] Node::_propagate_enter_tree (F:\dev\godot\scene\main\node.cpp:313)
[25] Node::_propagate_enter_tree (F:\dev\godot\scene\main\node.cpp:313)
[26] Node::_set_tree (F:\dev\godot\scene\main\node.cpp:3164)
[27] SceneTree::initialize (F:\dev\godot\scene\main\scene_tree.cpp:449)
[28] OS_Windows::run (F:\dev\godot\platform\windows\os_windows.cpp:1684)
[29] widechar_main (F:\dev\godot\platform\windows\godot_windows.cpp:181)
[30] _main (F:\dev\godot\platform\windows\godot_windows.cpp:206)
[31] main (F:\dev\godot\platform\windows\godot_windows.cpp:220)
[32] WinMain (F:\dev\godot\platform\windows\godot_windows.cpp:234)
[33] __scrt_common_main_seh (D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[34] <couldn't map PC to fn name>
-- END OF BACKTRACE --
================================================================

[DarioSamo]

otherwise, the creation fails and also the debug layer complains about it

It doesn't do it for me either with and without Agility SDK. Is it dependent on the driver? I recall having trouble with this elsewhere on other systems but couldn't find any details on the specification about why COMMON is banned from being used in upload (and state promotion is supposed to take care of it AFAIK). I'm just confused as to why the validation says nothing to me in this case.

Either way I'm not against the change in the PR, just trying to understand the reason behind the problem further.

[RandomShaper]

I found this at https://learn.microsoft.com/en-us/windows/win32/api/d3d12/ne-d3d12-d3d12_heap_type:

D3D12_HEAP_TYPE_UPLOAD
[...]
Resources in this heap must be created with D3D12_RESOURCE_STATE_GENERIC_READ and cannot be changed away from this. The CPU address for such heaps is commonly not efficient for CPU reads.

And also this, I've just found the debug layer complains about in my case as well:

D3D12_HEAP_TYPE_READBACK
[...]
Resources in this heap must be created with D3D12_RESOURCE_STATE_COPY_DEST, and cannot be changed away from this.

I've updated the PR so we meet this latter requirement as well. I no longer get errors about that. @clayjohn, can you confirm this fixes the issue for you, too?

[DarioSamo]

I found this at https://learn.microsoft.com/en-us/windows/win32/api/d3d12/ne-d3d12-d3d12_heap_type:

Seems like a good enough reason to change it then, apologies for the change to common then. I can't help but wonder why the debug layer accepts it on some systems and not in others though. I'll ask if the validation was added at some point.

[RandomShaper]

Seems like a good enough reason to change it then, apologies for the change to common then. I can't help but wonder why the debug layer accepts it on some systems and not in others though. I'll ask if the validation was added at some point.

No need to apologize! We do changes to our best judgement. I'm curious about the validation difference, too.

[clayjohn]

Looks good to me.

I have tested with the original MRP and with Desert Light and neither are crashing now!

[akien-mga]

Thanks!

[TCROC]

I can confirm this PR seems to be fixing the crash we were experiencing in #93670 on our NVIDIA 1070 Windows 10 computer. When we get back home, I'll test on our AMD Radeon Windows 10 computer and see if it is fixed there as well! Thanks everyone and great work! :)

[TCROC]

Can confirm the PR fixes both AMD and NVIDIA issues on Windows 10 :)