Reduce view add/removes when loading photos

Two fixes to reduce flicker when loading images. First, adjust
ViewItem instances in-place instead of creating new ViewItems
which included adding and removing views from the hierarchy.
It turned out we were doing an update for every photo due to
touching PanoramaMetadata and reporting back that metadata
was adjusted, whether or not a photo was a panorama. Now
that data is reported. New photo captures are essentially
added and removed from the view hierarchy three times before
this change, after an ImageView is added once, with its
contents changed as the photo is loaded.

More testing is needed but I cannot repro the elusive filmstrip
flicker with these changes.

Bug: 18977838
Bug: 17905863
Change-Id: I5a42c0baf87a8cef0ee4dbcf9f03d40715e6aa2d

diff --git a/src/com/android/camera/data/MetadataLoader.java b/src/com/android/camera/data/MetadataLoader.java
index 61eca76..6c03386 100644 (file)
--- a/src/com/android/camera/data/MetadataLoader.java
+++ b/src/com/android/camera/data/MetadataLoader.java
@@ -39,14 +39,12 @@ public class MetadataLoader {
     public static boolean loadMetadata(final Context context, final FilmstripItem data) {
         boolean metadataAdded = false;
         if (data.getAttributes().isImage()) {
-            PanoramaMetadataLoader.loadPanoramaMetadata(context, data.getData().getUri(),
-                    data.getMetadata());
-            RgbzMetadataLoader.loadRgbzMetadata(context, data.getData().getUri(),
-                  data.getMetadata());
-            metadataAdded = true;
+            metadataAdded |= PanoramaMetadataLoader.loadPanoramaMetadata(
+                    context, data.getData().getUri(), data.getMetadata());
+            metadataAdded |=  RgbzMetadataLoader.loadRgbzMetadata(
+                    context, data.getData().getUri(), data.getMetadata());
         } else if (data.getAttributes().isVideo()) {
-            VideoRotationMetadataLoader.loadRotationMetadata(data);
-            metadataAdded = true;
+            metadataAdded = VideoRotationMetadataLoader.loadRotationMetadata(data);
         }
         data.getMetadata().setLoaded(true);
         return metadataAdded;

diff --git a/src/com/android/camera/data/PanoramaMetadataLoader.java b/src/com/android/camera/data/PanoramaMetadataLoader.java
index 13aa3c8..ca196e7 100644 (file)
--- a/src/com/android/camera/data/PanoramaMetadataLoader.java
+++ b/src/com/android/camera/data/PanoramaMetadataLoader.java
@@ -29,18 +29,20 @@ public class PanoramaMetadataLoader {
      * Extracts panorama metadata from the item with the given URI and fills the
      * {@code metadata}.
      */
-    public static void loadPanoramaMetadata(final Context context, Uri contentUri,
+    public static boolean loadPanoramaMetadata(final Context context, Uri contentUri,
             Metadata metadata) {
         PhotoSphereHelper.PanoramaMetadata panoramaMetadata =
               PhotoSphereHelper.getPanoramaMetadata(context, contentUri);
-        if (panoramaMetadata == null) {
-            return;
+        // Note: The use of '==' here is in purpose as this is a singleton that
+        // is returned if this is not a panorama, so pointer comparison works.
+        if (panoramaMetadata == null || panoramaMetadata == PhotoSphereHelper.NOT_PANORAMA) {
+            return false;
         }
 
-        // Note: The use of '!=' here is in purpose as this is a singleton that
-        // is returned if this is not a panorama, so pointer comparison works.
-        metadata.setPanorama(panoramaMetadata != PhotoSphereHelper.NOT_PANORAMA);
+        metadata.setPanorama(true);
         metadata.setPanorama360(panoramaMetadata.mIsPanorama360);
         metadata.setUsePanoramaViewer(panoramaMetadata.mUsePanoramaViewer);
+
+        return true;
     }
 }

diff --git a/src/com/android/camera/data/RgbzMetadataLoader.java b/src/com/android/camera/data/RgbzMetadataLoader.java
index e7fcae7..e625c36 100644 (file)
--- a/src/com/android/camera/data/RgbzMetadataLoader.java
+++ b/src/com/android/camera/data/RgbzMetadataLoader.java
@@ -31,9 +31,12 @@ public class RgbzMetadataLoader {
      *
      * @param context  The app context.
      */
-    public static void loadRgbzMetadata(final Context context, Uri contentUri, Metadata metadata) {
+    public static boolean loadRgbzMetadata(
+            final Context context, Uri contentUri, Metadata metadata) {
         if (RefocusHelper.isRGBZ(context, contentUri)) {
             metadata.setHasRgbzData(true);
+            return true;
         }
+        return false;
     }
 }

diff --git a/src/com/android/camera/data/VideoRotationMetadataLoader.java b/src/com/android/camera/data/VideoRotationMetadataLoader.java
index 3ae2234..c5185f3 100644 (file)
--- a/src/com/android/camera/data/VideoRotationMetadataLoader.java
+++ b/src/com/android/camera/data/VideoRotationMetadataLoader.java
@@ -31,7 +31,7 @@ public class VideoRotationMetadataLoader {
         return ROTATE_90.equals(rotation) || ROTATE_270.equals(rotation);
     }
 
-    static void loadRotationMetadata(final FilmstripItem data) {
+    static boolean loadRotationMetadata(final FilmstripItem data) {
         final String path = data.getData().getFilePath();
         MediaMetadataRetriever retriever = new MediaMetadataRetriever();
         try {
@@ -53,5 +53,6 @@ public class VideoRotationMetadataLoader {
             // IllegalArgumentException. e.g: data contain *.avi file.
             Log.e(TAG, "MediaMetdataRetriever.setDataSource() fail", ex);
         }
+        return true;
     }
 }

diff --git a/src/com/android/camera/widget/FilmstripView.java b/src/com/android/camera/widget/FilmstripView.java
index 1f4b225..897bd36 100644 (file)
--- a/src/com/android/camera/widget/FilmstripView.java
+++ b/src/com/android/camera/widget/FilmstripView.java
@@ -154,7 +154,7 @@ public class FilmstripView extends ViewGroup {
         /** The position of the left of the view in the whole filmstrip. */
         private int mLeftPosition;
         private final View mView;
-        private final FilmstripItem mData;
+        private FilmstripItem mData;
         private final RectF mViewArea;
         private boolean mMaximumBitmapRequested;
 
@@ -182,6 +182,11 @@ public class FilmstripView extends ViewGroup {
             mFilmstrip = filmstrip;
         }
 
+        public void setData(FilmstripItem item) {
+            mData = item;
+            mMaximumBitmapRequested = false;
+        }
+
         public boolean isMaximumBitmapRequested() {
             return mMaximumBitmapRequested;
         }
@@ -1721,17 +1726,27 @@ public class FilmstripView extends ViewGroup {
             Log.w(TAG, "updateViewItem() - Trying to update an null item!");
             return;
         }
-        item.removeViewFromHierarchy(true);
 
-        ViewItem newItem = buildViewItemAt(item.getAdapterIndex());
-        if (newItem == null) {
-            Log.w(TAG, "updateViewItem() - New item is null!");
-            // keep using the old data.
-            item.addViewToHierarchy();
+        int adapterIndex = item.getAdapterIndex();
+        FilmstripItem filmstripItem = mDataAdapter.getFilmstripItemAt(adapterIndex);
+        if (filmstripItem == null) {
+            Log.w(TAG, "updateViewItem() - Trying to update item with null FilmstripItem!");
             return;
         }
-        newItem.copyAttributes(item);
-        mViewItems[bufferIndex] = newItem;
+
+        // In case the underlying data item is changed (commonly from
+        // SessionItem to PhotoItem for an image requiring processing), set the
+        // new FilmstripItem on the ViewItem
+        item.setData(filmstripItem);
+
+        // In case state changed from a new FilmStripItem or the existing one,
+        // redraw the View contents. We call getView here as it will refill the
+        // view contents, but it is not clear as we are not using the documented
+        // method intent to get a View, we know that this always uses the view
+        // passed in to populate it.
+        // TODO: refactor 'getView' to more explicitly just update view contents
+        mDataAdapter.getView(item.getView(), adapterIndex, mVideoClickedCallback);
+
         mZoomView.resetDecoder();
 
         boolean stopScroll = clampCenterX();
@@ -1753,7 +1768,7 @@ public class FilmstripView extends ViewGroup {
         adjustChildZOrder();
         invalidate();
         if (mListener != null) {
-            mListener.onDataUpdated(newItem.getAdapterIndex());
+            mListener.onDataUpdated(adapterIndex);
         }
     }