Fix several render issues found while debugging XR #68102
Conversation
|
There is one reported error I haven't fixed yet:
This one is really really weird. This happens on a PICO VR headset, the reason seems to be that we initially create a swapchain for output to screen using the screen resolution of the device, which seems to be The Vulkan driver reports that the maximum supported size is The fix here I would suggest is to build in checks for this in our Vulkan driver and give a normal warning in the output log and possibly clamp the image size of the swapchain to the That said, it puts up some questions on how the PICO can output the end result to screen when their Vulkan driver does not support their output resolution. Unless off course these are really two |
|
Test results I built this on top of the current state of #68023, on which I originally observed the validation errors. With this PR applied, all but two of the errors before the failed pipeline creation disappear: remaining errors are the unfixed error @BastiaanOlij mentioned in the previous comment (VUID-VkSwapchainCreateInfoKHR-imageExtent-01274) and
Regarding VUID-VkSwapchainCreateInfoKHR-imageExtent-01274: I did try clamping in my experiments, that did cause subsequent issues as we do get a valid surface in the (invalid) size, validation layers then complain that we use a different size than the surface... This PR also fixes most validation errors in scenes that do render properly. Only two total warnings remain there: VUID-VkSwapchainCreateInfoKHR-imageExtent-01274 similar to the failed pipeline scenario (I thus don't think it is relevant for proper rendering) and a single validation error for each frame:
This error also occurs on the failed pipeline scenario, buried in the vast mass of errors that obviously happen when godot tries to use a pipeline that failed to create. |
|
Well I call that progress I guess :) Not sure why it would be getting that last one though, unless there is another point where we're not setting the layout correctly. |
There was a problem hiding this comment.
The changes look reasonable to me. I would like @RandomShaper to take a look as well as he is more familiar with some of the state than I am
|
Correction regarding tests in which rendering works: I do still (again?) get other per-frame errors accompanying VUID-VkPresentInfoKHR-pImageIndices-01296. I'm not sure why I did not notice / get them in the test I did earlier, but I don't think the changed environment should cause them (I did switch from template apk to custom build with android plugin). So I guess I'm to blame. Both well-rendering and broken scenes start with the following warnings: Then repeats blocks of: A scene not rendering properly has the exact same warnings (in terms of IDs, I did not check messages), but the pipeline creation fails before the first repeating block, and within each block at the indicated position. Edit with concrete warnings from a run that did not render properly: Startup:
Relevant logcat message: First loop:
Relevant logcat message:
Second loop:
Relevant logcat message:
|
|
Hmmm, I think that kind of makes sense, it all sets of a chain of events from the moment it can't create the swapchain, it then can't create the pipeline, it then can't create the shaders or something. That poses an interesting dilemma for us at some point wanting to skip over the initialisation code all together because on standalone XR that swapchain and the associated windowing code is never used, but without it it doesn't set up the rendering engine correctly. Anyway, as a test I wonder if we can just rig it to not create a swapchain bigger then 4096x4096 and see if that makes the issue go away. |
|
@clayjohn @rsjtdrjgfuzkfg btw, seeing this PR fixes the validation errors mentioned in the OP, I think we should merge this PR if we all agree on the fixes (I too would like @RandomShaper to review, hopefully he'll react soon). That swapchain size issue probably will require further experimentation and would be better to fix in a separate PR. |
Given that I get the exact same warning even when it does render properly (simple scene or forward clustered renderer), I think we're still missing something. I did isolate the issue to having any data pass from vertex to fragment shaders causing the shaders to stop linking when using the mobile renderer, but don't see what could cause it. I dumped the full GLSL shaders and the locations and data types did match up. That said, of course it would be good to skip the "normal" initialization and thus fix the warning. I'm happy to test things if you have an idea how. :)
I agree that this PC should get merged; it is self-contained and we can fix any additional issues in other PRs. |
|
For me this |
|
Thanks! |
This PR fixes various issues reported by
--gpu-validationfound by @rsjtdrjgfuzkfgExternal textures were setting mipmap levels to 0, not 1.
When multiple formats are used,
VK_IMAGE_CREATE_MUTABLE_FORMAT_BITmust be set, we do this for normal swap chains but in OpenXR this requires addingXR_SWAPCHAIN_USAGE_MUTABLE_FORMAT_BITwhen we create the swap chainOur external texture logic was not setting the image layout value at all.
When we originally implemented multiview we did so for
vkCreateRenderPasswhich requires inclusion of this struct.Now that we use
vkCreateRenderPass2KHRthe information is already provided and the additional multiview struct is no longer needed.This took a bit of trickery because we implemented a fallback solution to have
vkCreateRenderPass2KHRfall back tovkCreateRenderPass. I'm not including this struct only if we expect to fallback onvkCreateRenderPass.Since switching to
vkCreateRenderPass2KHRwe now also need to supply theaspectMask, this was not implemented properly. I'm very surprised this was not reported earlier as it would also effect desktop rendering. My best guess is that many Vulkan drivers ignore this mask invkCreateRenderPass2KHR.Anyway, this should set them properly.
I suspect this is fixed by the above fix, but I may still have overlooked something. Since I'm not getting these validation errors I can't verify.