Modify

Opened 3 years ago

Closed 2 months ago

Last modified 2 months 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 (17)

comment:1 Changed 22 months ago by simon04

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

comment:2 Changed 22 months ago by simon04

Cc: naoliv added

comment:3 Changed 22 months ago by Klumbumbus

Cc: Klumbumbus added

comment:4 Changed 3 months 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 3 months ago by Atalanttore

Thanks for working on my suggestion.

comment:6 Changed 2 months 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 2 months 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 2 months 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 2 months 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 2 months ago by bastiK

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

comment:11 Changed 2 months ago by Don-vip

Milestone: 17.09

comment:12 Changed 2 months ago by naoliv

Great! Thank you!

comment:13 Changed 2 months ago by Don-vip

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

comment:14 Changed 2 months ago by ika-chan! on OpenStreetMap

Hi, I wonder if this can be enhanced further to still allow me to dictate the exact number of nodes? I use JOSM to do some local map experiments, but in some cases I wanted to create a circle an exact number of nodes (e.g. 360 or 720). Thanks in advance.

comment:15 Changed 2 months ago by Klumbumbus

You could:

  • create a circle at the radius that gives you approxiamtely 360 nodes
  • manually delete or add a few nods to match exactly 360
  • press O to make it a circle again
  • resize it (ctrl + alt + mouse move)

comment:16 Changed 2 months ago by ika-chan! on OpenStreetMap

Hi Klumbumbus, with all due respect I have a obsession with accuracy. How would the change allow me to ensure that the radius is of an exact value (e.g. 5 km or 10 km)? I like the new way of creating circles, but I also like an advanced mode for exact values. ;-)

comment:17 Changed 2 months ago by naoliv

@ika-chan
Install https://wiki.openstreetmap.org/wiki/JOSM/Plugins/CommandLine and modify the example circle.xml to allow a higher value in maxvalue (ie, instead maxvalue="144" you can set maxvalue="1024")

You will be able to specify exactly the radius and number of nodes that you want.

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.