Fix clamping on zoom out

[MeoMix]

Hello! 👋 Thank you for your plugin. I discovered a bug in it when trying to adopt it and have a proposed fix. I'm very new to Rust/Bevy, so my proposed solution is probably bad and wrong, but hopefully it's at least clear enough to highlight the issue and motivate a proper solution.

Consider the following scenario:

• bevy_pancam plugin is instantiated and configured with min/max clamping
• user zooms in
• user pans to the top
• user zooms out

In this scenario, clamping is not applied when the user zooms out and the clamp bounds are not respected. Then, when the user next clicks, the clamping value is enforced.

Here is a video of this issue reproducing on my system: https://www.loom.com/share/1f963e13433248019f46bfc598528a00

This occurs because the camera_movement system enforces clamping, but it no-ops if no mouse buttons are pressed. Since the user has zoomed out by scrolling the mouse wheel - no mouse buttons are pressed and thus no clamping occurs.

Initially, I tried to fix this by modifying camera_movement. My initial solution was to change query to utilize Ref<OrthographicProjection> and to loosen the no-op conditional to || projection.is_changed(). This solution does work, but introduces a one frame lag which visualizes as slight flickering in the UI.

I did not find this flickering to be an acceptable solution. So, I continued to explore. I then decided that the core of the camera_movement logic should run immediately from the camera_zoom system. I added this code, but found it to no-op because projection.area.size() was not returning an updated value.

So, I dug into Bevy's camera code a bit and copied a snippet from their internal camera_system system:

        if let Some(normalized_target) = camera.target.normalize(primary_window) {
            if normalized_target.is_changed(&changed_window_ids, &changed_image_handles)
                || camera.is_added()
                || camera_projection.is_changed()
                || camera.computed.old_viewport_size != viewport_size
            {
                camera.computed.target_info =
                    normalized_target.get_render_target_info(&windows, &images);
                if let Some(size) = camera.logical_viewport_size() {
                    camera_projection.update(size.x, size.y);
                    camera.computed.projection_matrix = camera_projection.get_projection_matrix();
                }
            }
        }

Note that camera.computed.projection_matrix is private so I'm not able to update it. I do not know if omitting this has unintended side-effects, but I do not believe that to be true because camera_projection.update is public and so I assume calling update is supported.

After calling camera.update, and then immediately running the core camera_movement clamping logic, the bug no longer reproduced.

Here is a video of the issue not reproducing after applying the fix proposed in this PR: https://www.loom.com/share/c9ced63b910c469d85276eedc92f2f0a

[johanhelsing]

Hi, thanks a lot for looking into this, and the detailed write-up. I'm trying to reproduce, so I can follow your reasoning. I tried with the clamped_borders example, but there I do get proper clamping when panning to the bottom and then zooming out... Do you have a wheel with continuous scrolling? Mine is discrete?

Or perhaps it's something about the example?

[johanhelsing]

Sorry, but I don't understand what the issue is... please re-open if/when it's still an issue you have the time to explain.

[MeoMix]

@johanhelsing I opened a couple of other issues and tagged you on 'em, that should clarify.

[johanhelsing]

Oh, sorry. Somehow i missed that there were related issues. I'll have a look when i get the time