Skip to content

Commit

Permalink
annihilate legacy Color (as much as humanly possible)
Browse files Browse the repository at this point in the history
  • Loading branch information
teh-cmc committed Jul 24, 2023
1 parent f2523a7 commit 07b6138
Show file tree
Hide file tree
Showing 41 changed files with 319 additions and 263 deletions.
5 changes: 3 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 7 additions & 1 deletion crates/re_components/src/color.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
/// in sRGB gamma space with linear alpha.
///
/// ```
/// use re_components::ColorRGBA;
/// use re_types::components::ColorRGBA;
/// use arrow2_convert::field::ArrowField;
/// use arrow2::datatypes::{DataType, Field};
///
Expand Down Expand Up @@ -33,6 +33,12 @@ impl From<LegacyColor> for re_types::components::Color {
}
}

impl From<re_types::components::Color> for LegacyColor {
fn from(val: re_types::components::Color) -> Self {
LegacyColor(val.0)
}
}

impl LegacyColor {
#[inline]
pub fn from_rgb(r: u8, g: u8, b: u8) -> Self {
Expand Down
4 changes: 3 additions & 1 deletion crates/re_components/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,12 @@ pub mod datagen;
// TODO: stuff we keep for annotation context
pub(crate) use self::{class_id::LegacyClassId, keypoint_id::LegacyKeypointId, label::LegacyLabel};

// TODO: has to be fully public cause we have something weird
pub use self::color::LegacyColor;

pub use self::{
arrow::Arrow3D,
bbox::Box3D,
color::LegacyColor,
context::{AnnotationContext, AnnotationInfo, ClassDescription},
coordinates::ViewCoordinates,
disconnected_space::DisconnectedSpace,
Expand Down
5 changes: 2 additions & 3 deletions crates/re_components/tests/data_row.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use re_components::LegacyColor;
use re_log_types::{DataRow, DataRowError, EntityPath, RowId, TimePoint};
use re_types::{
components::{Label, Point2D},
components::{Color, Label, Point2D},
Loggable as _,
};

Expand All @@ -12,7 +11,7 @@ fn data_row_error_num_instances() {

let num_instances = 2;
let points: &[Point2D] = &[[10.0, 10.0].into(), [20.0, 20.0].into()];
let colors: &[_] = &[LegacyColor::from_rgb(128, 128, 128)];
let colors: &[_] = &[Color::from_rgb(128, 128, 128)];
let labels: &[Label] = &[];

// 0 = clear: legal
Expand Down
9 changes: 4 additions & 5 deletions crates/re_components/tests/data_table_batcher.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
use crossbeam::{channel::TryRecvError, select};
use itertools::Itertools as _;

use re_components::LegacyColor;
use re_log_types::{
DataRow, DataTableBatcher, DataTableBatcherConfig, RowId, SizeBytes, TimePoint, Timeline,
};
use re_log_types::{DataTable, TableId, Time};
use re_types::components::{Label, Point2D};
use re_types::components::{Color, Label, Point2D};

#[test]
fn manual_trigger() {
Expand Down Expand Up @@ -283,7 +282,7 @@ fn create_table() -> DataTable {
let row0 = {
let num_instances = 2;
let points: &[Point2D] = &[[10.0, 10.0].into(), [20.0, 20.0].into()];
let colors: &[_] = &[LegacyColor::from_rgb(128, 128, 128)];
let colors: &[_] = &[Color::from_rgb(128, 128, 128)];
let labels: &[Label] = &[];

DataRow::from_cells3(
Expand All @@ -297,14 +296,14 @@ fn create_table() -> DataTable {

let row1 = {
let num_instances = 0;
let colors: &[LegacyColor] = &[];
let colors: &[Color] = &[];

DataRow::from_cells1(RowId::random(), "b", timepoint(1), num_instances, colors)
};

let row2 = {
let num_instances = 1;
let colors: &[_] = &[LegacyColor::from_rgb(255, 255, 255)];
let colors: &[_] = &[Color::from_rgb(255, 255, 255)];
let labels: &[_] = &[Label("hey".into())];

DataRow::from_cells2(
Expand Down
2 changes: 1 addition & 1 deletion crates/re_data_ui/src/component_ui_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub fn create_component_ui_registry() -> ComponentUiRegistry {
// add::<re_components::Arrow3D>(&mut registry);
// add::<re_components::Box3D>(&mut registry);
add::<re_types::components::ClassId>(&mut registry);
add::<re_components::LegacyColor>(&mut registry);
add::<re_types::components::Color>(&mut registry);
add::<re_types::components::KeypointId>(&mut registry);
// add::<re_components::Label>(&mut registry);
add::<re_components::LineStrip2D>(&mut registry);
Expand Down
5 changes: 3 additions & 2 deletions crates/re_data_ui/src/data.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use egui::Vec2;

use re_components::{
LegacyColor, LineStrip2D, LineStrip3D, Mat3x3, Rect2D, Vec2D, Vec3D, Vec4D, ViewCoordinates,
LineStrip2D, LineStrip3D, Mat3x3, Rect2D, Vec2D, Vec3D, Vec4D, ViewCoordinates,
};
use re_format::format_f32;
use re_types::components::Color;
use re_viewer_context::{UiVerbosity, ViewerContext};

use super::DataUi;
Expand Down Expand Up @@ -31,7 +32,7 @@ impl DataUi for [u8; 4] {
}
}

impl DataUi for LegacyColor {
impl DataUi for Color {
fn data_ui(
&self,
_ctx: &mut ViewerContext<'_>,
Expand Down
10 changes: 5 additions & 5 deletions crates/re_query/benches/query_benchmark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ use itertools::Itertools;
use re_arrow_store::{DataStore, LatestAtQuery};
use re_components::{
datagen::{build_frame_nr, build_some_colors, build_some_point2d, build_some_vec3d},
LegacyColor, Vec3D,
Vec3D,
};
use re_log_types::{entity_path, DataRow, EntityPath, Index, RowId, TimeType, Timeline};
use re_query::query_entity_with_primary;
use re_types::{
components::{InstanceKey, Point2D},
components::{Color, InstanceKey, Point2D},
Loggable as _,
};

Expand Down Expand Up @@ -175,7 +175,7 @@ fn insert_rows<'a>(msgs: impl Iterator<Item = &'a DataRow>) -> DataStore {

struct SavePoint {
_pos: Point2D,
_color: Option<LegacyColor>,
_color: Option<Color>,
}

fn query_and_visit_points(store: &mut DataStore, paths: &[EntityPath]) -> Vec<SavePoint> {
Expand All @@ -186,9 +186,9 @@ fn query_and_visit_points(store: &mut DataStore, paths: &[EntityPath]) -> Vec<Sa

// TODO(jleibs): Add Radius once we have support for it in field_types
for path in paths.iter() {
query_entity_with_primary::<Point2D>(store, &query, path, &[LegacyColor::name()])
query_entity_with_primary::<Point2D>(store, &query, path, &[Color::name()])
.and_then(|entity_view| {
entity_view.visit2(|_: InstanceKey, pos: Point2D, color: Option<LegacyColor>| {
entity_view.visit2(|_: InstanceKey, pos: Point2D, color: Option<Color>| {
points.push(SavePoint {
_pos: pos,
_color: color,
Expand Down
19 changes: 7 additions & 12 deletions crates/re_query/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,8 @@ pub fn query_archetype<A: Archetype>(

/// Helper used to create an example store we can use for querying in doctests
pub fn __populate_example_store() -> DataStore {
use re_components::{datagen::build_frame_nr, LegacyColor};
use re_types::components::Point2D;
use re_components::datagen::build_frame_nr;
use re_types::components::{Color, Point2D};

let mut store = DataStore::new(InstanceKey::name(), Default::default());

Expand All @@ -262,7 +262,7 @@ pub fn __populate_example_store() -> DataStore {
store.insert_row(&row).unwrap();

let instances = vec![InstanceKey(96)];
let colors = vec![LegacyColor(0xff000000)];
let colors = vec![Color(0xff000000)];

let row = DataRow::from_cells2_sized(
RowId::random(),
Expand Down Expand Up @@ -313,7 +313,6 @@ fn simple_get_component() {
#[test]
fn simple_query_entity() {
use re_arrow_store::LatestAtQuery;
use re_components::LegacyColor;
use re_log_types::Timeline;
use re_types::components::{Color, Point2D};

Expand All @@ -322,17 +321,13 @@ fn simple_query_entity() {
let ent_path = "point";
let query = LatestAtQuery::new(Timeline::new_sequence("frame_nr"), 123.into());

let entity_view = query_entity_with_primary::<Point2D>(
&store,
&query,
&ent_path.into(),
&[LegacyColor::name()],
)
.unwrap();
let entity_view =
query_entity_with_primary::<Point2D>(&store, &query, &ent_path.into(), &[Color::name()])
.unwrap();

#[cfg(feature = "polars")]
{
let df = entity_view.as_df2::<LegacyColor>().unwrap();
let df = entity_view.as_df2::<Color>().unwrap();
eprintln!("{df:?}");

let instances = vec![Some(InstanceKey(42)), Some(InstanceKey(96))];
Expand Down
38 changes: 19 additions & 19 deletions crates/re_query/tests/query_tests.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
mod common;

use re_arrow_store::DataStore;
use re_components::{datagen::build_frame_nr, LegacyColor};
use re_components::datagen::build_frame_nr;
use re_log_types::{DataRow, RowId};
use re_query::query_entity_with_primary;
use re_types::{
components::{InstanceKey, Point2D},
components::{Color, InstanceKey, Point2D},
Loggable as _,
};

Expand All @@ -23,7 +23,7 @@ fn simple_query() {

// Assign one of them a color with an explicit instance
let color_instances = vec![InstanceKey(1)];
let colors = vec![LegacyColor(0xff000000)];
let colors = vec![Color(0xff000000)];
let row = DataRow::from_cells2_sized(
RowId::random(),
ent_path,
Expand All @@ -40,7 +40,7 @@ fn simple_query() {
&store,
&timeline_query,
&ent_path.into(),
&[LegacyColor::name()],
&[Color::name()],
)
.unwrap();

Expand All @@ -62,13 +62,13 @@ fn simple_query() {
// Build expected df manually
let instances = vec![Some(InstanceKey(0)), Some(InstanceKey(1))];
let points = vec![Some(Point2D::new(1.0, 2.0)), Some(Point2D::new(3.0, 4.0))];
let colors = vec![None, Some(LegacyColor(0xff000000))];
let colors = vec![None, Some(Color(0xff000000))];
let expected = df_builder3(&instances, &points, &colors).unwrap();

//eprintln!("{df:?}");
//eprintln!("{expected:?}");

common::compare_df(&expected, &entity_view.as_df2::<LegacyColor>().unwrap());
common::compare_df(&expected, &entity_view.as_df2::<Color>().unwrap());
}
#[cfg(not(feature = "polars"))]
{
Expand All @@ -91,7 +91,7 @@ fn timeless_query() {

// Assign one of them a color with an explicit instance.. timelessly!
let color_instances = vec![InstanceKey(1)];
let colors = vec![LegacyColor(0xff000000)];
let colors = vec![Color(0xff000000)];
let row =
DataRow::from_cells2_sized(RowId::random(), ent_path, [], 1, (color_instances, colors));
store.insert_row(&row).unwrap();
Expand All @@ -103,7 +103,7 @@ fn timeless_query() {
&store,
&timeline_query,
&ent_path.into(),
&[LegacyColor::name()],
&[Color::name()],
)
.unwrap();

Expand All @@ -125,13 +125,13 @@ fn timeless_query() {
// Build expected df manually
let instances = vec![Some(InstanceKey(0)), Some(InstanceKey(1))];
let points = vec![Some(Point2D::new(1.0, 2.0)), Some(Point2D::new(3.0, 4.0))];
let colors = vec![None, Some(LegacyColor(0xff000000))];
let colors = vec![None, Some(Color(0xff000000))];
let expected = df_builder3(&instances, &points, &colors).unwrap();

//eprintln!("{df:?}");
//eprintln!("{expected:?}");

common::compare_df(&expected, &entity_view.as_df2::<LegacyColor>().unwrap());
common::compare_df(&expected, &entity_view.as_df2::<Color>().unwrap());
}
#[cfg(not(feature = "polars"))]
{
Expand All @@ -153,7 +153,7 @@ fn no_instance_join_query() {
store.insert_row(&row).unwrap();

// Assign them colors with explicit instances
let colors = vec![LegacyColor(0xff000000), LegacyColor(0x00ff0000)];
let colors = vec![Color(0xff000000), Color(0x00ff0000)];
let row = DataRow::from_cells1_sized(RowId::random(), ent_path, timepoint, 2, colors);
store.insert_row(&row).unwrap();

Expand All @@ -164,7 +164,7 @@ fn no_instance_join_query() {
&store,
&timeline_query,
&ent_path.into(),
&[LegacyColor::name()],
&[Color::name()],
)
.unwrap();

Expand All @@ -186,13 +186,13 @@ fn no_instance_join_query() {
// Build expected df manually
let instances = vec![Some(InstanceKey(0)), Some(InstanceKey(1))];
let points = vec![Some(Point2D::new(1.0, 2.0)), Some(Point2D::new(3.0, 4.0))];
let colors = vec![Some(LegacyColor(0xff000000)), Some(LegacyColor(0x00ff0000))];
let colors = vec![Some(Color(0xff000000)), Some(Color(0x00ff0000))];
let expected = df_builder3(&instances, &points, &colors).unwrap();

//eprintln!("{df:?}");
//eprintln!("{expected:?}");

common::compare_df(&expected, &entity_view.as_df2::<LegacyColor>().unwrap());
common::compare_df(&expected, &entity_view.as_df2::<Color>().unwrap());
}
#[cfg(not(feature = "polars"))]
{
Expand Down Expand Up @@ -220,7 +220,7 @@ fn missing_column_join_query() {
&store,
&timeline_query,
&ent_path.into(),
&[LegacyColor::name()],
&[Color::name()],
)
.unwrap();

Expand Down Expand Up @@ -270,7 +270,7 @@ fn splatted_query() {

// Assign all of them a color via splat
let color_instances = vec![InstanceKey::SPLAT];
let colors = vec![LegacyColor(0xff000000)];
let colors = vec![Color(0xff000000)];
let row = DataRow::from_cells2_sized(
RowId::random(),
ent_path,
Expand All @@ -287,7 +287,7 @@ fn splatted_query() {
&store,
&timeline_query,
&ent_path.into(),
&[LegacyColor::name()],
&[Color::name()],
)
.unwrap();

Expand All @@ -309,13 +309,13 @@ fn splatted_query() {
// Build expected df manually
let instances = vec![Some(InstanceKey(0)), Some(InstanceKey(1))];
let points = vec![Some(Point2D::new(1.0, 2.0)), Some(Point2D::new(3.0, 4.0))];
let colors = vec![Some(LegacyColor(0xff000000)), Some(LegacyColor(0xff000000))];
let colors = vec![Some(Color(0xff000000)), Some(Color(0xff000000))];
let expected = df_builder3(&instances, &points, &colors).unwrap();

//eprintln!("{df:?}");
//eprintln!("{expected:?}");

common::compare_df(&expected, &entity_view.as_df2::<LegacyColor>().unwrap());
common::compare_df(&expected, &entity_view.as_df2::<Color>().unwrap());
}
#[cfg(not(feature = "polars"))]
{
Expand Down
Loading

0 comments on commit 07b6138

Please sign in to comment.