Skip to content

Tensor & depth image value ranges can now be configured, from UI & code#7549

Merged
Wumpf merged 22 commits intomainfrom
andreas/tensor-data-range
Oct 1, 2024
Merged

Tensor & depth image value ranges can now be configured, from UI & code#7549
Wumpf merged 22 commits intomainfrom
andreas/tensor-data-range

Conversation

@Wumpf
Copy link
Copy Markdown
Member

@Wumpf Wumpf commented Sep 30, 2024 •
edited
Loading

What

Depth images (and their clouds):

depth.image.range.editing.mov

Tensors:

tensor_data_range.mov

Noteworth ripple effects & considerations in this PR:

  • turns out depth image constructor on Python missed a lot of parameters, fixed that
  • went with a new ValueRange component rather than reusing Range1D because the semantics (and names in the ui) of the later were just too unclear
    • we need a tagged component mechanism really badly!
    • decided against putting this on the tensor view as some notes indicated was the plan
  • ended up refactoring a few interesting things around image/tensor handling:
    • colormap is no longer part of ImageInfo, we already didn't use it for hashing it. Having now a range made it even more clear that this doesn't belong there
    • finite_range on tensor & image is now always present, using the datatypes' finite range as fallback. The None case handling was awkward at best and non-existant (error out) usually

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!
  • If have noted any breaking changes to the log API in CHANGELOG.md and the migration guide

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

Wumpf and others added 11 commits September 30, 2024 10:37
@Wumpf Wumpf added 📺 re_viewer affects re_viewer itself 🟦 blueprint The data that defines our UI include in changelog 🪵 Log & send APIs Affects the user-facing API for all languages labels Sep 30, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Sep 30, 2024 •
edited
Loading

Deployed docs

Commit Link
90eb9fd https://landing-do0mnbv92-rerun.vercel.app/docs

@emilk emilk self-requested a review October 1, 2024 09:00
emilk
emilk reviewed Oct 1, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Very good!

My only concern is that we don't end up with a bad estimate and then stick with it. In particular, what is the estimated range of an image of a new space view, created when the time cursor is before the image was logged? Maybe it doesn't run the fallback/estimator in that case, and all is good?

emilk
emilk approved these changes Oct 1, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Very good!

My only concern is that we don't end up with a bad estimate and then stick with it. In particular, what is the estimated range of an image of a new space view, created when the time cursor is before the image was logged? Maybe it doesn't run the fallback/estimator in that case, and all is good?

@Wumpf
Copy link
Copy Markdown
Member Author

Wumpf commented Oct 1, 2024 •
edited
Loading

My only concern is that we don't end up with a bad estimate and then stick with it. In particular, what is the estimated range of an image of a new space view, created when the time cursor is before the image was logged? Maybe it doesn't run the fallback/estimator in that case, and all is good?

The behavior shouldn't have changed to before: If no value is present, we evaluate the fallback and fallbacks are not sticky - they get evaluated every frame. This means that we'll always grab the image stats that are available for that frame and those (other than my change regarding how finite_range is defined) are unchanged to before. Which should give us the same behavior.
Notably, this also means that until someone either logs or overrides an image range, they will still get flickering behavior if their depth image or tensor range changes over time.

not 100% sure that answers the question - does it? :)

@emilk
Copy link
Copy Markdown
Member

emilk commented Oct 1, 2024

Oh, perfect answer - thank you!

@Wumpf Wumpf merged commit c4eb805 into main Oct 1, 2024
@Wumpf Wumpf deleted the andreas/tensor-data-range branch October 1, 2024 13:09
@emilk emilk changed the title Tensor & depth image value ranges can now be configured (from ui & code) Tensor & depth image value ranges can now be configured, from UI & code Oct 16, 2024
@rasmusgo rasmusgo mentioned this pull request Oct 18, 2024
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@emilk emilk emilk approved these changes

Assignees

No one assigned

Labels

🟦 blueprint The data that defines our UI include in changelog 🪵 Log & send APIs Affects the user-facing API for all languages 📺 re_viewer affects re_viewer itself

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Adjustable depth image range Allow to specify a tensor's data range via component

2 participants

@Wumpf @emilk