From 9d38189191ca028a2beffb1c247dfd3a2a64d6d8 Mon Sep 17 00:00:00 2001
From: Antoine Beyeler <49431240+abey79@users.noreply.github.com>
Date: Wed, 18 Dec 2024 11:40:35 +0100
Subject: [PATCH] Fix undo not working in some entity drag-and-drop instances
 (#8526)

This is a band-aid patch to address an issue with entity path filter
which would result in the following behaviours:

- the entity path filter would be perma-writen to the blueprint store
when the view is selected (in some cases)
- in such cases, this would break undo on user change (aka with entity
drag-and-drop)

The core issue is deeper and might cause panics with rust 1.81+. The
TL;DR is that the current `EntityPathFilter` type is ambiguous as to
whether its content has substitutions (aka $origin) applied or not. In
particular `Eq` and `Ord` apply to unsubstituted, resp. substituted
content (which can lead to the above panic). This should be further
cleaned by having two structures, one unsubstituted and another
substituted.

---------

Co-authored-by: Clement Rey <cr.rey.clement@gmail.com>
---
 crates/store/re_log_types/src/path/entity_path_filter.rs | 5 ++++-
 crates/viewer/re_selection_panel/src/selection_panel.rs  | 9 +++++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/crates/store/re_log_types/src/path/entity_path_filter.rs b/crates/store/re_log_types/src/path/entity_path_filter.rs
index a59629c1e089..9f94525e9bcc 100644
--- a/crates/store/re_log_types/src/path/entity_path_filter.rs
+++ b/crates/store/re_log_types/src/path/entity_path_filter.rs
@@ -16,8 +16,11 @@ pub enum EntityPathFilterParseError {
 }
 
 /// A set of substitutions for entity paths.
+///
+/// Important: the same substitutions must be used in every place we parse entity path filters.
+// TODO(ab): the above requirement is a foot-gun we already fell in and should be addressed.
 #[derive(Default)]
-pub struct EntityPathSubs(pub HashMap<String, String>);
+pub struct EntityPathSubs(HashMap<String, String>);
 
 impl EntityPathSubs {
     /// Create a new set of substitutions from a single origin.
diff --git a/crates/viewer/re_selection_panel/src/selection_panel.rs b/crates/viewer/re_selection_panel/src/selection_panel.rs
index 9d2cf9e3209c..4bec1216ff8d 100644
--- a/crates/viewer/re_selection_panel/src/selection_panel.rs
+++ b/crates/viewer/re_selection_panel/src/selection_panel.rs
@@ -7,7 +7,7 @@ use re_data_ui::{
     DataUi,
 };
 use re_entity_db::{EntityPath, InstancePath};
-use re_log_types::{ComponentPath, EntityPathFilter};
+use re_log_types::{ComponentPath, EntityPathFilter, EntityPathSubs};
 use re_types::blueprint::components::Interactive;
 use re_ui::{
     icons,
@@ -559,7 +559,12 @@ fn entity_path_filter_ui(
     }
 
     // Apply the edit.
-    let new_filter = EntityPathFilter::parse_forgiving(&filter_string, &Default::default());
+    //
+    // NOTE: The comparison of `EntityPathFilter` is done on the _expanded_ data (i.e. with variables substituted),
+    // so we must make sure to expand the new filter too before we compare it to the existing one.
+    // See <https://github.com/rerun-io/rerun/pull/8526>
+    let new_filter =
+        EntityPathFilter::parse_forgiving(&filter_string, &EntityPathSubs::new_with_origin(origin));
     if &new_filter == filter {
         None // no change
     } else {