Remove components if not extracted#15948
Conversation
|
Hm, in some cases, we also need to remove components inserted from |
|
Instead, I went ahead to add the config component ( |
There was a problem hiding this comment.
@rafalh are you able to review this PR for me in turn? :) This is closely related to #15946.
@alice-i-cecile I tried but I don't think I understand enough about the render graph to fully review this. I left some comments through. Most of changes looks okay to me.
crates/bevy_pbr/src/render/light.rs
Outdated
| .remove::<( | ||
| ExtractedDirectionalLight, | ||
| RenderCascadesVisibleEntities, | ||
| LightViewEntities, |
There was a problem hiding this comment.
LightViewEntities is added in an observer and not in this system, so it's not clear at the first glance why we remove it here. Maybe we should remove it in an observer too to be consistent? 🤔
There was a problem hiding this comment.
Added OnRemove version of the observer that removes LightViewEntities.
| _: &mut RenderGraphContext, | ||
| render_context: &mut RenderContext<'w>, | ||
| ( | ||
| _depth_of_field, |
There was a problem hiding this comment.
not sure why do we need this and it seems like a hack to me
There was a problem hiding this comment.
Right, changed to remove components in view query too.
tychedelia
left a comment
There was a problem hiding this comment.
Thanks for leaving the breadcrumbs to improve this later.
Objective
(Camera is done in Fix deactivated camera still being used in render world #15946)
Solution
SyncComponentPluginfor DOF, TAA, and SSAO(According to the documentation, this plugin "needs to be added for manual extraction implementations." We may need to check this is done.)
Testing
Modified example locally to add toggles if not exist.
depth_of_fieldexampleCamera.is_activeand TAA componentCamera.is_activeinmany_lightsexample (no crash/glitch even without this PR)Camera.is_activeinskybox(no crash/glitch even without this PR)VisibilityofDirectionalLightinlightingexampleCamera.is_activeand SSAO component inssaoexampleCamera.is_active(nop without Fix deactivated camera still being used in render world #15946 because UI defaults to some camera even ifDefaultCameraViewis not there)