improvements to ZoomLast, ZoomNext tools (extent history)

[smizuno]

I have found the addition of the view extent history using the ZoomLast/ZoomNext tools to be very useful. However, because there was no status indicated on the buttons it was not easy to tell whether there was an extent to zoom to.

Also, I found that loading a project did not set its extent in the history list.

I believe the extent history should be cleared when a project (new or existing) is loaded because the map extent is set to the extent of a project or the first layer loaded making this point the logical beginning for the extent history.

Some people might argue that the previous view extents might be useful with a new project or even an existing project, but I believe that it is more likely that these extents are of little use.

Note that on program start, the map extent is set twice - once by fileNew, then by the command-line extent processing - so there are two extents in the history instead of one.

I provide a patch to do the following:

Improvements-

+ to make the ZoomLast and ZoomNext buttons show whether there is an extent to zoom to (in various functions, signals are emitted to update the button state)

+ provide a new function to clear the extent history and set the current extent as the first item: QgsMapCanvas::clearExtentHistory()

+ clear the extent history on new project (this is mostly cosmetic as other actions will also clear the history)

+ clear the extent history when the first layer is loaded

+ clear the extent history on project loading (the extent is set in QgsMapRenderer, but the list of extents wasn't updated)

minor updates in QgsMapCanvas -

+ in zoomToPreviousExtent() and zoomToNextExtent(): refresh() was moved inside the if block so that it is called only if there is a previous or next extent (this doesn't mean much when the now disabled button won't allow a click event in such a case, but was an unnecessary map redraw before the button got status indication)

+ in zoomToPreviousExtent(): mLastExtentIndex was tested for >1 -- it should be >0.

qgsmapcanvas.sip is updated for the new function and signals, so Python can make use of them.

[smizuno]

patch for zoomlast/zoomnext tools

[pcav]

Any objection in applying this patch?

[lutra]

Hi,

I tried your patch, I applied it and qgis compiled fine and worked the first time, then it started to giving me segmentation faults when launching the program.

Can you please check it? Thanks

PS it is supposed to see different zoomnext soomlast buttons if there is an extent to zoom to?

[smizuno]

Replying to lutra:

I tried your patch, I applied it and qgis compiled fine and worked the first time, then it started to giving me segmentation faults when launching the program.

I have most recently applied the patch to r11179 and has been working OK on my Windows system. It has also worked on a Linux system, but I haven't tested it recently, due to problems with that system. Were you applying the patch to a more recent version? I will check further.

it is supposed to see different zoomnext soomlast buttons if there is an extent to zoom to?

The buttons should be enabled when there is an extent in the queue to use whichever direction you are navigating. The buttons are disabled (grayed) when you reach either end of the extent queue. There aren't different icons, they are just colored or grayed.

[lutra]

Replying to smizuno:

I will check further.

thanks.

As a fact I applied the patch to a more recent version (I don't remember exactly, I compile from source very day), but I had to make some changes manually as since you posted the patch the code has changed, so it is possible that I made something wrong (I also didn't see any changes in the zoom buttons).

[smizuno]

Replying to lutra:

As a fact I applied the patch to a more recent version (I don't remember exactly, I compile from source very day), but I had to make some changes manually as since you posted the patch the code has changed, so it is possible that I made something wrong (I also didn't see any changes in the zoom buttons).

Upon applying the patch to r11258 I found that one part for QgisApp patches the wrong place which caused some null pointers that were used before being initialized. The part that was affected was where signals are connected. I can't figure out why the patch is applied rather than failing to patch.

I have created a revised patch file.

The zoomlast button will be enabled and the zoomnext button is disabled when QGIS is first started. If you load a project both buttons will be disabled until some zooming or panning is done. Then you can navigate the history of the zoom/pan with the buttons.

[smizuno]

revised patch (r11258 is base)

[lutra]

works for me under r11270.

Thanks!

[marisn]

Works just fine with trunk r12240 (some parts fail to patch, still can be merged manually). This patch is a must for next release :)

Python changes not tested.

Only feature that seems to be missing for previous/next is automatic re-projection of history extents on projection change event with OTFR enabled. Test: Add data, zoom in/out, pan, enable OTFR to different CRS, zoom previous/next still will use old CRS extents :(

[smizuno]

Replying to marisn: While I was only fixing the extent history mechanics to enable/disable the buttons and track the history correctly, I considered what happens when the map CRS is changed.

While it would be a good idea to have the extent history track the map CRS, there are situations where a previous extent could 'blow up' a CRS transformation. This should be trapped so it doesn't cause a segfault, be what should QGIS behavior be in this case? Doing nothing is confusing. Popping an error dialog gets really annoying after the first one.

The most common scenario I have seen is forgetting to set the map CRS before loading data that has a UTM projection, for example, after starting QGIS. In this case the extents are more correct for the UTM, not the default EPSG:4326, if the map CRS is now changed to UTM. The extents are likely way out of bounds, as well.

I almost put a clear extent history on map CRS change, but decided not to because that behavior could be confusing and annoying.

I believe that all consequences of map CRS change on the extent history should be considered before doing anything further.

[smizuno]

I recently found a minor problem setting the last index incorrectly in certain situations, which has been fixed in my second updated patch.

Since it has been some time since the previous patch was submitted, several parts now fail in patching due to changes in the surrounding original source, I have revised the patch so r12262 is the base.

[smizuno]

minor fix, base is r12262

[mhugent]

Applied in r12279. Thanks!

Marco