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

Make all C++ archetypes eager serialized & provide generated update/clear APIs #8779

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Jan 22, 2025

Related

What

(see title)
On every archetype in C++ you can now use update_fields & clear_fields!

Commit by commit recommended - most of them are trivial! Outstanding are:

  • tensor
  • video_asset
    • needed to extract video data & media type from the serialized arrow data for a utility method. Nothing too scary, and no large copies involved
  • clear
    • needed a new cpp attribute to suppress the =default constructor as this one has its own (previous codegen wasn't as rigorous with default ctors!)

  • pass complete snippet comparison run locally
  • please the different compiler gods and their very individual preferences
  • succeed main ci

@Wumpf Wumpf force-pushed the andreas/cpp/more-eager branch from a79a4f3 to 728cf02 Compare January 22, 2025 15:21
@Wumpf
Copy link
Member Author

Wumpf commented Jan 22, 2025

image
got stuck... what happens if I make this ready for review and then draft it again.

@Wumpf Wumpf marked this pull request as ready for review January 22, 2025 15:30
@Wumpf Wumpf marked this pull request as draft January 22, 2025 15:31
@Wumpf Wumpf force-pushed the andreas/cpp/more-eager branch from 728cf02 to 95cd613 Compare January 22, 2025 15:33
@Wumpf
Copy link
Member Author

Wumpf commented Jan 22, 2025

nope, only pushing a new commit fixed it.

Copy link

github-actions bot commented Jan 22, 2025

Web viewer built successfully. If applicable, you should also test it:

  • I have tested the web viewer
Result Commit Link Manifest
a9f8004 https://rerun.io/viewer/pr/8779 +nightly +main

Note: This comment is updated whenever you push a commit.

@Wumpf Wumpf force-pushed the andreas/cpp/more-eager branch 2 times, most recently from 5335d4e to 549d4b6 Compare January 22, 2025 15:44
@Wumpf Wumpf force-pushed the andreas/cpp/more-eager branch 2 times, most recently from a6ae1e6 to 63f5486 Compare January 22, 2025 16:23
@Wumpf Wumpf force-pushed the andreas/cpp/more-eager branch from 63f5486 to 60da8d4 Compare January 22, 2025 16:40
@Wumpf Wumpf force-pushed the andreas/cpp/more-eager branch from 60da8d4 to a9f8004 Compare January 22, 2025 17:06
@Wumpf
Copy link
Member Author

Wumpf commented Jan 22, 2025

@rerun-bot full-check

Copy link

Started a full build: https://github.com/rerun-io/rerun/actions/runs/12913616811

@Wumpf Wumpf marked this pull request as ready for review January 22, 2025 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rerun::Collection borrows data too eagerly, making it very easy to cause segfaults & read of invalid data
1 participant