From: Alan Newberger Date: Thu, 22 Jan 2015 19:48:27 +0000 (-0800) Subject: Reduce view add/removes when loading photos X-Git-Tag: android-x86-6.0-r3~392^2 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=8e29072ca69229a25dc4a856e3635d131613f4ca;hp=93864cf39dfe25c27b41fb327d590dc31ff69fe5;p=android-x86%2Fpackages-apps-Camera2.git 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 61eca7648..6c03386f6 100644 --- 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 13aa3c8b9..ca196e7ab 100644 --- 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 e7fcae7b0..e625c36e0 100644 --- 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 3ae2234b3..c5185f38c 100644 --- 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 1f4b2259e..897bd3619 100644 --- 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); } }