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

Fortify the clear color iterator #2248

Closed
kvark opened this issue Jul 18, 2018 · 2 comments
Closed

Fortify the clear color iterator #2248

kvark opened this issue Jul 18, 2018 · 2 comments

Comments

@kvark
Copy link
Member

kvark commented Jul 18, 2018

This is another unsound aspect of today's HAL that would be great to fix.

In begin_render_pass we receive an iterator over clear values. It's semantics currently matches Vulkan - a value per attachment, but not necessarily enough for all attachment, and some values are ignored. This is rather sad from the API standpoint, and we should fix it.

Perhaps, we could say that the iterator has to only contain the values for cleared attachments? Given that framebuffer/renderpass creation is rare, we could have the portability layer building a mask of cleared attachments, and then filter the given clear value slice based on that. Shouldn't introduce any visible overhead.

@kocsis1david
Copy link
Contributor

My idea would be to have a key-value pair list, something like a Vec<(u32, ClearValueRaw)>, but it can be also a struct. The key would be the index of the attachment. This way you could build the VkRenderPassBeginInfo.pClearValues array without knowing the render pass. In my opinion it would be more explicit.

@kvark
Copy link
Member Author

kvark commented Jul 29, 2018

@kocsis1david that would certainly ease the life of gfx-portability a bit, at the cost of making the API a bit more verbose, but the user should already know these attachment indices, so it sounds like a good compromise.

@kvark kvark added this to the HAL 0.1 release milestone Dec 19, 2018
@kvark kvark self-assigned this Dec 19, 2018
@kvark kvark removed this from the HAL 0.1 release milestone Dec 27, 2018
@kvark kvark pinned this issue Dec 27, 2018
@kvark kvark unpinned this issue Dec 28, 2018
bors bot added a commit that referenced this issue Dec 29, 2018
2535: Attachment clear order API changes r=msiglreith a=kvark

Fixes #2248
PR checklist:
- [x] `make` succeeds (on *nix)
- [x] `make reftests` succeeds (after #2534)
- [ ] tested examples with the following backends: vulkan
- [ ] `rustfmt` run on changed code

Interestingly, DX11/12 backends were already treating the clear color iterator as specified for cleared attachments only (presumably because I implemented that and I was confused in the first place).

@msiglreith comments are welcome! I don't feel super strong either way, just want to make the semantics stronger.

Co-authored-by: Dzmitry Malyshau <dmalyshau@mozilla.com>
@bors bors bot closed this as completed in #2535 Dec 29, 2018
@kvark kvark added this to the HAL 0.2 release milestone Jan 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants