-
Notifications
You must be signed in to change notification settings - Fork 29.7k
[Impeller] fix barriers on PowerVR hardware / ensure Render pass cached on non-MSAA. #165497
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
Conversation
|
This should also impact the gaussian blur backdrop benchmark, since I switched those surfaces to be non-MSAA |
gaaclarke
left a comment
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.
Looks good modulo one missing assert in the test and we can drop the cyclomatic complexity by removing the "bool is_swapchain" arguments from methods.
| EXPECT_NE(texture_vk.GetCachedRenderPass(), nullptr); | ||
|
|
||
| render_pass->EncodeCommands(); | ||
| GetContext()->GetCommandQueue()->Submit({buffer}); |
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.
Assert the return result for Submit is a success. Maybe we should have [[nodiscard]] for that.
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.
Done
| SharedHandleVK<vk::RenderPass> RenderPassVK::CreateVKRenderPass( | ||
| const ContextVK& context, | ||
| const SharedHandleVK<vk::RenderPass>& recycled_renderpass, | ||
| const std::shared_ptr<CommandBufferVK>& command_buffer) const { | ||
| const std::shared_ptr<CommandBufferVK>& command_buffer, | ||
| bool is_swapchain) const { |
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.
Instead of passing in a bool can you just have CreateVKRenderPass() and CreateSwapchainVKRenderPass()?
| StoreAction store_action, | ||
| vk::ImageLayout current_layout) { | ||
| vk::ImageLayout current_layout, | ||
| bool is_swapchain) { |
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.
Instead of passing in a bool, why not pass in vk::ImageLayout final_layout?
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.
We still need the logic in the render_pass_builder because the MSAA texture and resolve always need separate layouts. I'd rather leave this determination in the render_pass_builder
gaaclarke
left a comment
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.
lgtm
|
autosubmit label was removed for flutter/flutter/165497, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label. |
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
Not to be landed until #165497 lands. We don't both to parse the model number until CXT and DXT as all prior models shouldn't be used.
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ass cached on non-MSAA. (flutter/flutter#165497)
…ed on non-MSAA. (flutter#165497) Work towards flutter#162033 - barriers for render pass are not correct, but only causes a problem on powervr/imagination. Added new external subpass dependencies that have better descriptions for what they do. We now use the subpass to transition images that are sampled to the final shaderReadOnlylayout, while keeping swapchain images in eGeneral. - missing cache for render pass objects when using non-msaa passes. This mostly impacts powervr hardware because render pass construction is much slower there.
Not to be landed until flutter#165497 lands. We don't both to parse the model number until CXT and DXT as all prior models shouldn't be used.
…ed on non-MSAA. (flutter#165497) Work towards flutter#162033 - barriers for render pass are not correct, but only causes a problem on powervr/imagination. Added new external subpass dependencies that have better descriptions for what they do. We now use the subpass to transition images that are sampled to the final shaderReadOnlylayout, while keeping swapchain images in eGeneral. - missing cache for render pass objects when using non-msaa passes. This mostly impacts powervr hardware because render pass construction is much slower there.
Not to be landed until flutter#165497 lands. We don't both to parse the model number until CXT and DXT as all prior models shouldn't be used.
Work towards #162033
barriers for render pass are not correct, but only causes a problem on powervr/imagination. Added new external subpass dependencies that have better descriptions for what they do. We now use the subpass to transition images that are sampled to the final shaderReadOnlylayout, while keeping swapchain images in eGeneral.
missing cache for render pass objects when using non-msaa passes. This mostly impacts powervr hardware because render pass construction is much slower there.