Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#2848 closed defect (fixed)

g.gui.vdigit: snapping distance seems to only be defined in map units, not screen pixels

Reported by: mlennert Owned by: grass-dev@…
Priority: major Milestone: 7.0.5
Component: wxGUI Version: svn-releasebranch70
Keywords: vector, digitize Cc:
CPU: Unspecified Platform: Unspecified

Description

Using g.gui.vdigit in trunk and in 7.0.3rc1, I only seem to be able to define snapping distance in map units, not in screen pixels:

  • Go to the digitizing parameters, 'General' tab
  • Change snapping threshold to any value, choosing screen pixels as unit

=> The translation to meters below always corresponds to the exact same number you set as threshold in screen pixels. And testing through actual digitization shows that the snapping threshold is in map units, whichever unit you chose in the parameters.

Setting priority to major as snapping is an important tool in topological vector editing, but the tool remains usable.

Change History (23)

comment:1 Changed 4 years ago by neteler

Milestone: 7.0.3

Ticket retargeted after milestone closed

comment:2 Changed 4 years ago by neteler

Milestone: 7.0.4

Ticket retargeted after 7.0.3 milestone closed

comment:3 Changed 3 years ago by martinl

Milestone: 7.0.47.0.5

comment:4 Changed 3 years ago by mlennert

Can anyone else confirm this issue ? I'm still seeing it.

comment:5 Changed 3 years ago by annakrat

In 69578:

GUI/vdigit: fix snapping units, see #2848

comment:6 Changed 3 years ago by annakrat

Please test. It was only problem when using different locale.

comment:7 in reply to:  6 ; Changed 3 years ago by mlennert

Replying to annakrat:

Please test. It was only problem when using different locale.

Right. It +/- works now, i.e. when I chose "screen pixels", I now get the correct value in map units below. However, when I change the value (i.e. the number of screen pixels), I get a wrong number below: the number of screen pixels instead of map units. I have to switch to map units and back to screen pixels for the depicted value in map units to be correct again.

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

Replying to mlennert:

Replying to annakrat:

Please test. It was only problem when using different locale.

Right. It +/- works now, i.e. when I chose "screen pixels", I now get the correct value in map units below. However, when I change the value (i.e. the number of screen pixels), I get a wrong number below: the number of screen pixels instead of map units. I have to switch to map units and back to screen pixels for the depicted value in map units to be correct again.

OK, I committed a new fix in r69579.

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

Replying to annakrat:

Replying to mlennert:

Replying to annakrat:

Please test. It was only problem when using different locale.

Right. It +/- works now, i.e. when I chose "screen pixels", I now get the correct value in map units below. However, when I change the value (i.e. the number of screen pixels), I get a wrong number below: the number of screen pixels instead of map units. I have to switch to map units and back to screen pixels for the depicted value in map units to be correct again.

OK, I committed a new fix in r69579.

Seems to work great, now ! Thanks.

Not sure if this is mature enough to backport to release70. You're a better judge of that.

comment:10 in reply to:  9 ; Changed 3 years ago by annakrat

Replying to mlennert:

Replying to annakrat:

Replying to mlennert:

Replying to annakrat:

Please test. It was only problem when using different locale.

Right. It +/- works now, i.e. when I chose "screen pixels", I now get the correct value in map units below. However, when I change the value (i.e. the number of screen pixels), I get a wrong number below: the number of screen pixels instead of map units. I have to switch to map units and back to screen pixels for the depicted value in map units to be correct again.

OK, I committed a new fix in r69579.

Seems to work great, now ! Thanks.

Not sure if this is mature enough to backport to release70. You're a better judge of that.

The only problem is in the settings. The old value should be ignored and then new value (0 or 1 is saved). You can test it by manually changing the values in the settings file. Since you didn't report any problem with the new patch I assume it ignores your saved settings value ("screen pixels" or the french version), but please check, you should have now there 0 or 1 if you saved the settings.

comment:11 in reply to:  10 Changed 3 years ago by mlennert

Replying to annakrat:

Replying to mlennert:

Replying to annakrat:

Replying to mlennert:

Replying to annakrat:

Please test. It was only problem when using different locale.

Right. It +/- works now, i.e. when I chose "screen pixels", I now get the correct value in map units below. However, when I change the value (i.e. the number of screen pixels), I get a wrong number below: the number of screen pixels instead of map units. I have to switch to map units and back to screen pixels for the depicted value in map units to be correct again.

OK, I committed a new fix in r69579.

Seems to work great, now ! Thanks.

Not sure if this is mature enough to backport to release70. You're a better judge of that.

The only problem is in the settings. The old value should be ignored and then new value (0 or 1 is saved). You can test it by manually changing the values in the settings file. Since you didn't report any problem with the new patch I assume it ignores your saved settings value ("screen pixels" or the french version), but please check, you should have now there 0 or 1 if you saved the settings.

I'm not sure I saved these settings before. Now, I do have

vdigit;snapping;units;0;value;20

However, when I now try to open the settings dialog in the grass 7.0.5 digitizer, I get:

Traceback (most recent call last):
  File
"/data/home/mlennert/SRC/GRASS/grass70_release/dist.x86_64
-pc-linux-gnu/gui/wxpython/vdigit/toolbars.py", line 528, in
OnSettings

self.settingsDialog = VDigitSettingsDialog(parent =
self.parent, giface = self._giface)
  File
"/data/home/mlennert/SRC/GRASS/grass70_release/dist.x86_64
-pc-linux-gnu/gui/wxpython/vdigit/preferences.py", line 43,
in __init__

self._createGeneralPage(notebook)
  File
"/data/home/mlennert/SRC/GRASS/grass70_release/dist.x86_64
-pc-linux-gnu/gui/wxpython/vdigit/preferences.py", line 168,
in _createGeneralPage

self.snappingUnit.SetStringSelection(UserSettings.Get(group
= 'vdigit', key = "snapping", subkey = 'units'))
  File "/usr/lib/python2.7/dist-
packages/wx-3.0-gtk2/wx/_core.py", line 13002, in
SetStringSelection

return _core_.ItemContainer_SetStringSelection(*args,
**kwargs)
TypeError
:
String or Unicode type required

so there is an issue of backwards compatibility....

comment:12 Changed 3 years ago by annakrat

OK, try r69580. First remove the snapping record in your settings file. Save it in trunk, and now open the dialog in older GRASS. Save it and open it again in trunk. If there are no problems, I will backport it.

comment:13 in reply to:  12 ; Changed 3 years ago by mlennert

Replying to annakrat:

OK, try r69580. First remove the snapping record in your settings file. Save it in trunk, and now open the dialog in older GRASS. Save it and open it again in trunk. If there are no problems, I will backport it.

This seems to work, except: when I chose map units in trunk, I get the following in the settings file:

vdigit;snapping;units;screen pixels;value;25;unit;1

which looks a bit contradictory ('unit: 1' should be 'units: map units', or ?).

When I open this in grass70, I get values corresponding to 25 pixels, although the drop-down list status does not show any choice. When I then choose pixels, I first get 25 pixels = 25 meters, but when I change the number of pixels, the meter info is correct again.

Trying to save from grass70, nothing is saved to the settings file if I choose map units. Choosing screen pixels, I only get:

vdigit;snapping;

in the settings file.

When I then open trunk, it gives me the default settings of 10 screen pixels.

comment:14 in reply to:  13 ; Changed 3 years ago by annakrat

Replying to mlennert:

Replying to annakrat:

OK, try r69580. First remove the snapping record in your settings file. Save it in trunk, and now open the dialog in older GRASS. Save it and open it again in trunk. If there are no problems, I will backport it.

This seems to work, except: when I chose map units in trunk, I get the following in the settings file:

vdigit;snapping;units;screen pixels;value;25;unit;1

which looks a bit contradictory ('unit: 1' should be 'units: map units', or ?).

That's correct, 'units' is the old name kept there for backwards compatibility, 'unit' is the new one (0 is screen pixels, 1 is map units).

When I open this in grass70, I get values corresponding to 25 pixels, although the drop-down list status does not show any choice. When I then choose pixels, I first get 25 pixels = 25 meters, but when I change the number of pixels, the meter info is correct again.

Trying to save from grass70, nothing is saved to the settings file if I choose map units. Choosing screen pixels, I only get:

vdigit;snapping;

in the settings file.

It is broken in GRASS70, the point is it shouldn't give you any error caused by the changes in the new settings file.

When I then open trunk, it gives me the default settings of 10 screen pixels.

That's correct. So I would say it behaves as I would expect.

comment:15 in reply to:  14 ; Changed 3 years ago by mlennert

Replying to annakrat:

Replying to mlennert:

Replying to annakrat:

OK, try r69580. First remove the snapping record in your settings file. Save it in trunk, and now open the dialog in older GRASS. Save it and open it again in trunk. If there are no problems, I will backport it.

This seems to work, except: when I chose map units in trunk, I get the following in the settings file:

vdigit;snapping;units;screen pixels;value;25;unit;1

which looks a bit contradictory ('unit: 1' should be 'units: map units', or ?).

That's correct, 'units' is the old name kept there for backwards compatibility, 'unit' is the new one (0 is screen pixels, 1 is map units).

Yes, I understand that, but if 1 is map units, then why does the line contain 'units;screen pixels' and 'unit;1' ? Shouldn't it be 'units;map units' ?

comment:16 in reply to:  15 ; Changed 3 years ago by annakrat

Replying to mlennert:

Replying to annakrat:

Replying to mlennert:

Replying to annakrat:

OK, try r69580. First remove the snapping record in your settings file. Save it in trunk, and now open the dialog in older GRASS. Save it and open it again in trunk. If there are no problems, I will backport it.

This seems to work, except: when I chose map units in trunk, I get the following in the settings file:

vdigit;snapping;units;screen pixels;value;25;unit;1

which looks a bit contradictory ('unit: 1' should be 'units: map units', or ?).

That's correct, 'units' is the old name kept there for backwards compatibility, 'unit' is the new one (0 is screen pixels, 1 is map units).

Yes, I understand that, but if 1 is map units, then why does the line contain 'units;screen pixels' and 'unit;1' ? Shouldn't it be 'units;map units' ?

I don't care about the value of 'units' anymore, I am not trying to set it, I just kept them there so that the settings file can be read without error in old versions.

comment:17 in reply to:  16 ; Changed 3 years ago by mlennert

Replying to annakrat:

Replying to mlennert:

Replying to annakrat:

Replying to mlennert:

Replying to annakrat:

OK, try r69580. First remove the snapping record in your settings file. Save it in trunk, and now open the dialog in older GRASS. Save it and open it again in trunk. If there are no problems, I will backport it.

This seems to work, except: when I chose map units in trunk, I get the following in the settings file:

vdigit;snapping;units;screen pixels;value;25;unit;1

which looks a bit contradictory ('unit: 1' should be 'units: map units', or ?).

That's correct, 'units' is the old name kept there for backwards compatibility, 'unit' is the new one (0 is screen pixels, 1 is map units).

Yes, I understand that, but if 1 is map units, then why does the line contain 'units;screen pixels' and 'unit;1' ? Shouldn't it be 'units;map units' ?

I don't care about the value of 'units' anymore, I am not trying to set it, I just kept them there so that the settings file can be read without error in old versions.

Then things look pretty good for me. I just want to make sure we don't create a regression if we backport this fix this late before the release of 7.0.5.

comment:18 in reply to:  17 Changed 3 years ago by annakrat

Replying to mlennert:

Then things look pretty good for me. I just want to make sure we don't create a regression if we backport this fix this late before the release of 7.0.5.

I understand, thanks for testing. I will backport later today.

comment:19 Changed 3 years ago by annakrat

In 69594:

GUI/vdigit: fix snapping units, see #2848 (merge from trunk r69578, r69579, r69580)

comment:20 Changed 3 years ago by annakrat

In 69595:

GUI/vdigit: fix snapping units, see #2848 (merge from trunk r69578, r69579, r69580)

comment:21 Changed 3 years ago by annakrat

Resolution: fixed
Status: newclosed

OK, backported, tested again and hopefully fixed now.

comment:22 Changed 3 years ago by martinl

In 70404:

wxGUI/vdigit: fix GetThreshold?() (see #2848)

comment:23 Changed 3 years ago by martinl

In 70405:

wxGUI/vdigit: fix GetThreshold?() (see #2848, see #3266)

Note: See TracTickets for help on using tickets.