Make map paint code use double coordinates

[michael2402]

This patch makes the map paint code use double coordinates.

It prevents the MapView from rounding. We use Path2D (GeneralPath) for drawing most of the time so there is no change in the computations needed, we simply skip a rounding step.

I moved some computations to a common place. I moved the code to compute a symbol shape to Symbol.

I use the MapViewPoint class in many places. It is an immutable Point2D that can be used to get the view and the east/north or lat/lon coordinates. That way, we do not need more intermediate objects. The MapPath2D has a convenience method to add such points.

A new ArrowHelper draws the arrows. You simply tell it the point the arrow should point to and the direction it should point from.

[Don-vip]

the patch does not apply anymore, can you please update it?

[michael2402]

I updated that one, too.

[Don-vip]

In 10827/josm:

fix #13306 - Make map paint code use double coordinates (patch by michael2402) - gsoc-core

[Don-vip]

In 10829/josm:

see #13306 - fix wrong version number

[Don-vip]

Do we really want to deprecate the getPoint() methods? It is used 90 times in JOSM, even more in plugins. I looked at the first occurence in JOSM code (ButtonMarker.java), there is no added value to replace

    @Override public boolean containsPoint(Point p) {
        Point screen = Main.map.mapView.getPoint(getEastNorth());
        buttonRectangle.setLocation(screen.x+4, screen.y+2);
        return buttonRectangle.contains(p);
    }

by:

    @Override public boolean containsPoint(Point p) {
        Point2D screen = Main.map.mapView.getPoint2D(getEastNorth());
        buttonRectangle.setLocation((int) screen.getX()+4, (int) screen.getY()+2);
        return buttonRectangle.contains(p);
    }

[michael2402]

... it depends on if we want to encourage such use. If we simply change the code like you suggested, we won't gain anything.

1. containsPoint is not able to handle multiple map views.
2. containsPoint does not respect the border
3. The click handling on the map view is a bit (or a lot) hacked right now - the best thing would be to register a single listener on the mapView. We could then add a new method to the layer, like handleClick() or handleContextMenu(). This would even allow us to overlay multiple context menus for the imagery layers.
4. We should compute the map position for every click first. So we have boolean containsPoint(MapViewPoint p)
5. You can change this method to use a local Rectangle2D - this gives the JIT the ability to inline more and optimize away the object creation
6. We can add a method MapViewPoint#rectAround(width, height) to quickly create that rectangle.

In plugins, I even found uses of mv.getPoint(Main.getProjection.latlon2eastnorth...)

Other examples that currently use the method:

• GPX: DrawArrows: Use the new draw helper class. We have several places where such arrows are drawn.
• GpxDrawHelper: In drawLines, we can use a Path2D
• GpxDrawHelper: In drawPoints, e.g. g.fill(screen.rectAround(largesize, largesize))
• DrawAction: It already uses a Path2D. Simply change to Point2D - or simply MapViewPoint, so you do not even need the .getX()

But you are right, there are some situations (e.g. painting the GUI buttons) where integer coordinates are easier to handle and potentially faster.

[Don-vip]

OK. Well until we remove all uses of these methods in JOSM core, I'm removing the deprecation warning, it causes too much noise. It can be added back at the same time the method is no more used in core.

[Don-vip]

In 10843/josm:

see #13306 - remove (temporarily ?) deprecation warning for getPoint methods, still massively used in JOSM core

[Don-vip]

I don't understand what's going on with LineClip. Your patch has modified it, but the class is not used anymore. As a result the optimized build process removes it, and we have #13392.

Is the class still needed?

[michael2402]

I don't know if any plugin uses it. As far as I can tell, the overflow problem is solved by using double instead of int coordinates, so it is not needed since Java2D does the right clipping on it's own.

[Don-vip]

Replying to michael2402:

it is not needed since Java2D does the right clipping on it's own.

Are we sure of that? I see in the removed code this mention which refers to #4289 and #4424:

                                /**
                                 * Do custom clipping to work around openjdk bug. It leads to
                                 * drawing artefacts when zooming in a lot. (#4289, #4424)
                                 * (Looks like int overflow.)
                                 */
                                LineClip clip = new LineClip(p1, p2, bounds);

Today we have #13385 which seems related, what do you think?

[michael2402]

#13385 is not related to clipping. LineClip did only clip the part of the line that was out of view, in #13385, all lines are inside the view.

[Don-vip]

OK let's remove it then.

[Don-vip]

In 10857/josm:

see #13306 - remove unused LineClip class

[Don-vip]

In 10858/josm:

see #13306, see #13392 - remove Main.isOpenJdk, not used anymore