Skip to content
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

Remove barrier transition of frame images #325

Closed
wants to merge 144 commits into from

Conversation

zakorgy
Copy link

@zakorgy zakorgy commented Oct 21, 2019

This is a prerequisite of #317.

  • Made the render pass creation dynamic. Previously we had 4 render passes, but now we moved the transition logic behind render passes so we need to create a variety of them.
  • Reduced the number of pipeline barriers on frame images. The remaining ones are related to blitting and the read pixel function. To remove these we would need a rebase for the latest WR, because it has solutions which can make this part easier for us.
  • Started using the clear load operation in places where it's possible.
  • Added a workaround for debug blits to avoid validation layer errors generated by these calls.

This change is Reviewable

emilio and others added 30 commits February 11, 2019 10:07
…relative to, and compute the right transform based on that. r=kats,kvark

I think this is as clean as it can get.

Differential Revision: https://phabricator.services.mozilla.com/D17848

[wrupdater] From https://hg.mozilla.org/mozilla-central/rev/d7a369cd3b4a1df9933fec5e4eba28da8d7745e2
Now that we no longer guarantee that a picture with perspective transform is rasterized in local space, we need to ensure that the shaders don't apply perspective correction to the texture coordinates twice.
For that to be the case, we pass an extra flag to the plane splitting shader, and un-do the perspective correction if it's not enabled.

Differential Revision: https://phabricator.services.mozilla.com/D17854

[wrupdater] From https://hg.mozilla.org/mozilla-central/rev/653f85bc0bf8113bb8c0e3d4635412620cfb5857
Calculate the frame_count based on the PresentMode.
Run with "--features=vulkan,headless_gfx" from wrench.
Build as usual. To run wrench in `headless` mode, add `--headless` argument.
Eg.: `cargo run --features=vulkan -- --headless reftest`
Copy link

@kvark kvark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR!

The most important thing we should keep in mind is render pass compatibility. You have to specify the exact subpass of a render pass when creating a graphics pipeline. The compatibility rules say that you can use that pipeline in any render pass/subpass that is compatible with the one it was created with.

It's mostly the LoadOp and StoreOp that you can change while retaining the compatibility. You can't have a different attachment format, or different layouts... Hence my suggestion to not include the layouts here.

Without the layouts, there aren't many render pass configurations left:

  • one that doesn't clear, separate for RGBA8 and R8
  • one that clears the color, separate for RGBA8 and R8
  • one that clears the color and depth, for RGBA8 only

So we only end up with 5 render passes in total. In which case they don't need to be dynamically created.

@@ -197,7 +223,8 @@ pub struct Device<B: hal::Backend> {
staging_buffer_pool: ArrayVec<[BufferPool<B>; FRAME_COUNT_MAILBOX]>,
pub swap_chain: Option<B::Swapchain>,
frames: ArrayVec<[Frame<B>; FRAME_COUNT_MAILBOX]>,
render_passes: HalRenderPasses<B>,
render_pass_manager: RenderPassManager<B>,
clear_values: FastHashMap<FBOId, (/*color*/Option<ClearValueRaw>, /*depth*/Option<ClearValueRaw>)>,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's have a structure defined for the values (better to have fields than comments here:) )

#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)]
pub(super) struct AttachmentState {
pub format: Format,
pub src_layout: Layout,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have to put the layout in here. Instead, we can always create the render passes with, say ColorAttachmentOptimal .. ShaderReadOnlyOptimal and then do the transitions outside (using pipeline_barrier) if we need a different layout at the end, or we want to start with a different layout. That would allow us to have much fewer render passes in the end, not to mention the compatibility concerns (to be explained)

@kvark kvark mentioned this pull request Oct 24, 2019
@zakorgy zakorgy force-pushed the master branch 6 times, most recently from 84c0d18 to abc0cd8 Compare November 5, 2019 09:24
@zakorgy
Copy link
Author

zakorgy commented Nov 21, 2019

This was implemented in #331

@zakorgy zakorgy closed this Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants