Modify

Opened 9 months ago

Last modified 6 days 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 skyper)

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:

  1. 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 #18863:comment:35
      • 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.
  2. 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 #18596#comment:60 and needs to be checked for boundary
  3. 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 (2)

17898-download-areas.patch (8.3 KB) - added by GerdP 7 days ago.
patch implemets the download areas strategy, also contains 18863-detect-broken.2.patch
17898.2.patch (26.4 KB) - added by GerdP 6 days ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 2 months ago by skyper

Description: modified (diff)

See #18596 for the first step.

comment:2 Changed 2 weeks ago by skyper

Cc: JeroenHoek added
Description: modified (diff)
Keywords: way download added

In #18596 (r15943) the warning for some relation types were added including an option to download all members but only for already known/downloaded relations.

Last edited 8 days ago by skyper (previous) (diff)

comment:3 Changed 2 weeks ago by JeroenHoek

in #18596 the whole relation is downloaded

The patch introduced in #18596 only downloads the neighbouring members. I think the patch in JOSM r15943 and newer already does most of what you ask. I'm not sure what it is exactly you are requesting. Perhaps you could provide a use case?

comment:4 Changed 2 weeks ago by skyper

Description: modified (diff)

comment:5 Changed 2 weeks ago by skyper

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?

  1. Download a way with route relation
  2. In relation list: download incomplete members
  3. Select any node with parent way completely outside download area
  4. split way

Here we need the warning. What happens afterwards is to be discussed and optional.

comment:6 Changed 8 days ago by skyper

Ticket #8082 has been marked as a duplicate of this ticket.

comment:7 Changed 8 days ago by skyper

Description: modified (diff)

comment:8 Changed 8 days ago by skyper

Description: modified (diff)

comment:9 Changed 7 days ago by skyper

Description: modified (diff)

comment:10 in reply to:  5 Changed 7 days ago by GerdP

Replying to skyper:

What steps will reproduce the problem?

  1. Download a way with route relation
  2. In relation list: download incomplete members
  3. Select any node with parent way completely outside download area
  4. 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)

comment:11 Changed 7 days ago by 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.

Changed 7 days ago by GerdP

Attachment: 17898-download-areas.patch added

patch implemets the download areas strategy, also contains 18863-detect-broken.2.patch

comment:12 in reply to:  11 Changed 6 days ago by skyper

With the possibility of not creating any download area and the knowledge I gained in #17898, 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 Changed 6 days ago by GerdP

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)

Changed 6 days ago by GerdP

Attachment: 17898.2.patch added

comment:14 Changed 6 days ago by GerdP

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...

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain team.
as The resolution will be set.
to The owner will be changed from team to the specified user.
The owner will change to skyper
as duplicate The resolution will be set to duplicate.The specified ticket will be cross-referenced with this ticket
The owner will be changed from team to anonymous.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.