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

2D visualizers are no longer applicable to paths that are behind a pinhole camera other than the space's origin #4728

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Jan 8, 2024

What

Fixes broken heuristics for structure_from_motion.

This is a bit of a hacky addition. We need to solve this with a proper SpatialTopology data structure that can tell us efficiently if we're inside a 2d/3d space and how far it reaches.

See also

image

picture showing that a 2D space at camera is no longer suggested since it wouldn't be able to show anything: camera/image has a pinhole projection, therefore camera -> camera/pinhole is a 3D->2D projection but by having a 2D space at camera we implied camera to have an eye pinhole.

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

@Wumpf Wumpf added 🪳 bug Something isn't working exclude from changelog PRs with this won't show up in CHANGELOG.md labels Jan 8, 2024
pub has_pinhole_at_origin: bool,

/// All subtrees that are invalid since they're behind a pinhole that's not at the origin.
pub invalid_subtrees: Vec<EntityPath>,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pub invalid_subtrees: Vec<EntityPath>,
pub nested_pinholes: Vec<EntityPath>,

Copy link
Member

Choose a reason for hiding this comment

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

I retract this suggestion -- there are other things that could be invalid

@Wumpf
Copy link
Member Author

Wumpf commented Jan 8, 2024

checked performance on python ./examples/python/open_photogrammetry_format/main.py --no-frames: It's not free, but ~0.5ms while generally pretty bad don't make much of a dent here and we already know how to make this significantly better

@Wumpf Wumpf merged commit 0f7216e into main Jan 8, 2024
42 of 44 checks passed
@Wumpf Wumpf deleted the andreas/fix-double-pinhole-issue branch January 8, 2024 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪳 bug Something isn't working exclude from changelog PRs with this won't show up in CHANGELOG.md
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants