Tests: Fixes for backend tests when using Vulkan #9031
Conversation
- Make sure the defaultRenderTarget is created before calling makeCurrent. - Add some missing TextureUsage flags to textures - Add a finish to the RenderFrame destructor, so it guarantess that the GPU work is completed, prevents some synchronization issues.
filament/backend/test/Lifetimes.cpp
Outdated
| @@ -25,6 +25,7 @@ RenderFrame::RenderFrame(filament::backend::DriverApi& api): mApi(api) { | |||
|
|
|||
| RenderFrame::~RenderFrame() { | |||
| mApi.endFrame(0); | |||
| mApi.finish(); | |||
There was a problem hiding this comment.
Can you elaborate on the synchronization issues you were experiencing? Was it only with the Vulkan backend?
There was a problem hiding this comment.
I can only saw it with the vulkan backend (the only one I tested) and it was just the report of the validation layers, the source of this is coming when doing readPixels which is not synchronized properly with vulkan, cant comment on metal.
So I just made the safe approach to make sure the work is completed before trying to read the data, which in the case of tests should be fine.
There was a problem hiding this comment.
We can't "fix the tests" to workaround bugs in the backends -- that's the whole point of having tests. readPixels should work without having to call finish() and it should work from within a renderpass. If the vk backend needs fixing, I prefer we file a buganizer, prioritize and fix rather an working around in the tests.
There was a problem hiding this comment.
I removed the finish calls.
Will create a bug for the readPixels synchronization issue.
| @@ -48,7 +48,7 @@ layout(location = 0) out vec4 fragColor; | |||
|
|
|||
| // Filament's Vulkan backend requires a descriptor set index of 1 for all samplers. | |||
| // This parameter is ignored for other backends. | |||
| layout(location = 0, set = 0) uniform {samplerType} test_tex; | |||
| layout(binding = 0, set = 0) uniform {samplerType} test_tex; | |||
There was a problem hiding this comment.
are we sure this doesn't break the other backends?
There was a problem hiding this comment.
Using location here is wrong GLSL code, the correct one is binding
There was a problem hiding this comment.
so there's glsl for opengl and then glsl for spirv conversion. Could this be one of places where they differ?
There was a problem hiding this comment.
I checked here https://www.khronos.org/opengl/wiki/Layout_Qualifier_(GLSL)
The only thing extra in vulkan is the "set" parameter, but that one seems to be ignored in GL
Uh oh!
There was an error while loading. Please reload this page.