[GRASS-dev] [GRASS GIS] #1257: Georectifier is broken

#1257: Georectifier is broken
--------------------------+-------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
     Type: defect | Status: new
Priority: major | Milestone: 6.5.0
Component: wxGUI | Version: unspecified
Keywords: georectifier | Platform: Unspecified
      Cpu: Unspecified |
--------------------------+-------------------------------------------------
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
}}}

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1257&gt;
GRASS GIS <http://grass.osgeo.org>

#1257: Georectifier is broken
--------------------------+-------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
     Type: defect | Status: new
Priority: blocker | Milestone: 6.4.1
Component: wxGUI | Version: unspecified
Keywords: georectifier | Platform: Unspecified
      Cpu: Unspecified |
--------------------------+-------------------------------------------------
Changes (by cmbarton):

  * priority: major => blocker
  * milestone: 6.5.0 => 6.4.1

Comment:

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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1257#comment:1&gt;
GRASS GIS <http://grass.osgeo.org>

#1257: Georectifier is broken
--------------------------+-------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
     Type: defect | Status: new
Priority: blocker | Milestone: 6.4.1
Component: wxGUI | Version: unspecified
Keywords: georectifier | Platform: Unspecified
      Cpu: Unspecified |
--------------------------+-------------------------------------------------

Comment(by mmetz):

Replying to [comment:1 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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1257#comment:2&gt;
GRASS GIS <http://grass.osgeo.org>

#1257: Georectifier is broken
--------------------------+-------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
     Type: defect | Status: new
Priority: blocker | Milestone: 6.4.1
Component: wxGUI | Version: unspecified
Keywords: georectifier | Platform: Unspecified
      Cpu: Unspecified |
--------------------------+-------------------------------------------------

Comment(by 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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1257#comment:3&gt;
GRASS GIS <http://grass.osgeo.org>

#1257: Georectifier is broken
--------------------------+-------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
     Type: defect | Status: new
Priority: blocker | Milestone: 6.4.1
Component: wxGUI | Version: unspecified
Keywords: georectifier | Platform: Unspecified
      Cpu: Unspecified |
--------------------------+-------------------------------------------------

Comment(by 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.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1257#comment:4&gt;
GRASS GIS <http://grass.osgeo.org>

#1257: Georectifier is broken
--------------------------+-------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
     Type: defect | Status: new
Priority: blocker | Milestone: 6.4.1
Component: wxGUI | Version: unspecified
Keywords: georectifier | Platform: Unspecified
      Cpu: Unspecified |
--------------------------+-------------------------------------------------

Comment(by 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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1257#comment:5&gt;
GRASS GIS <http://grass.osgeo.org>

#1257: Georectifier is broken
--------------------------+-------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
     Type: defect | Status: new
Priority: blocker | Milestone: 6.4.1
Component: wxGUI | Version: unspecified
Keywords: georectifier | Platform: Unspecified
      Cpu: Unspecified |
--------------------------+-------------------------------------------------

Comment(by hellik):

Replying to [comment:5 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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1257#comment:6&gt;
GRASS GIS <http://grass.osgeo.org>

#1257: Georectifier is broken
--------------------------+-------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
     Type: defect | Status: new
Priority: blocker | Milestone: 6.4.1
Component: wxGUI | Version: unspecified
Keywords: georectifier | Platform: Unspecified
      Cpu: Unspecified |
--------------------------+-------------------------------------------------

Comment(by hellik):

Replying to [comment:6 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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1257#comment:7&gt;
GRASS GIS <http://grass.osgeo.org>

#1257: Georectifier is broken
--------------------------+-------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
     Type: defect | Status: new
Priority: blocker | Milestone: 6.4.1
Component: wxGUI | Version: unspecified
Keywords: georectifier | Platform: Unspecified
      Cpu: Unspecified |
--------------------------+-------------------------------------------------

Comment(by hellik):

Replying to [comment:7 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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1257#comment:8&gt;
GRASS GIS <http://grass.osgeo.org>

#1257: Georectifier is broken
--------------------------+-------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
     Type: defect | Status: new
Priority: blocker | Milestone: 6.4.1
Component: wxGUI | Version: unspecified
Keywords: georectifier | Platform: MacOSX
      Cpu: OSX/Intel |
--------------------------+-------------------------------------------------
Changes (by cmbarton):

  * platform: Unspecified => MacOSX
  * cpu: Unspecified => OSX/Intel

Comment:

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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1257#comment:9&gt;
GRASS GIS <http://grass.osgeo.org>

#1257: Georectifier is broken
--------------------------+-------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
     Type: defect | Status: new
Priority: blocker | Milestone: 6.4.1
Component: wxGUI | Version: unspecified
Keywords: georectifier | Platform: MacOSX
      Cpu: OSX/Intel |
--------------------------+-------------------------------------------------

Comment(by mmetz):

Replying to [comment:9 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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1257#comment:10&gt;
GRASS GIS <http://grass.osgeo.org>

#1257: Georectifier is broken
--------------------------+-------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
     Type: defect | Status: new
Priority: blocker | Milestone: 6.4.1
Component: wxGUI | Version: unspecified
Keywords: georectifier | Platform: MacOSX
      Cpu: OSX/Intel |
--------------------------+-------------------------------------------------

Comment(by 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...[[BR]]

self.OnZoomToMap(None) in line 870

and a similar call of ...[[BR]]

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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1257#comment:11&gt;
GRASS GIS <http://grass.osgeo.org>

#1257: Georectifier is broken
--------------------------+-------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
     Type: defect | Status: new
Priority: blocker | Milestone: 6.4.1
Component: wxGUI | Version: unspecified
Keywords: georectifier | Platform: MacOSX
      Cpu: OSX/Intel |
--------------------------+-------------------------------------------------

Comment(by 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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1257#comment:12&gt;
GRASS GIS <http://grass.osgeo.org>

#1257: Georectifier is broken
--------------------------+-------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
     Type: defect | Status: new
Priority: blocker | Milestone: 6.4.1
Component: wxGUI | Version: unspecified
Keywords: georectifier | Platform: MacOSX
      Cpu: OSX/Intel |
--------------------------+-------------------------------------------------

Comment(by mmetz):

Replying to [comment:11 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...[[BR]]
>
>
> 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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1257#comment:13&gt;
GRASS GIS <http://grass.osgeo.org>

#1257: Georectifier is broken
--------------------------+-------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
     Type: defect | Status: new
Priority: blocker | Milestone: 6.4.1
Component: wxGUI | Version: unspecified
Keywords: georectifier | Platform: MacOSX
      Cpu: OSX/Intel |
--------------------------+-------------------------------------------------

Comment(by mmetz):

Replying to [comment:13 mmetz]:
> Replying to [comment:11 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...[[BR]]
> >
> >
> > 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?

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1257#comment:14&gt;
GRASS GIS <http://grass.osgeo.org>

#1257: Georectifier is broken
--------------------------+-------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
     Type: defect | Status: new
Priority: blocker | Milestone: 6.4.1
Component: wxGUI | Version: unspecified
Keywords: georectifier | Platform: MacOSX
      Cpu: OSX/Intel |
--------------------------+-------------------------------------------------

Comment(by mmetz):

Replying to [comment:11 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...[[BR]]
>
>
> 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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1257#comment:15&gt;
GRASS GIS <http://grass.osgeo.org>

#1257: Georectifier is broken
--------------------------+-------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
     Type: defect | Status: new
Priority: blocker | Milestone: 6.4.1
Component: wxGUI | Version: unspecified
Keywords: georectifier | Platform: MacOSX
      Cpu: OSX/Intel |
--------------------------+-------------------------------------------------

Comment(by cmbarton):

Thanks. I'll this out later today.

Michael

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1257#comment:16&gt;
GRASS GIS <http://grass.osgeo.org>

#1257: Georectifier is broken
--------------------------+-------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
     Type: defect | Status: new
Priority: blocker | Milestone: 6.4.1
Component: wxGUI | Version: unspecified
Keywords: georectifier | Platform: MacOSX
      Cpu: OSX/Intel |
--------------------------+-------------------------------------------------

Comment(by cmbarton):

Replying to [comment:15 mmetz]:
> Replying to [comment:11 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...[[BR]]
> >
> >
> > 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.[[BR]]

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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1257#comment:17&gt;
GRASS GIS <http://grass.osgeo.org>

#1257: Georectifier is broken
--------------------------+-------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
     Type: defect | Status: new
Priority: blocker | Milestone: 6.4.1
Component: wxGUI | Version: unspecified
Keywords: georectifier | Platform: MacOSX
      Cpu: OSX/Intel |
--------------------------+-------------------------------------------------

Comment(by 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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1257#comment:18&gt;
GRASS GIS <http://grass.osgeo.org>

#1257: Georectifier is broken
--------------------------+-------------------------------------------------
Reporter: cmbarton | Owner: grass-dev@…
     Type: defect | Status: new
Priority: blocker | Milestone: 6.4.1
Component: wxGUI | Version: unspecified
Keywords: georectifier | Platform: MacOSX
      Cpu: OSX/Intel |
--------------------------+-------------------------------------------------

Comment(by mmetz):

Replying to [comment:18 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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/1257#comment:19&gt;
GRASS GIS <http://grass.osgeo.org>