Bug report #11811

rotation regression: zoom to mouse cursor using mouse wheel is broken

Added by Mathieu Pellerin - nIRV almost 5 years ago. Updated almost 5 years ago.

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:
  1. Make sure that the mouse wheel action setting is set to "Zoom to mouse cursor" (in the Options' Map Tools panel)
  2. Open a project
  3. Place your mouse cursor at a corner edge of the map canvas, and zoom using the mouse wheel
  4. Notice how the zoomed area does not zoom to mouse cursor

Setting the priority to blocker as this is a regression.

zoom_error.mp4 (2.11 MB) Mathieu Pellerin - nIRV, 2014-12-09 10:25 PM

zoom_master.mp4 (1.84 MB) Giovanni Manghi, 2014-12-10 12:16 AM


Related issues

Related to QGIS Application - Bug report #11817: Grid decoration meaningless with rotated map Closed 2014-12-09

History

#1 Updated by Sandro Santilli almost 5 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 5 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 5 years ago

Also not confirmed here...

#4 Updated by Mathieu Pellerin - nIRV almost 5 years ago

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 5 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 5 years ago

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 5 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 5 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 5 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 5 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 5 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 5 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 5 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 5 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 5 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 5 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 5 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 5 years ago

  • Resolution set to fixed/implemented
  • Status changed from Open to Closed

Merged as 40e0d78b42474c698cc66e549bfb18d9867167b2

#19 Updated by Sandro Santilli almost 5 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 5 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 5 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 5 years ago

Giovanni, Mathieu, could you please test https://github.com/qgis/QGIS/pull/1734 ?

#23 Updated by Sandro Santilli almost 5 years ago

  • Status changed from Reopened to Closed

PR merged by @wonder-sk with 1d9eb8f66d3616b91d1fdb9927fa35f898028daa

Also available in: Atom PDF