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

Split Image visualizer into Image/DepthImage/SegmentationImage #6654

Merged
merged 14 commits into from
Jun 28, 2024

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Jun 26, 2024

What

Lots of copy & paste between the three visualizers left. Didn't want to remove it entirely to not make it too hard to have them diverge more in the future. But thanks to some clean-up passes it's not too bad (imho).

image
image
image

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 the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!

To run all checks from main, comment on the PR with @rerun-bot full-check.

@Wumpf Wumpf added 📺 re_viewer affects re_viewer itself exclude from changelog PRs with this won't show up in CHANGELOG.md labels Jun 26, 2024
Copy link

github-actions bot commented Jun 26, 2024

Deployed docs

Commit Link
0db2e98 https://landing-fstt216ct-rerun.vercel.app/docs

Copy link
Member

@jleibs jleibs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like how this worked out!

I agree the duplication is generally clearer than the mega-visualizer.

One observation (not actionable yet): the 2D view of the depth image visualizer still feels slightly strange to me, since it basically falls back to acting like a normal image and depth-meter, fill ratio, etc. stop being applicable. I'm still wondering if ther'es an opportunity to have the 2D view end up using the normal Image visualizer in this case (once we support colormaps in all images)

crates/re_space_view_spatial/src/ui.rs Outdated Show resolved Hide resolved
@Wumpf Wumpf added the do-not-merge Do not merge this PR label Jun 28, 2024
@Wumpf
Copy link
Member Author

Wumpf commented Jun 28, 2024

the 2D view of the depth image visualizer still feels slightly strange to me

💯 felt the same about it!

Base automatically changed from andreas/fix-draworder-and-make-it-editable to main June 28, 2024 09:12
@Wumpf Wumpf force-pushed the andreas/split-image-visualizer branch from 8735ef9 to 99a4fd8 Compare June 28, 2024 09:16
@Wumpf Wumpf removed the do-not-merge Do not merge this PR label Jun 28, 2024
@Wumpf Wumpf merged commit 4dada3c into main Jun 28, 2024
39 of 40 checks passed
@Wumpf Wumpf deleted the andreas/split-image-visualizer branch June 28, 2024 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude from changelog PRs with this won't show up in CHANGELOG.md 📺 re_viewer affects re_viewer itself
Projects
None yet
2 participants