Georectifier is broken

[cmbarton]

The georectifier is broken in the most recent svn versions of GRASS 6.5 and 7 (maybe earlier ones too, but I don't know).

Starting up the wizard works fine all the way to the end when it is supposed to launch the interface for setting and managing GCP's. This generates an error:

Traceback (most recent call last):
  File "/Applications/GRASS/GRASS-6.5.app/Contents/MacOS/etc
/wxpython/wxgui.py", line 253, in OnGCPManager

gcpmanager.GCPWizard(self)
  File "/Applications/GRASS/GRASS-6.5.app/Contents/MacOS/etc
/wxpython/gui_modules/gcpmanager.py", line 237, in __init__

Map=self.SrcMap, lmgr=self.parent)
  File "/Applications/GRASS/GRASS-6.5.app/Contents/MacOS/etc
/wxpython/gui_modules/gcpmanager.py", line 870, in __init__

self.OnZoomToMap(None)
  File "/Applications/GRASS/GRASS-6.5.app/Contents/MacOS/etc
/wxpython/gui_modules/gcpmapdisp.py", line 1030, in
OnZoomToMap

self.MapWindow.ZoomToMap(layers =
self.Map.GetListOfLayers())
  File "/Applications/GRASS/GRASS-6.5.app/Contents/MacOS/etc
/wxpython/gui_modules/mapdisp_window.py", line 2773, in
ZoomToMap

self.UpdateMap()
  File "/Applications/GRASS/GRASS-6.5.app/Contents/MacOS/etc
/wxpython/gui_modules/mapdisp_window.py", line 773, in
UpdateMap

windres = windres)
  File "/Applications/GRASS/GRASS-6.5.app/Contents/MacOS/etc
/wxpython/gui_modules/render.py", line 866, in Render

os.environ["GRASS_REGION"] = self.SetRegion(windres)
  File "/Applications/GRASS/GRASS-6.5.app/Contents/MacOS/etc
/wxpython/gui_modules/render.py", line 698, in SetRegion

region = self.AdjustRegion()
  File "/Applications/GRASS/GRASS-6.5.app/Contents/MacOS/etc
/wxpython/gui_modules/render.py", line 492, in AdjustRegion

self.region["nsres"] =  mapheight / self.height
ZeroDivisionError
:
float division

[cmbarton]

I just tested this in GRASS 6.4.1 and found out that it is broken there too. This is why I upgraded the priority. I don't have a copy of 6.4.0, but hope that we don't have a broken georectifier in the stable version.

I'm testing this on xy files that I generated from Spearfish and have used for several years in developing and testing the georectifier interfaces for GRASS.

Michael

[mmetz]

Replying to cmbarton:

I just tested this in GRASS 6.4.1 and found out that it is broken there too. This is why I upgraded the priority. I don't have a copy of 6.4.0, but hope that we don't have a broken georectifier in the stable version.

Can anybody else reproduce this? I tested on Linux and Windows and it works just fine.

Markus M

[cmbarton]

Did you test on 6.5 or only 6.4.0 (which I didn't test)?

Could be that this is Mac only, since I first heard about if from a colleague using a Mac. Then I tested and had the same problems.

Doesn't seem like a Mac problem given the error, however. The variable self.height is not being set, triggering a divide by zero error. Where is self.height coming from in this case?

Michael

[kyngchaos]

I found that it continues successfully to the CGP dialog if I do not specify a target map to display. When a target map is selected, no GCP dialog.

[cmbarton]

Following up on William's information. When I originally wrote this, the regular display was used for the target map for GCP creation and management. Looking at the GUI docs, it looks like this has been rewritten to have side by side displays now, with a target map in one and source map in the other. Perhaps someone can verify this.

If this is correct, the error is in the creation of the target display. The GCP manager window opens if no target map is selected, as William says. But there is no target map displayed either. The regular map display no longer seems to act like a target map now. So the target map canvas is bombing.

William documents that this seems OK in GRASS 6.4.0 but confirms that it is broken in the dev versions, including 6.4.1. So this needs to be fixed before any other release. Has anyone checked Windows on this?

Michael

[hellik]

GCP - WinGRASS-7.0.SVN-r45023-1-Setup.exe

[hellik]

Replying to cmbarton:

Following up on William's information. When I originally wrote this, the regular display was used for the target map for GCP creation and management. Looking at the GUI docs, it looks like this has been rewritten to have side by side displays now, with a target map in one and source map in the other. Perhaps someone can verify this.

If this is correct, the error is in the creation of the target display. The GCP manager window opens if no target map is selected, as William says. But there is no target map displayed either. The regular map display no longer seems to act like a target map now. So the target map canvas is bombing.

William documents that this seems OK in GRASS 6.4.0 but confirms that it is broken in the dev versions, including 6.4.1. So this needs to be fixed before any other release. Has anyone checked Windows on this?

Michael

tested with nightly-build WinGRASS-7.0.SVN-r45023-1-Setup.exe, georectifier is working (see screenshot). I will try later on a nightly wingrass6.4.1-build.

best regards Helmut

[hellik]

Replying to hellik:

tested with nightly-build WinGRASS-7.0.SVN-r45023-1-Setup.exe, georectifier is working (see screenshot). I will try later on a nightly wingrass6.4.1-build.

tested now with WinGRASS-6.4.SVN-r45020-1-Setup.exe from 2011/01/14, the georectifier is working

tested in two different ways:

(1) selecting source and target map in the gcp-wizzard => georectifying is working

(2) selecting only source map in the wizzard and later on set target map in gcp manager-settings => georectifying is working

best regards Helmut

[hellik]

Replying to hellik:

(2) selecting only source map in the wizzard and later on set target map in gcp manager-settings => georectifying is working

see sequence of added screenshots

Helmut

[cmbarton]

Thanks Helmut.

Helmut tried an alternate way of getting a target map that neither William nor I tried (image ending in 144249). When I replicate what he did in GRASS 6.5, I do not get a target map. The georectifier does not crash, but I get the same error as I reported above (and no target map).

self.height is not getting set. I'm not sure where this variable is coming from in the current version of the code. Does this variable get used in any other code than the GUI? If not, then we need to look at how its value is being acquired. If yes, then it is the way it is being used in this case--perhaps switching locations (from xy back to target) is not working right.

I'm happy to do some testing, but don't have time right now to completely reverse engineer the new code and so could use help in this.

Michael

[mmetz]

Replying to cmbarton:

Helmut tried an alternate way of getting a target map that neither William nor I tried (image ending in 144249). When I replicate what he did in GRASS 6.5, I do not get a target map. The georectifier does not crash, but I get the same error as I reported above (and no target map).

The Georectifier works fine in Linux and Windows. But I encountered cross-platform problems with the wxpython AUI Manager when developing the Georectifier and had to include a few msys hacks. Maybe the AUI Manager behaves different again on Mac?

Both the source and the target map display are buffered windows managed by an instance of the AUI Manager, just like the regular map display. The only difference is that the source map display is the center pane whereas - in the aui layout - the target map display the pane right of the center pane is.

self.height is not getting set. I'm not sure where this variable is coming from in the current version of the code.

This is set in render.py, independent of the georectifier:

​https://trac.osgeo.org/grass/browser/grass/branches/releasebranch_6_4/gui/wxpython/gui_modules/render.py#L383

Does this variable get used in any other code than the GUI? If not, then we need to look at how its value is being acquired. If yes, then it is the way it is being used in this case--perhaps switching locations (from xy back to target) is not working right.

Problems with switching locations could be a starting point. The wxGUI sometimes overrides the settings of the georectifier and switches back to its original location/mapset, although nobody told it to do so... That's why i.rectify can no longer be executed in the main wxGUI Layer Manager Output window, but instead runs in the background. That is somehow related with the wxGUI layer Manager (not the georectifier) having its own private copy of GISRC which is a relatively recent change.

I'm happy to do some testing, but don't have time right now to completely reverse engineer the new code and so could use help in this.

I hope that gives a bit more insight into the design of the new Georectifier. And there is still the manual:

​http://grass.osgeo.org/grass64/manuals/html64_user/wxGUI.GCP_Manager.html

Markus M

[cmbarton]

I found where the error is and can prevent it. But I don't yet know why this is a problem. Hopefully Markus M will have some ideas. The errors are generated by...

self.OnZoomToMap(None) in line 870

and a similar call of ...

self.parent.TgtMapWindow.ZoomToMap(layers = self.parent.TgtMap.GetListOfLayers()) in line 2739

If I comment out these lines, both source and target maps open fine. Initializing the target map window with a zoom to map extents is probably a good idea. So the root cause of this error still needs to be found. These both do something that causes a crash on the Mac.

Michael

[cmbarton]

Here is more info.

Both of the zoom lines call an instance of BufferedMap.ZoomToMap() in mapdisplay_window.py

ZoomToMap() calls UpdateMap, which does a GetClientSize(), which returns (0,0). This is the problem. Previous calls to UpdateMap return non-zero values for GetClientSize().

Prior to calling UpdateMap, ZoomToMap calls an instance of Map.GetRegion() in render.py. This seems to go OK, but I can't tell for sure. GetRegion does GISRC switching.

So any ideas here?

Michael

[mmetz]

Replying to cmbarton:

I found where the error is and can prevent it. But I don't yet know why this is a problem. Hopefully Markus M will have some ideas. The errors are generated by...

self.OnZoomToMap(None) in line 870

Yes, I fixed that 3 months ago in trunk, because you reported that error earlier. Please test trunk. If that works, the change can be backported to to 6.5. and 6.4.

Markus M

[mmetz]

Replying to mmetz:

Replying to cmbarton:

I found where the error is and can prevent it. But I don't yet know why this is a problem. Hopefully Markus M will have some ideas. The errors are generated by...

self.OnZoomToMap(None) in line 870

Yes, I fixed that 3 months ago in trunk, because you reported that error earlier. Please test trunk. If that works, the change can be backported to to 6.5. and 6.4.

Correction, it was also fixed in 6.5. Did you only test 6.4.1?

[mmetz]

Replying to cmbarton:

I found where the error is and can prevent it. But I don't yet know why this is a problem. Hopefully Markus M will have some ideas. The errors are generated by...

self.OnZoomToMap(None) in line 870

First case, a target map is specified when starting the georectifier:

in trunk, gcpmapdisp.py L309 I have commented out a call to update the AUI Manager. That line was active in the original georectifer and may be needed on Mac. On msys however, activating this line causes havoc. On Linux, it does not matter.

and a similar call of ...

self.parent.TgtMapWindow.ZoomToMap(layers = self.parent.TgtMap.GetListOfLayers()) in line 2739

Second case, the georectifier is started with a source map only and a target map is specified later on through the settings dialog:

in trunk, gcpmanager.py, you can try to move lines 2745 and 2753 up by 3 lines and insert them right after self.parent._mgr.GetPane(). The target map window (and its size) would now be updated by the AUI Manager before any call to ZoomToMap. Moving these lines does apparently not matter under msys or Linux.

Markus M

[cmbarton]

Thanks. I'll this out later today.

Michael

[cmbarton]

Replying to mmetz:

Replying to cmbarton:

I found where the error is and can prevent it. But I don't yet know why this is a problem. Hopefully Markus M will have some ideas. The errors are generated by...

self.OnZoomToMap(None) in line 870

First case, a target map is specified when starting the georectifier:

in trunk, gcpmapdisp.py L309 I have commented out a call to update the AUI Manager. That line was active in the original georectifer and may be needed on Mac. On msys however, activating this line causes havoc. On Linux, it does not matter.

Uncommenting Line 309 fixes the error. This needs to be conditionalized for windows.

Thanks for finding this.

Michael

and a similar call of ...

self.parent.TgtMapWindow.ZoomToMap(layers = self.parent.TgtMap.GetListOfLayers()) in line 2739

Second case, the georectifier is started with a source map only and a target map is specified later on through the settings dialog:

in trunk, gcpmanager.py, you can try to move lines 2745 and 2753 up by 3 lines and insert them right after self.parent._mgr.GetPane(). The target map window (and its size) would now be updated by the AUI Manager before any call to ZoomToMap. Moving these lines does apparently not matter under msys or Linux.

Markus M

[cmbarton]

Markus,

I just updated trunk from the svn and tested. Your revisions have fixed this problem. When backdated to 6.5 and 6.4.1, we can close this report. It would be good if someone could test this on Windows as it was a Window-related changed that caused this to break in the first place. Should work fine, though.

Michael

[mmetz]

Replying to cmbarton:

[...] It would be good if someone could test this on Windows as it was a Window-related changed that caused this to break in the first place.

It took me a bit of testing to figure out a solution that works on both Linux and Windows (can't test on Mac), IOW, the recent changes in the code seem to work fine on all officially supported platforms. And are already submitted to 6.5 and 6.4.1.

Markus M

[cmbarton]

I've tested this in 7.0, 6.5, and 6.4.1 and it works fine in all. Closing ticket.

Michael