Skip to content

Commit

Permalink
Fix error when trying to clear non-existant component (#7215)
Browse files Browse the repository at this point in the history
### What

* Fixes #7210

Changed the semantics of the method from "log empty" to "clear" which is
what we actually want in all cases. Being a clear, it's no longer an
error if there's nothing there to begin with as we don't have to write
anything then.

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7215?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7215?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!
* [x] If have noted any breaking changes to the log API in
`CHANGELOG.md` and the migration guide

- [PR Build Summary](https://build.rerun.io/pr/7215)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
  • Loading branch information
Wumpf authored Aug 16, 2024
1 parent 61dce72 commit 03c0d9a
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 14 deletions.
2 changes: 1 addition & 1 deletion crates/viewer/re_selection_panel/src/defaults_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ fn active_default_ui(
re_ui::list_item::PropertyContent::new(component_name.short_name())
.min_desired_width(150.0)
.action_button(&re_ui::icons::CLOSE, || {
ctx.save_empty_blueprint_component_by_name(
ctx.clear_blueprint_component_by_name(
&view.defaults_path,
component_name,
);
Expand Down
4 changes: 2 additions & 2 deletions crates/viewer/re_selection_panel/src/visualizer_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ fn editable_blueprint_component_list_item(
.action_button(&re_ui::icons::CLOSE, || {
query_ctx
.viewer_ctx
.save_empty_blueprint_component_by_name(blueprint_path, component);
.clear_blueprint_component_by_name(blueprint_path, component);
}),
)
}
Expand All @@ -457,7 +457,7 @@ fn menu_more(
.on_disabled_hover_text("There's no override active")
.clicked()
{
ctx.save_empty_blueprint_component_by_name(override_path, component_name);
ctx.clear_blueprint_component_by_name(override_path, component_name);
ui.close_menu();
}

Expand Down
2 changes: 1 addition & 1 deletion crates/viewer/re_space_view/src/view_property_ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ This has the same effect as not setting the value in the blueprint at all."
)
.on_disabled_hover_text("The property is already unset.");
if response.clicked() {
ctx.save_empty_blueprint_component_by_name(blueprint_path, component_name);
ctx.clear_blueprint_component_by_name(blueprint_path, component_name);
ui.close_menu();
}

Expand Down
11 changes: 4 additions & 7 deletions crates/viewer/re_viewer_context/src/blueprint_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,12 @@ impl ViewerContext<'_> {
{
self.save_blueprint_array(entity_path, component_name, default_value);
} else {
self.save_empty_blueprint_component_by_name(entity_path, component_name);
self.clear_blueprint_component_by_name(entity_path, component_name);
}
}

/// Helper to save a component to the blueprint store.
pub fn save_empty_blueprint_component_by_name(
/// Clears a component in the blueprint store by logging an empty array if it exists.
pub fn clear_blueprint_component_by_name(
&self,
entity_path: &EntityPath,
component_name: ComponentName,
Expand All @@ -164,10 +164,7 @@ impl ViewerContext<'_> {
.map(|array| array.data_type().clone())
})
else {
re_log::error!(
"Tried to clear a component with unknown type: {}",
component_name
);
// There's no component at this path yet, so there's nothing to clear.
return;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,14 +151,15 @@ impl<'a> ViewContext<'a> {
.reset_blueprint_component_by_name(entity_path, component_name);
}

/// Clears a component in the blueprint store by logging an empty array if it exists.
#[inline]
pub fn save_empty_blueprint_component_by_name(
pub fn clear_blueprint_component_by_name(
&self,
entity_path: &EntityPath,
component_name: ComponentName,
) {
self.viewer_ctx
.save_empty_blueprint_component_by_name(entity_path, component_name);
.clear_blueprint_component_by_name(entity_path, component_name);
}

#[inline]
Expand Down
2 changes: 1 addition & 1 deletion crates/viewer/re_viewport_blueprint/src/view_properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ impl<'a> ViewProperty<'a> {
/// Resets all components to empty values, i.e. the fallback.
pub fn reset_all_components_to_empty(&self, ctx: &'a ViewerContext<'a>) {
for &component_name in self.query_results.components.keys() {
ctx.save_empty_blueprint_component_by_name(&self.blueprint_store_path, component_name);
ctx.clear_blueprint_component_by_name(&self.blueprint_store_path, component_name);
}
}

Expand Down

0 comments on commit 03c0d9a

Please sign in to comment.