Merging Bart's changes.

MyTracks/res/values/keys.xml

@@ -38,7 +38,6 @@ limitations under the License.
   <string name="share_url_only_key">shareUrlOnly</string>
   <string name="signal_sampling_frequency_key">signalSamplingFrequency</string>
   <string name="split_frequency_key">splitFrequency</string>
-  <string name="debug_draw_bounds_key">debugDrawBounds</string>
 
   <!-- Preference keys which keep transient state (not user preferences) - DO NOT TRANSLATE -->
   <string name="auto_resume_track_current_retry_key">autoResumeTrackCurrentRetry</string>

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/MyTracksMap.java

@@ -51,14 +51,14 @@ import android.os.HandlerThread;
 import android.provider.Settings;
 import android.util.Log;
 import android.view.ContextMenu;
-import android.view.ContextMenu.ContextMenuInfo;
 import android.view.Menu;
 import android.view.MenuItem;
 import android.view.MotionEvent;
 import android.view.SubMenu;
 import android.view.View;
-import android.view.View.OnCreateContextMenuListener;
 import android.view.Window;
+import android.view.ContextMenu.ContextMenuInfo;
+import android.view.View.OnCreateContextMenuListener;
 import android.widget.ImageButton;
 import android.widget.LinearLayout;
 import android.widget.RelativeLayout;
@@ -140,14 +140,16 @@ public class MyTracksMap extends MapActivity
    */
   private HandlerThread updateTrackThread;
 
-  /** Handler for updateTrackThread */
+  /**
+   * Handler for updateTrackThread.
+   */
   private Handler updateTrackHandler;
 
   private MyTracksProviderUtils providerUtils;
  
   /**
    * A runnable that updates the track from the provider (looking for points
-   * added after "lastSeenLocationId".
+   * added after "lastSeenLocationId").
    */
   private final Runnable updateTrackRunnable = new Runnable() {
     @Override
@@ -155,6 +157,7 @@ public class MyTracksMap extends MapActivity
       if (!isATrackSelected()) {
         return;
       }
+      
       readAllNewTrackPoints();
     }
   };
@@ -168,6 +171,7 @@ public class MyTracksMap extends MapActivity
       if (!isATrackSelected()) {
         return;
       }
+      
       mapOverlay.clearPoints();
       firstSeenLocationId = -1;
       lastSeenLocationId = -1;
@@ -193,13 +197,10 @@ public class MyTracksMap extends MapActivity
         cursor = providerUtils.getWaypointsCursor(
             selectedTrackId, 0,
             MyTracksConstants.MAX_DISPLAYED_WAYPOINTS_POINTS);
-        if (cursor != null) {
+        if (cursor != null && cursor.moveToFirst()) {
-          if (cursor.moveToFirst()) {
+          do {
-            do {
+            mapOverlay.addWaypoint(providerUtils.createWaypoint(cursor));
-              Waypoint wpt = providerUtils.createWaypoint(cursor);
+          } while (cursor.moveToNext());
-              mapOverlay.addWaypoint(wpt);
-            } while (cursor.moveToNext());
-          }
         }
       } catch (RuntimeException e) {
         Log.w(MyTracksConstants.TAG, "Caught an unexpected exception.", e);
@@ -213,9 +214,9 @@ public class MyTracksMap extends MapActivity
   };
 
   /**
-   * A runnable intended to be posted to the updateTrackThread after the
+   * A runnable intended to be posted to the {@code #updateTrackThread} after
-   * selected track changes. It will post to the ui thread to update the screen
+   * the selected track changes.  It will post to the UI thread to update
-   * elements and move the map to show the selected track.
+   * the screen elements and move the map to show the selected track.
    */
   private final Runnable setSelectedTrackRunnable = new Runnable() {
     @Override
@@ -375,12 +376,20 @@ public class MyTracksMap extends MapActivity
         setSelectedTrack(selectedTrackId);
       }
       updateOptionsButton();
-      mapOverlay.setDrawBounds(preferences.getBoolean(
-          getString(R.string.debug_draw_bounds_key), false));
       preferences.registerOnSharedPreferenceChangeListener(this);
     }
   }
 
+  @Override
+  protected void onDestroy() {
+    Log.d(MyTracksConstants.TAG, "MyTracksMap.onDestroy");
+    if (updateTrackThread != null) {
+      updateTrackThread.quit();
+    }
+    
+    super.onDestroy();
+  }
+
   /**
    * Returns whether there's a track currently selected for display.
    */
@@ -896,8 +905,7 @@ public class MyTracksMap extends MapActivity
 
   @Override
   public void onSharedPreferenceChanged(
-      final SharedPreferences sharedPreferences,
+      final SharedPreferences sharedPreferences, final String key) {
-      final String key) {
     if (key != null) {
       uiHandler.post(new Runnable() {
         @Override
@@ -921,10 +929,6 @@ public class MyTracksMap extends MapActivity
               mapOverlay.setShowEndMarker(!isRecordingSelected());
               mapView.postInvalidate();
             }
-          } else if (key.equals(getString(R.string.debug_draw_bounds_key))) {
-            mapOverlay.setDrawBounds(
-                sharedPreferences.getBoolean(
-                    getString(R.string.debug_draw_bounds_key), false));
           }
         }
       });
@@ -953,8 +957,8 @@ public class MyTracksMap extends MapActivity
     public void onLocationChanged(Location location) {
       if (location.getProvider().equals(MyTracksConstants.GPS_PROVIDER)) {
         // Recalculate the variation if there was a jump in location > 1km:
-        if (currentLocation == null
+        if (currentLocation == null ||
-            || location.distanceTo(currentLocation) > 1000) {
+            location.distanceTo(currentLocation) > 1000) {
           setVariation(location);
         }
         currentLocation = location;
@@ -1042,8 +1046,8 @@ public class MyTracksMap extends MapActivity
             // Now we already have at least one point, calculate the sampling
             // frequency
             long numTotalPoints = lastStoredLocationId - firstSeenLocationId;
-            samplingFrequency = (int) (1 +
+            samplingFrequency = (int) (1 + numTotalPoints
-                numTotalPoints / MyTracksConstants.TARGET_DISPLAYED_TRACK_POINTS);
+                / MyTracksConstants.TARGET_DISPLAYED_TRACK_POINTS);
           }
 
           providerUtils.fillLocation(cursor, location);
@@ -1055,8 +1059,8 @@ public class MyTracksMap extends MapActivity
           if (numPoints % samplingFrequency == 0 ||
               (!isRecordingSelected() && locationId == lastStoredLocationId) ||
               !MyTracksUtils.isValidLocation(location)) {
-            // Only allocate a new location if it is going to be kept around.
+            // No need to allocate a new location (we can safely reuse the existing).
-            mapOverlay.addLocation(new Location(location));
+            mapOverlay.addLocation(location);
           }
 
           numPoints++;

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

@@ -20,20 +20,24 @@ 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;
 import android.content.Intent;
+import android.content.res.Resources;
 import android.graphics.Canvas;
 import android.graphics.Paint;
 import android.graphics.Path;
 import android.graphics.Point;
 import android.graphics.Rect;
-import android.graphics.Region;
 import android.graphics.drawable.Drawable;
 import android.location.Location;
 
 import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ArrayBlockingQueue;
+import java.util.concurrent.BlockingQueue;
 
 /**
  * A map overlay that displays a "MyLocation" arrow, an error circle, the
@@ -43,7 +47,7 @@ import java.util.ArrayList;
  */
 public class MyTracksOverlay extends Overlay {
 
-  private final Drawable arrow[] = new Drawable[18];
+  private final Drawable[] arrows;
   private final int arrowWidth, arrowHeight;
   private final Drawable statsMarker;
   private final Drawable waypointMarker;
@@ -53,68 +57,94 @@ public class MyTracksOverlay extends Overlay {
   private final Paint selectedTrackPaint;
   private final Paint errorCirclePaint;
   private final Context context;
-  private final ArrayList<Waypoint> waypoints = new ArrayList<Waypoint>();
+  private final List<Waypoint> waypoints;
-  private final ArrayList<Location> points = new ArrayList<Location>(1024);
+  private final List<CachedLocation> points;
+  private final BlockingQueue<CachedLocation> pendingPoints;
 
   private boolean trackDrawingEnabled;
   private int lastHeading = 0;
   private Location myLocation;
   private boolean showEndMarker = true;
-  private boolean drawBounds;
+  // TODO: Remove it completely after completing performance tests.
+  private boolean alwaysVisible = true;
+
+  private GeoPoint lastReferencePoint;
+  private Rect lastViewRect;
+  private Path lastPath;
+  
+  /**
+   * Represents a pre-processed {@code Location} to speed up drawing.
+   * This class is more like a data object and doesn't provide accessors.
+   */
+  private static class CachedLocation {
+    public final boolean valid;
+    public final GeoPoint geoPoint;
+    
+    public CachedLocation(Location location) {
+      this.valid = MyTracksUtils.isValidLocation(location);
+      this.geoPoint = valid ? MyTracksUtils.getGeoPoint(location) : null; 
+    }
+  };
   
   public MyTracksOverlay(Context context) {
     this.context = context;
     
+    this.waypoints = new ArrayList<Waypoint>();
+    this.points = new ArrayList<CachedLocation>(1024);
+    this.pendingPoints = new ArrayBlockingQueue<CachedLocation>(
+        MyTracksConstants.MAX_DISPLAYED_TRACK_POINTS, true);
+
     // TODO: Can we use a FrameAnimation or similar here rather
     // than individual resources for each arrow direction?
-
+    final Resources resources = context.getResources();
-    arrow[0] = context.getResources().getDrawable(R.drawable.arrow_0);
+    arrows = new Drawable[] {
-    arrow[1] = context.getResources().getDrawable(R.drawable.arrow_20);
+        resources.getDrawable(R.drawable.arrow_0),
-    arrow[2] = context.getResources().getDrawable(R.drawable.arrow_40);
+        resources.getDrawable(R.drawable.arrow_20),
-    arrow[3] = context.getResources().getDrawable(R.drawable.arrow_60);
+        resources.getDrawable(R.drawable.arrow_40),
-    arrow[4] = context.getResources().getDrawable(R.drawable.arrow_80);
+        resources.getDrawable(R.drawable.arrow_60),
-    arrow[5] = context.getResources().getDrawable(R.drawable.arrow_100);
+        resources.getDrawable(R.drawable.arrow_80),
-    arrow[6] = context.getResources().getDrawable(R.drawable.arrow_120);
+        resources.getDrawable(R.drawable.arrow_100),
-    arrow[7] = context.getResources().getDrawable(R.drawable.arrow_140);
+        resources.getDrawable(R.drawable.arrow_120),
-    arrow[8] = context.getResources().getDrawable(R.drawable.arrow_160);
+        resources.getDrawable(R.drawable.arrow_140),
-    arrow[9] = context.getResources().getDrawable(R.drawable.arrow_180);
+        resources.getDrawable(R.drawable.arrow_160),
-    arrow[10] = context.getResources().getDrawable(R.drawable.arrow_200);
+        resources.getDrawable(R.drawable.arrow_180),
-    arrow[11] = context.getResources().getDrawable(R.drawable.arrow_220);
+        resources.getDrawable(R.drawable.arrow_200),
-    arrow[12] = context.getResources().getDrawable(R.drawable.arrow_240);
+        resources.getDrawable(R.drawable.arrow_220),
-    arrow[13] = context.getResources().getDrawable(R.drawable.arrow_260);
+        resources.getDrawable(R.drawable.arrow_240),
-    arrow[14] = context.getResources().getDrawable(R.drawable.arrow_280);
+        resources.getDrawable(R.drawable.arrow_260),
-    arrow[15] = context.getResources().getDrawable(R.drawable.arrow_300);
+        resources.getDrawable(R.drawable.arrow_280),
-    arrow[16] = context.getResources().getDrawable(R.drawable.arrow_320);
+        resources.getDrawable(R.drawable.arrow_300),
-    arrow[17] = context.getResources().getDrawable(R.drawable.arrow_340);
+        resources.getDrawable(R.drawable.arrow_320),
-    arrowWidth = arrow[lastHeading].getIntrinsicWidth();
+        resources.getDrawable(R.drawable.arrow_340)
-    arrowHeight = arrow[lastHeading].getIntrinsicHeight();
+    };
-    for (int i = 0; i <= 17; i++) {
+    arrowWidth = arrows[lastHeading].getIntrinsicWidth();
-      arrow[i].setBounds(0, 0, arrowWidth, arrowHeight);
+    arrowHeight = arrows[lastHeading].getIntrinsicHeight();
+    for (Drawable arrow : arrows) {
+      arrow.setBounds(0, 0, arrowWidth, arrowHeight);
     }
 
-    statsMarker = context.getResources().getDrawable(R.drawable.ylw_pushpin);
+    statsMarker = resources.getDrawable(R.drawable.ylw_pushpin);
     markerWidth = statsMarker.getIntrinsicWidth();
     markerHeight = statsMarker.getIntrinsicHeight();
     statsMarker.setBounds(0, 0, markerWidth, markerHeight);
 
-    startMarker = context.getResources().getDrawable(R.drawable.green_dot);
+    startMarker = resources.getDrawable(R.drawable.green_dot);
     startMarker.setBounds(0, 0, markerWidth, markerHeight);
 
-    endMarker = context.getResources().getDrawable(R.drawable.red_dot);
+    endMarker = resources.getDrawable(R.drawable.red_dot);
     endMarker.setBounds(0, 0, markerWidth, markerHeight);
 
-    waypointMarker =
+    waypointMarker = resources.getDrawable(R.drawable.blue_pushpin);
-        context.getResources().getDrawable(R.drawable.blue_pushpin);
     waypointMarker.setBounds(0, 0, markerWidth, markerHeight);
 
     selectedTrackPaint = new Paint();
-    selectedTrackPaint.setColor(context.getResources().getColor(R.color.red));
+    selectedTrackPaint.setColor(resources.getColor(R.color.red));
     selectedTrackPaint.setStrokeWidth(3);
     selectedTrackPaint.setStyle(Paint.Style.STROKE);
     selectedTrackPaint.setAntiAlias(true);
 
     errorCirclePaint = new Paint();
-    errorCirclePaint.setColor(context.getResources().getColor(R.color.blue));
+    errorCirclePaint.setColor(resources.getColor(R.color.blue));
     errorCirclePaint.setStyle(Paint.Style.STROKE);
     errorCirclePaint.setStrokeWidth(3);
     errorCirclePaint.setAlpha(127);
@@ -124,35 +154,50 @@ public class MyTracksOverlay extends Overlay {
   /**
    * Add a location to the map overlay.
    * 
-   * NOTE: this method takes ownership of this location and may change it.
+   * NOTE: This method doesn't take ownership of the given location, so it is
+   * safe to reuse the same location while calling this method.
    *
-   * @param l the location to add
+   * @param l the location to add.
    */
   public void addLocation(Location l) {
-    if (l != null) {
+    // Queue up in the pending queue until it's merged with {@code #points}.
-      synchronized (points) {
+    pendingPoints.offer(new CachedLocation(l));
-        points.add(l);
+  }
+
+  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);
       }
     }
   }
   
-  public void addWaypoint(Waypoint wpt) {
-    waypoints.add(wpt);
-  }
-
   public int getNumLocations() {
     synchronized (points) {
-      return points.size();
+      return points.size() + pendingPoints.size();
     }
   }
 
-  public void clearWaypoints() {
+  // Visible for testing.
-    waypoints.clear();
+  int getNumWaypoints() {
+    synchronized (waypoints) {
+      return waypoints.size();
+    }
   }
   
   public void clearPoints() {
     synchronized (points) {
       points.clear();
+      pendingPoints.clear();
+      lastPath = null;
+      lastViewRect = null;
+    }
+  }
+
+  public void clearWaypoints() {
+    synchronized (waypoints) {
+      waypoints.clear();
     }
   }
 
@@ -170,230 +215,175 @@ 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);
   }
   
-  private void drawWaypoints(Canvas canvas, MapView mapView) {
+  // Visible for testing.
-    ArrayList<Waypoint> currentWaypoints = waypoints;
+  Projection getMapProjection(MapView mapView) {
-    for (int i = 1; i < currentWaypoints.size(); i++) {
+    return mapView.getProjection();
-      Waypoint wpt = currentWaypoints.get(i);
-      if (wpt == null) {
-        continue;
-      }
-      Location loc = wpt.getLocation();
-      if (loc == null) {
-        continue;
-      }
-      GeoPoint geoPoint = MyTracksUtils.getGeoPoint(loc);
-      Point pt = new Point();
-      mapView.getProjection().toPixels(geoPoint, pt);
-      canvas.save();
-      canvas.translate(pt.x - (markerWidth / 2) + 3, pt.y - (markerHeight));
-      if (wpt.getType() == Waypoint.TYPE_STATISTICS) {
-        statsMarker.draw(canvas);
-      } else {
-        waypointMarker.draw(canvas);
-      }
-      canvas.restore();
-    }
   }
   
-  private void drawMyLocation(Canvas canvas, MapView mapView) {
+  // Visible for testing.
-    // Draw the arrow icon:
+  Rect getMapViewRect(MapView mapView) {
-    if (myLocation == null) {
-      return;
-    }
-
-    GeoPoint geoPoint = new GeoPoint(
-        (int) (myLocation.getLatitude() * 1E6),
-        (int) (myLocation.getLongitude() * 1E6));
-    Point pt = new Point();
-    mapView.getProjection().toPixels(geoPoint, pt);
-    canvas.save();
-    canvas.translate(pt.x - (arrowWidth / 2), pt.y - (arrowHeight / 2));
-    arrow[lastHeading].draw(canvas);
-    canvas.restore();
-
-    // Draw the error circle:
-    float radius =
-        mapView.getProjection().metersToEquatorPixels(myLocation.getAccuracy());
-    canvas.drawCircle(pt.x, pt.y, radius, errorCirclePaint);
-  }
-
-  private void drawTrack(Canvas canvas, MapView mapView) {
-    Path path;
-    Point pt = new Point();
-    Location lastValidLocation;
-    int locLon = 0, locLat = 0;
-    GeoPoint firstGeoPoint = null;
-    boolean lastLocValid;
-
-    // Get the current viewing window:
     int w = mapView.getLongitudeSpan();
     int h = mapView.getLatitudeSpan();
     int cx = mapView.getMapCenter().getLongitudeE6();
     int cy = mapView.getMapCenter().getLatitudeE6();
-    Rect viewRect = new Rect(cx - w, cy - h, cx + w, cy + h);
+    return new Rect(cx - w / 2, cy - h / 2, cx + w / 2, cy + h / 2);
+  }
   
-    // Global bounding box, including points not visible
+  // Visible for testing.
-    int allMinLat, allMinLon, allMaxLat, allMaxLon;
+  Path newPath() {
+    return new Path();
+  }
   
+  // Visible for testing.
+  Path getLastPath() {
+    return lastPath;
+  }
+
+  private void drawWaypoints(Canvas canvas, Projection projection) {
+    synchronized (waypoints) {;
+      for (Waypoint wpt : waypoints) {
+        Location loc = wpt.getLocation();
+        drawElement(canvas, projection, MyTracksUtils.getGeoPoint(loc),
+            wpt.getType() == Waypoint.TYPE_STATISTICS ? statsMarker
+                : waypointMarker, -(markerWidth / 2) + 3, -markerHeight);
+      }
+    }
+  }
+
+  private void drawMyLocation(Canvas canvas, Projection projection) {
+    // Draw the arrow icon.
+    if (myLocation == null) {
+      return;
+    }
+
+    Point pt = drawElement(canvas, projection,
+        MyTracksUtils.getGeoPoint(myLocation), arrows[lastHeading],
+        -(arrowWidth / 2) + 3, -(arrowHeight / 2));
+    // Draw the error circle.
+    float radius = projection.metersToEquatorPixels(myLocation.getAccuracy());
+    canvas.drawCircle(pt.x, pt.y, radius, errorCirclePaint);
+  }
+
+  private void drawTrack(Canvas canvas, Projection projection, Rect viewRect) {
+    Path path;
     synchronized (points) {
-      int numPoints = points.size();
+      // Merge the pending points with the list of cached locations.
-      if (numPoints < 2) {
+      final GeoPoint referencePoint = projection.fromPixels(0, 0);
-        return;
+      int newPoints = pendingPoints.drainTo(points); 
-      }
+      boolean newProjection = !viewRect.equals(lastViewRect) ||
-
+          !referencePoint.equals(lastReferencePoint); 
-      GeoPoint geoPoint;
+      if (newPoints == 0 && lastPath != null && !newProjection) {
-      Location loc;
+        // No need to recreate path (same points and viewing area).
-      int minLon, maxLon, minLat, maxLat;
+        path = lastPath;
-      lastValidLocation = points.get(0);
+      } else {
-      int lastLocLon = allMinLon = allMaxLon = (int) (lastValidLocation.getLongitude() * 1E6);
+        int numPoints = points.size();
-      int lastLocLat = allMinLat = allMaxLat = (int) (lastValidLocation.getLatitude() * 1E6);
+        if (numPoints < 2) {
-      lastLocValid = MyTracksUtils.isValidLocation(lastValidLocation);
+          // Not enough points to draw a path.
-      boolean lastLocVisible = false;
+          path = null;
-      path = new Path();
+        } else if (lastPath != null && !newProjection) {
-
+          // Incremental update of the path, without repositioning the view.
-      // Loop over track points:
+          path = lastPath;
-      path.incReserve(numPoints);
+          updatePath(projection, viewRect, path, numPoints - newPoints); 
-      for (int i = 1; i < numPoints; i++) {
+        } else {
-        loc = points.get(i);
+          // The view has changed so we have to start from scratch.
-
+          path = newPath();
-        boolean locValid = MyTracksUtils.isValidLocation(loc);
+          path.incReserve(numPoints);
-        boolean locVisible = false;
+          updatePath(projection, viewRect, path, 0); 
-        if (locValid) {
+        }
-          locLon = (int) (loc.getLongitude() * 1E6);
+        lastPath = path;
-          locLat = (int) (loc.getLatitude() * 1E6);
+      }
-
+      lastReferencePoint = referencePoint;
-          if (firstGeoPoint == null) {
+      lastViewRect = viewRect;
-            // Found the starting point
+    }
-            firstGeoPoint = new GeoPoint(locLat, locLon);
+    if (path != null) {
-          }
+      canvas.drawPath(path, selectedTrackPaint);
-
+    }
-          // If both the current and previous locations were valid
+
-          if (lastLocValid) {
+    // Draw the "End" marker.
-            lastValidLocation = loc;
+    if (showEndMarker) {
-  
+      for (int i = points.size() - 1; i >= 0; --i) {
-            // Get the bounding box of the segment about to be drawn
+        if (points.get(i).valid) {
-            if (locLon > lastLocLon) {
+          drawElement(canvas, projection, points.get(i).geoPoint, endMarker,
-              minLon = lastLocLon;
+              -markerWidth / 2, -markerHeight);
-              maxLon = locLon;
+          break;
-            } else {
-              minLon = locLon;
-              maxLon = lastLocLon;
-            }
-            if (locLat > lastLocLat) {
-              minLat = lastLocLat;
-              maxLat = locLat;
-            } else {
-              minLat = locLat;
-              maxLat = lastLocLat;
-            }
-
-            if (drawBounds) {
-              allMaxLat = Math.max(allMaxLat, maxLat);
-              allMinLat = Math.min(allMinLat, minLat);
-              allMaxLon = Math.max(allMaxLon, maxLon);
-              allMinLon = Math.min(allMinLon, minLon);
-            }
-
-            // See if that bounding box intersects the viewable bounding box
-            // Assume that if it does, the location is visible
-            locVisible = viewRect.intersects(minLon, minLat, maxLon, maxLat);
-            if (locVisible) {
-              // If the previous point wasn't drawn, start at its position
-              if (!lastLocVisible) {
-                geoPoint = new GeoPoint(lastLocLat, lastLocLon);
-                mapView.getProjection().toPixels(geoPoint, pt);
-                path.moveTo(pt.x, pt.y);
-              }
-  
-              // Draw a line to the new point
-              geoPoint = new GeoPoint(locLat, locLon);
-              mapView.getProjection().toPixels(geoPoint, pt);
-              path.lineTo(pt.x, pt.y);
-            }
-          }
         }
-        lastLocLon = locLon;
-        lastLocLat = locLat;
-        lastLocValid = locValid;
-        lastLocVisible = locVisible;
       }
     }
     
-    canvas.drawPath(path, selectedTrackPaint);
+    // Draw the "Start" marker.
-
+    for (int i = 0; i < points.size(); ++i) {
-    if (drawBounds) {
+      if (points.get(i).valid) {
-      drawBoundingBox(canvas, mapView,
+        drawElement(canvas, projection, points.get(i).geoPoint, startMarker,
-          allMinLat, allMinLon, allMaxLat, allMaxLon);
+            -markerWidth / 2, -markerHeight);
-    }
+        break;
-
+      }
-    // Draw the "End" marker:
-    if (showEndMarker && lastValidLocation != null) {
-      canvas.save();
-      GeoPoint geoPoint = MyTracksUtils.getGeoPoint(lastValidLocation);
-      mapView.getProjection().toPixels(geoPoint, pt);
-      canvas.translate(pt.x - (markerWidth / 2), pt.y - markerHeight);
-      endMarker.draw(canvas);
-      canvas.restore();
-    }
-
-    // Draw the "Start" marker:
-    if (firstGeoPoint != null) {
-      mapView.getProjection().toPixels(firstGeoPoint, pt);
-      canvas.save();
-      canvas.translate(pt.x - (markerWidth / 2), pt.y - markerHeight);
-      startMarker.draw(canvas);
-      canvas.restore();
     }
   }
   
-  private void drawBoundingBox(Canvas canvas, MapView mapView, int allMinLat,
+  private void updatePath(Projection projection, Rect viewRect, Path path,
-      int allMinLon, int allMaxLat, int allMaxLon) {
+      int startLocationIdx) {
-    // Transform coordinates
+    // Whether to start a new segment on new valid and visible point.
-    GeoPoint maxPoint = new GeoPoint(allMaxLat, allMaxLon);
+    boolean newSegment =
-    GeoPoint minPoint = new GeoPoint(allMinLat, allMinLon);
+        startLocationIdx > 0 ? !points.get(startLocationIdx - 1).valid : true;
-    Point minPt = new Point();
+    boolean lastVisible = !newSegment;
-    Point maxPt = new Point();
+    final Point pt = new Point();
-    mapView.getProjection().toPixels(minPoint, minPt);
+    // Loop over track points.
-    mapView.getProjection().toPixels(maxPoint, maxPt);
+    for (int i = startLocationIdx; i < points.size(); ++i) {
-    Rect allBounds = new Rect(minPt.x, minPt.y, maxPt.x, maxPt.y);
+      CachedLocation loc = points.get(i);
+      // Check if valid, if not then indicate a new segment.
+      if (!loc.valid) {
+        newSegment = true;
+        continue;
+      }
       
-    // Prepare a green paint
+      final GeoPoint geoPoint = loc.geoPoint;
-    Paint boundingBoxPaint = new Paint();
+      // Check if this breaks the existing segment.
-    boundingBoxPaint.setColor(context.getResources().getColor(R.color.green));
+      boolean visible = alwaysVisible || viewRect.contains(
-    boundingBoxPaint.setStrokeWidth(2);
+          geoPoint.getLongitudeE6(), geoPoint.getLatitudeE6());
-    boundingBoxPaint.setStyle(Paint.Style.STROKE);
+      if (!visible && !lastVisible) {
-    boundingBoxPaint.setAntiAlias(true);
+        // This is a point outside view not connected to a visible one.
+        newSegment = true;
+      }
+      lastVisible = visible;
       
-    // Disable clipping
+      // 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);
+      }
+    }
+  }
+  
+  // Visible for testing.
+  Point drawElement(Canvas canvas, Projection projection, GeoPoint geoPoint,
+      Drawable element, int offsetX, int offsetY) {
+    Point pt = new Point();
+    projection.toPixels(geoPoint, pt);
     canvas.save();
-    canvas.clipRect(Float.NEGATIVE_INFINITY, Float.NEGATIVE_INFINITY,
+    canvas.translate(pt.x + offsetX, pt.y + offsetY);
-                    Float.POSITIVE_INFINITY, Float.POSITIVE_INFINITY,
+    element.draw(canvas);
-                    Region.Op.REPLACE);
-
-    // Draw the bounding box
-    canvas.drawRect(allBounds, boundingBoxPaint);
-
-    // Re-enable clipping
     canvas.restore();
-  }
+    return pt;
-
-  @Override
-  public boolean draw(Canvas canvas, MapView mapView, boolean shadow,
-      long when) {
-    draw(canvas, mapView, shadow);
-    return false;
   }
 
   /**
@@ -436,20 +426,22 @@ public class MyTracksOverlay extends Overlay {
     final Location tapLocation = MyTracksUtils.getLocation(p);
     double dmin = Double.MAX_VALUE;
     Waypoint waypoint = null;
-    for (int i = 0; i < waypoints.size(); i++) {
+    synchronized (waypoints) {
-      final Location waypointLocation = waypoints.get(i).getLocation();
+      for (int i = 0; i < waypoints.size(); i++) {
-      if (waypointLocation == null) {
+        final Location waypointLocation = waypoints.get(i).getLocation();
-        continue;
+        if (waypointLocation == null) {
-      }
+          continue;
-      final double d = waypointLocation.distanceTo(tapLocation);
+        }
-      if (d < dmin) {
+        final double d = waypointLocation.distanceTo(tapLocation);
-        dmin = d;
+        if (d < dmin) {
-        waypoint = waypoints.get(i);
+          dmin = d;
+          waypoint = waypoints.get(i);
+        }
       }
     }
 
-    if (waypoint != null
+    if (waypoint != null &&
-        && dmin < 15000000 / Math.pow(2, mapView.getZoomLevel())) {
+        dmin < 15000000 / Math.pow(2, mapView.getZoomLevel())) {
       Intent intent = new Intent(context, MyTracksWaypointDetails.class);
       intent.putExtra("waypointid", waypoint.getId());
       context.startActivity(intent);
@@ -457,8 +449,4 @@ public class MyTracksOverlay extends Overlay {
     }
     return super.onTap(p, mapView);
   }
-
-  public void setDrawBounds(boolean drawBounds) {
-    this.drawBounds = drawBounds;
-  }
 }

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

@@ -0,0 +1,243 @@
+/*
+ * Copyright 2010 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+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.
+ * 
+ * @author Bartlomiej Niechwiej
+ */
+public class MyTracksOverlayTest extends AndroidTestCase {
+  private Canvas canvas;
+  private MockMyTracksOverlay myTracksOverlay;
+  private MapView mockView;
+  private Projection mockProjection;
+  
+  /**
+   * A mock version of {@code MyTracksOverlay} that does not use
+   * {@class MapView}. 
+   */
+  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();
+    }
+  }
+  
+  /**
+   * A mock class that intercepts {@code Path}'s and records calls to
+   * {@code #moveTo()} and {@code #lineTo()}.
+   */
+  private static class MockPath extends Path {
+    /** A list of disjoined path segments. */
+    public final List<List<PointF>> segments = new LinkedList<List<PointF>>();
+    /** The total number of points in this path. */
+    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++;
+    }
+  }
+
+  /**
+   * A mock {@code Projection} that acts as the identity matrix.
+   */
+  private static class MockProjection implements 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);
+    }
+  }
+  
+  @Override
+  protected void setUp() throws Exception {
+    super.setUp();
+    canvas = new Canvas();
+    myTracksOverlay = new MockMyTracksOverlay(getContext());
+    // Enable drawing.
+    myTracksOverlay.setTrackDrawingEnabled(true);
+    mockView = null;
+    mockProjection = new MockProjection();
+  }
+
+  public void testAddLocation() throws Exception {
+    Location location = new Location("gps");
+    location.setLatitude(10);
+    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 {
+    Location location = new Location("gps");
+    location.setLatitude(10);
+    location.setLongitude(20);
+    myTracksOverlay.addLocation(location);
+    assertEquals(1, myTracksOverlay.getNumLocations());
+    myTracksOverlay.clearPoints();
+    assertEquals(0, myTracksOverlay.getNumLocations());
+
+    // Same after drawing on canvas.
+    final int locations = 100;
+    for (int i = 0; i < locations; ++i) {
+      myTracksOverlay.addLocation(location);
+    }
+    assertEquals(locations, myTracksOverlay.getNumLocations());
+    myTracksOverlay.draw(canvas, mockView, false);
+    myTracksOverlay.draw(canvas, mockView, true);
+    myTracksOverlay.clearPoints();
+    assertEquals(0, myTracksOverlay.getNumLocations());
+  }
+
+  public void testAddWaypoint() throws Exception {
+    Location location = new Location("gps");
+    location.setLatitude(10);
+    location.setLongitude(20);
+    Waypoint waypoint = new Waypoint();
+    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) {
+      waypoint = new Waypoint();
+      waypoint.setLocation(location);
+      myTracksOverlay.addWaypoint(waypoint);
+    }
+    assertEquals(1 + waypoints, myTracksOverlay.getNumWaypoints());
+    assertEquals(0, myTracksOverlay.getNumLocations());
+    assertNull(myTracksOverlay.getLastPath());
+  }
+
+  public void testClearWaypoints() throws Exception {
+    Location location = new Location("gps");
+    location.setLatitude(10);
+    location.setLongitude(20);
+    Waypoint waypoint = new Waypoint();
+    waypoint.setLocation(location);
+    myTracksOverlay.addWaypoint(waypoint);
+    assertEquals(1, myTracksOverlay.getNumWaypoints());
+    myTracksOverlay.clearWaypoints();
+    assertEquals(0, myTracksOverlay.getNumWaypoints());
+  }
+  
+  public void testDrawing() {
+    Location location = new Location("gps");
+    location.setLatitude(10);
+    for (int i = 0; i < 40; ++i) {
+      location.setLongitude(20 + i);
+      Waypoint waypoint = new Waypoint();
+      waypoint.setLocation(location);
+      myTracksOverlay.addWaypoint(waypoint);
+    }
+    for (int i = 0; i < 100; ++i) {
+      location = new Location("gps");
+      location.setLatitude(20 + i / 2);
+      location.setLongitude(150 - i);
+      myTracksOverlay.addLocation(location);
+    }
+    
+    // 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).
+  }
+}