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.

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

Warn if no resolution provided to Pinhole #3923

Merged
merged 5 commits into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 17 additions & 1 deletion crates/re_space_view_spatial/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ mod ui;
mod ui_2d;
mod ui_3d;

use re_types::components::{Resolution, TensorData};
pub use space_view_2d::SpatialSpaceView2D;
pub use space_view_3d::SpatialSpaceView3D;

Expand All @@ -33,6 +34,20 @@ mod view_kind {
}
}

fn resolution_from_tensor(
store: &re_arrow_store::DataStore,
query: &re_arrow_store::LatestAtQuery,
entity_path: &re_log_types::EntityPath,
) -> Option<Resolution> {
store
.query_latest_component::<TensorData>(entity_path, query)
.and_then(|tensor| {
tensor
.image_height_width_channels()
.map(|hwc| Resolution([hwc[1] as f32, hwc[0] as f32].into()))
})
}

/// Utility for querying a pinhole archetype instance.
///
/// TODO(andreas): It should be possible to convert [`re_query::ArchetypeView`] to its corresponding Archetype for situations like this.
Expand All @@ -48,7 +63,8 @@ fn query_pinhole(
image_from_camera: image_from_camera.value,
resolution: store
.query_latest_component(entity_path, query)
.map(|c| c.value),
.map(|c| c.value)
.or_else(|| resolution_from_tensor(store, query, entity_path)),
camera_xyz: store
.query_latest_component(entity_path, query)
.map(|c| c.value),
Expand Down
35 changes: 20 additions & 15 deletions rerun_py/rerun_sdk/rerun/archetypes/pinhole_ext.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,18 +88,27 @@ def __init__(

# TODO(andreas): Use a union type for the Pinhole component instead ~Zof converting to a matrix here
if image_from_camera is None:
# Resolution is needed for various fallbacks/error cases below.
if resolution is None:
resolution = [1.0, 1.0]
resolution = Vec2D(resolution)
width = cast(float, resolution.xy[0])
height = cast(float, resolution.xy[1])
if resolution is not None:
res_vec = Vec2D(resolution)
width = cast(float, res_vec.xy[0])
height = cast(float, res_vec.xy[1])
else:
width = None
height = None

if focal_length is None:
_send_warning_or_raise("either image_from_camera or focal_length must be set", 1)
focal_length = (width * height) ** 0.5 # a reasonable default
if height is None or width is None:
raise ValueError("Either image_from_camera or focal_length must be set")
else:
_send_warning_or_raise("Either image_from_camera or focal_length must be set", 1)
focal_length = (width * height) ** 0.5 # a reasonable best-effort default

if principal_point is None:
principal_point = [width / 2, height / 2]
if height is not None and width is not None:
principal_point = [width / 2, height / 2]
else:
raise ValueError("Must provide one of principal_point, resolution, or width/height")

if type(focal_length) in (int, float):
fl_x = focal_length
fl_y = focal_length
Expand All @@ -109,17 +118,13 @@ def __init__(
fl_x = focal_length[0] # type: ignore[index]
fl_y = focal_length[1] # type: ignore[index]
except Exception:
_send_warning_or_raise("Expected focal_length to be one or two floats", 1)
fl_x = width / 2
fl_y = fl_x
raise ValueError("Expected focal_length to be one or two floats")

try:
u_cen = principal_point[0] # type: ignore[index]
v_cen = principal_point[1] # type: ignore[index]
except Exception:
_send_warning_or_raise("Expected principal_point to be one or two floats", 1)
u_cen = width / 2
v_cen = height / 2
raise ValueError("Expected principal_point to be one or two floats")

image_from_camera = [[fl_x, 0, u_cen], [0, fl_y, v_cen], [0, 0, 1]] # type: ignore[assignment]
else:
Expand Down
4 changes: 4 additions & 0 deletions rerun_py/tests/unit/test_expected_warnings.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ def test_expected_warnings() -> None:
rr.log("test_transform", rr.TranslationAndMat3x3(translation=[1, 0, 0])), # type: ignore[arg-type]
"Expected an object implementing rerun.AsComponents or an iterable of rerun.ComponentBatchLike, but got",
),
(
rr.log("world/image", rr.Pinhole(focal_length=3)),
"Must provide one of principal_point, resolution, or width/height)",
),
]

assert len(warnings) == len(expected_warnings)
Expand Down
Loading