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

Fix combining outline mask for selection & hover #1552

Merged
merged 2 commits into from
Mar 10, 2023
Merged

Fix combining outline mask for selection & hover #1552

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d7d9fac
Fix combining outline mask for selection & hover
Wumpf Mar 10, 2023
a3a5f8b
Rename `combine` to `with_fallback_to` and fix orderings
Wumpf Mar 10, 2023
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
20 changes: 18 additions & 2 deletions crates/re_renderer/src/renderer/outlines.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ use smallvec::smallvec;
/// Each channel can distinguish up 255 different objects, each getting their own outline.
///
/// Object index 0 is special: It is the default background of each outline channel, thus rendering with it
/// is a form of "active no outline", effectively subtracting from the outline channel.
#[derive(Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord)]
/// is a form of "active no outline", effectively subtracting from any outline channel.
#[derive(Clone, Copy, Default, PartialEq, Eq, Debug)]
pub struct OutlineMaskPreference(pub Option<[u8; 2]>);

impl OutlineMaskPreference {
Expand All @@ -87,6 +87,22 @@ impl OutlineMaskPreference {
pub fn is_none(self) -> bool {
self.0.is_none()
}

#[inline]
pub fn combine(self, other: Self) -> Self {
Wumpf marked this conversation as resolved.
Show resolved Hide resolved
if let Some([a, b]) = self.0 {
if let Some([other_a, other_b]) = other.0 {
OutlineMaskPreference::some(
if a == 0 { other_a } else { a },
if b == 0 { other_b } else { b },
)
} else {
self
}
} else {
other
}
}
}

#[derive(Clone, Debug)]
Expand Down
27 changes: 12 additions & 15 deletions crates/re_viewer/src/misc/selection_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ impl<'a> OptionalSpaceViewOutlineMask<'a> {
.get(&instance_key)
.cloned()
.unwrap_or_default()
.max(entity_outline_mask.overall)
.combine(entity_outline_mask.overall)
})
}

Expand Down Expand Up @@ -367,15 +367,15 @@ impl SelectionState {

let mut selection_mask_index: u8 = 0;
let mut hover_mask_index: u8 = 0;
let mut next_selection_mask_index = || {
let mut next_selection_mask = || {
// We don't expect to overflow u8, but if we do, don't use the "background mask".
selection_mask_index = selection_mask_index.wrapping_add(1).at_least(1);
selection_mask_index
OutlineMaskPreference::some(selection_mask_index, 0)
};
let mut next_hover_mask_index = || {
let mut next_hover_mask = || {
// We don't expect to overflow u8, but if we do, don't use the "background mask".
hover_mask_index = hover_mask_index.wrapping_add(1).at_least(1);
hover_mask_index
OutlineMaskPreference::some(0, hover_mask_index)
};

for current_selection in self.selection.iter() {
Expand All @@ -387,8 +387,7 @@ impl SelectionState {
if let Some(space_view) = space_views.get(group_space_view_id) {
// Everything in the same group should receive the same selection outline.
// (Due to the way outline masks work in re_renderer, we can't leave the hover channel empty)
let selection_mask =
OutlineMaskPreference::some(next_selection_mask_index(), 0);
let selection_mask = next_selection_mask();

space_view.data_blueprint.visit_group_entities_recursively(
*group_handle,
Expand All @@ -400,7 +399,8 @@ impl SelectionState {
.selection = SelectionHighlight::SiblingSelection;
let outline_mask =
outlines_masks.entry(entity_path.hash()).or_default();
outline_mask.overall = outline_mask.overall.max(selection_mask);
outline_mask.overall =
outline_mask.overall.combine(selection_mask);
outline_mask.any_selection_highlight = true;
},
);
Expand Down Expand Up @@ -444,8 +444,7 @@ impl SelectionState {
} else {
&mut outline_mask.overall
};
*outline_mask_target = (*outline_mask_target)
.max(OutlineMaskPreference::some(next_selection_mask_index(), 0));
*outline_mask_target = outline_mask_target.combine(next_selection_mask());
}
}
};
Expand All @@ -461,8 +460,7 @@ impl SelectionState {
if *group_space_view_id == space_view_id {
if let Some(space_view) = space_views.get(group_space_view_id) {
// Everything in the same group should receive the same selection outline.
let hover_mask =
OutlineMaskPreference::some(0, next_hover_mask_index());
let hover_mask = next_hover_mask();

space_view.data_blueprint.visit_group_entities_recursively(
*group_handle,
Expand All @@ -474,7 +472,7 @@ impl SelectionState {
.hover = HoverHighlight::Hovered;
let mask =
outlines_masks.entry(entity_path.hash()).or_default();
mask.overall = mask.overall.max(hover_mask);
mask.overall = mask.overall.combine(hover_mask);
},
);
}
Expand Down Expand Up @@ -511,8 +509,7 @@ impl SelectionState {
} else {
&mut outlined_entity.overall
};
*outline_mask_target = (*outline_mask_target)
.max(OutlineMaskPreference::some(0, next_selection_mask_index()));
*outline_mask_target = outline_mask_target.combine(next_hover_mask());
}
}
};
Expand Down
12 changes: 6 additions & 6 deletions crates/re_viewer/src/ui/view_spatial/scene/scene_part/meshes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,19 @@ impl MeshPart {

let _default_color = DefaultColor::EntityPath(ent_path);
let world_from_obj_affine = glam::Affine3A::from_mat4(world_from_obj);
let entity_outlines = highlights.entity_outline_mask(ent_path.hash());
let entity_highlight = highlights.entity_outline_mask(ent_path.hash());
Copy link
Member

Choose a reason for hiding this comment

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

The intermixing of "highlight" and "outline" as concepts could probably use some attention at some point.

Copy link
Member Author

Choose a reason for hiding this comment

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

as you can see I went back and forth a bit: They are a form of highlights and soon the only one that's why I went back to calling it highlight 🤔


let visitor =
|instance_key: InstanceKey, mesh: re_log_types::Mesh3D, _color: Option<ColorRGBA>| {
let instance_path_hash = instance_path_hash_for_picking(
let picking_instance_hash = instance_path_hash_for_picking(
ent_path,
instance_key,
entity_view,
props,
entity_outlines.any_selection_highlight(),
entity_highlight.any_selection_highlight(),
);

let outline_mask = entity_outlines.index_outline_mask(instance_key);
let outline_mask = entity_highlight.index_outline_mask(instance_key);
scene.primitives.any_outlines |= outline_mask.is_some();

if let Some(mesh) = ctx
Expand All @@ -61,10 +61,10 @@ impl MeshPart {
ctx.render_ctx,
)
.map(|cpu_mesh| MeshSource {
instance_path_hash,
instance_path_hash: picking_instance_hash,
world_from_mesh: world_from_obj_affine,
mesh: cpu_mesh,
outline_mask: entity_outlines.index_outline_mask(instance_key),
outline_mask,
})
{
scene.primitives.meshes.push(mesh);
Expand Down