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

Panic when editor camera 2d viewport does not fit inside of bounds #90

Open
Dokkae6949 opened this issue Oct 18, 2024 · 3 comments
Open

Comments

@Dokkae6949
Copy link
Contributor

PR #88 introduced a crash by using the Aabb2d::shrink method since that is not safe to use and will panic if the viewport does not fit inside the bounds. The PR removed the old custom implementation of the shrink method which removed the panic and handled it manually. This needs to be reverted.

Here the change in question

@Dokkae6949
Copy link
Contributor Author

Also we might want the tests which were removed in the PR #88 to come back 👀

@tim-blackbird
Copy link
Contributor

tim-blackbird commented Oct 18, 2024

Can you reproduce the panic with the current code?
constrain_proj_scale should prevent Aabb2d::shrink from ever panicking, if it does ever panic that means there's a bug in constrain_proj_scale and I'd rather fix that instead.

Sorry, I removed the tests because I wasn't confident they were correct, though I have to admit I didn't look at them closely.
The zoom to cursor feature was not working right with the function (clamp_area_to_bound) those tests were for, which is why I removed the tests.

I'm currently looking at bevy_editor_cam and seeing if we can make use of that for both 3D and 2D

@Dokkae6949
Copy link
Contributor Author

If you didn't update anything since I opened the issue then yes the panic happens whenever you zoom out too much every time. So the zoom limit function is broken. About the test they were at least correct for the old code. But I changed a lot of the logic from the external crate and simplified it / fixed weird issues so the tests might not be correct for the code now. Or the code now is not correct. (I assume both are probably not correct xD)

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

No branches or pull requests

2 participants