Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

render/vulkan: Defer to renderpass clears when possible #3269

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

misyltoad
Copy link
Contributor

Signed-off-by: Joshua Ashton joshua@froggi.es

Signed-off-by: Joshua Ashton <joshua@froggi.es>
@emersion
Copy link
Member

cc @nyorain

renderer->render_pass_clear_color = clear_color;
renderer->pending_render_pass_clear = true;
} else {
vulkan_begin_renderpass(renderer, cb);
Copy link
Contributor

Choose a reason for hiding this comment

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

is this really needed? we only land here if renderer->in_render_pass == true in which case this call is a no-op right?

@@ -490,7 +491,7 @@ static struct wlr_vk_render_buffer *create_render_buffer(
fb_info.width = dmabuf.width;
fb_info.height = dmabuf.height;
fb_info.layers = 1u;
fb_info.renderPass = buffer->render_setup->render_pass;
fb_info.renderPass = buffer->render_setup->render_pass_load;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd add a comment here stating that we can use either VkRenderPass since they are compatible due to only loadOp being different.

Copy link
Contributor

@nyorain nyorain left a comment

Choose a reason for hiding this comment

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

Aside from the nitpicks this makes sense. It's just a workaround for the mismatch between efficient vulkan usage and the wlr api though and we should probably rather adjust the wlr api on the long run (allowing to optionally pass clear color and scissor rect to begin already, for instance).

@emersion
Copy link
Member

emersion commented Nov 1, 2021

wlroots has migrated to gitlab.freedesktop.org. This pull request has been moved to:

https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/3269

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants