Opened 8 years ago

Closed 8 years ago

#1300 closed defect (fixed)

WxGUI measure tool gives wrong results

Reported by: marisn Owned by: martinl
Priority: blocker Milestone: 6.4.1
Component: wxGUI Version: svn-releasebranch64
Keywords: measure tool Cc: grass-dev@…
CPU: Unspecified Platform: MSWindows Vista

Description

Steps to reproduce:

  • Add North arrow and Scale bar to map display
  • Measure scale bar length

gis.m gives correct result (4.9), wxgui allways gives too short result. 4.2Km instead of 5km is a bit too short for simple "shaking hand" error. Individual bars are given not as 1km, but only 800m.

WinGRASS-6.4.SVN-r45559-1

Attachments (2)

wxgui_measure_fail.jpg (105.9 KB) - added by marisn 8 years ago.
Screenshot with both GUIs measuring scale bar
GRASS_measure_problem.png (58.7 KB) - added by marisn 8 years ago.
Spearfish example on Linux. 6.4.1 r43636 (Did I sed that current measure tool mouse bindings suck?)

Download all attachments as: .zip

Change History (15)

Changed 8 years ago by marisn

Attachment: wxgui_measure_fail.jpg added

Screenshot with both GUIs measuring scale bar

comment:1 Changed 8 years ago by hamish

strange, works for me in spearfish. can you try putting up a 5 km grid as well? (layer manager button to the left of remove) also drag the barscale over the grid to make sure those are the same.

I notice it is a bit odd if you add the barscale when the measure tool is on, after that the measure tool is randomly starting on the page (ref'd to the barscale overlay canvas instead of the background canvas?) until you reselect the measure/barscale tools, then it seems to correct itself. ??

Hamish

comment:2 Changed 8 years ago by hamish

also, right-click(?) to restart measure tool ("Do another ...") would be a nice addition. instead of double left-click to quit tool and then to the menus to start the tool again.

double left-click seems more natural to me actually, more like a stopwatch's lap-time vs reset.

Hamish

Changed 8 years ago by marisn

Attachment: GRASS_measure_problem.png added

Spearfish example on Linux. 6.4.1 r43636 (Did I sed that current measure tool mouse bindings suck?)

comment:3 Changed 8 years ago by marisn

Agh! Note to myself: "Never use g.version -r". Failing example on Linux was more like r45560.

comment:4 Changed 8 years ago by hamish

Keywords: measure tool added

Ok, I can reproduce this on Linux using the latest 6.4.1svn, 6.5svn & spearfish.

notice that it works in the y-direction but not the x-direction.

well, as long as the white bands are on the left-right, not top-bottom. if you resize the map window so that the white out-of-region bands are on the left,right sides, then the measure tool works along the x-asix but not the y-axis.

so it seems to be using window size not region size, and the bigger the out-of-region bands, the bigger the d.measure error.

go into preferences and tick the 'constrain' box and it starts to work better.

also in 6.5svn I notice if I mouse wheel zoom out, the 10km barscale no longer exactly matches the d.grid lines. (some mm out on the screen) maybe a d.barscale bug, but I've only known that to be 1px out on the right end. ?

(Did I sed that current measure tool mouse bindings suck?)

please suggest what would be better. (thoughts on comment:2 ?)

... also, I notice in 6.5svn all maps are missing from d.rast map name GUI:

GSelect: invalid item: descriptor 'lower' requires a
'unicode' object but received a 'str'

Hamish

Hamish

comment:5 Changed 8 years ago by hamish

Priority: majorcritical

measure tool bad results also present in 6.4.0+42329 (rc6++), so not a reversion. I'm surprised we haven't noticed this earlier. Bumping bug priority as we shouldn't knowingly report bad data, someone might try and use the results for something important.

alterbug- I do not get the following with a reasonably recent 6.5.svn45558 when trying to add new maps, will need to check back on my other machine.

GSelect: invalid item: descriptor 'lower' requires a
'unicode' object but received a 'str'

Hamish

comment:6 Changed 8 years ago by hamish

unicode error filed as #1304.

comment:7 Changed 8 years ago by hamish

Priority: criticalblocker

We are reporting incorrect results, aka data corruption. Bumping this to blocker.

comment:8 in reply to:  7 ; Changed 8 years ago by martinl

Cc: grass-dev@… added
Owner: changed from grass-dev@… to martinl
Status: newassigned

Replying to hamish:

We are reporting incorrect results, aka data corruption. Bumping this to blocker.

hopefully fixed in r45883 (and backported to devbr6).

comment:9 in reply to:  8 ; Changed 8 years ago by hamish

Replying to martinl:

hopefully fixed in r45883 (and backported to devbr6).

thanks, that passes the 'd.grid 5000' test in spearfish,simple_xy loc'ns + latest 6.5svn. no difference with using the 'constrain' tick box in prefs AFAICT.

lat/lon is still reporting meaningless results, in grass7 it should use m.measure to do the calc so geodesic distance is automatically used if needed. (which is basically what I wrote the earlier m.distance.py SWIG version for*; see also discussion with Radim in the last week about adding a small sleep to QGIS's r.what mouse-over queries so process doesn't get launched 1000 times per mouse movement) [*] source:grass/trunk/swig/python/examples/m.distance.py@43200

cheers, Hamish

comment:10 in reply to:  9 ; Changed 8 years ago by martinl

Replying to hamish:

thanks, that passes the 'd.grid 5000' test in spearfish,simple_xy loc'ns + latest 6.5svn. no difference with using the 'constrain' tick box in prefs AFAICT.

backported in r45891

lat/lon is still reporting meaningless results, in grass7 it should use m.measure to do the calc so geodesic distance is automatically used if needed.

probably we should warn user about that. Would be

Warning: Measuring reports meaningless results for LL projection (geodetic distance ignored). Check future versions.

be OK?

comment:11 in reply to:  10 ; Changed 8 years ago by hamish

Replying to martinl:

backported in r45891

cheers

lat/lon is still reporting meaningless results, in grass7 it should use m.measure to do the calc so geodesic distance is automatically used if needed.

probably we should warn user about that. Would be

> Warning: Measuring reports meaningless results for LL projection (geodetic
> distance ignored). Check future versions.

be OK?

sounds fine. I'd consider the additional step of disabling the tool after showing that message. Maybe with "Geodesic distance is not yet supported by this tool, try d.measure or m.measure.", or something. (apple usability guide: an error message should say what the trouble was, and give a hint about where the user should look next/for more info)

veering off-topic a little, but along the same reporting seemingly valid (but in fact broken) numbers to the user, I've been trying to follow how the mapscale setting in the drop down menu on the bottom bar of the Map Display window does its job. Does .ppm[0], .ppm[1] refer to image width,height? Map scale is fundamentally unknowable if you don't know the correct pixels-per-mm ratio for the user's particular monitor. X11,OSX,Window's reports for that have in the past not been very good, maybe with modern versions it has improved? (being optimistic..) We get away with it in ps.map because we know the exact size of the paper, and PostScript? exists in that reference frame. GIMP provides a calibration tool (Prefs->Display->Calibrate) where the user can press a ruler up to their screen and type in the distance covered by x,y pixel bars, then hope the user doesn't change screen resolution.

Hamish

ps- At some point I'd like to add the v.in.geodesic addon to trunk, but I'd like feedback on options & assumptions made before finalizing it and porting it to python, or turning it into a new C module reusing proj4's geod code if that is preferred.

comment:12 in reply to:  11 ; Changed 8 years ago by martinl

Replying to hamish:

sounds fine. I'd consider the additional step of disabling the tool after showing that message. Maybe with "Geodesic distance is not yet supported by this tool, try d.measure or m.measure.", or something. (apple usability guide: an error message should say what the trouble was, and give a hint about where the user should look next/for more info)

Bearing in mind that we don't expect that wxGUI users know Xmons I wouldn't note d.measure or m.measure (no ctypes in relbr64). Would be r45898 enough for now?

BTW, I have updated m.distance in r45895, check also r45897 for wxGUI measuring tool in LL-locations.

comment:13 in reply to:  12 Changed 8 years ago by hamish

Resolution: fixed
Status: assignedclosed

Replying to martinl:

Bearing in mind that we don't expect that wxGUI users know Xmons I wouldn't note d.measure or m.measure

or necessarily have access to them- point taken, but the user picked up the tool to get an answer and is now stranded without an answer, and we do actually have the capability to answer it somehow ... Mainly I'm trying to think of ways to connect them with the answer they want. shrug

Would be r45898 enough for now?

sure, and r45897,9 looks like it will be great.

BTW, I have updated m.distance in r45895,

thanks.

Hamish

Note: See TracTickets for help on using tickets.