Bug report #11811
rotation regression: zoom to mouse cursor using mouse wheel is broken
Status: | Closed | ||
---|---|---|---|
Priority: | Severe/Regression | ||
Assignee: | Sandro Santilli | ||
Category: | - | ||
Affected QGIS version: | master | Regression?: | No |
Operating System: | Easy fix?: | No | |
Pull Request or Patch supplied: | No | Resolution: | fixed/implemented |
Crashes QGIS or corrupts data: | No | Copied to github as #: | 20035 |
Description
The map rotation feature commit introduced a serious regression. When zooming in and out using the mouse wheel over the main canvas, it fails to properly center the zoomed area to mouse cursor.
Steps to reproduce:- Make sure that the mouse wheel action setting is set to "Zoom to mouse cursor" (in the Options' Map Tools panel)
- Open a project
- Place your mouse cursor at a corner edge of the map canvas, and zoom using the mouse wheel
- Notice how the zoomed area does not zoom to mouse cursor
Setting the priority to blocker as this is a regression.
Related issues
History
#1 Updated by Sandro Santilli almost 10 years ago
Actually it works correctly for me, what could I be missing ? I'm testing with 2 layers, a vector one and a raster one. The raster is "band3_int15_noct_epsg4326" from the testuite, the vector is just two lines on the diagonals of that raster.
I can zoom out and place the tiny square over any of the corners and then use the mouse wheel to zoom in while the cursor is on the same corner of the raster to get it fill the viewport and back. Settings Options Map Tool Mouse wheel action is set to "Zoom to mouse cursor". This is as of commit ce8a9ba
#2 Updated by Richard Duivenvoorde almost 10 years ago
I cannot reproduce this too. I've tried it with current master and qgis 2.6 here. To me they behave the same?
#3 Updated by Matthias Kuhn almost 10 years ago
Also not confirmed here...
#4 Updated by Mathieu Pellerin - nIRV almost 10 years ago
- File zoom_error.mp4 added
Here's a video recording of the regression. The map canvas does actually get zoomed to the mouse cursor, the regression seems to have to do with the cache drawing while zooming in / out (i.e. the canvas drawing gets shifted to left / right prior to re-rendering). It's a bit hard to explain, hoping the attached video will make it easier.
#5 Updated by Richard Duivenvoorde almost 10 years ago
yes, I have seen that behaviour, that it is not exactly centering on your cursor, but I also tried it in 2.4 and 2.2 and to me the behaviour looked exactly the same. I know the zoomin (double click) recenters the canvas, but I'm not sure that the mousewheel has done this before?
So plz check with older versions to be sure it is a regression (I think it is not).
If it is not we can off course create a feature request, it seems a logical choice to let mousewheel also recenter to cursor.
#6 Updated by Giovanni Manghi almost 10 years ago
- File zoom_master.mp4 added
Richard Duivenvoorde wrote:
yes, I have seen that behaviour, that it is not exactly centering on your cursor, but I also tried it in 2.4 and 2.2 and to me the behaviour looked exactly the same. I know the zoomin (double click) recenters the canvas, but I'm not sure that the mousewheel has done this before?
So plz check with older versions to be sure it is a regression (I think it is not).If it is not we can off course create a feature request, it seems a logical choice to let mousewheel also recenter to cursor.
I have also made a screencast to show the issue (attached).
#7 Updated by Mathieu Pellerin - nIRV almost 10 years ago
Giovanni, excellent video, it does a much better job than my video at showing what's wrong.
#8 Updated by Richard Duivenvoorde almost 10 years ago
ok, I see there is a painting artefact. I can reproduce that here by the way, best with a simple vector layer.
It seems like the old image is first centered to the map, drawn, and then the new image goes over it.
You do not see this with 2.6.1 indeed.
But I was looking for "it fails to properly center the zoomed area to mouse cursor", I still do not see any difference between master and 2.6? Or am I still missing the point?
#9 Updated by Sandro Santilli almost 10 years ago
I've seen the effect of first zooming to center too, but I saw it also before the rotation change, can anyone confirm that ?
The rotation commit is a single one so easy to test before ce8a9ba4afed849e7d85e9c0773fbc029b24bd15
#10 Updated by Giovanni Manghi almost 10 years ago
Sandro Santilli wrote:
I've seen the effect of first zooming to center too, but I saw it also before the rotation change, can anyone confirm that ?
The rotation commit is a single one so easy to test before ce8a9ba4afed849e7d85e9c0773fbc029b24bd15
ahh, it may be. Anyone can test master pre-rotation commit?
#11 Updated by Sandro Santilli almost 10 years ago
- Tag set to rotation
I've tested b774853 (right before rotation change) and the problem does not occur. So this is really related to the rotation commit :(
#12 Updated by Sandro Santilli almost 10 years ago
I've found that QgsMapCanvasMap::pain gets called twice on every zoom-wheel event.
The first time it gets called with image size == item size, the second with different sizes. Second time handling is correct.
#13 Updated by Sandro Santilli almost 10 years ago
Example output on zoom out:
src/gui/qgsmapcanvasmap.cpp: 51: (paint) [4ms] XXXXX QgsMapCanvasMap::paint, img 781,819 item 391,409
src/gui/qgsmapcanvasmap.cpp: 51: (paint) [2ms] XXXXX QgsMapCanvasMap::paint, img 781,819 item 781,819
The first one has bogus item size
#14 Updated by Sandro Santilli almost 10 years ago
In-between there's the ::setContent call, but it is like the item size is lost on next ::paint:
src/gui/qgsmapcanvasmap.cpp: 53: (paint) [7ms] XXXXX QgsMapCanvasMap::paint, img 781,819 item 391,409
src/gui/qgsmapcanvasmap.cpp: 39: (setContent) [1ms] XXX rect is 1606014.4615725926123559,4832382.2548933466896415 : 1669852.5080358437262475,4899326.3778810584917665
src/gui/qgsmapcanvasmap.cpp: 53: (paint) [1ms] XXXXX QgsMapCanvasMap::paint, img 781,819 item 781,819
- wheel zoom event here **
src/gui/qgsmapcanvasmap.cpp: 53: (paint) [3ms] XXXXX QgsMapCanvasMap::paint, img 781,819 item 391,410
src/gui/qgsmapcanvasmap.cpp: 39: (setContent) [1ms] XXX rect is 1605115.3341576175298542,4766418.9981765169650316 : 1732791.4270841197576374,4900307.2441519405692816
src/gui/qgsmapcanvasmap.cpp: 53: (paint) [1ms] XXXXX QgsMapCanvasMap::paint, img 781,819 item 781,819
#15 Updated by Sandro Santilli almost 10 years ago
The bogus "item" size is obtained via boundingRect, which is a QT standard method:
http://qt-project.org/doc/qt-4.8/qgraphicsitem.html#boundingRect
the call to ::setContent does invoke the setter of it (::setRect -- http://qt-project.org/doc/qt-4.8/qgraphicsrectitem.html#setRect)
#16 Updated by Sandro Santilli almost 10 years ago
Alright the problem is in the assumption made in QgsMapCanvasMap::paint that a different size between item and image mean the canvas map was rotated while another case is that the zoom just changed before the rendered had a chance to complete.
One fix could be to always pass a pre-clipped image from QgsMapCanvas::rendererJobFinished and drop the special handling from QgsMapCanvasMap::paint, possibly the cleanest one.
#17 Updated by Sandro Santilli almost 10 years ago
A more hackish approach is in test (by Travis, already tested locally) here: https://github.com/qgis/QGIS/pull/1732
#18 Updated by Sandro Santilli almost 10 years ago
- Resolution set to fixed/implemented
- Status changed from Open to Closed
Merged as 40e0d78b42474c698cc66e549bfb18d9867167b2
#19 Updated by Sandro Santilli almost 10 years ago
- Status changed from Closed to Reopened
Actually the fix is invalid in that it breaks proper rendering of rotation :(
#20 Updated by Sandro Santilli almost 10 years ago
So the "fix" was reverted with fd15a168b23a1d05531e65281d786f65e52be8d4 and I'm now looking at how to better implement handling of rotation at QgsMapCanvasMap::paint time. Right now the image is never stretched but rather offsetted. Instead we want it offsetted when rendered and stretched otherwise (for the pre-rendering feedback). Is that right @wonder-sk ?
I'm trying to implement the offsetting from the caller (QgsMapCanvas::rendererJobFinished) but it it still not clear to me what the meaning of the rectangle passed to ::setRect is.
#21 Updated by Sandro Santilli almost 10 years ago
Another attempt at fixing the regression w/out breaking the rendering of rotated map:
https://github.com/qgis/QGIS/pull/1734
Please review. I know there must be a better way but I don't have time to figure it out.
\\cc Martin Dobias
#22 Updated by Sandro Santilli almost 10 years ago
Giovanni, Mathieu, could you please test https://github.com/qgis/QGIS/pull/1734 ?
#23 Updated by Sandro Santilli almost 10 years ago
- Status changed from Reopened to Closed
PR merged by @wonder-sk with 1d9eb8f66d3616b91d1fdb9927fa35f898028daa