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

Planar projection camera #45

Merged
merged 4 commits into from
Dec 13, 2024
Merged

Conversation

thatcomputerguy0101
Copy link
Contributor

Revision of #42 to not depend on the zooming functionality of #40. Keeping the near and far planes on the correct side of the focal point is tricky with this logic, resulting in the additional check and adjustment in set_planar_projection. Reimplementing this projection using the Viewer trait externally would allow for the alternative zoom logic to make that check unnecessary. However, that solution would make the planar projection incompatible with the built-in control samples.

Copy link
Owner

@asny asny left a comment

Choose a reason for hiding this comment

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

Looks good except the change in screen2ray 👇

src/camera.rs Outdated Show resolved Hide resolved
@thatcomputerguy0101
Copy link
Contributor Author

To resolve the problem that led to the focal point check, maybe the near and far plane distances should be marked as either absolute or relative, where absolute is the current behavior and relative gets multiplied by the zoom factor (matching how the old zoom refactor effectively handled the plane distances). This reduces the complexity of the planar projection by making the relative mode the recommended mode, but increases the complexity of the perspective projection which would need to be recalculated if either depth uses relative mode.

@thatcomputerguy0101
Copy link
Contributor Author

Regarding the absolute/relative (world/camera?) distance units, does it make more sense to force z_near and z_far to share the same unit, or should the units be individually specified?

@asny
Copy link
Owner

asny commented Dec 13, 2024

I think this is fine, I still think using a planar camera is a bit of an edge case, so I doubt if a lot of people care. I think the check in set_planar_projection is not super nice, but it's also not a huge issue IMO. I think it's more important to keep it simple and still use absolute values for z_near and z_far.

@asny asny merged commit 23dec6f into asny:main Dec 13, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants