From 490b932eabce182cd81e2523cde945c34965767c Mon Sep 17 00:00:00 2001
From: lucaosten <luca.osten@hotmail.com>
Date: Thu, 16 Nov 2017 13:16:25 +0100
Subject: [PATCH] Code improvements (#446) + reduced memory usage of
 RenderBucket by making "type" byte instead of int + improved code readability
 in TextItem by renaming variable + added latitude/longitude validation
 methods to GeoPointUtils + solved Java double precision problem in GeoPoint +
 made GeoPoint Serializable for easier Android usage + made
 BoundingBox.toString() human readable for better debugging

---
 vtm/src/org/oscim/core/BoundingBox.java       |  9 ++--
 vtm/src/org/oscim/core/GeoPoint.java          | 26 ++++++++--
 .../layers/tile/vector/labeling/Label.java    |  8 ++--
 .../tile/vector/labeling/LabelPlacement.java  |  6 +--
 .../tile/vector/labeling/LabelPool.java       |  2 +-
 .../tile/vector/labeling/WayDecorator.java    |  8 ++--
 vtm/src/org/oscim/map/ViewController.java     |  1 +
 vtm/src/org/oscim/map/Viewport.java           |  1 +
 .../org/oscim/renderer/bucket/LineBucket.java |  2 +-
 .../oscim/renderer/bucket/RenderBucket.java   | 23 ++++-----
 .../org/oscim/renderer/bucket/TextBucket.java | 12 ++---
 .../org/oscim/renderer/bucket/TextItem.java   | 13 ++---
 .../oscim/renderer/bucket/TextureBucket.java  |  2 +-
 vtm/src/org/oscim/utils/GeoPointUtils.java    | 48 +++++++++++++++++++
 14 files changed, 116 insertions(+), 45 deletions(-)

diff --git a/vtm/src/org/oscim/core/BoundingBox.java b/vtm/src/org/oscim/core/BoundingBox.java
index 4de0b816..81119197 100644
--- a/vtm/src/org/oscim/core/BoundingBox.java
+++ b/vtm/src/org/oscim/core/BoundingBox.java
@@ -2,6 +2,7 @@
  * Copyright 2010, 2011, 2012 mapsforge.org
  * Copyright 2014 Hannes Janetzek
  * Copyright 2016-2017 devemux86
+ * Copyright 2017 Luca Osten
  *
  * This file is part of the OpenScienceMap project (http://www.opensciencemap.org).
  *
@@ -365,13 +366,13 @@ public class BoundingBox {
     public String toString() {
         return new StringBuilder()
                 .append("BoundingBox [minLat=")
-                .append(minLatitudeE6)
+                .append(getMinLatitude())
                 .append(", minLon=")
-                .append(minLongitudeE6)
+                .append(getMinLongitude())
                 .append(", maxLat=")
-                .append(maxLatitudeE6)
+                .append(getMaxLatitude())
                 .append(", maxLon=")
-                .append(maxLongitudeE6)
+                .append(getMaxLongitude())
                 .append("]")
                 .toString();
     }
diff --git a/vtm/src/org/oscim/core/GeoPoint.java b/vtm/src/org/oscim/core/GeoPoint.java
index 2ec99fde..bc553c33 100644
--- a/vtm/src/org/oscim/core/GeoPoint.java
+++ b/vtm/src/org/oscim/core/GeoPoint.java
@@ -3,6 +3,7 @@
  * Copyright 2012 Hannes Janetzek
  * Copyright 2016 Andrey Novikov
  * Copyright 2016 devemux86
+ * Copyright 2017 Luca Osten
  *
  * This file is part of the OpenScienceMap project (http://www.opensciencemap.org).
  *
@@ -21,11 +22,18 @@ package org.oscim.core;
 
 import org.oscim.utils.FastMath;
 
+import java.io.Serializable;
+
 /**
  * A GeoPoint represents an immutable pair of latitude and longitude
  * coordinates.
  */
-public class GeoPoint implements Comparable<GeoPoint> {
+public class GeoPoint implements Comparable<GeoPoint>, Serializable {
+    /**
+     * Generated serial version UID
+     */
+    private static final long serialVersionUID = 8965378345755560352L;
+
     /**
      * Conversion factor from degrees to microdegrees.
      */
@@ -109,7 +117,10 @@ public class GeoPoint implements Comparable<GeoPoint> {
 
     @Override
     public int compareTo(GeoPoint geoPoint) {
-        if (this.longitudeE6 > geoPoint.longitudeE6) {
+        // equals method will resolve Java double precision problem (see equals method)
+        if (this.equals(geoPoint)) {
+            return 0;
+        } else if (this.longitudeE6 > geoPoint.longitudeE6) {
             return 1;
         } else if (this.longitudeE6 < geoPoint.longitudeE6) {
             return -1;
@@ -162,10 +173,17 @@ public class GeoPoint implements Comparable<GeoPoint> {
         } else if (!(obj instanceof GeoPoint)) {
             return false;
         }
+        /*
+         * problem is that the Java double precision problem can cause two coordinates that represent 
+         * the same geographical position to have a different latitudeE6/longitudeE6. therefore a difference 
+         * of 1 in the latitudeE6/longitudeE6 can be the result of this rounding effect
+         * see https://en.wikipedia.org/wiki/Double-precision_floating-point_format
+         * see https://stackoverflow.com/questions/179427/how-to-resolve-a-java-rounding-double-issue
+         */
         GeoPoint other = (GeoPoint) obj;
-        if (this.latitudeE6 != other.latitudeE6) {
+        if (Math.abs(this.latitudeE6 - other.latitudeE6) > 1) {
             return false;
-        } else if (this.longitudeE6 != other.longitudeE6) {
+        } else if (Math.abs(this.longitudeE6 - other.longitudeE6) > 1) {
             return false;
         }
         return true;
diff --git a/vtm/src/org/oscim/layers/tile/vector/labeling/Label.java b/vtm/src/org/oscim/layers/tile/vector/labeling/Label.java
index 3e624a94..dcc14abd 100644
--- a/vtm/src/org/oscim/layers/tile/vector/labeling/Label.java
+++ b/vtm/src/org/oscim/layers/tile/vector/labeling/Label.java
@@ -35,7 +35,7 @@ final class Label extends TextItem {
     public OBB2D bbox;
 
     public Label clone(TextItem ti) {
-        this.string = ti.string;
+        this.label = ti.label;
         this.text = ti.text;
         this.width = ti.width;
         this.length = ti.length;
@@ -51,12 +51,12 @@ final class Label extends TextItem {
         if (l.text != ll.text)
             return false;
 
-        if (l.string == ll.string)
+        if (l.label == ll.label)
             return true;
 
-        if (l.string.equals(ll.string)) {
+        if (l.label.equals(ll.label)) {
             // make strings unique, should be done only once..
-            l.string = ll.string;
+            l.label = ll.label;
             return true;
         }
 
diff --git a/vtm/src/org/oscim/layers/tile/vector/labeling/LabelPlacement.java b/vtm/src/org/oscim/layers/tile/vector/labeling/LabelPlacement.java
index efddf395..d4e61e0b 100644
--- a/vtm/src/org/oscim/layers/tile/vector/labeling/LabelPlacement.java
+++ b/vtm/src/org/oscim/layers/tile/vector/labeling/LabelPlacement.java
@@ -520,15 +520,15 @@ public class LabelPlacement {
             /* iterate through following */
             for (Label l = (Label) cur.next; l != null; l = (Label) l.next) {
 
-                if (w != l.width || t != l.text || !cur.string.equals(l.string)) {
+                if (w != l.width || t != l.text || !cur.label.equals(l.label)) {
                     p = l;
                     continue;
                 } else if (cur.next == l) {
-                    l.string = cur.string;
+                    l.label = cur.label;
                     p = l;
                     continue;
                 }
-                l.string = cur.string;
+                l.label = cur.label;
 
                 /* insert l after cur */
                 Label tmp = (Label) cur.next;
diff --git a/vtm/src/org/oscim/layers/tile/vector/labeling/LabelPool.java b/vtm/src/org/oscim/layers/tile/vector/labeling/LabelPool.java
index 5d78c72c..26560c79 100644
--- a/vtm/src/org/oscim/layers/tile/vector/labeling/LabelPool.java
+++ b/vtm/src/org/oscim/layers/tile/vector/labeling/LabelPool.java
@@ -10,7 +10,7 @@ final class LabelPool extends Pool<TextItem> {
 
         // drop references
         l.item = null;
-        l.string = null;
+        l.label = null;
         Label ret = (Label) l.next;
 
         // ignore warning
diff --git a/vtm/src/org/oscim/layers/tile/vector/labeling/WayDecorator.java b/vtm/src/org/oscim/layers/tile/vector/labeling/WayDecorator.java
index a76c0e53..226eebb6 100644
--- a/vtm/src/org/oscim/layers/tile/vector/labeling/WayDecorator.java
+++ b/vtm/src/org/oscim/layers/tile/vector/labeling/WayDecorator.java
@@ -25,7 +25,7 @@ import org.oscim.utils.geom.LineClipper;
 
 public final class WayDecorator {
 
-    public static void renderText(LineClipper clipper, float[] coordinates, String string,
+    public static void renderText(LineClipper clipper, float[] coordinates, String label,
                                   TextStyle text, int pos, int len, LabelTileData ld) {
         //TextItem items = textItems;
         TextItem t = null;
@@ -171,14 +171,14 @@ public final class WayDecorator {
                 }
 
                 if (labelWidth < 0) {
-                    labelWidth = text.paint.measureText(string);
+                    labelWidth = text.paint.measureText(label);
                 }
 
                 if (segmentLength < labelWidth * 0.50) {
                     continue;
                 }
             } else if (labelWidth < 0) {
-                labelWidth = text.paint.measureText(string);
+                labelWidth = text.paint.measureText(label);
             }
 
             float x1, y1, x2, y2;
@@ -205,7 +205,7 @@ public final class WayDecorator {
             t = n;
             t.x = x1 + (x2 - x1) / 2f;
             t.y = y1 + (y2 - y1) / 2f;
-            t.string = string;
+            t.label = label;
             t.text = text;
             t.width = labelWidth;
             t.x1 = x1;
diff --git a/vtm/src/org/oscim/map/ViewController.java b/vtm/src/org/oscim/map/ViewController.java
index 7758bcd4..12f39702 100644
--- a/vtm/src/org/oscim/map/ViewController.java
+++ b/vtm/src/org/oscim/map/ViewController.java
@@ -1,5 +1,6 @@
 /*
  * Copyright 2016 devemux86
+ * Copyright 2017 Luca Osten
  *
  * This file is part of the OpenScienceMap project (http://www.opensciencemap.org).
  *
diff --git a/vtm/src/org/oscim/map/Viewport.java b/vtm/src/org/oscim/map/Viewport.java
index a4477026..5c8062d0 100644
--- a/vtm/src/org/oscim/map/Viewport.java
+++ b/vtm/src/org/oscim/map/Viewport.java
@@ -2,6 +2,7 @@
  * Copyright 2012 Hannes Janetzek
  * Copyright 2016 devemux86
  * Copyright 2016 Erik Duisters
+ * Copyright 2017 Luca Osten
  *
  * This file is part of the OpenScienceMap project (http://www.opensciencemap.org).
  *
diff --git a/vtm/src/org/oscim/renderer/bucket/LineBucket.java b/vtm/src/org/oscim/renderer/bucket/LineBucket.java
index b437382f..6b5f3495 100644
--- a/vtm/src/org/oscim/renderer/bucket/LineBucket.java
+++ b/vtm/src/org/oscim/renderer/bucket/LineBucket.java
@@ -81,7 +81,7 @@ public class LineBucket extends RenderBucket {
         this.level = layer;
     }
 
-    LineBucket(int type, boolean indexed, boolean quads) {
+    LineBucket(byte type, boolean indexed, boolean quads) {
         super(type, indexed, quads);
     }
 
diff --git a/vtm/src/org/oscim/renderer/bucket/RenderBucket.java b/vtm/src/org/oscim/renderer/bucket/RenderBucket.java
index 5f048728..65b34fdc 100644
--- a/vtm/src/org/oscim/renderer/bucket/RenderBucket.java
+++ b/vtm/src/org/oscim/renderer/bucket/RenderBucket.java
@@ -1,6 +1,7 @@
 /*
  * Copyright 2012, 2013 Hannes Janetzek
  * Copyright 2016 Stephan Leuschner 
+ * Copyright 2017 Luca Osten
  *
  * This file is part of the OpenScienceMap project (http://www.opensciencemap.org).
  *
@@ -23,17 +24,17 @@ import java.nio.ShortBuffer;
 
 public abstract class RenderBucket extends Inlist<RenderBucket> {
 
-    public static final int LINE = 0;
-    public static final int TEXLINE = 1;
-    public static final int POLYGON = 2;
-    public static final int MESH = 3;
-    public static final int EXTRUSION = 4;
-    public static final int HAIRLINE = 5;
-    public static final int SYMBOL = 6;
-    public static final int BITMAP = 7;
-    public static final int CIRCLE = 8;
+    public static final byte LINE = 0;
+    public static final byte TEXLINE = 1;
+    public static final byte POLYGON = 2;
+    public static final byte MESH = 3;
+    public static final byte EXTRUSION = 4;
+    public static final byte HAIRLINE = 5;
+    public static final byte SYMBOL = 6;
+    public static final byte BITMAP = 7;
+    public static final byte CIRCLE = 8;
 
-    public final int type;
+    public final byte type;
 
     /**
      * Drawing order from bottom to top.
@@ -55,7 +56,7 @@ public abstract class RenderBucket extends Inlist<RenderBucket> {
     final static VertexData EMPTY = new VertexData();
     final boolean quads;
 
-    protected RenderBucket(int type, boolean indexed, boolean quads) {
+    protected RenderBucket(byte type, boolean indexed, boolean quads) {
         this.type = type;
         vertexItems = new VertexData();
         if (indexed)
diff --git a/vtm/src/org/oscim/renderer/bucket/TextBucket.java b/vtm/src/org/oscim/renderer/bucket/TextBucket.java
index cf71ee15..6f37db96 100644
--- a/vtm/src/org/oscim/renderer/bucket/TextBucket.java
+++ b/vtm/src/org/oscim/renderer/bucket/TextBucket.java
@@ -59,16 +59,16 @@ public class TextBucket extends TextureBucket {
                         /* break if next item uses different text style */
                         && item.text == it.next.text
                         /* check same string instance */
-                        && item.string != it.string
+                        && item.label != it.label
                         /* check same string */
-                        && !item.string.equals(it.string))
+                        && !item.label.equals(it.label))
                     it = it.next;
 
                 /* unify duplicate string
                  * // Note: this is required for 'packing test' in prepare to
                  * work! */
-                if (item.string != it.string && item.string.equals(it.string))
-                    item.string = it.string;
+                if (item.label != it.label && item.label.equals(it.label))
+                    item.label = it.label;
 
                 /* insert after text of same type and/or before same string */
                 item.next = it.next;
@@ -129,7 +129,7 @@ public class TextBucket extends TextureBucket {
 
             yy = y + height - it.text.fontDescent;
 
-            mCanvas.drawText(it.string, x, yy, it.text.paint, it.text.stroke);
+            mCanvas.drawText(it.label, x, yy, it.text.paint, it.text.stroke);
 
             // FIXME !!!
             if (width > TEXTURE_WIDTH)
@@ -144,7 +144,7 @@ public class TextBucket extends TextureBucket {
 
                 if (it.next == null
                         || (it.next.text != it.text)
-                        || (it.next.string != it.string)) {
+                        || (it.next.label != it.label)) {
                     it = it.next;
                     break;
                 }
diff --git a/vtm/src/org/oscim/renderer/bucket/TextItem.java b/vtm/src/org/oscim/renderer/bucket/TextItem.java
index 717d746f..3a3a8a12 100644
--- a/vtm/src/org/oscim/renderer/bucket/TextItem.java
+++ b/vtm/src/org/oscim/renderer/bucket/TextItem.java
@@ -1,5 +1,6 @@
 /*
  * Copyright 2012 Hannes Janetzek
+ * Copyright 2017 Luca Osten
  *
  * This file is part of the OpenScienceMap project (http://www.opensciencemap.org).
  *
@@ -34,7 +35,7 @@ public class TextItem extends Inlist<TextItem> {
         @Override
         protected boolean clearItem(TextItem ti) {
             // drop references
-            ti.string = null;
+            ti.label = null;
             ti.text = null;
             //ti.n1 = null;
             //ti.n2 = null;
@@ -57,16 +58,16 @@ public class TextItem extends Inlist<TextItem> {
         return ti;
     }
 
-    public TextItem set(float x, float y, String string, TextStyle text) {
+    public TextItem set(float x, float y, String label, TextStyle text) {
         this.x = x;
         this.y = y;
-        this.string = string;
+        this.label = label;
         this.text = text;
         this.x1 = 0;
         this.y1 = 0;
         this.x2 = 1;
         this.y2 = 0;
-        this.width = text.paint.measureText(string);
+        this.width = text.paint.measureText(label);
         return this;
     }
 
@@ -74,7 +75,7 @@ public class TextItem extends Inlist<TextItem> {
     public float x, y;
 
     // label text
-    public String string;
+    public String label;
 
     // text style
     public TextStyle text;
@@ -96,6 +97,6 @@ public class TextItem extends Inlist<TextItem> {
 
     @Override
     public String toString() {
-        return x + " " + y + " " + string;
+        return x + " " + y + " " + label;
     }
 }
diff --git a/vtm/src/org/oscim/renderer/bucket/TextureBucket.java b/vtm/src/org/oscim/renderer/bucket/TextureBucket.java
index 228bbc5b..85858010 100644
--- a/vtm/src/org/oscim/renderer/bucket/TextureBucket.java
+++ b/vtm/src/org/oscim/renderer/bucket/TextureBucket.java
@@ -52,7 +52,7 @@ public class TextureBucket extends RenderBucket {
             TEXTURE_HEIGHT,
             false);
 
-    public TextureBucket(int type) {
+    public TextureBucket(byte type) {
         super(type, false, true);
     }
 
diff --git a/vtm/src/org/oscim/utils/GeoPointUtils.java b/vtm/src/org/oscim/utils/GeoPointUtils.java
index 9f6db6b6..72cb90c2 100644
--- a/vtm/src/org/oscim/utils/GeoPointUtils.java
+++ b/vtm/src/org/oscim/utils/GeoPointUtils.java
@@ -1,4 +1,5 @@
 /*
+ * Copyright 2010, 2011, 2012, 2013 mapsforge.org
  * Copyright 2017 devemux86
  *
  * This program is free software: you can redistribute it and/or modify it under the
@@ -17,8 +18,31 @@ package org.oscim.utils;
 import org.oscim.core.GeoPoint;
 import org.oscim.core.Point;
 
+/**
+ * The coordinate validation functions come from Mapsforge <a href="https://github.com/mapsforge/mapsforge/blob/master/mapsforge-core/src/main/java/org/mapsforge/core/util/LatLongUtils.java">LatLongUtils</a> class.
+ */
 public final class GeoPointUtils {
 
+    /**
+     * Maximum possible latitude coordinate.
+     */
+    public static final double LATITUDE_MAX = 90;
+
+    /**
+     * Minimum possible latitude coordinate.
+     */
+    public static final double LATITUDE_MIN = -LATITUDE_MAX;
+
+    /**
+     * Maximum possible longitude coordinate.
+     */
+    public static final double LONGITUDE_MAX = 180;
+
+    /**
+     * Minimum possible longitude coordinate.
+     */
+    public static final double LONGITUDE_MIN = -LONGITUDE_MAX;
+
     /**
      * Find if the given point lies within this polygon.
      * <p>
@@ -73,6 +97,30 @@ public final class GeoPointUtils {
         return new Point(startX + t * (endX - startX), startY + t * (endY - startY));
     }
 
+    /**
+     * @param latitude the latitude coordinate in degrees which should be validated.
+     * @return the latitude value
+     * @throws IllegalArgumentException if the latitude coordinate is invalid or {@link Double#NaN}.
+     */
+    public static double validateLatitude(double latitude) {
+        if (Double.isNaN(latitude) || latitude < LATITUDE_MIN || latitude > LATITUDE_MAX) {
+            throw new IllegalArgumentException("invalid latitude: " + latitude);
+        }
+        return latitude;
+    }
+
+    /**
+     * @param longitude the longitude coordinate in degrees which should be validated.
+     * @return the longitude value
+     * @throws IllegalArgumentException if the longitude coordinate is invalid or {@link Double#NaN}.
+     */
+    public static double validateLongitude(double longitude) {
+        if (Double.isNaN(longitude) || longitude < LONGITUDE_MIN || longitude > LONGITUDE_MAX) {
+            throw new IllegalArgumentException("invalid longitude: " + longitude);
+        }
+        return longitude;
+    }
+
     private GeoPointUtils() {
         throw new IllegalStateException();
     }