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.

Sign up for GitHub

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

Attempt to remove component from render world if not extracted. #15904

Merged
merged 3 commits into from
Oct 15, 2024
Merged

Attempt to remove component from render world if not extracted. #15904

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9ec9c7b
Attempt to remove component from render world if not extracted.
tychedelia Oct 14, 2024
28c19ac
Correct docs.
tychedelia Oct 15, 2024
f791bf3
Merge branch 'main' into 15871-conditional-extraction
tychedelia Oct 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions crates/bevy_render/src/extract_component.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@ pub trait ExtractComponent: Component {

/// The output from extraction.
///
/// Returning `None` based on the queried item can allow early optimization,
/// for example if there is an `enabled: bool` field on `Self`, or by only accepting
/// values within certain thresholds.
/// Returning `None` based on the queried item will remove the component from the entity in
/// the render world. This can be used, for example, to conditionally extract camera settings
/// in order to disable a rendering feature on the basis of those settings, without removing
/// the component from the entity in the main world.
///
/// The output may be different from the queried component.
/// This can be useful for example if only a subset of the fields are useful
Expand Down Expand Up @@ -205,6 +206,8 @@ fn extract_components<C: ExtractComponent>(
for (entity, query_item) in &query {
if let Some(component) = C::extract_component(query_item) {
values.push((entity, component));
} else {
commands.entity(entity).remove::<C>();
Copy link
Contributor

Choose a reason for hiding this comment

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

@alice-i-cecile What is the behavior of remove() when the entity lacks the component already? No-op, or panic?

The docs don't say: https://dev-docs.bevyengine.org/bevy_ecs/system/commands/struct.EntityCommands.html#method.remove

Copy link
Member

Choose a reason for hiding this comment

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

No-op.

Copy link
Contributor

Choose a reason for hiding this comment

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

C: ExtractComponent inserts C::Out instead of C so this might need to be remove::<C::Out>.

bevy/crates/bevy_core_pipeline/src/contrast_adaptive_sharpening/mod.rs

Lines 82 to 85 in 813c759

impl ExtractComponent for ContrastAdaptiveSharpening {
type QueryData = &'static Self;
type QueryFilter = With<Camera>;
type Out = (DenoiseCas, CasUniform);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gosh duh. In many cases they're the same but good catch.

}
}
*previous_len = values.len();
Expand All @@ -222,6 +225,8 @@ fn extract_visible_components<C: ExtractComponent>(
if view_visibility.get() {
if let Some(component) = C::extract_component(query_item) {
values.push((entity, component));
} else {
commands.entity(entity).remove::<C>();
}
}
}
Expand Down