[PATCH] There should be some way to determine how many nodes are superflous in a way

[taylor.smock]

This can be used to tell the user roughly how many nodes will be removed (although, since it uses the same algorithm as the actual node removal, it should be the same).

The primary reason I want it is so that I can easily tell how many nodes will be removed, if the user simplifies the way, although this patch isn't necessary for my needs (I can just directly call the commands with specified parameters).

I do think it will be a useful UI component, regardless (as seen in the patch).

[taylor.smock]

This was an attempt to allow the mapview display how the maximum error (threshold) actually affected the way. This does not work, since the EDT is blocked.

[taylor.smock]

Is there any interest for this patch?

[simon04]

In 16566/josm:

fix #19199 - SimplifyWayAction: display how many nodes are going to be removed (patch by taylor.smock, modified)

[GerdP]

Nice! I miss two points:
1) some kind of percentage would be nice, e.g. "about 1,232 nodes (30%) to remove"
2) Shouldn't convert gpx to data use the same code and thus show the same dialog?

[taylor.smock]

Replying to GerdP:

Nice! I miss two points:
1) some kind of percentage would be nice, e.g. "about 1,232 nodes (30%) to remove"

I think I did look at adding a percentage. I don't remember why I didn't (possibly due to line length -- I probably didn't want to try to do a line-wrap).

2) Shouldn't convert gpx to data use the same code and thus show the same dialog?

I've never used that feature -- my local area has recent imagery (~1 year old at this point). I would expect them to use the same dialog.

That being said, the nodes to remove only appear if the calling function sends the ways that are being modified to the dialog. Otherwise, it cannot determine the number of nodes that will be removed.

I'll be uploading a patch for that here in a minute.

[taylor.smock]

Send ways that will be simplified due to converting from GPX to OSM data

[GerdP]

In 16570/josm:

see #19199: There should be some way to determine how many nodes are superflous in a way

• show number of nodes also when converting gpx to data layer

19199.gpx_to_osm.patch by taylor.smock

[taylor.smock]

It looks like the 19199.1.patch was the patch that was merged. If I (or someone else) ever gets around to allowing the map screen to repaint while a dialog window is up, some of the code used will no longer be dead (it is effectively dead -- it is still called, but doesn't do what it is supposed to do). It was supposed to show how the way changed as the threshold was modified.

See https://josm.openstreetmap.de/browser/josm/trunk/src/org/openstreetmap/josm/actions/SimplifyWayAction.java?rev=16566#L499 for the code in question.

Calling code could probably be modified to do something like this:

MainApplication.worker.submit(() -> {
    int ret = SimplifyWayAction.askSimplifyWays(ways, text, auto);
    realizeSimplifyReturn(ret);
});

There are probably some EDT violations in there somewhere, and there might be some other locations where the EDT is blocked by the simplify way dialog.

[Bjoeni]

This needs to be moved to another thread. When converting big GPX files to datasets JOSM becomes unresponsive for a long time (more than 20 seconds) just figuring out how many nodes are to be removed (same when changing the max-error).

[taylor.smock]

Add threading

[taylor.smock]

@Bjoeni: Can you apply that patch and see if it fixes your problem? I don't have a large enough GPX file for it to show any real difference on my machine.

[Bjoeni]

Yep, that works perfectly fine. Thank you!

[taylor.smock]

(Don't apply that patch to JOSM core yet -- it may have undesirable side effects, for example, what happens if it is still calculating when the dialog is closed? I really need to find something that takes a good amount of time for testing purposes, but worst case scenario, it will run the command and place it in the undo/redo buffer.)

[Bjoeni]

what happens if it is still calculating when the dialog is closed?

For me that didn't cause any issues. nodesToRemove.setText() goes through without any issues even if the dialog is closed already. Also doesn't cause any issues even if the same dialog is opened again while the closed one is still calculating.

Unfortunately I can't provide you with the original GPX file I'm currently using, but you could just put a Thread.sleep() call in updateNodesToRemove. Or you download a lot of raw GPS data from the server and convert that (doesn't take that long either but long enough to test).

[Bjoeni]

@taylor.smock
What's the state of this patch? It works for me, and makes things much faster (just tested it without the patch on a 250MB file, JOSM froze for 55 secs before showing the dialog and also each time the value was changed).

[taylor.smock]

Replying to Bjoeni:

@taylor.smock
What's the state of this patch? It works for me, and makes things much faster (just tested it without the patch on a 250MB file, JOSM froze for 55 secs before showing the dialog and also each time the value was changed).

That is a good question. I should have remembered to ping the ticket every two weeks or so (it probably just fell out of the active ticket range).

Anyway, I'll upload a patch that applies cleanly (there was a slight line-number change).

[taylor.smock]

Update patch to current source (line number change only)

[GerdP]

I have a file containing my collected recent GPX tracks. I created it by merging ~340 tracks, it's about ~23.5 MB. I load that file and choose "Convert to data layer". When I click on convert the unpatched version shows a small delay and than the "Simplify way" dialog while at the same time a new command command is added to the "Command stack window".
With the patch it is even more confusing because the command stack is updated while I'm looking at the popup.
Why is the command added to the UndoRedoHandler before I've even decided what to do?

[taylor.smock]

To answer your question, I wanted to set it up such that the user could see how the way was changing as they modified the number. (See comment:7). That didn't end up working since the dialog window currently blocks the EDT. However, if we unblock the EDT, the user will be able to see how the way changes as they increase/decrease the number (and it finishes calculating).

Since I didn't want to accidentally leave the way in an inconsistent state if something happened, I added the change command to the undo/redo stack. This meant that if the window crashed, the change could still be undone.

Reading back (comment:12),

(Don't apply that patch to JOSM core yet -- it may have undesirable side effects, for example, what happens if it is still calculating when the dialog is closed? I really need to find something that takes a good amount of time for testing purposes, but worst case scenario, it will run the command and place it in the undo/redo buffer.)

I'll see if I can create a large simplify action to test the corner cases.

[GerdP]

OK, so two different, probably conflicting goals:
1) Visualize the changes
2) Compute the number (I'd prefer percentage) of removed nodes without changing anything, preferably as fast as possible (multi-threaded?)

Maybe the overall number of way nodes can be used to decide what is more important?

[taylor.smock]

In order to do either, I first need to calculate the nodes that will be removed. Either one should probably be async, regardless.

For (1), I need to do something so that the EDT isn't blocked by the dialog, so I would need to push the calculation to another thread anyway. For (2), I still have to move it to another thread (multiple), and return the number of nodes that will be removed.

So, I don't think that they are necessarily conflicting goals. But I could be wrong.

[taylor.smock]

Add methods to allow for async ExtendedDialogs (so we can update the map), don't update mapview when there are more than 10 ways selected

[GerdP]

I assume this is WIP? Patch contains mods in .project.

[taylor.smock]

To answer your question, I *think* it's done. It just needs a little bit of testing.

For example, I *think* I fixed a bug where, when changing the max error quickly, would cause a command to not be undone. I wasn't able to repro again after adding syncs, but I wanted to double check when my computer wasn't under intense memory pressure.

The .project mod was an accidental add to the patch.

As soon as I'm on a real computer, and not my phone, I'll edit the patch.

[GerdP]

With the patch the command is still added to the command stack before I've decided what to do when using the normal mode (Shift+Y)
Do we really need all the new code? ExtendedDialog has a parameter reg. modal/non-modal. Isn't that enough?

[taylor.smock]

Remove .project file

[taylor.smock]

Replying to GerdP:

With the patch the command is still added to the command stack before I've decided what to do when using the normal mode (Shift+Y)
Do we really need all the new code? ExtendedDialog has a parameter reg. modal/non-modal. Isn't that enough?

I added the listener code since I didn't see a method to listen for the close of the modal. I'll double check that.

EDIT: As far as adding the command to the command stack goes, that is largely a fail-safe. Once you close the window, it will undo the command. Unless you have made some other modifications in the mapview prior to closing the window.

[taylor.smock]

Drop listener code in ExtendedDialog

[GerdP]

I really don't like the behaviour that the command stack changes while I'm just looking at the popup. Gives me the feeling that it doesn't really matter what I am doing in that dialog, somehow creepy.
My approach would be to execute the dialog in a try / finally block so that the finally block can decide if the command should be undone or added to the undo handler.
OTOH: If nobody else feels that way I can commit the patch since I don't use that function often.

[taylor.smock]

Replying to GerdP:

I really don't like the behaviour that the command stack changes while I'm just looking at the popup. Gives me the feeling that it doesn't really matter what I am doing in that dialog, somehow creepy.

There is a ghost in the machine...
But more seriously, it is changing as you make changes in the dialog, but maybe you are right.

My approach would be to execute the dialog in a try / finally block so that the finally block can decide if the command should be undone or added to the undo handler.

I probably should have done this, but I've made the assumption that I (or someone else) will make a mistake somewhere, and overwrite the command so that it wouldn't be undone in a finally block (since the stored command is not final). If it is in the command stack, at least the user can at least see it and undo it.

Also, like I indicated in comment:25, the user can make map modifications with the window open. If I don't have the command in the undo/redo stack, what should happen when the user makes a modification to the simplified way?

OTOH: If nobody else feels that way I can commit the patch since I don't use that function often.

EDIT: I should probably find a study group. I don't use the function all that often either.

[Friendly_Ghost]

Hello,

Taylor Smock requested that I share this here.
I just made a quintuple changeset by simplifying >42,000 nodes all at once. I noticed a significant performance drop with so many objects loaded in, and an even larger performance drop while I was in the process of simplifying it. Taylor said it could be worth looking into.

The changesets:

• ​https://www.openstreetmap.org/changeset/127414795
• ​https://www.openstreetmap.org/changeset/127414809
• ​https://www.openstreetmap.org/changeset/127414818
• ​https://www.openstreetmap.org/changeset/127414822
• ​https://www.openstreetmap.org/changeset/127414876

Status report: ​https://drive.google.com/file/d/1-gLvdT0d9u3p6pf2x_erTFlJVwgEUcOQ/view?usp=sharing

[taylor.smock]

To clarify, based off of chat, this was both in the map view and in the simplify way dialog.

@Friendly_Ghost: we haven't applied the patch in this ticket, largely because we don't know what users would expect the behavior to be. If you see something in the map view change while you are modifying values in the simplify way dialog, do you expect to be able to undo it if you decide to do something else in the map view?

If it would help, I can upload a patched jar file for you to play around with.

[Friendly_Ghost]

I would not show any changes in the dialog, because people may fiddle with the numbers repeatedly and quite drastically by adding or removing zeroes, and displaying the changes each time may lead to more lag, especially at the scale on which I'm editing today.
I would only show the change after confirmation of the change. After that people are of course free to undo & redo it. I think that's good enough.

If you end up showing the changes in the preview anyway, I suggest that you delay this process enough for slow computers to keep up with the rendering of it.