Changeset 31838 in osm for applications/editors/josm/plugins

Timestamp:

2015-12-16T17:17:00+01:00 (9 years ago)

Author:

floscher

Message:

[mapillary] Performance improvements by e.g. using StringBuilder and avoiding initializations to defaults

Location:

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary

Files:

• MapillaryDataListener.java (modified) (2 diffs)
• actions/MapillaryWalkAction.java (modified) (1 diff)
• actions/WalkListener.java (modified) (1 diff)
• actions/WalkThread.java (modified) (9 diffs)
• gui/MapillaryMainDialog.java (modified) (3 diffs)
• utils/MapillaryUtils.java (modified) (3 diffs)
• utils/PluginState.java (modified) (5 diffs)

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/MapillaryDataListener.java

@@ -13 +13 @@
    * Fired when any image is added to the database.
    */
-  public void imagesAdded();
+  void imagesAdded();
 
   /**
@@ -24 +24 @@
    *          New selected {@link MapillaryAbstractImage}
    */
-  public void selectedImageChanged(MapillaryAbstractImage oldImage,
+  void selectedImageChanged(MapillaryAbstractImage oldImage,
       MapillaryAbstractImage newImage);
 }

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/actions/MapillaryWalkAction.java

@@ -97 +97 @@
 
   @Override
-  public void selectedImageChanged(MapillaryAbstractImage oldImage,
-      MapillaryAbstractImage newImage) {
-    if (newImage != null)
-      MapillaryPlugin.setMenuEnabled(MapillaryPlugin.getWalkMenu(), true);
-    else
-      MapillaryPlugin.setMenuEnabled(MapillaryPlugin.getWalkMenu(), false);
+  public void selectedImageChanged(MapillaryAbstractImage oldImage, MapillaryAbstractImage newImage) {
+      MapillaryPlugin.setMenuEnabled(MapillaryPlugin.getWalkMenu(), newImage != null);
   }
 

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/actions/WalkListener.java

@@ -16 +16 @@
    * @param thread The thread executing the walk.
    */
-  public void walkStarted(WalkThread thread);
+  void walkStarted(WalkThread thread);
 }

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/actions/WalkThread.java

@@ -23 +23 @@
   private final int interval;
   private final MapillaryData data;
-  private boolean end = false;
+  private boolean end;
   private final boolean waitForFullQuality;
   private final boolean followSelected;
   private final boolean goForward;
   private BufferedImage lastImage;
-  private volatile boolean paused = false;
+  private volatile boolean paused;
 
   /**
@@ -61 +61 @@
           // Predownload next 10 thumbnails.
           for (int i = 0; i < 10; i++) {
-            if (image.next() == null)
+            if (image.next() == null) {
               break;
+            }
             image = image.next();
-            CacheUtils.downloadPicture((MapillaryImage) image,
-                CacheUtils.PICTURE.THUMBNAIL);
+            CacheUtils.downloadPicture((MapillaryImage) image, CacheUtils.PICTURE.THUMBNAIL);
           }
           if (this.waitForFullQuality)
             // Start downloading 3 next full images.
             for (int i = 0; i < 3; i++) {
-              if (image.next() == null)
+              if (image.next() == null) {
                 break;
+              }
               image = image.next();
-              CacheUtils.downloadPicture((MapillaryImage) image,
-                  CacheUtils.PICTURE.FULL_IMAGE);
+              CacheUtils.downloadPicture((MapillaryImage) image, CacheUtils.PICTURE.FULL_IMAGE);
             }
         }
@@ -80 +80 @@
           synchronized (this) {
             // Waits for full quality picture.
+            final BufferedImage displayImage = MapillaryMainDialog.getInstance().mapillaryImageDisplay.getImage();
             if (this.waitForFullQuality && image instanceof MapillaryImage) {
-              while (MapillaryMainDialog.getInstance().mapillaryImageDisplay
-                  .getImage() == this.lastImage
-                  || MapillaryMainDialog.getInstance().mapillaryImageDisplay
-                      .getImage() == null
-                  || MapillaryMainDialog.getInstance().mapillaryImageDisplay
-                      .getImage().getWidth() < 2048)
+              while ( displayImage == this.lastImage || displayImage == null || displayImage.getWidth() < 2048) {
                 wait(100);
+              }
             }
             // Waits for thumbnail.
             else {
-              while (MapillaryMainDialog.getInstance().mapillaryImageDisplay
-                  .getImage() == this.lastImage
-                  || MapillaryMainDialog.getInstance().mapillaryImageDisplay
-                      .getImage() == null
-                  || MapillaryMainDialog.getInstance().mapillaryImageDisplay
-                      .getImage().getWidth() < 320)
+              while (displayImage == this.lastImage || displayImage == null || displayImage.getWidth() < 320) {
                 wait(100);
+              }
             }
-            while (this.paused)
+            while (this.paused) {
               wait(100);
+            }
             wait(this.interval);
-            while (this.paused)
+            while (this.paused) {
               wait(100);
+            }
           }
-          this.lastImage = MapillaryMainDialog.getInstance().mapillaryImageDisplay
-              .getImage();
+          this.lastImage = MapillaryMainDialog.getInstance().mapillaryImageDisplay.getImage();
           synchronized (this) {
-            if (this.goForward)
+            if (this.goForward) {
               this.data.selectNext(this.followSelected);
-            else
+            } else {
               this.data.selectPrevious(this.followSelected);
+            }
           }
         } catch (InterruptedException e) {
@@ -118 +113 @@
       }
     } catch (NullPointerException e) {
+      // TODO: Avoid NPEs instead of waiting until they are thrown and then catching them
       return;
     }
@@ -129 +125 @@
 
   @Override
-  public void selectedImageChanged(MapillaryAbstractImage oldImage,
-      MapillaryAbstractImage newImage) {
+  public void selectedImageChanged(MapillaryAbstractImage oldImage, MapillaryAbstractImage newImage) {
     if (newImage != oldImage.next()) {
       end();
@@ -155 +150 @@
    */
   public void stopWalk() {
-    if (!SwingUtilities.isEventDispatchThread()) {
+    if (SwingUtilities.isEventDispatchThread()) {
+      end();
+      this.interrupt();
+    } else {
       SwingUtilities.invokeLater(new Runnable() {
         @Override
@@ -162 +160 @@
         }
       });
-    } else {
-      end();
-      this.interrupt();
     }
   }
@@ -172 +167 @@
    */
   public void end() {
-    if (!SwingUtilities.isEventDispatchThread()) {
+    if (SwingUtilities.isEventDispatchThread()) {
+      this.end = true;
+      this.data.removeListener(this);
+      MapillaryMainDialog.getInstance().setMode(MapillaryMainDialog.MODE.NORMAL);
+    } else {
       SwingUtilities.invokeLater(new Runnable() {
         @Override
@@ -179 +178 @@
         }
       });
-    } else {
-      this.end = true;
-      this.data.removeListener(this);
-      MapillaryMainDialog.getInstance()
-          .setMode(MapillaryMainDialog.MODE.NORMAL);
     }
   }

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/gui/MapillaryMainDialog.java

@@ -308 +308 @@
       });
     } else if (this.image != null) {
+      StringBuilder title = new StringBuilder(tr(BASE_TITLE));
       if (this.image instanceof MapillaryImage) {
         MapillaryImage mapillaryImage = (MapillaryImage) this.image;
-        String title = tr(BASE_TITLE);
         if (mapillaryImage.getUser() != null)
-          title += " -- " + mapillaryImage.getUser();
+          title.append(" — ").append(mapillaryImage.getUser());
         if (mapillaryImage.getCapturedAt() != 0)
-          title += " -- " + mapillaryImage.getDate();
-        setTitle(title);
+          title.append(" — ").append(mapillaryImage.getDate());
+        setTitle(title.toString());
       } else if (this.image instanceof MapillaryImportedImage) {
         MapillaryImportedImage mapillaryImportedImage = (MapillaryImportedImage) this.image;
-        String title = tr(BASE_TITLE);
-        title += " -- " + mapillaryImportedImage.getFile().getName();
-        title += " -- " + mapillaryImportedImage.getDate();
-        setTitle(title);
+        title.append(" — ").append(mapillaryImportedImage.getFile().getName());
+        title.append(" — ").append(mapillaryImportedImage.getDate());
+        setTitle(title.toString());
       }
     }
@@ -518 +517 @@
     } else if (data != null && result == LoadResult.SUCCESS) {
       try {
-        BufferedImage img = ImageIO.read(new ByteArrayInputStream(data
-            .getContent()));
-        if (img == null)
+        BufferedImage img = ImageIO.read(new ByteArrayInputStream(data.getContent()));
+        if (img == null) {
           return;
-        if (this.mapillaryImageDisplay.getImage() == null)
+        }
+        if (this.mapillaryImageDisplay.getImage() == null) {
           this.mapillaryImageDisplay.setImage(img);
-        else if (img.getHeight() > this.mapillaryImageDisplay.getImage()
-            .getHeight()) {
+        } else if (img.getHeight() > this.mapillaryImageDisplay.getImage().getHeight()) {
           this.mapillaryImageDisplay.setImage(img);
         }
@@ -563 +561 @@
       panel.add(this.buttonsPanel, BorderLayout.NORTH);
       createLayout(panel, true, null);
-    } else
+    } else {
       createLayout(panel, true, buttons);
+    }
     this.add(this.titleBar, BorderLayout.NORTH);
   }

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/utils/MapillaryUtils.java

@@ -15 +15 @@
 import java.util.Date;
 import java.util.List;
+import java.util.Locale;
 
 import javax.swing.SwingUtilities;
@@ -163 +164 @@
    * @throws ParseException
    */
-  public static long getEpoch(String date, String format)
-      throws ParseException {
-    SimpleDateFormat formatter = new SimpleDateFormat(format);
+  public static long getEpoch(String date, String format) throws ParseException {
+    SimpleDateFormat formatter = new SimpleDateFormat(format, Locale.UK);
     Date dateTime = formatter.parse(date);
     return dateTime.getTime();
@@ -460 +460 @@
    */
   public static void updateHelpText() {
-    String ret = "";
-    if (PluginState.isDownloading())
-      ret += tr("Downloading Mapillary images");
-    else if (MapillaryLayer.getInstance().getData().size() > 0)
-      ret += tr("Total Mapillary images: {0}",
-          MapillaryLayer.getInstance().getData().size());
-    else
-      ret += tr("No images found");
-    if (MapillaryLayer.getInstance().mode != null)
-      ret += " -- " + tr(MapillaryLayer.getInstance().mode.toString());
-    if (PluginState.isUploading())
-      ret += " -- " + PluginState.getUploadString();
+    StringBuilder ret = new StringBuilder();
+    if (PluginState.isDownloading()) {
+      ret .append(tr("Downloading Mapillary images"));
+    } else if (MapillaryLayer.getInstance().getData().size() > 0) {
+      ret.append(tr("Total Mapillary images: {0}", MapillaryLayer.getInstance().getData().size()));
+    } else {
+      ret.append(tr("No images found"));
+    }
+    if (MapillaryLayer.getInstance().mode != null) {
+      ret.append(" — ").append(tr(MapillaryLayer.getInstance().mode.toString()));
+    }
+    if (PluginState.isUploading()) {
+      ret.append(" — ").append(PluginState.getUploadString());
+    }
     synchronized (MapillaryUtils.class) {
-      Main.map.statusLine.setHelpText(ret);
+      Main.map.statusLine.setHelpText(ret.toString());
     }
   }

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/utils/PluginState.java

@@ -17 +17 @@
 public class PluginState {
 
-  private static int runningDownloads = 0;
+  private static int runningDownloads;
   /** Images that have to be uploaded. */
-  protected static int imagesToUpload = 0;
+  protected static int imagesToUpload;
   /** Images that have been uploaded. */
-  public static int imagesUploaded = 0;
+  public static int imagesUploaded;
+
+  private PluginState() {
+    // Empty constructor to avoid instantiation
+  }
 
   /**
@@ -77 +81 @@
   public static void imageUploaded() {
     imagesUploaded++;
-    if (imagesToUpload == imagesUploaded) {
-      if (Main.main != null)
+    if (imagesToUpload == imagesUploaded && Main.main != null) {
         finishedUploadDialog();
     }
@@ -85 +88 @@
   private static void finishedUploadDialog() {
     if (!SwingUtilities.isEventDispatchThread()) {
+      JOptionPane pane = new JOptionPane();
+      pane.setMessage(new FinishedUploadDialog());
+      JDialog dlg = pane.createDialog(Main.parent, tr("Finished upload"));
+      dlg.setVisible(true);
+    } else {
       SwingUtilities.invokeLater(new Runnable() {
         @Override
@@ -91 +99 @@
         }
       });
-    } else {
-      JOptionPane pane = new JOptionPane();
-      pane.setMessage(new FinishedUploadDialog());
-      JDialog dlg = pane.createDialog(Main.parent, tr("Finished upload"));
-      dlg.setVisible(true);
     }
   }
@@ -105 +108 @@
    */
   public static String getUploadString() {
-    return tr("Uploading: {0}", "(" + imagesUploaded + "/" + imagesToUpload
-        + ")");
+    return tr("Uploading: {0}", "(" + imagesUploaded + "/" + imagesToUpload + ")");
   }
 }