Duplicated Multimesh breaks when TAA is active #67287
Comments
|
The relation between multimesh and TAA sounds very surprising. |
|
I had this issue that generated this report during import, everytime I tried to re-import the gltfs with the MRP post-import script to generate multimeshes, only when disabling TAA it imports ok, where as if TAA was enabled the transforms of the multimeshs were all messed up, see gif below.
The last multimesh has distorted transforms, the weird part is if I keep re-importing the distortions keep changing, and sometimes it doesn't even generate the transforms for the multimeshes, the multimesh buffer array keeps everything as 0.0, and no errors popped up, I am also not running any toolscripts. |
|
Those messed up transforms happen without TAA as well, in my project they are stretched in the editor and generate an error every frame:
However, the addon
|
|
I can confirm this on 4.1.dev 31eccb5 (Linux, GeForce RTX 4090 with NVIDIA 525.89.02). TAA disabled
TAA enabled
This is spammed in the console when running the project: |
|
Godot v4.0 rc6 win64: E 0:00:50:0746 draw_list_bind_uniform_set: Condition "!uniform_set" is true.<Source Code C++>drivers/vulkan/rendering_device_vulkan.cpp:7199 @ draw_list_bind_uniform_set() One MultiMeshInstance3D doesn't appear drawn on the scene, but two different MultiMeshInstance3D are drawn fine. |
|
On windows on recent master with TAA enabled and shared MultiMeshes I get the same uniform set error spam. When I set the MultiMesh resources to Note that everything including the rendering is set to run singlethreaded in the ProjectSettings.
|
|
It seems the link behind TAA breaking this and the Multimesh error spam is this particular bit of code. Particularly enabling the use of motion vectors on the color pass. if (p_color_pass_flags & COLOR_PASS_FLAG_MOTION_VECTORS) {
if ((flags & (INSTANCE_DATA_FLAG_MULTIMESH | INSTANCE_DATA_FLAG_PARTICLES)) == INSTANCE_DATA_FLAG_MULTIMESH && RendererRD::MeshStorage::get_singleton()->_multimesh_enable_motion_vectors(inst->data->base)) {
inst->transforms_uniform_set = mesh_storage->multimesh_get_3d_uniform_set(inst->data->base, scene_shader.default_shader_rd, TRANSFORMS_UNIFORM_SET);
}
}The instance already has a valid Removing this code in particular makes the error go away, but I'm not sure what the implications are or if it breaks anything else. @JFonS is shown as the last author of that particular bit of code along with a few commit messages explaining the reasoning behind it. Maybe it's worth taking a second look at this fix in this particular issue as to why it's replacing a valid uniform set and causing the rendering error? |
|
After further investigation I'd have to say there's a bit of a deeper problem in how multimesh handles the update of the buffer that makes this quite hard to fix. My understanding of the problem so far is:
However, when it comes to where this is handled, it seems the renderer itself will forcefully update the mesh storage to duplicate the buffer size, and therefore invalidating the previous buffer. This poses a few problems in the case of duplicated instances, as only the first instance where this happens will determine it's necessary to create a new uniform and the rest will have the old resource bound. This is why the duplicated versions of the multimesh fail to bind the uniform correctly the first time. A first temporary solution would be to track that this is necessary for all instances that invalidate the multimesh buffer and re-recreate them, fixing this particular bug (which would require creating some sort of invalidation set during the render list creation, which seems ugly). Another one could be to move this validation to I'll attempt the temporary fix, but there's some things that don't sit right with me with the current solution as it seems a bit messy for the renderer to be able to change the mesh storage instead of just requesting a separate buffer. EDIT: |
Fixes godotengine#67287. There was a subtle error where due to how enabling motion vectors for multi-meshes was handled, only the first instance would have a valid transforms buffer and the rest would point to an invalid buffer. This change moves over the responsibility of enabling motion vectors only when changes happen to the individual 3D transforms or the entire buffer itself. It also fixes an unnecessary download of the existing buffer that'd get overwritten by the current cache if it exists. Another fix is handling the case where the buffer was not set, and enabling motion vectors would not cause the buffer to be recreated correctly.
Fixes godotengine#67287. There was a subtle error where due to how enabling motion vectors for multi-meshes was handled, only the first instance would have a valid transforms buffer and the rest would point to an invalid buffer. This change moves over the responsibility of enabling motion vectors only when changes happen to the individual 3D transforms or the entire buffer itself. It also fixes an unnecessary download of the existing buffer that'd get overwritten by the current cache if it exists. Another fix is handling the case where the buffer was not set, and enabling motion vectors would not cause the buffer to be recreated correctly.
Fixes godotengine#67287. There was a subtle error where due to how enabling motion vectors for multi-meshes was handled, only the first instance would have a valid transforms buffer and the rest would point to an invalid buffer. This change moves over the responsibility of enabling motion vectors only when changes happen to the individual 3D transforms or the entire buffer itself. It also fixes an unnecessary download of the existing buffer that'd get overwritten by the current cache if it exists. Another fix is handling the case where the buffer was not set, and enabling motion vectors would not cause the buffer to be recreated correctly.
Fixes godotengine#67287. There was a subtle error where due to how enabling motion vectors for multi-meshes was handled, only the first instance would have a valid transforms buffer and the rest would point to an invalid buffer. This change moves over the responsibility of enabling motion vectors only when changes happen to the individual 3D transforms or the entire buffer itself. It also fixes an unnecessary download of the existing buffer that'd get overwritten by the current cache if it exists. Another fix is handling the case where the buffer was not set, and enabling motion vectors would not cause the buffer to be recreated correctly.
Fixes godotengine#67287. There was a subtle error where due to how enabling motion vectors for multi-meshes was handled, only the first instance would have a valid transforms buffer and the rest would point to an invalid buffer. This change moves over the responsibility of enabling motion vectors only when changes happen to the individual 3D transforms or the entire buffer itself. It also fixes an unnecessary download of the existing buffer that'd get overwritten by the current cache if it exists. Another fix is handling the case where the buffer was not set, and enabling motion vectors would not cause the buffer to be recreated correctly.
Fixes godotengine#67287. There was a subtle error where due to how enabling motion vectors for multi-meshes was handled, only the first instance would have a valid transforms buffer and the rest would point to an invalid buffer. This change moves over the responsibility of enabling motion vectors only when changes happen to the individual 3D transforms or the entire buffer itself. It also fixes an unnecessary download of the existing buffer that'd get overwritten by the current cache if it exists. Another fix is handling the case where the buffer was not set, and enabling motion vectors would not cause the buffer to be recreated correctly.
Fixes godotengine#67287. There was a subtle error where due to how enabling motion vectors for multi-meshes was handled, only the first instance would have a valid transforms buffer and the rest would point to an invalid buffer. This change moves over the responsibility of enabling motion vectors only when changes happen to the individual 3D transforms or the entire buffer itself. It also fixes an unnecessary download of the existing buffer that'd get overwritten by the current cache if it exists. Another fix is handling the case where the buffer was not set, and enabling motion vectors would not cause the buffer to be recreated correctly. (cherry picked from commit 5155870)
Fixes godotengine#67287. There was a subtle error where due to how enabling motion vectors for multi-meshes was handled, only the first instance would have a valid transforms buffer and the rest would point to an invalid buffer. This change moves over the responsibility of enabling motion vectors only when changes happen to the individual 3D transforms or the entire buffer itself. It also fixes an unnecessary download of the existing buffer that'd get overwritten by the current cache if it exists. Another fix is handling the case where the buffer was not set, and enabling motion vectors would not cause the buffer to be recreated correctly.
Godot version
v4.0.beta2.official [f8745f2]
System information
Windows 10, Vulkan, Nvidia Geforce GTX 1050 TI
Issue description
It seems that TAA breaks when duplicated multimeshs are visible, see the MRP, if only one instance of the multimesh is visible it works properly, however when TAA is active and the multimeshs duplicated are visible, reload the project to continue testing,
Steps to reproduce
The MRP was created with the follow steps:
1 - Import multiple meshes as a multimesh using a post-import script, (included in MRP)
2 - Duplicate the post-imported gltf in the scene now as multimesh,
3 - Enable TAA in Antialias render in project settings.
To test the issue with the MRP, just open the MRP and activate the TAA in the project settings, should spam with errors the output window.
Minimal reproduction project
Multimesh Flicker Godot4 Beta2.zip
The text was updated successfully, but these errors were encountered: