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

[0.3/dx12] split explicit external pass barrier access for depth attachments #3013

Closed
wants to merge 1 commit into from

Conversation

zakorgy
Copy link
Contributor

@zakorgy zakorgy commented Sep 13, 2019

Follow up for #3010

Fixes #3009
PR checklist:

  • make succeeds (on *nix)
  • make reftests succeeds
  • tested examples with the following backends:
  • rustfmt run on changed code

Handle the case for depth access provided in the explicit external barriers separately.
Also set the end in case of Range { start: Sr::External, end: Sr::Pass(_) } since there is no guarantee that there is always a Range { start: Sr::Pass(_), end: Sr::External } for transitioning back the resources, at least Vulkan doesn't need it.
cc @kvark

@msiglreith
Copy link
Contributor

Would you mind expanding on why handling depth stencil attachments differently is required?

Copy link
Member

@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.

I started reviewing and then realized there are many things that need to be changed, so I'm working on another iteration of the barrier handling in D3D12. Should be out soon!

@kvark
Copy link
Member

kvark commented Sep 13, 2019

Would you mind expanding on why handling depth stencil attachments differently is required?

The access flags specified in an external barrier apply to the whole sub-pass, so it's a union of flags needed for depth and color and resolve etc. When we try to map them to D3D12 states, the conversion assumes the access flags to be required specifically, so it's confused and it's producing the wrong initial state.

@kvark
Copy link
Member

kvark commented Sep 13, 2019

See #3014 . Please let me know if this works for you!

bors bot added a commit that referenced this pull request Sep 14, 2019
3014: [0.3/dx12] rework resource state conversion, better external barriers r=msiglreith a=kvark

Follow-up to #3010 that tries to address #3009 better. Replaces #3013
PR checklist:
- [ ] `make` succeeds (on *nix)
- [ ] `make reftests` succeeds
- [x] tested examples with the following backends: dx12
- [ ] `rustfmt` run on changed code

Contains 2 ideas:
  1. we try to extract more information out of the image layouts, which is more robust
  2. we are very selective in the external access dependencies, only taking ones that are related to where the attachment is used first or last.

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
bors bot added a commit that referenced this pull request Sep 17, 2019
3014: [0.3/dx12] rework resource state conversion, better external barriers r=msiglreith a=kvark

Follow-up to #3010 that tries to address #3009 better. Replaces #3013
Edit: this combines parts 2 and 3 of the big dx12 barrier refactor :)

PR checklist:
- [ ] `make` succeeds (on *nix)
- [ ] `make reftests` succeeds
- [x] tested examples with the following backends: dx12
- [ ] `rustfmt` run on changed code

Contains 2 ideas:
  1. we try to extract more information out of the image layouts, which is more robust
  2. we are very selective in the external access dependencies, only taking ones that are related to where the attachment is used first or last.

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
bors bot added a commit that referenced this pull request Sep 17, 2019
3014: [0.3/dx12] rework resource state conversion, better external barriers r=msiglreith a=kvark

Follow-up to #3010 that tries to address #3009 better. Replaces #3013
Edit: this combines parts 2 and 3 of the big dx12 barrier refactor :)

PR checklist:
- [ ] `make` succeeds (on *nix)
- [ ] `make reftests` succeeds
- [x] tested examples with the following backends: dx12
- [ ] `rustfmt` run on changed code

Contains 2 ideas:
  1. we try to extract more information out of the image layouts, which is more robust
  2. we are very selective in the external access dependencies, only taking ones that are related to where the attachment is used first or last.

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
@zakorgy
Copy link
Contributor Author

zakorgy commented Sep 17, 2019

Closing in favor of #3014

@zakorgy zakorgy closed this Sep 17, 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.

None yet

3 participants