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

C++: Failure to serialize rotations (code-examples/transform3d_simple) #3865

Closed
Tracked by #2919
emilk opened this issue Oct 16, 2023 · 3 comments · Fixed by #3935
Closed
Tracked by #2919

C++: Failure to serialize rotations (code-examples/transform3d_simple) #3865

emilk opened this issue Oct 16, 2023 · 3 comments · Fixed by #3935
Assignees
Labels
🌊 C++ API C/C++ API specific
Milestone

Comments

@emilk
Copy link
Member

emilk commented Oct 16, 2023

Repro: ./docs/code-examples/roundtrips.py transform3d_simple

This might be related to why just cpp-test fails

@emilk emilk mentioned this issue Oct 16, 2023
33 tasks
@emilk emilk changed the title Failure to serialize rotations (code-examples/transform3d_simple) C++: Failure to serialize rotations (code-examples/transform3d_simple) Oct 16, 2023
@emilk emilk added the 🌊 C++ API C/C++ API specific label Oct 16, 2023
@emilk emilk self-assigned this Oct 17, 2023
@emilk
Copy link
Member Author

emilk commented Oct 17, 2023

It is specifically rotations that fail to serialize - scales work.

    rec.log(
        "base/rotated",
        rerun::Transform3D(
            rrd::RotationAxisAngle({0.0f, 0.0f, 1.0f}, rrd::Angle::radians(TAU / 8.0f))
        )
    );

I wonder if this is a case of nested enums gone awry - rerun::datatypes::Transform3D is an enum, and so is rerun::datatypes::Rotation3D.

@Wumpf
Copy link
Member

Wumpf commented Oct 17, 2023

scales are also enums though, no? 🤔

@emilk emilk removed their assignment Oct 18, 2023
@emilk
Copy link
Member Author

emilk commented Oct 18, 2023

My printf-debugging yesterday showed me that the serialization code for the Rotation is being called, but the rotation doesn't show up when you just print output.rrd

@Wumpf Wumpf self-assigned this Oct 18, 2023
@Wumpf Wumpf added this to the 0.10 C++ milestone Oct 18, 2023
Wumpf added a commit that referenced this issue Oct 20, 2023
### What

* Fixes #3865

It seems this was caused by undefined behavior via calling an assignment
operator on uninitialized memory!

Bulldozered the entire responsible codegen section over to make it super
simple :)
(which we can do now in part because recent std::array use - should have
done so from the start, makes things way simpler!)

### 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/3935) (if
applicable)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/3935)
- [Docs
preview](https://rerun.io/preview/b0a17e547eb59de9f2be8982e8ae1d01ca15afb5/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/b0a17e547eb59de9f2be8982e8ae1d01ca15afb5/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌊 C++ API C/C++ API specific
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants