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

Transform3D + DisconnectedTransform migration to archetypes #2791

Closed
12 of 16 tasks
Tracked by #2795
teh-cmc opened this issue Jul 24, 2023 · 0 comments · Fixed by #2940
Closed
12 of 16 tasks
Tracked by #2795

Transform3D + DisconnectedTransform migration to archetypes #2791

teh-cmc opened this issue Jul 24, 2023 · 0 comments · Fixed by #2940
Assignees
Labels
🏹 arrow concerning arrow 🌊 C++ API C/C++ API specific codegen/idl 🐍 Python API Python logging API 🦀 Rust API Rust logging API

Comments

@teh-cmc
Copy link
Member

teh-cmc commented Jul 24, 2023

Turn into sub-issues as needed.

  • Python support
    • Extensions
    • Unit test (constructors, serialization)
    • Doc example
  • Rust support
    • Extensions
    • Unit test (constructors, rust-to-rust roundtrip)
    • Doc example
  • C++ support
    • Extensions
    • Unit test (constructors, serialization)
    • Doc example
  • Cross-language roundtrip test
  • Remove old components
    • Clean up re_components
    • Port viewer to use new types and queries
@teh-cmc teh-cmc added 🐍 Python API Python logging API 🏹 arrow concerning arrow 🦀 Rust API Rust logging API codegen/idl 🌊 C++ API C/C++ API specific labels Jul 24, 2023
@teh-cmc teh-cmc self-assigned this Jul 25, 2023
@teh-cmc teh-cmc changed the title Transform3D+Pinhole migration to archetypes Transform3D + DisconnectedTransform migration to archetypes Jul 25, 2023
teh-cmc added a commit that referenced this issue Jul 28, 2023
What the title says.

This does **not** migrate off of the legacy `DisconnectedSpace`, that's
for a future PR.

Part of #2791

### What

### 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 [demo.rerun.io](https://demo.rerun.io/pr/2833) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2833)
- [Docs
preview](https://rerun.io/preview/pr%3Acmc%2Froundtrippable_disconnected_transform/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Acmc%2Froundtrippable_disconnected_transform/examples)
teh-cmc added a commit that referenced this issue Jul 28, 2023
This PR removes the old `DisconnectedSpace` entirely.

The legacy `rr.disconnected_space()` have been rewritten in terms of the
new `DisconnectedSpace` archetype.

---

Part of #2791 
Requires #2833 

### What

### 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 [demo.rerun.io](https://demo.rerun.io/pr/2835) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2835)
- [Docs
preview](https://rerun.io/preview/pr%3Acmc%2Fgoodbye_old_disconnected_space/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Acmc%2Fgoodbye_old_disconnected_space/examples)
teh-cmc added a commit that referenced this issue Jul 31, 2023
This PR annihilates the legacy `Transform3D` family of component and
data types, and deprecates the legacy `VecND` & `MatNxN` types as much
as possible (there are still many other components that we have to
migrate first).

The legacy `log_transform3d` API still exists, but now simply delegates
to the new `Transform3D` archetype.

---

This PR also includes some improved error reporting on the
deserialization path, as well as a workaround for `pa.nulls` misbehaving
in some cases, see #2871 for the full story.

---

This PR also removes any traces of datatypes being registered as UI
components, as it historically did not make sense and was never used,
and is simply impossible to do anymore since `Datatype` and `Component`
are now different traits.

Part of #2791

### What

### 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 [demo.rerun.io](https://demo.rerun.io/pr/2846) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2846)
- [Docs
preview](https://rerun.io/preview/pr%3Acmc%2Fgoodbye_old_transform/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Acmc%2Fgoodbye_old_transform/examples)
@teh-cmc teh-cmc added this to the 0.9 milestone Aug 1, 2023
Wumpf added a commit that referenced this issue Aug 4, 2023
)

### What

Adds C++ example build setup and examples for:

* Point3D  #2789
* Arrow3D #2785
* DisconnectedSpace #2791

as before Point2D example is blocked on rects for the moment.

Generating a bunch more methods now for better usability. Still lacking
user extensions though, in particular Point3D construction is poor right
now

Examples can be built in bulk with `
./tests/cpp/build_all_doc_examples.sh`
Then run with `./build/tests/cpp/doc_example_point3d_EXAMPLE_NAME`
They all connect to localhost on default port for now.


Point3D Simple:
<img width="1336" alt="image"
src="https://github.com/rerun-io/rerun/assets/1220815/61ca8dba-690f-4bcf-96d2-25cb010881fb">

Point3D Random:
<img width="1341" alt="image"
src="https://github.com/rerun-io/rerun/assets/1220815/48fa902d-7858-4580-b64b-69815dab47e3">

Arrow3D:
<img width="1360" alt="image"
src="https://github.com/rerun-io/rerun/assets/1220815/131d0efa-af94-418a-b08f-653c1d9eafb0">

Disconnected Space:
<img width="1339" alt="image"
src="https://github.com/rerun-io/rerun/assets/1220815/5da1bb64-984a-4ff5-903a-1ea060dd296a">

Dependent on #2906 

### 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 [demo.rerun.io](https://demo.rerun.io/pr/2908) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2908)
- [Docs
preview](https://rerun.io/preview/pr%3Aandreas%2Fcpp%2Fexamplesv2/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Aandreas%2Fcpp%2Fexamplesv2/examples)
Wumpf added a commit that referenced this issue Aug 7, 2023
### What

Introduces a simple extension system for C++ codegen: Add an extra cpp
file that will be compiled as part of the SDK. A section between two
markes is copied into the generated hpp as part of generation (typically
this section is removed from compilation via #ifdef)

Adds extensions to:
* color
* vec2/vec3/vec4
* quaternion
* origin3d
* point2d
* point3d
* arrow3d

... and uses them to simplify examples and test code!

All fully supported archetypes now have a simple test that checks that
the base interface works and that we can serialize out to arrow without
issues.

Additionally, there's tests for vecN/quaternion/color to check that
their various constructors work as expected (not being a C++ expert it's
fairly hard to predict)

Extends codegen with single-array-field-constructors.


---------

* part of #2647
* Fixes #2798
* Fixes #2785
* Missing only 2D example: #2789
* Adds to #2791 


### 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 [demo.rerun.io](https://demo.rerun.io/pr/2916) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2916)
- [Docs
preview](https://rerun.io/preview/pr%3Aandreas%2Fcpp%2Fcustom-extensions/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Aandreas%2Fcpp%2Fcustom-extensions/examples)
Wumpf added a commit that referenced this issue Aug 9, 2023
…p test (#2937)

Part of:
* #2919 
* #2791

### What

Major facelift of the C++ codegen as it got restructured to support more
support more nested kind of serialization and moved away entirely from
using the arrow type registry at all (this happened originally due to
c&p from Rust without understanding the details; this change makes a lot
of the code a lot simpler!)

In order to test the new nested serialization this also enables the
transform 3d roundtrip test which required adding some extensions on
various datatypes to make the test code readable.
**These are not the transform related final extensions.** More is under
way in a future PR to make transform usable.

The resulting transforms can't yet be loaded in the viewer due to #2871

The only remaining cases that don't produce (presumably until tested)
correct serialization now are:
* nullable component/datatype inside a transparent component/datatype
* lists/vectors inside as a union variant

(they are not particularly hard to solve, just left out of this 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 [demo.rerun.io](https://demo.rerun.io/pr/2937) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2937)
- [Docs
preview](https://rerun.io/preview/pr%3Aandreas%2Fcpp%2Funion-support/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Aandreas%2Fcpp%2Funion-support/examples)
Wumpf added a commit that referenced this issue Aug 9, 2023
### What

* depends on #2937
* part of #2919 
* fixes #2791

Roundtrip test is part of #2937
Does not yet work in viewer because of
#2871

C++ extensions now get additional includes copied into the generated
header

Extensions circle around making use of the `Transform3D` archetype easy.
Note that the willingness of a c++ compiler to convert arrays and floats
to bool causes us quite a lot of extra overloads (luckily the added unit
tests catches these and there are warnings emitted).
The amount of overloads introduced leaves a bit of a bad taste, but the
expectation is that this is rather special to `Transform3D`. Some of the
provided overloads may be generated in the future if a pattern emerges.

A bunch of code example added to demonstrate what it looks like now to
use Transform3D:
```cpp
// translation only
rr_stream.log("translated", rr::artchetypes::Transform3D({1.0f, 0.0f, 0.0f}));

// translation/rotation/uniform-scale
rr_stream.log(
    "rotated_scaled",
    rr::archetypes::Transform3D(
        rrd::RotationAxisAngle({0.0f, 0.0f, 1.0f}, rrd::Angle::radians(pi / 4.0f)),
        2.0f
    )
);

// 3x3 matrix
rec_stream.log(
    "translation_and_mat3x3/rotation",
    rr::archetypes::Transform3D({
        {1.0f, 4.0f, 7.0f},
        {2.0f, 5.0f, 8.0f},
        {3.0f, 6.0f, 9.0f},
    })
);
```

### 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 [demo.rerun.io](https://demo.rerun.io/pr/2940) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2940)
- [Docs
preview](https://rerun.io/preview/pr%3Aandreas%2Fcpp%2Ftransform3d/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Aandreas%2Fcpp%2Ftransform3d/examples)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏹 arrow concerning arrow 🌊 C++ API C/C++ API specific codegen/idl 🐍 Python API Python logging API 🦀 Rust API Rust logging API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant