Skip to content

Commit

Permalink
Sanity-check that an archetype has a component (#7330)
Browse files Browse the repository at this point in the history
### What
We're starting to build in a lot of assumptions in our code about what
archetypes has what components.
This happens here and there where we query for components directly.

At first I added the checks at runtime using reflection, before I
realized we can do it at compile-time instead:

```rs
    fn _check_encoded_image_has_blob(image: &archetypes::EncodedImage) {
        let _: components::Blob = image.blob;
    }
```

This means we will be reminded to update the code if we ever change the
archetype to e.g. use a `EncodedImageBuffer` component or similar in the
future!

### 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/7188?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/7188?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

- [PR Build Summary](https://build.rerun.io/pr/7188)
- [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`.
  • Loading branch information
emilk authored Sep 16, 2024
1 parent ecb20e7 commit a45b13c
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 3 deletions.
47 changes: 47 additions & 0 deletions crates/store/re_types_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,3 +144,50 @@ pub mod external {
pub use arrow2;
pub use re_tuid;
}

/// Useful macro for staticlly asserting that a `struct` contains some specific fields.
///
/// In particular, this is useful to statcially check that an archetype
/// has a specific component.
///
/// ```
/// # #[macro_use] extern crate re_types_core;
/// struct Data {
/// x: f32,
/// y: String,
/// z: u32,
/// }
///
/// static_assert_struct_has_fields!(Data, x: f32, y: String);
/// ```
///
/// This will fail to compile because the type is wrong:
///
/// ```compile_fail
/// # #[macro_use] extern crate re_types_core;
/// struct Data {
/// x: f32,
/// }
///
/// static_assert_struct_has_fields!(Data, x: u32);
/// ```
///
/// This will fail to compile because the field is missing:
///
/// ```compile_fail
/// # #[macro_use] extern crate re_types_core;
/// struct Data {
/// x: f32,
/// }
///
/// static_assert_struct_has_fields!(Data, nosuch: f32);
/// ```
///
#[macro_export]
macro_rules! static_assert_struct_has_fields {
($strct:ty, $($field:ident: $field_typ:ty),+) => {
const _: fn(&$strct) = |s: &$strct| {
$(let _: &$field_typ = &s.$field;)+
};
}
}
19 changes: 18 additions & 1 deletion crates/viewer/re_data_ui/src/instance_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use re_types::{
archetypes, components,
datatypes::{ChannelDatatype, ColorModel},
image::ImageKind,
Archetype, ComponentName, Loggable,
static_assert_struct_has_fields, Archetype, ComponentName, Loggable,
};
use re_ui::{ContextExt as _, UiExt as _};
use re_viewer_context::{
Expand Down Expand Up @@ -251,6 +251,23 @@ fn preview_if_image_ui(
entity_path: &re_log_types::EntityPath,
component_map: &IntMap<ComponentName, UnitChunkShared>,
) -> Option<()> {
// First check assumptions:
static_assert_struct_has_fields!(
archetypes::Image,
buffer: components::ImageBuffer,
format: components::ImageFormat
);
static_assert_struct_has_fields!(
archetypes::DepthImage,
buffer: components::ImageBuffer,
format: components::ImageFormat
);
static_assert_struct_has_fields!(
archetypes::SegmentationImage,
buffer: components::ImageBuffer,
format: components::ImageFormat
);

let image_buffer = component_map.get(&components::ImageBuffer::name())?;
let buffer_row_id = image_buffer.row_id()?;
let image_buffer = image_buffer
Expand Down
12 changes: 10 additions & 2 deletions crates/viewer/re_space_view_spatial/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,12 @@ use re_space_view::DataResultQuery as _;
use re_viewer_context::{ImageDecodeCache, ViewContext, ViewerContext};

use re_renderer::RenderContext;
use re_types::components::{Color, MediaType, Resolution};
use re_types::{blueprint::components::BackgroundKind, components::ImageFormat};
use re_types::{
archetypes,
blueprint::components::BackgroundKind,
components::{self, Color, ImageFormat, MediaType, Resolution},
static_assert_struct_has_fields,
};
use re_viewport_blueprint::{ViewProperty, ViewPropertyQueryError};

mod view_kind {
Expand All @@ -57,6 +61,10 @@ fn resolution_of_image_at(
query: &re_chunk_store::LatestAtQuery,
entity_path: &re_log_types::EntityPath,
) -> Option<Resolution> {
// First check assumptions:
static_assert_struct_has_fields!(archetypes::Image, format: components::ImageFormat);
static_assert_struct_has_fields!(archetypes::EncodedImage, blob: components::Blob);

let db = ctx.recording();

if let Some((_, image_format)) = db.latest_at_component::<ImageFormat>(entity_path, query) {
Expand Down

0 comments on commit a45b13c

Please sign in to comment.