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

Only grow 3D view bounds with projected image if image plane has been user-edited #3749

Merged
merged 2 commits into from Oct 10, 2023

Conversation

jleibs
Copy link
Member

@jleibs jleibs commented Oct 9, 2023

What

Resolves: #3728

Previously if just logging a pinhole, the projected image rectangle would cause the accumulated bounding box to increase, but then the heuristic would cause it to shrink again.

This changes that behavior to only happen in the event that the image plane has been user-edited (in which case the heuristic no longer applies). This gives us somewhat of the best of both worlds where the bounds can't impact themselves, but if a user increases them manually then the bounding box will still grow appropriately.

Before:
image

After:
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 demo.rerun.io (if applicable)
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG

@jleibs jleibs changed the title Only update view bounds with image if image plane has been edited Only grow 3d view bounds with projected image if image plane has been user-edited Oct 9, 2023
@jleibs jleibs added 📺 re_viewer affects re_viewer itself include in changelog labels Oct 9, 2023
@jleibs jleibs marked this pull request as ready for review October 9, 2023 23:41
@jleibs jleibs added this to the 0.9.1 milestone Oct 10, 2023
// Only update the bounding box if the image_plane_distance is not auto.
// This is because in auto mode the bounds will change the image plane heuristic,
// and there's no good reason to grow the size of the bounds only to the shrink
// the image plane as a result.
Copy link
Member

Choose a reason for hiding this comment

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

More to the point: it causes a cyclic relationship, where the auto-distance depend on the bounding box, and the bounding box is affected by the distance

Copy link
Member Author

Choose a reason for hiding this comment

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

It's only lightly cyclic though -- since net result is to shrink the plane it doesn't continue the cycle.

@jleibs jleibs merged commit 9ad4f4e into main Oct 10, 2023
30 checks passed
@jleibs jleibs deleted the jleibs/pinhole_bounds branch October 10, 2023 11:59
@emilk emilk changed the title Only grow 3d view bounds with projected image if image plane has been user-edited Only grow 3D view bounds with projected image if image plane has been user-edited Oct 12, 2023
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.

Image plane immediately shrinks to too small of a value for pinhole cameras.
3 participants