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

Optimize gathering of point cloud colors #3730

Merged
merged 16 commits into from
Oct 10, 2023
Merged

Optimize gathering of point cloud colors #3730

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
4fab5fe
Optimize gathering colors for point clouds: inline common case
emilk Oct 7, 2023
39cc5a7
Remove unnecessary reference
emilk Oct 7, 2023
d0e4791
Clean up the points2d/3d code paths and make them more similar to eac…
emilk Oct 9, 2023
d0dc7fb
Move labels last
emilk Oct 9, 2023
fd71873
Pass in less powerful objects as parameters
emilk Oct 9, 2023
ce410d0
Break out the CPU-specific code so we can benchmark it
emilk Oct 9, 2023
16825ee
Make it easy to construct DataRow and DataCell from archetypes
emilk Oct 9, 2023
144387b
Make it easier to benchmark individual parts
emilk Oct 9, 2023
00417f1
Add a high-level benchmark of Points3D rendering
emilk Oct 9, 2023
7d6c14e
Compute the size of the row
emilk Oct 10, 2023
f727a72
Add profile-scopes around the `collect` calls
emilk Oct 10, 2023
42a3b2e
Fix docstring
emilk Oct 10, 2023
8a82d46
`#[inline]`
emilk Oct 10, 2023
f25d0b8
backticks
emilk Oct 10, 2023
4c7298e
Make `from_component_batches` more complicated and add `from_archetype`
emilk Oct 10, 2023
a55c079
Remove unused import
emilk Oct 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
15 changes: 3 additions & 12 deletions crates/re_space_view_spatial/src/parts/images.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,10 +259,7 @@ impl ImagesPart {
.annotations
.resolved_class_description(None)
.annotation_info()
.color(
color.map(|c| c.to_array()).as_ref(),
DefaultColor::OpaqueWhite,
);
.color(color.map(|c| c.to_array()), DefaultColor::OpaqueWhite);

if let Some(textured_rect) = to_textured_rect(
ctx,
Expand Down Expand Up @@ -379,10 +376,7 @@ impl ImagesPart {
.annotations
.resolved_class_description(None)
.annotation_info()
.color(
color.map(|c| c.to_array()).as_ref(),
DefaultColor::OpaqueWhite,
);
.color(color.map(|c| c.to_array()), DefaultColor::OpaqueWhite);

if let Some(textured_rect) = to_textured_rect(
ctx,
Expand Down Expand Up @@ -467,10 +461,7 @@ impl ImagesPart {
.annotations
.resolved_class_description(None)
.annotation_info()
.color(
color.map(|c| c.to_array()).as_ref(),
DefaultColor::OpaqueWhite,
);
.color(color.map(|c| c.to_array()), DefaultColor::OpaqueWhite);

if let Some(textured_rect) = to_textured_rect(
ctx,
Expand Down
6 changes: 2 additions & 4 deletions crates/re_space_view_spatial/src/parts/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ pub fn process_colors<'a, A: Archetype>(
arch_view.iter_optional_component::<Color>()?,
)
.map(move |(annotation_info, color)| {
annotation_info.color(color.map(move |c| c.to_array()).as_ref(), default_color)
annotation_info.color(color.map(|c| c.to_array()), default_color)
}))
}

Expand All @@ -134,9 +134,7 @@ pub fn process_labels<'a, A: Archetype>(
annotation_infos.iter(),
arch_view.iter_optional_component::<Text>()?,
)
.map(move |(annotation_info, text)| {
annotation_info.label(text.as_ref().map(move |t| t.as_str()))
}))
.map(move |(annotation_info, text)| annotation_info.label(text.as_ref().map(|t| t.as_str()))))
}

/// Process [`re_types::components::Radius`] components to [`re_renderer::Size`] using auto size
Expand Down
3 changes: 1 addition & 2 deletions crates/re_space_view_time_series/src/view_part_system.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,7 @@ impl TimeSeriesSystem {
arch_view.iter_optional_component::<Radius>()?,
arch_view.iter_optional_component::<Text>()?,
) {
let color =
annotation_info.color(color.map(|c| c.to_array()).as_ref(), default_color);
let color = annotation_info.color(color.map(|c| c.to_array()), default_color);
let label = annotation_info.label(label.as_ref().map(|l| l.as_str()));

const DEFAULT_RADIUS: f32 = 0.75;
Expand Down
11 changes: 9 additions & 2 deletions crates/re_viewer_context/src/annotations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,19 @@ pub struct ResolvedAnnotationInfo {

impl ResolvedAnnotationInfo {
/// `rgba` should be unmultiplied
#[inline]
pub fn color(
&self,
rgba: Option<&[u8; 4]>,
rgba: Option<[u8; 4]>,
default_color: DefaultColor<'_>,
) -> re_renderer::Color32 {
if let Some([r, g, b, a]) = rgba {
re_renderer::Color32::from_rgba_unmultiplied(*r, *g, *b, *a)
if a == 255 {
// Common-case optimization
re_renderer::Color32::from_rgb(r, g, b)
} else {
re_renderer::Color32::from_rgba_unmultiplied(r, g, b, a)
}
Comment on lines +161 to +166
Copy link
Member

@Wumpf Wumpf Oct 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

now that you marked all color methods inline emilk/egui@38b4234 and given that from_rgba_unmultiplied already has this short-circuit, this shouldn't actually be here, amiright? :)

Copy link
Member Author

Choose a reason for hiding this comment

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

It depends on when we update to egui, and wether or not the compiler will actually inline it, and what effect that will have, yadda yadda

} else if let Some(color) = self.annotation_info.as_ref().and_then(|info| {
info.color
.map(|c| c.into())
Expand All @@ -168,6 +174,7 @@ impl ResolvedAnnotationInfo {
}
}

#[inline]
pub fn label(&self, label: Option<&str>) -> Option<String> {
if let Some(label) = label {
Some(label.to_owned())
Expand Down
Loading