Closed jorgeag-google closed 5 months ago
Thanks Jorge! Any chance we can split this into separate PRs for each commit/meaningful change instead? This much code is a bit daunting to review.
I have split the PR into the original commits. The last one also included my won addition to integrate. I am looking into the shader compilation problem on Linux
Since you already have this in separate commits, would it be possible to split into PRs? It would be much easier/faster to review, and address review feedback, feedback is supposed to be addressed as separate commits.
I follows Aliya's advice, I update this PR to contain only one of the CL in the original merge. I create the nexts ones when this gets merged.
Thanks Jorge for taking care of merging these.
I don't remember exactly what benchmarking you're planning to do, so I just want to make sure these functions are actually needed. I added them specifically to test FDM and VRS independent of OpenXR foveation. These let the application use it's own density patterns.
In normal OpenXR foveation, the foveation pattern comes from OpenXR, via the application via XrSwapchainImageFoveationVulkanFB
in the next chain of xrEnumerateSwapchainImages
. For this use case, we would want to build a ShadingRatePattern
around the density image from OpenXR.
Hi Benson,
Yes, the idea is to port all your changes that lead to the foveation_benchmark
app.
To my undestanding these is a CL that is part of four required ones (correct me if I am worng here)
I want to integrate all four of these:
To answer the other part of your your comment, I want fixed foveated rendering as a first step. So, if I am able to set a fix pattern of two concentric elipses manually, that will be perfect for my use case. Maybe later I would want to query the pattern from the app using OpenXR, but in the meantime fixed patter will work fine.
The first of a series of CL that integrates the commits from:
https://github.com/bjoeris/bigwheels/tree/experimental-foveation
into BW main's branch.
The series will have 4 CL, this is the first ona that integrates the following commit