Skip to content

Commit

Permalink
Rewrite Image archetype (rerun-io#6942)
Browse files Browse the repository at this point in the history
### What
* Part of rerun-io#6844
* Closes rerun-io#6386

### Breaking changes
* `Image(…).compress(…)` is now `ImageEncoded.compres(…)`
* `ImageEncoded` has very different constructor compared to 0.17, and
only supports chroma-downsampled images

### TODO
* [x] Fix how enums are handled in Python (I don't understand it). In
particular, this code is broken when given a string as a color model:
https://github.com/rerun-io/rerun/blob/8974966ef17c71574df04a77e9ffee0125b32063/rerun_py/rerun_sdk/rerun/archetypes/image_encoded_ext.py#L150-L157
* [x] Fix this test:
https://github.com/rerun-io/rerun/blob/4f12d7f6f47a15e0583c5de9e2883840b571e21e/rerun_py/tests/unit/test_image.py#L16-L45
* [x] Test `detect_and_track_objects`
* [x] Test a bunch of other examples
* ~~[ ] Log a warning if `color_model` is missing, so that we can in the
future migrate to having BGR be the default~~
* BGR should never be the default. This has haunted opencv for over a
decade. We need not inherit this trauma.
* ~~[ ] Consider re-implementing `image.compress(…)` (with a forward
call to `ImageEncoded.compress`~~
* TODO: I don't understand why this was moved in the first place. We
should move it back, but it can happen in a follow-on PR.

### 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/6942?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/6942?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:
  * rerun-io#7031

- [PR Build Summary](https://build.rerun.io/pr/6942)
- [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`.

---------

Co-authored-by: Jeremy Leibs <[email protected]>
  • Loading branch information
emilk and jleibs authored Aug 1, 2024
1 parent 821c26c commit 102a226
Show file tree
Hide file tree
Showing 238 changed files with 3,833 additions and 4,309 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
# ⚠️ Breaking changes
* `mesh_material: Material` has been renamed to `albedo_factor: AlbedoFactor` [#6841](https://github.com/rerun-io/rerun/pull/6841)
* 3D transform APIs: Previously, the transform component was represented as one of several variants (an Arrow union, `enum` in Rust) depending on how the transform was expressed. Instead, there are now several components for translation/scale/rotation/matrices that can live side-by-side in the 3D transform archetype.
* Python: `NV12/YUY2` are now logged with the new `ImageChromaDownsampled`
* Python: `NV12/YUY2` are now logged with `Image`
* `Image.compress` has been replaced by `ImageEncoded.compress`
* [`ImageEncoded`](https://rerun.io/docs/reference/types/archetypes/image_encoded?speculative-link):s `format` parameter has been replaced with `media_type` (MIME)
* [`DepthImage`](https://rerun.io/docs/reference/types/archetypes/depth_image) and [`SegmentationImage`](https://rerun.io/docs/reference/types/archetypes/segmentation_image) are no longer encoded as a tensors, and expects its shape in `[width, height]` order

Expand Down
14 changes: 9 additions & 5 deletions crates/build/re_types_builder/src/codegen/cpp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,17 +336,21 @@ fn hpp_type_extensions(
remaining_content = &remaining_content[end + COPY_TO_HEADER_END_MARKER.len()..];
}

let comment = quote_comment(&format!(
"Extensions to generated type defined in '{}'",
let start_comment = quote_comment(&format!(
"START of extensions from {}:",
extension_file.file_name().unwrap()
));
let end_comment = quote_comment(&format!(
"END of extensions from {}, start of generated code:",
extension_file.file_name().unwrap()
));
let hpp_type_extensions = quote! {
public:
#NEWLINE_TOKEN
#comment
public: #start_comment
#NEWLINE_TOKEN
#HEADER_EXTENSION_TOKEN
#NEWLINE_TOKEN
#end_comment
#NEWLINE_TOKEN
};

(hpp_type_extensions, Some(hpp_extension_string))
Expand Down
97 changes: 71 additions & 26 deletions crates/build/re_types_builder/src/codegen/python/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ impl PythonCodeGenerator {
let obj_code = match obj.class {
crate::objects::ObjectClass::Struct => {
if obj.kind == ObjectKind::View {
code_for_view(reporter, objects, obj)
code_for_view(reporter, objects, &ext_class, obj)
} else {
code_for_struct(reporter, arrow_registry, &ext_class, objects, obj)
}
Expand Down Expand Up @@ -827,7 +827,9 @@ fn code_for_enum(
ObjectKind::Datatype | ObjectKind::Component
));

let Object { name, .. } = obj;
let Object {
name: enum_name, ..
} = obj;

let mut code = String::new();

Expand All @@ -837,17 +839,28 @@ fn code_for_enum(
code.push_unindented(format!(r#"@deprecated("""{deprecation_notice}""")"#), 1);
}

code.push_str(&format!("class {name}(Enum):\n"));
let superclasses = {
let mut superclasses = vec![];
if ext_class.found {
// Extension class needs to come first, so its __init__ method is called if there is one.
superclasses.push(ext_class.name.clone());
}
superclasses.push("Enum".to_owned());
superclasses.join(",")
};
code.push_str(&format!("class {enum_name}({superclasses}):\n"));
code.push_indented(1, quote_obj_docs(reporter, objects, obj), 0);

for (i, variant) in obj.fields.iter().enumerate() {
let arrow_type_index = 1 + i; // plus-one to leave room for zero == `_null_markers`

// NOTE: we use PascalCase for the enum variants for consistency across:
// NOTE: we keep the casing of the enum variants exactly as specified in the .fbs file,
// or else `RGBA` would become `Rgba` and so on.
// Note that we want consistency across:
// * all languages (C++, Python, Rust)
// * the arrow datatype
// * the GUI
let variant_name = variant.pascal_case_name();
let variant_name = &variant.name;
code.push_indented(1, format!("{variant_name} = {arrow_type_index}"), 1);

// Generating docs for all the fields creates A LOT of visual noise in the API docs.
Expand All @@ -871,20 +884,50 @@ fn code_for_enum(
}
}

// -------------------------------------------------------

// OVerload `__str__`:
code.push_indented(1, "def __str__(self) -> str:", 1);
code.push_indented(2, "'''Returns the variant name'''", 1);

for (i, variant) in obj.fields.iter().enumerate() {
let variant_name = &variant.name;
if i == 0 {
code.push_indented(2, format!("if self == {enum_name}.{variant_name}:"), 1);
} else {
code.push_indented(2, format!("elif self == {enum_name}.{variant_name}:"), 1);
}
code.push_indented(3, format!("return '{variant_name}'"), 1);
}
code.push_indented(2, "else:", 1);
code.push_indented(3, "raise ValueError('Unknown enum variant')", 3);

// -------------------------------------------------------

let variants = format!(
"Literal[{}]",
obj.fields
.iter()
.map(|v| format!("{:?}", v.pascal_case_name().to_lowercase()))
.join(", ")
itertools::chain!(
// We always accept the original casing
obj.fields.iter().map(|v| format!("{:?}", v.name)),
// We also accept the lowercase variant, for historical reasons (and maybe others?)
obj.fields
.iter()
.map(|v| format!("{:?}", v.name.to_lowercase()))
)
.sorted()
.dedup()
.join(", ")
);
code.push_unindented(
format!("{enum_name}Like = Union[{enum_name}, {variants}]"),
1,
);
code.push_unindented(format!("{name}Like = Union[{name}, {variants}]"), 1);
code.push_unindented(
format!(
r#"
{name}ArrayLike = Union[
{name}Like,
Sequence[{name}Like]
{enum_name}ArrayLike = Union[
{enum_name}Like,
Sequence[{enum_name}Like]
]
"#,
),
Expand Down Expand Up @@ -933,21 +976,23 @@ fn code_for_union(
String::new()
};

let mut superclasses = vec![];
let superclass_decl = {
let mut superclasses = vec![];

// Extension class needs to come first, so its __init__ method is called if there is one.
if ext_class.found {
superclasses.push(ext_class.name.as_str());
}
// Extension class needs to come first, so its __init__ method is called if there is one.
if ext_class.found {
superclasses.push(ext_class.name.as_str());
}

if *kind == ObjectKind::Archetype {
superclasses.push("Archetype");
}
if *kind == ObjectKind::Archetype {
superclasses.push("Archetype");
}

let superclass_decl = if superclasses.is_empty() {
String::new()
} else {
format!("({})", superclasses.join(","))
if superclasses.is_empty() {
String::new()
} else {
format!("({})", superclasses.join(","))
}
};

if let Some(deprecation_notice) = obj.deprecation_notice() {
Expand Down Expand Up @@ -1999,7 +2044,7 @@ fn quote_arrow_serialization(
.iter()
.map(|f| {
let newline = '\n';
let variant = f.pascal_case_name();
let variant = &f.name;
let lowercase_variant = variant.to_lowercase();
format!(
r#"elif value.lower() == "{lowercase_variant}":{newline} types.append({name}.{variant}.value)"#
Expand Down
20 changes: 18 additions & 2 deletions crates/build/re_types_builder/src/codegen/python/views.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,14 @@ use crate::{
Object, Objects, Reporter, ATTR_PYTHON_ALIASES, ATTR_RERUN_VIEW_IDENTIFIER,
};

pub fn code_for_view(reporter: &Reporter, objects: &Objects, obj: &Object) -> String {
use super::ExtensionClass;

pub fn code_for_view(
reporter: &Reporter,
objects: &Objects,
ext_class: &ExtensionClass,
obj: &Object,
) -> String {
assert!(obj.is_struct());

let mut code = String::new();
Expand All @@ -27,7 +34,16 @@ from ..api import SpaceView, SpaceViewContentsLike
);
code.push('\n');

code.push_indented(0, format!("class {}(SpaceView):", obj.name), 1);
let superclasses = {
let mut superclasses = vec![];
if ext_class.found {
// Extension class needs to come first, so its __init__ method is called if there is one.
superclasses.push(ext_class.name.clone());
}
superclasses.push("SpaceView".to_owned());
superclasses.join(",")
};
code.push_indented(0, format!("class {}({superclasses}):", obj.name), 1);
code.push_indented(1, quote_obj_docs(reporter, objects, obj), 1);

code.push_indented(1, init_method(reporter, objects, obj), 1);
Expand Down
24 changes: 19 additions & 5 deletions crates/build/re_types_builder/src/codegen/rust/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,8 +500,15 @@ fn quote_enum(
quote!(#derive)
});

// NOTE: we keep the casing of the enum variants exactly as specified in the .fbs file,
// or else `RGBA` would become `Rgba` and so on.
// Note that we want consistency across:
// * all languages (C++, Python, Rust)
// * the arrow datatype
// * the GUI

let quoted_fields = fields.iter().enumerate().map(|(i, field)| {
let name = format_ident!("{}", field.pascal_case_name());
let name = format_ident!("{}", field.name);

let quoted_doc = quote_field_docs(reporter, objects, field);

Expand All @@ -514,27 +521,34 @@ fn quote_enum(
quote!()
};

let clippy_attrs = if field.name == field.pascal_case_name() {
quote!()
} else {
quote!(#[allow(clippy::upper_case_acronyms)]) // e.g. for `ColorModel::RGBA`
};

quote! {
#quoted_doc
#default_attr
#clippy_attrs
#name = #arrow_type_index
}
});

let quoted_trait_impls = quote_trait_impls_from_obj(reporter, arrow_registry, objects, obj);

let all = fields.iter().map(|field| {
let name = format_ident!("{}", field.pascal_case_name());
let name = format_ident!("{}", field.name);
quote!(Self::#name)
});

let display_match_arms = fields.iter().map(|field| {
let name = field.pascal_case_name();
let quoted_name = format_ident!("{name}");
let name = &field.name;
let quoted_name = format_ident!("{}", name);
quote!(Self::#quoted_name => write!(f, #name))
});
let docstring_md_match_arms = fields.iter().map(|field| {
let quoted_name = format_ident!("{}", field.pascal_case_name());
let quoted_name = format_ident!("{}", field.name);
let docstring_md = doc_as_lines(
reporter,
objects,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ pub fn quote_arrow_deserializer(
let quoted_branches = obj.fields.iter().enumerate().map(|(typ, obj_field)| {
let arrow_type_index = Literal::i8_unsuffixed(typ as i8 + 1); // 0 is reserved for `_null_markers`

let quoted_obj_field_type = format_ident!("{}", obj_field.pascal_case_name());
let quoted_obj_field_type = format_ident!("{}", obj_field.name);
quote! {
#arrow_type_index => Ok(Some(Self::#quoted_obj_field_type))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ table DepthImage (
resolution: rerun.components.Resolution2D ("attr.rerun.component_required", order: 1500);

/// The data type of the depth image data (U16, F32, …).
data_type: rerun.components.ChannelDataType ("attr.rerun.component_required", order: 2000);
datatype: rerun.components.ChannelDatatype ("attr.rerun.component_required", order: 2000);

// --- Optional ---

Expand Down Expand Up @@ -57,5 +57,4 @@ table DepthImage (
///
/// Objects with higher values are drawn on top of those with lower values.
draw_order: rerun.components.DrawOrder ("attr.rerun.component_optional", nullable, order: 3400);

}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ namespace rerun.archetypes;
///
/// This archetype is for ellipsoids or spheres whose size is a key part of the data
/// (e.g. a bounding sphere).
/// For points whose radii are for the sake of visualization, use `Points3D` instead.
/// For points whose radii are for the sake of visualization, use [archetypes.Points3D] instead.
///
/// Currently, ellipsoids are always rendered as wireframes.
/// Opaque and transparent rendering will be supported later.
Expand Down Expand Up @@ -53,7 +53,7 @@ table Ellipsoids (
/// Optional text labels for the ellipsoids.
labels: [rerun.components.Text] ("attr.rerun.component_optional", nullable, order: 3200);

/// Optional `ClassId`s for the ellipsoids.
/// Optional class ID for the ellipsoids.
///
/// The class ID provides colors and labels if not specified explicitly.
class_ids: [rerun.components.ClassId] ("attr.rerun.component_optional", nullable, order: 3300);
Expand Down
48 changes: 31 additions & 17 deletions crates/store/re_types/definitions/rerun/archetypes/image.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,19 @@ namespace rerun.archetypes;

/// A monochrome or color image.
///
/// The order of dimensions in the underlying [components.TensorData] follows the typical
/// row-major, interleaved-pixel image format. Additionally, Rerun orders the
/// [datatypes.TensorDimension]s within the shape description from outer-most to inner-most.
/// See also [archetypes.DepthImage] and [archetypes.SegmentationImage].
///
/// As such, the shape of the [components.TensorData] must be mappable to:
/// - A `HxW` tensor, treated as a grayscale image.
/// - A `HxWx3` tensor, treated as an RGB image.
/// - A `HxWx4` tensor, treated as an RGBA image.
/// The raw image data is stored as a single buffer of bytes in a [rerun.components.Blob].
/// The meaning of these bytes is determined by the `ImageFormat` which specifies the resolution
/// and the pixel format (e.g. RGB, RGBA, …).
///
/// Leading and trailing unit-dimensions are ignored, so that
/// `1x480x640x3x1` is treated as a `480x640x3` RGB image.
/// The order of dimensions in the underlying [components.Blob] follows the typical
/// row-major, interleaved-pixel image format.
///
/// Rerun also supports compressed images (JPEG, PNG, …), using [archetypes.ImageEncoded].
/// Compressing images can save a lot of bandwidth and memory.
///
/// \py You can compress an image using [`rerun.Image.compress`][].
/// \py To pass in a chroma-encoded image (NV12, YUY2), use [`rerun.ImageChromaDownsampled`][].
///
/// See also [components.TensorData] and [datatypes.TensorBuffer].
///
/// \cpp Since the underlying `rerun::datatypes::TensorData` uses `rerun::Collection` internally,
/// \cpp Since the underlying [rerun::components::Blob] uses `rerun::Collection` internally,
/// \cpp data can be passed in without a copy from raw pointers or by reference from `std::vector`/`std::array`/c-arrays.
/// \cpp If needed, this "borrow-behavior" can be extended by defining your own `rerun::CollectionAdapter`.
///
Expand All @@ -36,8 +28,30 @@ table Image (
) {
// --- Required ---

/// The image data. Should always be a 2- or 3-dimensional tensor.
data: rerun.components.TensorData ("attr.rerun.component_required", order: 1000);
/// The raw image data.
data: rerun.components.Blob ("attr.rerun.component_required", order: 1000);

/// The size of the image.
///
/// For chroma downsampled formats, this is the size of the full image (the luminance channel).
resolution: rerun.components.Resolution2D ("attr.rerun.component_required", order: 1200);

// --- Image format ---

/// Used mainly for chroma downsampled formats and differing number of bits per channel.
///
/// If specified, this takes precedence over both [components.ColorModel] and [components.ChannelDatatype] (which are ignored).
pixel_format: rerun.components.PixelFormat ("attr.rerun.component_optional", nullable, order: 2000);

/// L, RGB, RGBA, …
///
/// Also requires a [components.ChannelDatatype] to fully specify the pixel format.
color_model: rerun.components.ColorModel ("attr.rerun.component_optional", nullable, order: 2100);

/// The data type of each channel (e.g. the red channel) of the image data (U8, F16, …).
///
/// Also requires a [components.ColorModel] to fully specify the pixel format.
datatype: rerun.components.ChannelDatatype ("attr.rerun.component_optional", nullable, order: 2200);

// --- Optional ---

Expand Down
Loading

0 comments on commit 102a226

Please sign in to comment.