-
Notifications
You must be signed in to change notification settings - Fork 755
New Fragment Density Map Sample #1411
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Build fails for me at GLSL compilation level. Sadly not getting any meaningful error to look for. Any ideas? |
I just checked and I can recreate the spv on my machine. Can you share more details? Sorry, no idea how to reproduce it. |
I'm using the most recent SDK (1.4.328.1). I do not get any meaningful error other than the GLSL shader project for the new sample fails :/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, I have no HW supporting FDM. Thus I could not really test.
|
||
Below is a simple pipeline arrangement used on this sample. | ||
|
||
1. The density map is computed once outside the render loop and reused each frame. This is done in a separate command-buffer, all the othe commands will go to the same command buffer. The sample also includes a UI option to update the fragment density map attachment every frame using `fragmentDensityMapDynamic`. If this option is selected, an additional pass at the beginning of the main command-buffer produces the fragment density map attachment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: othe -> other
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
struct | ||
{ | ||
ImageData image{}; | ||
VkExtent2D extend{32 * 32 * 4, 23 * 32 * 4}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo? 23 -> 32
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s actually not used, so I reset it to 0. The actual value is calculated from the swap chain resolution.
Previously, it was intentionally set to 23. Interestingly, 23 × 32 × 4 = 2944, matches the resolution I observed on my test device.
if (!available_options.supports_fdm) | ||
{ | ||
current_options.enable_fdm = false; | ||
LOGW("Fragment density map is not supported"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be a LOGE
?
The sample does not run without FDM support anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, change to LOGE
Hi, I’m not very familiar with how the project compiles GLSL to SPV, but everything looks identical to other samples. I assume the Windows CI is also compiling the GLSL shaders, so any GLSL compilation problems should be caught there? Or are we avoiding compiling shaders on the CI? I am unsure about next steps. Are you sure this isn’t an issue with your local setup? |
Afaik the CI for non-desktop platforms doesn't compile shaders. If I'm the only one with that issue there is no need to further investigate. Aside from that I can't really test the sample as I don't have a device that supports said extensions. It does compile fine and properly exits out with an error. |
By code inspection, this looks good. |
Description
Add a new sample to demonstrate the use of the extension Fragment Density Map(FDM). This new sample renders a glTF scene with and without FDM to show the benefits of this extension
Note
We made the following changes to the framework.
VkComponentMapping
render_pass = VK_NULL_HANDLE
inapi_vulkan_sample.h
. Is best practice to initialize variables and this avoids trying to delete it when I overwrite the resize function.Gui::show_stats
public so that I can use it on my API sample.VkPhysicalDeviceFragmentDensityMapFeaturesEXT
inframework/vulkan_type_mapping.h
Testing
General Checklist:
Please ensure the following points are checked:
Note: The Samples CI runs a number of checks including:
If this PR contains framework changes:
batch
command line argument to make sure all samples still work properlySample Checklist
If your PR contains a new or modified sample, these further checks must be carried out in addition to the General Checklist: