Fix 3D zoom wheel distance formula #60778
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🪟 Windows buildsDownload Windows builds of this PR for testing. 🪟 Windows Qt6 buildsDownload Windows Qt6 builds of this PR for testing. |
Tests failed for Qt 5One or more tests failed using the build from commit e5407c5 depth_wheel_action_4 (testDepthBuffer)depth_wheel_action_4Test failed at testDepthBuffer at tests/src/3d/testqgs3drendering.cpp:2169 Rendered image did not match tests/testdata/control_images/3d/expected_depth_wheel_action_4/expected_depth_wheel_action_4.png (found 648 pixels different) The full test report (included comparison of rendered vs expected images) can be found here. Further documentation on the QGIS test infrastructure can be found in the Developer's Guide. |
Tests failed for Qt 6One or more tests failed using the build from commit e5407c5 depth_wheel_action_2 (testDepthBuffer)depth_wheel_action_2Test failed at testDepthBuffer at tests/src/3d/testqgs3drendering.cpp:2135 Rendered image did not match tests/testdata/control_images/3d/expected_depth_wheel_action_2/expected_depth_wheel_action_2.png (found 9 pixels different) The full test report (included comparison of rendered vs expected images) can be found here. Further documentation on the QGIS test infrastructure can be found in the Developer's Guide. |
|
@dvdkon Peek.2025-02-28.09-31.mp4 |
Hm, I can't reproduce this. You're testing with DEM terrain, right? Could you please film a new video with the debug pane open? I'd like to see what the coordinates look like in the blanked sections. Maybe a minimum distance to the zoom point could fix this. Candidate patch follows: diff --git a/src/3d/qgscameracontroller.cpp b/src/3d/qgscameracontroller.cpp
index 9efedd3caf0..bf1cc495abc 100644
--- a/src/3d/qgscameracontroller.cpp
+++ b/src/3d/qgscameracontroller.cpp
@@ -524,6 +524,11 @@ void QgsCameraController::handleTerrainNavigationWheelZoom()
double oldDist = ( mZoomPoint - mCameraBefore->position() ).length();
// Each step of the scroll wheel decreases distance by 20%
double newDist = std::pow( 0.8, mCumulatedWheelY ) * oldDist;
+ if ( newDist < 1 )
+ {
+ // Make sure we don't clip the thing we're zooming to.
+ newDist = 1;
+ }
double zoomFactor = newDist / oldDist;
zoomCameraAroundPivot( mCameraBefore->position(), zoomFactor, mZoomPoint ); |
|
@dvdkon unfortunately that patch didn't help. I added some extra debug code, and here's what I see when the quick zoom glitches out: Heres' what I see in a normal slow wheel zoom: Debug panel before glitchy zoom: after: |
|
Thanks for the debug logs @nyalldawson. It looks like you somehow haven't applied this PR? The |
The previous distance coefficient would go negative for high enough accumulated values, throwing the user somewhere far below the terrain. The expected behaviour here is that successive scroll wheel events, accumulated or not, bring the camera closer to the zoom point, but never beyond.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes an issue with the 3D camera controller identified by #60700, where multiple quick scroll wheel movements would result in jumping to some unexpected position. The previous distance coefficient would go negative for high enough accumulated values, throwing the user somewhere far below the terrain.
I feel the need to point out that this issue wasn't caused by #60246. It only became more prominent, because previously the performance of handling input events was so abysmal, that enough scroll wheel travel usually didn't accumulated before a frame.