Simplify sketch paths using Douglas-Peuker algorithm #150

danielworkman · 2021-06-09T18:44:36Z

Added an implementation of the Douglas-Peuker algorithm for simplifying polylines/paths
This gets run on a sketch path a) each time a user lifts their finger after making a new sketch and b) when loading a saved survey. This means that older surveys saved before the addition of this enhancement can get the same performance gains.
The calculated epsilon value is quite crude but can be improved after additional testing if necessary
Added a "debug mode" option to the "Dev" menu to toggle the drawing of path points which is handy when analysing epsilon values
Using the current epsilon calculation there was a reduction of 54% of the total sketch coordinates after loading a large test project

… also some basic unit tests

…path and b) on loading of JSON so that older survey files can also make use of the performance improvement. Also added debug drawing of path points which makes deriving sensible epsilon values easier

danielworkman · 2021-06-09T18:45:01Z

app/src/main/java/org/hwyl/sexytopo/control/activity/SexyTopoActivity.java

@@ -62,11 +62,12 @@
 public abstract class SexyTopoActivity extends AppCompatActivity {


-    protected static SurveyManager dataManager;


Android studio claimed this was a memory leak

Ah yep, it does whinge about that. It's because it's holding a reference to a Context. I don't think it's an issue - the only context we're setting is an ApplicationContext, which is (apparently) fine.

Looks like not a problem making this non-static though.

danielworkman · 2021-06-09T18:46:52Z

app/src/main/java/org/hwyl/sexytopo/control/util/Space2DUtils.java

+        double distMax = 0;
+
+        for (int i = 1; i != pathSize; ++i) {
+            double dist = getDistanceFromLine(path.get(i), path.get(0), path.get(pathSize - 1));


Considering how much this function gets called we should really implement a version that does not perform the square root and compare against the square of the epsilon but we can optimise when and where necessary

danielworkman · 2021-06-09T18:48:58Z

app/src/main/java/org/hwyl/sexytopo/control/util/Space2DUtils.java

+
+        return DouglasPeukerIteration(path, epsilon);
+    }
+


These methods may need moving to more appropriate homes but I'll leave that decision to you :)

Fair enough - not a terrible place for them to live for now. Although they should have lowercase first letters because the initial caps looks a bit weird in Java.

richsmith · 2021-06-09T22:21:54Z

app/src/main/java/org/hwyl/sexytopo/control/io/basic/SketchJsonTranslater.java

+
+        double epsilon = Space2DUtils.SketchEpsilon(pathDetail);
+        List<Coord2D> simplifiedPath = Space2DUtils.Simplify(path, epsilon);
+        pathDetail.setPath(simplifiedPath);


I'm wondering whether we should set this behind a version guard clause. That is, only do this if the file was saved with a SexyTopo earlier than the current version. Although that would be a pain in the bum to do that the way the file is currently written. Guess this Douggie-Puker algo is idempotent? May well not be worth it.

Assuming you use the same epsilon then yes, you'll get exactly the same result each time

You probably understood what I meant, but just checking: we don't just need the result to be the same for the same input, but we also need the result to be the same for feeding the output of the first pass back into the algorithm. That is, we don't want the paths to get progressively simpler every time the file gets loaded.

richsmith · 2021-06-09T22:26:44Z

app/src/main/res/menu/action_bar.xml

@@ -88,6 +88,8 @@
          android:orderInCategory="100"
          app:showAsAction="never">
        <menu>
+            <item android:id="@+id/action_debug_mode"


I'm going to hide this dev menu behind a settings flag as it looks a bit messy.

(Just an FYI - you don't need to do anything. I've been fiddling with the menus in my Bric4 branch to add custom entries for instrument ops).

richsmith

Great job! Just needs those methods uncapitalised ;)

danielworkman · 2021-06-10T08:00:52Z

If you look at the implementation of the algorithm you can see that that will not be the case. You could run the algo 1000 times on a geometry simplified just once and nothing would happen

…

On Thu, 10 Jun 2021 at 08:36, Rich Smith ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In app/src/main/java/org/hwyl/sexytopo/control/io/basic/SketchJsonTranslater.java <#150 (comment)>: > @@ -161,6 +160,11 @@ public static PathDetail toPathDetail(JSONObject json) throws JSONException { } PathDetail pathDetail = new PathDetail(path, colour); + + double epsilon = Space2DUtils.SketchEpsilon(pathDetail); + List<Coord2D> simplifiedPath = Space2DUtils.Simplify(path, epsilon); + pathDetail.setPath(simplifiedPath); You probably understood what I meant, but just checking: we don't just need the result to be the same for the same input, but we also need the result to be the same for feeding the output of the first pass back into the algorithm. That is, we don't want the paths to get progressively simpler every time the file gets loaded. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#150 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGM4DA6BWCBTHVWJZQWI27DTSBTRHANCNFSM46MTJVHQ> .

richsmith · 2021-06-10T11:01:31Z

If you look at the implementation of the algorithm you can see that that will not be the case. You could run the algo 1000 times on a geometry simplified just once and nothing would happen

I guess I should stop being lazy and understand what the algo does!

[...]

OK, think I understand it now. Quite elegant. Yep, should be fine.

danielworkman · 2021-06-11T18:56:19Z

app/src/main/java/org/hwyl/sexytopo/control/activity/SexyTopoActivity.java

@@ -985,6 +983,15 @@ public void onClick(DialogInterface dialog, int id) {
    }


+    private void toggleDebugMode() {
+        debugMode = !debugMode;
+        dataManager.broadcastSurveyUpdated();


I really just want to call invalidate on the canvas so that you instantly see the path points but this will call the sync method hence how I noticed how long it would take to refresh. It's fine as long as the table has never been loaded though so I just don't do that. Worth broadcasting an invalidate style message instead?

Ahah - are you saying that the table view gets updated even if it's not currently active? That does make sense - it would explain a couple of bug reports about things hanging when updating data. And since I rarely use it that might explain why I didn't see the issue.

I assumed it wouldn't do anything f it's not currently running. (Well actually I suppose I hadn't thought about it.)

I don't think it's worth a separate broadcast option. There's probably a command to tell the activity to redraw itself.

However, it's a bit useless having the table update itself even when you can't see it. I think we need to add a check at the start of the sync methods to not bother if the activity is not Resumed and then sync in onResume().

Yep to repro:

Load that survey you sent me

Load table view

Load plan view

Pan away from the main survey (to discount draw time)

Draw a small sketch

Toggle debug mode

danielworkman added 3 commits June 9, 2021 19:18

Add a menu option for toggling debug drawing on/off

6a2ba5d

Added the Douglas-Peuker simplification algorithm to Space2DUtils and…

5ac3d0e

… also some basic unit tests

Made use of the simplification algorithm a) after finishing a sketch …

1f912e7

…path and b) on loading of JSON so that older survey files can also make use of the performance improvement. Also added debug drawing of path points which makes deriving sensible epsilon values easier

danielworkman commented Jun 9, 2021

View reviewed changes

Replace calls to Simplify with calls to DouglasPeukerIteration

2db468c

richsmith reviewed Jun 9, 2021

View reviewed changes

Code review lowercase methods

e1c9350

danielworkman commented Jun 11, 2021

View reviewed changes

richsmith approved these changes Jun 12, 2021

View reviewed changes

richsmith merged commit 67cfd4a into richsmith:master Jun 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify sketch paths using Douglas-Peuker algorithm #150

Simplify sketch paths using Douglas-Peuker algorithm #150

danielworkman commented Jun 9, 2021

danielworkman Jun 9, 2021

richsmith Jun 9, 2021

danielworkman Jun 9, 2021

danielworkman Jun 9, 2021

richsmith Jun 9, 2021

richsmith Jun 9, 2021

danielworkman Jun 9, 2021

richsmith Jun 10, 2021

richsmith Jun 9, 2021

richsmith Jun 9, 2021

richsmith left a comment

danielworkman commented Jun 10, 2021 via email

richsmith commented Jun 10, 2021

danielworkman Jun 11, 2021

richsmith Jun 12, 2021

danielworkman Jun 12, 2021

		@@ -62,11 +62,12 @@
		public abstract class SexyTopoActivity extends AppCompatActivity {


		protected static SurveyManager dataManager;

Simplify sketch paths using Douglas-Peuker algorithm #150

Simplify sketch paths using Douglas-Peuker algorithm #150

Conversation

danielworkman commented Jun 9, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richsmith left a comment

Choose a reason for hiding this comment

danielworkman commented Jun 10, 2021 via email

richsmith commented Jun 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment