Use RenderStartup for order independent transparency#20170
Use RenderStartup for order independent transparency#20170andriyDev wants to merge 2 commits intobevyengine:mainfrom
RenderStartup for order independent transparency#20170Conversation
andriyDev
changed the title
RenderStartup for order independent transparency
andriyDev
added
A-Rendering
|
|
||
| /// A resource to indicate that OIT resolve is supported. | ||
| #[derive(Resource)] | ||
| struct OitResolveSupported; |
There was a problem hiding this comment.
I would prefer if this was called OitSupported, same for the SystemSet. The fact that everything has resolve in the name right now is more a side effect of the implementation but it shouldn't leak at that level because it will eventually do more than that.
There was a problem hiding this comment.
I looked at the code again and I misrepresented this a bit, but I still think this resource should be renamed.
I believe pretty much everything in OrderIndependentTransparencyPlugin should be skipped if the check is false. I don't know why I didn't do it like this originally.
There was a problem hiding this comment.
To be clear, I think a refactor that move the check to that plugin should be separate, for this PR just keep Resolve out of the name.
There was a problem hiding this comment.
Done. Renamed the new types to omit the Resolve suffixes.
| render_device: Res<RenderDevice>, | ||
| render_adapter: Res<RenderAdapter>, | ||
| ) { | ||
| if is_oit_supported(&render_adapter, &render_device, true) { |
There was a problem hiding this comment.
It might require a bit more work, but this function is only used in one other place, I think we could instead inline it here and pipe the resource to that other place that needs to know if OIT is supported.
There was a problem hiding this comment.
I would like to make this a followup rather than blocking this here. Minimal diff is better for these refactors IMO!
There was a problem hiding this comment.
Especially in rendering code it's better to avoid sprawling changes because everyone touches everything all the time. The other usage site (mesh_view_bindings.rs) would require modifying more than one function, as you'd have to grab the OIT support state from the world and pass it around.
There was a problem hiding this comment.
Ah, right, I didn't realize it would be that complex, definitely follow up work then
andriyDev
force-pushed
the
startup-oit-conditional
branch
from
e950cc7 to
df6766a
Compare
nicopap
left a comment
nicopap
left a comment
There was a problem hiding this comment.
It's just a really good changeset in line with the goal of minimizing the use of finish().
I also do like the code.
andriyDev
added
S-Ready-For-Final-Review
atlv24
added
S-Ready-For-Final-Review
andriyDev
added
S-Blocked
|
Blocking on #20206 |
4 participants
Objective
Solution
Testing
order_independent_transparencyexample and it works!