Improve log_pinhole and support non-RDF pinholes

[emilk]

Closes #2244, closes #2589

• ViewCoordinates are not working properly for camera frustums #2244
• Specify Pinhole without an explicit matrix #2589

What

log_pinhole is now much easier to use. You only need to set focal length and optional principal point instead of setting the full 3x3 matrix.

There is also a new argument to it camera_xyz for setting the coordinate system. The default is RDF (the old default). This affects the visible camera frustum, how rays are projected when hovering a 2D image, and how depth clouds are projected.

There is also a bunch of bug fixes along the way, including an accurate camera frustum which works for non-centered principal points.

This simplifies some examples, especially examples/python/open_photogrammetry_format/main.py which uses RUB.

Tests

There is a new test: tests/python/view_coordinates/main.py creates a bunch of pinholes with RGB and Depth.

I've also run

Checklist

• I have read and agree to Contributor Guide and the Code of Conduct
• I've included a screenshot or gif (if applicable)
• I have tested demo.rerun.io (if applicable)

• PR Build Summary
• Docs preview
• Examples preview

[jleibs]

Is there a hard constraint forcing this, or do we have the option to (in the future) make the default view coordinates of re_renderer RDF instead? This seems like it would cut down on some of the book-keeping around whether a camera is a "real camera" vs a "re-renderer camera." Having one standard set of conventions for cameras would reduce ambiguity, and given that we position ourselves as a robotics-and-computer-vision product, I think RDF is the right choice here.

[emilk]

I agree it would be nice to use RDF as the default everywhere. We could probably switch re_renderer with some work, though it should be noted that RDF is quite unusual for rendering. @Wumpf can probably add some more nuance here.

[jleibs]

Suggested change

	// View-coordinates are relevant here because without them we have no notion of what the image plane is.
	// (it's not a given that e.g. XY is the camera image plane!)
	// First transform to the "standard RUB" 3D camera and then from there to the image plane coordinate system.
	// The pinhole view-coordinates are important here because they define how the image plane is aligned
	// with the camera coordinate system. It is not a given that a user wants the image-plane aligned with the
	// XY-plane in camera space.
	//
	// Because the [`Pinhole`] component currently assumes an input in the default `image_view_coordinates`
	// we need to pre-transform the data from the user-defined `pinhole_view_coordiantes` to the required
	// `image_view_coordinates`. We do this by converting the points into and out of RUB.
	//
	// TODO(...): When Pinhole is an archetype instead of a component, `pinhole.project` should do this
	// internally.

gripe: still don't love RUB being the "standard" here.

[emilk]

a.from_rub() * b.to_rub() would be equivalent to a.from_rdf() * b.to_rdf(). I'd be happy to change to the latter in most places. We could even though a.from_other(b) to not even have an inbetween step

[emilk]

Done in 62fad9c

[jleibs]

Can't wait for Archetype-queries. Would be much less error-prone to have this be an associated component of the Pinhole archetype. Then we could account for it inside of Pinhole.project instead of needing to pre-condition the data before calling the project function.

[emilk]

Yeah, it would be much nicer. I also look forward splitting ViewCoordinates into several different components

[jleibs]

Why is this an empty file...?

[emilk]

The file isn't empty - the diff is. Or rather: the diff is that I did fd main.py | xargs chmod +x because I don't have the patience of writing python in the terminal when running scripts :)

[jleibs]

did you want to add a "camera_xyz" here?

[emilk]

I'm not sure.

We can either add camera_xyz=RDF to all examples to be explicit. This could either be taken as an encouragement to users to use RDF (good), or as an invitation to change it to something else (perhaps less good).

If we instead omit it when not needed it could lead to questions of what the default is and how to change it.

[jleibs]

This definition of "re-oriented" is vague as it doesn't do a great job of describing what happens to the direction of pixel-x and pixel-y directions. I believe in essence this is more of "remapping" the axes than re-orienting (which will be important once we allow right-to-left conversions).

I believe the actual definition of this is:

• Whichever axis is labeled F will be the positive projection direction (B means negative projection values are "into" the image).
• Whichever axis is labeled R is positive pixel-x (increasing column index). L means we flip the image horizontally.
• Whichever axis is labeled D is the positive pixel-y (increasing row index). U means we flip the image vertically.

[jleibs]

I guess L / U doesn't actually mean we flip the image exactly... but we flip the indexing of the image. So increasing axis = drecreasing pixel-x from "right" to "left" across the image (which is technically the same as increasing pixel coordinates across a flipped copy of the image).

[emilk]

I've expanded the docstring

[emilk]

Ok, I think this is good enough now.

I'm not 100% married to this idea of implicit transforms, but I think the overall code is a lot clearer now, and changing to another approach will yield a much smaller diff.

In particular we have:

• A great test
• Improved log_pinhole ui
• An easy-to-use interface for when we don't want to use RDF (e.g. the open_photogrammetry_format example)
• Much more explicit Rust code
• Encouraging users to use the camera_xyz argument of log_pinhole instead of using the free-standing log_view_coordinates function.
• Simpler and bug-free frustum code