Improved test coverage and minor refactoring/improvements.

MyTracks/res/xml/preferences.xml

@@ -133,13 +133,4 @@ limitations under the License.
           android:persistent="false" />
     </PreferenceCategory>
   </PreferenceScreen>
-
-  <!-- Debug settings - uncomment this while debugging only -->
-  <!-- 
-  <PreferenceScreen android:title="Debug">
-    <CheckBoxPreference android:key="@string/debug_draw_bounds_key"
-                        android:title="Draw bounding box"
-                        android:defaultValue="false" />
-  </PreferenceScreen>
-  -->
 </PreferenceScreen>

MyTracks/src/com/google/android/apps/mytracks/MyTracksOverlay.java

@@ -20,6 +20,7 @@ import com.google.android.apps.mytracks.util.MyTracksUtils;
 import com.google.android.maps.GeoPoint;
 import com.google.android.maps.MapView;
 import com.google.android.maps.Overlay;
+import com.google.android.maps.Projection;
 import com.google.android.maps.mytracks.R;
 
 import android.content.Context;
@@ -64,15 +65,22 @@ public class MyTracksOverlay extends Overlay {
   private int lastHeading = 0;
   private Location myLocation;
   private boolean showEndMarker = true;
-  // TODO: Remove it completely.  If this is true, drawing is faster by 5%.
+  // TODO: Remove it completely after completing performance tests.
   private boolean alwaysVisible = true;
   
+  private Rect lastViewRect;
+  private Path lastPath;
+  
   /**
-   * Represents a pre-processed {@code Location} to speed up drawing. 
+   * Represents a pre-processed {@code Location} to speed up drawing.
+   * This class is more like a data object and doesn't provide getters/setters.
    */
   private static class CachedLocation {
     public final boolean valid;
     public final GeoPoint geoPoint;
+    // Cached point in the screen's coordinate system.
+    // TODO: Use it in drawTrack.
+    public Point point;
     
     public CachedLocation(Location location) {
       this.valid = MyTracksUtils.isValidLocation(location);
@@ -84,7 +92,7 @@ public class MyTracksOverlay extends Overlay {
     this.context = context;
     
     this.waypoints = new ArrayList<Waypoint>();
-    this.points = new ArrayList<CachedLocation>(256);
+    this.points = new ArrayList<CachedLocation>(1024);
     this.pendingPoints = new ArrayBlockingQueue<CachedLocation>(
         MyTracksConstants.MAX_DISPLAYED_TRACK_POINTS, true);
 
@@ -159,6 +167,7 @@ public class MyTracksOverlay extends Overlay {
   }
 
   public void addWaypoint(Waypoint wpt) {
+    // Note: We don't cache waypoints, because it's not worth the effort.
     if (wpt != null && wpt.getLocation() != null) {
       synchronized (waypoints) {
         waypoints.add(wpt);
@@ -183,6 +192,8 @@ public class MyTracksOverlay extends Overlay {
     synchronized (points) {
       points.clear();
       pendingPoints.clear();
+      lastPath = null;
+      lastViewRect = null;
     }
   }
 
@@ -206,126 +217,152 @@ public class MyTracksOverlay extends Overlay {
       return;
     }
 
+    // It's safe to keep projection within a single draw operation.
+    final Projection projection = getMapProjection(mapView);
+    // Get the current viewing window.
     if (trackDrawingEnabled) {
+      Rect viewRect = getMapViewRect(mapView);
+      
       // Draw the selected track:
-      drawTrack(canvas, mapView);
+      drawTrack(canvas, projection, viewRect);
 
       // Draw the waypoints:
-      drawWaypoints(canvas, mapView);
+      drawWaypoints(canvas, projection);
     }
 
     // Draw the current location
-    drawMyLocation(canvas, mapView);
+    drawMyLocation(canvas, projection);
+  }
+  
+  // Visible for testing.
+  Projection getMapProjection(MapView mapView) {
+    return mapView.getProjection();
+  }
+  
+  // Visible for testing.
+  Rect getMapViewRect(MapView mapView) {
+    int w = mapView.getLongitudeSpan();
+    int h = mapView.getLatitudeSpan();
+    int cx = mapView.getMapCenter().getLongitudeE6();
+    int cy = mapView.getMapCenter().getLatitudeE6();
+    return new Rect(cx - w / 2, cy - h / 2, cx + w / 2, cy + h / 2);
+  }
+  
+  // Visible for testing.
+  Path newPath() {
+    return new Path();
+  }
+  
+  // Visible for testing.
+  Path getLastPath() {
+    return lastPath;
   }
 
-  private void drawWaypoints(Canvas canvas, MapView mapView) {
+  private void drawWaypoints(Canvas canvas, Projection projection) {
     synchronized (waypoints) {;
       for (Waypoint wpt : waypoints) {
         Location loc = wpt.getLocation();
-        drawElement(canvas, mapView, MyTracksUtils.getGeoPoint(loc),
+        drawElement(canvas, projection, MyTracksUtils.getGeoPoint(loc),
             wpt.getType() == Waypoint.TYPE_STATISTICS ? statsMarker
                 : waypointMarker, -(markerWidth / 2) + 3, -markerHeight);
       }
     }
   }
 
-  private void drawMyLocation(Canvas canvas, MapView mapView) {
+  private void drawMyLocation(Canvas canvas, Projection projection) {
-    // Draw the arrow icon:
+    // Draw the arrow icon.
     if (myLocation == null) {
       return;
     }
 
-    Point pt = drawElement(canvas, mapView,
+    Point pt = drawElement(canvas, projection,
         MyTracksUtils.getGeoPoint(myLocation), arrows[lastHeading],
         -(arrowWidth / 2) + 3, -(arrowHeight / 2));
-    // Draw the error circle:
+    // Draw the error circle.
-    float radius =
+    float radius = projection.metersToEquatorPixels(myLocation.getAccuracy());
-        mapView.getProjection().metersToEquatorPixels(myLocation.getAccuracy());
     canvas.drawCircle(pt.x, pt.y, radius, errorCirclePaint);
   }
 
-  private void drawTrack(Canvas canvas, MapView mapView) {
+  private void drawTrack(Canvas canvas, Projection projection, Rect viewRect) {
+    Path path;
+    final Point pt = new Point();
     GeoPoint firstGeoPoint = null;
     GeoPoint lastGeoPoint = null;
-
-    // Get the current viewing window.
-    Rect viewRect = null;
-    if (!alwaysVisible) {
-      int w = mapView.getLongitudeSpan();
-      int h = mapView.getLatitudeSpan();
-      int cx = mapView.getMapCenter().getLongitudeE6();
-      int cy = mapView.getMapCenter().getLatitudeE6();
-      viewRect = new Rect(cx - w / 2, cy - h / 2, cx + w / 2, cy + h / 2);
-    }
-    
-    final Path path = new Path();
-    final Point pt = new Point();
     synchronized (points) {
       // Merge the pending points with the list of cached locations. 
-      pendingPoints.drainTo(points);
+      if (pendingPoints.drainTo(points) == 0 && lastPath != null &&
-      
+          viewRect.equals(lastViewRect)) {
-      int numPoints = points.size();
+        // No need to recreate path (same points and viewing area).
-      if (numPoints < 2) {
+        path = lastPath;
-        return;
+      } else {
-      }
+        // Regenerate the whole path.
-      path.incReserve(numPoints);
+        int numPoints = points.size();
-
+        if (numPoints < 2) {
-      // Whether to start a new segment on new valid and visible point.
+          return;
-      boolean newSegment = true;
-      boolean lastVisible = false;
-      // Loop over track points:
-      for (CachedLocation loc : points) {
-        // Check if valid, if not then indicate a new segment.
-        if (!loc.valid) {
-          newSegment = true;
-          continue;
         }
-        
+        path = newPath();
-        final GeoPoint geoPoint = loc.geoPoint;
+        path.incReserve(numPoints);
-        if (firstGeoPoint == null) {
+  
-          // Found the starting point.
+        // Whether to start a new segment on new valid and visible point.
-          firstGeoPoint = geoPoint;
+        boolean newSegment = true;
-        }
+        boolean lastVisible = false;
-        lastGeoPoint = geoPoint;
+        // Loop over track points:
-
+        for (CachedLocation loc : points) {
-        // Check if break the existing segment.
+          // Check if valid, if not then indicate a new segment.
-        boolean visible = alwaysVisible || viewRect.contains(
+          if (!loc.valid) {
-            geoPoint.getLongitudeE6(), geoPoint.getLatitudeE6());
+            newSegment = true;
-        if (!visible && !lastVisible) {
+            continue;
-          // So this is a point outside view not connected to a visible one.
+          }
-          newSegment = true;
+          
-        }
+          final GeoPoint geoPoint = loc.geoPoint;
-        lastVisible = visible;
+          if (firstGeoPoint == null) {
-        
+            // Found the starting point.
-        // Either move to beginning of a new segment or continue the old one.
+            firstGeoPoint = geoPoint;
-        mapView.getProjection().toPixels(geoPoint, pt);
+          }
-        if (newSegment) {
+          lastGeoPoint = geoPoint;
-          path.moveTo(pt.x, pt.y);
+  
-          newSegment = false;
+          // Check if this breaks the existing segment.
-        } else {
+          boolean visible = alwaysVisible || viewRect.contains(
-          path.lineTo(pt.x, pt.y);
+              geoPoint.getLongitudeE6(), geoPoint.getLatitudeE6());
+          if (!visible && !lastVisible) {
+            // This is a point outside view not connected to a visible one.
+            newSegment = true;
+          }
+          lastVisible = visible;
+          
+          // Either move to beginning of a new segment or continue the old one.
+          projection.toPixels(geoPoint, pt);
+          if (newSegment) {
+            path.moveTo(pt.x, pt.y);
+            newSegment = false;
+          } else {
+            path.lineTo(pt.x, pt.y);
+          }
         }
+        lastPath = path;
+        lastViewRect = viewRect;
       }
     }
     canvas.drawPath(path, selectedTrackPaint);
 
     // Draw the "End" marker.
     if (showEndMarker && lastGeoPoint != null) {
-      drawElement(canvas, mapView, lastGeoPoint, endMarker, -markerWidth / 2,
+      drawElement(canvas, projection, lastGeoPoint, endMarker, -markerWidth / 2,
           -markerHeight);
     }
     
     // Draw the "Start" marker:
     if (firstGeoPoint != null) {
-      drawElement(canvas, mapView, firstGeoPoint, startMarker, -markerWidth / 2,
+      drawElement(canvas, projection, firstGeoPoint,
-          -markerHeight);
+          startMarker, -markerWidth / 2, -markerHeight);
     }
   }
   
-  private Point drawElement(Canvas canvas, MapView mapView, GeoPoint geoPoint,
+  // Visible for testing.
+  Point drawElement(Canvas canvas, Projection projection, GeoPoint geoPoint,
       Drawable element, int offsetX, int offsetY) {
     Point pt = new Point();
-    mapView.getProjection().toPixels(geoPoint, pt);
+    projection.toPixels(geoPoint, pt);
     canvas.save();
     canvas.translate(pt.x + offsetX, pt.y + offsetY);
     element.draw(canvas);

MyTracksTest/src/com/google/android/apps/mytracks/MyTracksOverlayTest.java

@@ -16,12 +16,24 @@
 package com.google.android.apps.mytracks;
 
 import com.google.android.apps.mytracks.content.Waypoint;
+import com.google.android.maps.GeoPoint;
 import com.google.android.maps.MapView;
+import com.google.android.maps.Projection;
 
+import android.content.Context;
 import android.graphics.Canvas;
+import android.graphics.Path;
+import android.graphics.Point;
+import android.graphics.PointF;
+import android.graphics.Rect;
 import android.location.Location;
 import android.test.AndroidTestCase;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.LinkedList;
+import java.util.List;
+
 /**
  * Tests for the MyTracks map overlay.
  * 
@@ -29,16 +41,71 @@ import android.test.AndroidTestCase;
  */
 public class MyTracksOverlayTest extends AndroidTestCase {
   private Canvas canvas;
-  private MyTracksOverlay myTracksOverlay;
+  private MockMyTracksOverlay myTracksOverlay;
   private MapView mockView;
+  private Projection mockProjection;
+  
+  private class MockMyTracksOverlay extends MyTracksOverlay {
+    public MockMyTracksOverlay(Context context) {
+      super(context);
+    }
+    @Override
+    Projection getMapProjection(MapView mapView) {
+      return mockProjection;
+    }
+    @Override
+    Rect getMapViewRect(MapView mapView) {
+      return new Rect(0, 0, 100, 100);
+    }
+    @Override
+    Path newPath() {
+      return new MockPath();
+    }
+  }
+  
+  private static class MockPath extends Path {
+    public final List<List<PointF>> segments = new LinkedList<List<PointF>>();
+    public int totalPoints;
+    private List<PointF> currentSegment;
+
+    @Override
+    public void lineTo(float x, float y) {
+      super.lineTo(x, y);
+      assertNotNull(currentSegment);
+      currentSegment.add(new PointF(x, y));
+      totalPoints++;
+    }
+    @Override
+    public void moveTo(float x, float y) {
+      super.moveTo(x, y);
+      segments.add(currentSegment =
+          new ArrayList<PointF>(Arrays.asList(new PointF(x, y))));
+      totalPoints++;
+    }
+  }
   
   @Override
   protected void setUp() throws Exception {
     super.setUp();
     canvas = new Canvas();
-    myTracksOverlay = new MyTracksOverlay(getContext());
+    myTracksOverlay = new MockMyTracksOverlay(getContext());
-    // TODO: add real mock view and enable track drawing.
+    // Enable drawing.
+    myTracksOverlay.setTrackDrawingEnabled(true);
     mockView = null;
+    mockProjection = new Projection() {
+      @Override
+      public Point toPixels(GeoPoint in, Point out) {
+        return out;
+      }
+      @Override
+      public float metersToEquatorPixels(float meters) {
+        return meters;
+      }
+      @Override
+      public GeoPoint fromPixels(int x, int y) {
+        return new GeoPoint(y, x);
+      }
+    };
   }
 
   public void testAddLocation() throws Exception {
@@ -47,17 +114,26 @@ public class MyTracksOverlayTest extends AndroidTestCase {
     location.setLongitude(20);
     myTracksOverlay.addLocation(location);
     assertEquals(1, myTracksOverlay.getNumLocations());
+    assertEquals(0, myTracksOverlay.getNumWaypoints());
     
     location.setLatitude(20);
     location.setLongitude(30);
     myTracksOverlay.addLocation(location);
     assertEquals(2, myTracksOverlay.getNumLocations());
+    assertEquals(0, myTracksOverlay.getNumWaypoints());
+    assertNull(myTracksOverlay.getLastPath());
     
     // Draw and make sure that we don't lose any point.
     myTracksOverlay.draw(canvas, mockView, false);
     assertEquals(2, myTracksOverlay.getNumLocations());
+    assertEquals(0, myTracksOverlay.getNumWaypoints());
+    assertNotNull(myTracksOverlay.getLastPath());
+    assertEquals(2, ((MockPath)myTracksOverlay.getLastPath()).totalPoints);
+    
     myTracksOverlay.draw(canvas, mockView, true);
     assertEquals(2, myTracksOverlay.getNumLocations());
+    assertEquals(0, myTracksOverlay.getNumWaypoints());
+    assertNotNull(myTracksOverlay.getLastPath());
   }
   
   public void testClearPoints() throws Exception {
@@ -89,6 +165,8 @@ public class MyTracksOverlayTest extends AndroidTestCase {
     waypoint.setLocation(location);
     myTracksOverlay.addWaypoint(waypoint);
     assertEquals(1, myTracksOverlay.getNumWaypoints());
+    assertEquals(0, myTracksOverlay.getNumLocations());
+    assertNull(myTracksOverlay.getLastPath());
     
     final int waypoints = 10;
     for (int i = 0; i < waypoints; ++i) {
@@ -97,6 +175,8 @@ public class MyTracksOverlayTest extends AndroidTestCase {
       myTracksOverlay.addWaypoint(waypoint);
     }
     assertEquals(1 + waypoints, myTracksOverlay.getNumWaypoints());
+    assertEquals(0, myTracksOverlay.getNumLocations());
+    assertNull(myTracksOverlay.getLastPath());
   }
 
   public void testClearWaypoints() throws Exception {
@@ -122,12 +202,26 @@ public class MyTracksOverlayTest extends AndroidTestCase {
     }
     for (int i = 0; i < 100; ++i) {
       location = new Location("gps");
-      location.setLatitude(50 + i);
+      location.setLatitude(20 + i / 2);
       location.setLongitude(150 - i);
       myTracksOverlay.addLocation(location);
     }
     
-    myTracksOverlay.draw(canvas, mockView, false);
+    // Shadow.
     myTracksOverlay.draw(canvas, mockView, true);
+    // We don't expect to do anything if  
+    assertNull(myTracksOverlay.getLastPath());
+    assertEquals(40, myTracksOverlay.getNumWaypoints());
+    assertEquals(100, myTracksOverlay.getNumLocations());
+
+    // No shadow.
+    myTracksOverlay.draw(canvas, mockView, false);
+    assertNotNull(myTracksOverlay.getLastPath());
+    assertTrue(myTracksOverlay.getLastPath() instanceof MockPath);
+    MockPath path = (MockPath) myTracksOverlay.getLastPath();
+    assertEquals(40, myTracksOverlay.getNumWaypoints());
+    assertEquals(100, myTracksOverlay.getNumLocations());
+    assertEquals(100, path.totalPoints);
+    // TODO: Check the points from the path (and the segments).
   }
 }