Opened 6 years ago
Last modified 2 years ago
#17898 new enhancement
better protection of relation member order
Reported by: | skyper | Owned by: | team |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | Core | Version: | latest |
Keywords: | split way relation order download | Cc: | JeroenHoek |
Description (last modified by )
Right now, you do not get any information if you split a way of an incompletely downloaded relation and it is not possible to determine the right position of the new member (before or after) in order to keep the order correct.
What I ask for:
- a warning that user can intervene and manually download at least one more member.
- in #18596 for
type=route/multipolygon
relation a dialog was added but only if the relation is already known/downloaded.- the way to determine needed downloads need to be adjusted/changed, see #18863.
the neighbors in the relation's member list are used instead of the connected ways in data.see comment 35 on #18863
- support for more types is needed see #10808
- is there any relation which does not benefit from preserving the order and has no special treatment.
- the way to determine needed downloads need to be adjusted/changed, see #18863.
- in #18596 for
- an option to automatically download one or two members to determine the correct position.
- introduced for
type=route/multipolygon/boundary
in #18596- does not work for all multipolygon see comment 60 on #18863 and needs to be checked for boundary
- introduced for
- a silent, temporally, partial download of the direct neighbors to automatically find the right position.
- discussion?
Not sure if you really need both neighbors.
Attachments (7)
Change History (39)
comment:1 by , 5 years ago
Description: | modified (diff) |
---|
comment:2 by , 5 years ago
Cc: | added |
---|---|
Description: | modified (diff) |
Keywords: | way download added |
comment:3 by , 5 years ago
comment:4 by , 5 years ago
Description: | modified (diff) |
---|
follow-up: 10 comment:5 by , 5 years ago
Description: | modified (diff) |
---|
- To solve 1. split way needs to check if all parent relations are known.
ATM, this means that at least one child node with positive id needs to be within a download area bbox.
Otherwise a least a warning should be displayed about possibly damaging unknown relations. Optional is the option to download one of the selected nodes or, maybe better, both end nodes, see 3. - temporally downloads as in 3. are not used in JOSM, atm. There is no virtual/cache data layer. I thought of a solution to download the needed members only to determine the proper position in the member list and then purge/forget about them again
What steps will reproduce the problem?
- Download a way with route relation
- In relation list: download incomplete members
- Select any node with parent way completely outside download area
- split way
Here we need the warning. What happens afterwards is to be discussed and optional.
comment:7 by , 5 years ago
Description: | modified (diff) |
---|
comment:8 by , 5 years ago
Description: | modified (diff) |
---|
comment:9 by , 5 years ago
Description: | modified (diff) |
---|
comment:10 by , 5 years ago
Replying to skyper:
What steps will reproduce the problem?
- Download a way with route relation
- In relation list: download incomplete members
- Select any node with parent way completely outside download area
- split way
Here we need the warning. What happens afterwards is to be discussed and optional.
When I follow the first two steps I don't have a download area. Anyhow, whenever a way is split AND there is either no download area OR all nodes of the way are outside of any download area we can't be sure that we know all parent relations.
I see three possible strategies when this happens:
1) Show dialog that asks for confirmation to download the parents of the way.
2) Show dialog that asks for confirmation to download a minimal area around one (or both) endnodes. Do we ever need both?
3) Show dialog that asks for confirmation to use "Download Along" with a distance of 1m and a small download area.
Pros & Cons:
pro 1):
- no download area is added (which might be what users want if they get into this situation)
- only neccessary data is downloaded.
pro 2):
- the added download area tells us that the parents are known, so we will not ask again,
- also we should know the connected ways and thus have all needed information to determine the order of the members
pro 3):
- User has all the data that is relevant for the way
- Relations are known and order can be determined without further popups
con 1):
- no download areas are added, we may show the dialog again and may download the same parents again
- if parent relations exist and order is important a 2nd dialog may ask if missing members should be downloaded (#18596)
con 2):
- other, maybe unrelated ways and relations might be downloaded
- if the added download area is the first the rendering changes completely (dashes everywhere)
- the minimal download area is invisible (not rendered), zoom to download area shows the node
con 3)
- if the way is long the download of the areas might take long
- a lot more data is downloaded compared to the other strategies
I think I prefer 2), and maybe we can even remove all the code which was added to download missing members(#18596)
(Typically, I use strategy 3) before I consider to split a way)
follow-up: 12 comment:11 by , 5 years ago
download a minimal area around one (or both) endnodes. Do we ever need both?
Yes, the split-way might be the first or last way in a route relation.
by , 5 years ago
Attachment: | 17898-download-areas.patch added |
---|
patch implemets the download areas strategy, also contains 18863-detect-broken.2.patch
comment:12 by , 5 years ago
With the possibility of not creating any download area and the knowledge I gained in #18863, I would prefer 1) but only one dialog for all actions, e.g.:
1a) Show a dialog that asks for confirmation to download the parents of the way and, possibly, up to two additional members of each relation.
Replying to GerdP:
download a minimal area around one (or both) endnodes. Do we ever need both?
Yes, the split-way might be the first or last way in a route relation.
- PTv1 allows to add variants which start and end ways are only connected to one neighbor.
- Ordered dual-way routes can have members with role
forward/backward/link
which are only connected to one neighbor. - If the order of any relation is not correct only one neighbor might be connected.
I tend to be save and always download both neighbors if one end node is outside downloaded area.
comment:13 by , 5 years ago
OK, I 'll implement the 1a) solution. If users complain we might add a preference key to select the strategy.
We have probably more actions which should ask for missing parent relations, question is if we should ask that for each action or if we can store the decision in a general key like action_download_needed_data
(current solution uses split_way_download_missing_members
)
by , 5 years ago
Attachment: | 17898.2.patch added |
---|
comment:14 by , 5 years ago
17898.2.patch introduces new class MissingDataHelper
which should be usable for all actions which might change relation data. Besides that it tries to implement the 1a) solution from comment:11
I am not yet sure how much more code we can move into this class. I assume other actions require more or less the same checks.
The strings in the dialog are rather generic now.
Work in progress...
comment:15 by , 5 years ago
How about "downloading parents" if none of the split-way's child node is inside a downloaded area? I do this manually right now before splitting but this is the job of a machine.
Pro:
- no downloaded area will be added
- no unneeded member/object is downloaded
Contra:
- split way might need more members to find the correct place in the member list and has to download again.
comment:16 by , 5 years ago
I think that is what the patch does. It contains this
+ if (!way.isNew() && (way.getDataSet().getDataSourceBounds().isEmpty() + || way.getNodes().stream().allMatch(Node::isOutsideDownloadArea))) {
comment:17 by , 5 years ago
Description: | modified (diff) |
---|
by , 5 years ago
Attachment: | 17898.3.patch added |
---|
by , 5 years ago
Attachment: | split-missing-parent.PNG added |
---|
by , 5 years ago
Attachment: | combine-missing-parent.PNG added |
---|
follow-up: 25 comment:18 by , 5 years ago
new patch
- corrects a problem that the download dialog was shown although user decided to always download
- also offers to download possibly missing parents when ways are combined (see #19394)
Dialogs need fine tuning reg. the texts. Which variant is better?
comment:20 by , 5 years ago
Selected ways are not inside a downloaded area. Parent relations might be corrupted combining the ways.
or
Not all selected ways are inside a downloaded area. Their parent relations might be corrupted combining the ways.
Do we use operation
or action
(shorter) ?
It helps a lot for documentation, if context sensitive help support is added right away to new dialogs. Then F1
can lead to a wiki page explaining the message and a help buttons can be added to the dialog if wanted/needed.
comment:21 by , 5 years ago
OK, I like the 2nd, but I think a by
is missing.
Not all selected ways are inside a downloaded area. Their parent relations might be corrupted by combining the ways.
I have no idea for a better text regarding split ways. Maybe
The selected node is not inside a download area. Relations might be corrupted by this action.
Not all selected nodes are inside a download area. Relations might be corrupted by this action.
The source also contains this very generic message which could be used for all actions. We may also use a single preference key to store the user decision:
Relations might be corrupted by this action.
follow-up: 24 comment:22 by , 5 years ago
As a non-native speaker, I would say both are correct, with or without "by".
The way intended to split is not inside a downloaded area. Its parent relations might be corrupted by this action
Is it "download" or "downloaded" area?
I do not understand, if you try to make it a more general message, to be used with other actions, or if you plan to have one message per action?
Where is the general message Relations might be corrupted by this action.
used?. Could/should it be changed to Objects' parent relations might be corrupted by this action.
comment:23 by , 5 years ago
Oh, we can then use Download parents from server?
or Download parent relations from server?
.
comment:24 by , 5 years ago
Replying to skyper:
As a non-native speaker, I would say both are correct, with or without "by".
The way intended to split is not inside a downloaded area. Its parent relations might be corrupted by this action
Is it "download" or "downloaded" area?
Good question. In JOSM sources "download area" is used for different things:
- a single rectangle that should be downloaded from the server
- a single rectangle that was downloaded
- the area covered by (possibly overlapping) rectangles which were downloaded.
Is it explained in the wiki?
I do not understand, if you try to make it a more general message, to be used with other actions, or if you plan to have one message per action?
I think about these alternatives and wonder what would be better.
Where is the general message
Relations might be corrupted by this action.
used?. Could/should it be changed toObjects' parent relations might be corrupted by this action.
Currently it is not used, both actions overwrite the default, but I wonder if this really improves the UI. I presume that we can add a similar dialog for more actions. The messages will be very similar and users might not understand why they see the dialog again and again although they selected the "don't show again" option.
I think it could be good to have a single dialog for all actions which need more data.
comment:25 by , 5 years ago
Replying to GerdP:
Dialogs need fine tuning reg. the texts. Which variant is better?
To be somewhat consistent with:
I suggest to:
- add ok icon to the left button
- add cancel icons to the middle and right button
- label the the middle button "No, cancel"
- switch the positions of the middle and left button
- add help button
comment:26 by , 5 years ago
switch the positions of the middle and left button
You want to make Cancel the default?
follow-up: 30 comment:28 by , 5 years ago
OK, I already guessed that ;)
What's your opinion about the message texts? Should there be a different text for each reason that a download is needed?
So far we want to download either the parent relations or incomplete members of already known relations (the first might trigger the 2nd). I've removed the word missing
because the popup might also appear when parents relations are all known.
So, this would be the generic popup that could be displayed for all actions which might require more data about relations:
by , 5 years ago
Attachment: | 17898.4.patch added |
---|
patch implements generic popup and a single preference key for both (all) actions
follow-up: 31 comment:29 by , 5 years ago
A general question is if we really want to go this way. When we show this dialog for some actions users are encouraged to work without download areas or outside of them. I already see complains like "JOSM did not warn so I thought it was OK" or "Why didn't it download the data automatically?"
comment:30 by , 5 years ago
Replying to GerdP:
Should there be a different text for each reason that a download is needed?
No thats not needed imo.
comment:31 by , 4 years ago
Replying to GerdP:
A general question is if we really want to go this way. When we show this dialog for some actions users are encouraged to work without download areas or outside of them. I already see complains like "JOSM did not warn so I thought it was OK" or "Why didn't it download the data automatically?"
We warn about deleting and about ungluing nodes outside download area. A new user did ask about it on the German forum.
You always have to be careful, even working on the edge inside download area and there will be a lot of cases where JOSM will not warn but in this case, I think, a warning is helpful and will lead to less damage by experienced users.
See #18596 for the first step.