Modify

Opened 3 years ago

Closed 10 days ago

Last modified 10 days ago

#10777 closed enhancement (fixed)

[patch] Use a multiplier instead of a fixed "createcircle.nodecount"

Reported by: Atalanttore Owned by: team
Priority: normal Milestone: 17.09
Component: Core Version: latest
Keywords: nodecount multiplier circle Cc: naoliv, Klumbumbus

Description

Hi,

I would prefer a multiplier instead of a fixed createcircle.nodecount with a default value of 16. Big circles often need more nodes than 16 and small ones probably less than 16.

My idea is to calculate the necessary nodecount of the circle depending on the distance between the two or three selected nodes for creating a circle.

In short: distance between the selected nodes * multiplier = nodecount

Best Regards,
Ettore Atalan

Attachments (0)

Change History (12)

comment:1 Changed 20 months ago by simon04

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

comment:2 Changed 20 months ago by simon04

Cc: naoliv added

comment:3 Changed 20 months ago by Klumbumbus

Cc: Klumbumbus added

comment:4 Changed 2 weeks ago by naoliv

Empirically I tested the radius value with a (what I consider to be a) satisfactory number of nodes:

r  → n
2  → 8
4  → 12
8  → 16
16 → 24
32 → 32
64 → 48

ie, a radius of 16 meters should give us 24 nodes in the circle.

And found a formula that gives a good approximation for this, as 6 * r ^ (1/2)

https://i.imgur.com/BdWKklC.png

Of course you will ignore that I don't know Java and consider this as a proof of concept, but I got some very good results by using

numberOfNodesInCircle = (int) Math.ceil(6.0 * Math.pow(r, 0.5));

right after the radius is calculated (at https://josm.openstreetmap.de/browser/josm/trunk/src/org/openstreetmap/josm/actions/CreateCircleAction.java#L195)

Is it possible to properly implement this?

comment:5 Changed 2 weeks ago by Atalanttore

Thanks for working on my suggestion.

comment:6 Changed 11 days ago by naoliv

Functional proof-of-concept patch:

  • src/org/openstreetmap/josm/actions/CreateCircleAction.java

     
    130130        if (!isEnabled())
    131131            return;
    132132
    133         int numberOfNodesInCircle = Main.pref.getInteger("createcircle.nodecount", 16);
    134         if (numberOfNodesInCircle < 1) {
    135             numberOfNodesInCircle = 1;
    136         } else if (numberOfNodesInCircle > 100) {
    137             numberOfNodesInCircle = 100;
    138         }
    139 
    140133        DataSet ds = getLayerManager().getEditDataSet();
    141134        Collection<OsmPrimitive> sel = ds.getSelected();
    142135        List<Node> nodes = OsmPrimitive.getFilteredList(sel, Node.class);
     
    193186        double r = Math.sqrt(Math.pow(center.east()-n1.east(), 2) +
    194187                Math.pow(center.north()-n1.north(), 2));
    195188
     189        // minimum circle will have 6 nodes when using a way with length ≥ 1.4m
     190        if (r < 0.7) {
     191            new Notification(
     192                tr("Nodes are too close to create a circle."))
     193                .setIcon(JOptionPane.INFORMATION_MESSAGE)
     194                .setDuration(Notification.TIME_LONG)
     195                .show();
     196            return;
     197        }
     198
     199        // see #10777
     200        int numberOfNodesInCircle = (int) Math.ceil(6.0 * Math.pow(r, 0.5));
     201
    196202        // Order nodes by angle
    197203        PolarNode[] angles = new PolarNode[nodes.size()];
    198204        for (int i = 0; i < nodes.size(); i++) {

My view: we don't need to have a default value anymore (since it's dynamically calculated using the radius) nor we should create too tiny circles.

comment:7 Changed 11 days ago by bastiK

Summary: Use a multiplier instead of a fixed "createcircle.nodecount"[patch] Use a multiplier instead of a fixed "createcircle.nodecount"

Thanks for the patch! Just two remarks:

  • The variable r is not in meter. To get a value in meter, you can use LatLon.greatCircleDistance.
  • This is a matter of taste, but why not just take a minimum of 6 points, instead of refusing to create a small circle?

comment:8 Changed 10 days ago by naoliv

New version:

  • src/org/openstreetmap/josm/actions/CreateCircleAction.java

     
    130130        if (!isEnabled())
    131131            return;
    132132
    133         int numberOfNodesInCircle = Main.pref.getInteger("createcircle.nodecount", 16);
    134         if (numberOfNodesInCircle < 1) {
    135             numberOfNodesInCircle = 1;
    136         } else if (numberOfNodesInCircle > 100) {
    137             numberOfNodesInCircle = 100;
    138         }
    139 
    140133        DataSet ds = getLayerManager().getEditDataSet();
    141134        Collection<OsmPrimitive> sel = ds.getSelected();
    142135        List<Node> nodes = OsmPrimitive.getFilteredList(sel, Node.class);
     
    193186        double r = Math.sqrt(Math.pow(center.east()-n1.east(), 2) +
    194187                Math.pow(center.north()-n1.north(), 2));
    195188
     189        // see #10777
     190        LatLon ll1 = Main.getProjection().eastNorth2latlon(n1);
     191        LatLon ll2 = Main.getProjection().eastNorth2latlon(center);
     192
     193        double radiusInMeters = ll1.greatCircleDistance(ll2);
     194
     195        int numberOfNodesInCircle = (int) Math.ceil(6.0 * Math.pow(radiusInMeters, 0.5));
     196        if (numberOfNodesInCircle < 6) {
     197            numberOfNodesInCircle = 6;
     198        }
     199
    196200        // Order nodes by angle
    197201        PolarNode[] angles = new PolarNode[nodes.size()];
    198202        for (int i = 0; i < nodes.size(); i++) {

comment:9 Changed 10 days ago by bastiK

Resolution: fixed
Status: newclosed

In 12837/josm:

fixed #10777 - new algorithm to determine the node count in CreateCircleAction (patch by naoliv)

comment:10 Changed 10 days ago by bastiK

Thanks for the patch, your square root formula works quite well!

comment:11 Changed 10 days ago by Don-vip

Milestone: 17.09

comment:12 Changed 10 days ago by naoliv

Great! Thank you!

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


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

 
Note: See TracTickets for help on using tickets.