From c6e089b29703c8506b7b85c5af796e8c340b0818 Mon Sep 17 00:00:00 2001 From: Chih-Chung Chang Date: Fri, 22 Jun 2012 20:56:04 +0800 Subject: Fix the behavior of deleting the last picture. Also fix the jank while deleting multiple pictures quickly. Bug: 6713932, 6712555 Change-Id: I9d64d8bbdcd4ec0dc9a447a51d50f88ff27363b4 --- src/com/android/gallery3d/app/PhotoPage.java | 20 +- .../android/gallery3d/data/FilterDeleteSet.java | 218 ++++++++++++++++----- src/com/android/gallery3d/ui/PhotoView.java | 66 +++++-- 3 files changed, 235 insertions(+), 69 deletions(-) diff --git a/src/com/android/gallery3d/app/PhotoPage.java b/src/com/android/gallery3d/app/PhotoPage.java index c94454352..568f5901f 100644 --- a/src/com/android/gallery3d/app/PhotoPage.java +++ b/src/com/android/gallery3d/app/PhotoPage.java @@ -218,10 +218,12 @@ public class PhotoPage extends ActivityState implements mShowBars = false; } + MediaSet originalSet = mActivity.getDataManager() + .getMediaSet(mSetPathString); + mSelectionManager.setSourceMediaSet(originalSet); mSetPathString = "/filter/delete/{" + mSetPathString + "}"; mMediaSet = (FilterDeleteSet) mActivity.getDataManager() .getMediaSet(mSetPathString); - mSelectionManager.setSourceMediaSet(mMediaSet); mCurrentIndex = data.getInt(KEY_INDEX_HINT, 0); if (mMediaSet == null) { Log.w(TAG, "failed to restore " + mSetPathString); @@ -717,31 +719,24 @@ public class PhotoPage extends ActivityState implements // the deletion, we then actually delete the media item. @Override public void onDeleteImage(Path path, int offset) { - commitDeletion(); // commit the previous deletion + onCommitDeleteImage(); // commit the previous deletion mDeletePath = path; mDeleteIsFocus = (offset == 0); - mMediaSet.setDeletion(path, mCurrentIndex + offset); - mPhotoView.showUndoBar(); + mMediaSet.addDeletion(path, mCurrentIndex + offset); } @Override public void onUndoDeleteImage() { + if (mDeletePath == null) return; // If the deletion was done on the focused item, we want the model to // focus on it when it is undeleted. if (mDeleteIsFocus) mModel.setFocusHintPath(mDeletePath); - mMediaSet.setDeletion(null, 0); + mMediaSet.removeDeletion(mDeletePath); mDeletePath = null; - mPhotoView.hideUndoBar(); } @Override public void onCommitDeleteImage() { - if (mDeletePath == null) return; - commitDeletion(); - mPhotoView.hideUndoBar(); - } - - private void commitDeletion() { if (mDeletePath == null) return; mSelectionManager.deSelectAll(); mSelectionManager.toggle(mDeletePath); @@ -857,6 +852,7 @@ public class PhotoPage extends ActivityState implements onCommitDeleteImage(); mMenuExecutor.pause(); + if (mMediaSet != null) mMediaSet.clearDeletion(); } @Override diff --git a/src/com/android/gallery3d/data/FilterDeleteSet.java b/src/com/android/gallery3d/data/FilterDeleteSet.java index fc94eb8e0..97bbc1256 100644 --- a/src/com/android/gallery3d/data/FilterDeleteSet.java +++ b/src/com/android/gallery3d/data/FilterDeleteSet.java @@ -18,25 +18,50 @@ package com.android.gallery3d.data; import java.util.ArrayList; -// FilterDeleteSet filters a base MediaSet to remove a deletion item. The user -// can use the following method to change the deletion item: +// FilterDeleteSet filters a base MediaSet to remove some deletion items (we +// expect the number to be small). The user can use the following methods to +// add/remove deletion items: // -// void setDeletion(Path path, int index); +// void addDeletion(Path path, int index); +// void removeDelection(Path path); +// void clearDeletion(); // -// If the path is null, there is no deletion item. public class FilterDeleteSet extends MediaSet implements ContentListener { private static final String TAG = "FilterDeleteSet"; + private static final int REQUEST_ADD = 1; + private static final int REQUEST_REMOVE = 2; + private static final int REQUEST_CLEAR = 3; + + private static class Request { + int type; // one of the REQUEST_* constants + Path path; + int indexHint; + public Request(int type, Path path, int indexHint) { + this.type = type; + this.path = path; + this.indexHint = indexHint; + } + } + + private static class Deletion { + Path path; + int index; + public Deletion(Path path, int index) { + this.path = path; + this.index = index; + } + } + + // The underlying MediaSet private final MediaSet mBaseSet; - private Path mDeletionPath; - private int mDeletionIndexHint; - private boolean mNewDeletionSettingPending = false; - // This is set to true or false in reload(), so we know if the given - // mDelectionPath is still in the mBaseSet, and if so we can adjust the - // index and items. - private boolean mDeletionInEffect; - private int mDeletionIndex; + // Pending Requests + private ArrayList mRequests = new ArrayList(); + + // Deletions currently in effect, ordered by index + private ArrayList mCurrent = new ArrayList(); + private int mMediaItemCount; public FilterDeleteSet(Path path, MediaSet baseSet) { super(path, INVALID_DATA_VERSION); @@ -51,65 +76,170 @@ public class FilterDeleteSet extends MediaSet implements ContentListener { @Override public int getMediaItemCount() { - if (mDeletionInEffect) { - return mBaseSet.getMediaItemCount() - 1; - } else { - return mBaseSet.getMediaItemCount(); - } + return mMediaItemCount; } + // Gets the MediaItems whose (post-deletion) index are in the range [start, + // start + count). Because we remove some of the MediaItems, the index need + // to be adjusted. + // + // For example, if there are 12 items in total. The deleted items are 3, 5, + // 10, and the the requested range is [3, 7]: + // + // The original index: 0 1 2 3 4 5 6 7 8 9 A B C + // The deleted items: X X X + // The new index: 0 1 2 3 4 5 6 7 8 9 + // Requested: * * * * * + // + // We need to figure out the [3, 7] actually maps to the original index 4, + // 6, 7, 8, 9. + // + // We can break the MediaItems into segments, each segment other than the + // last one ends in a deleted item. The difference between the new index and + // the original index increases with each segment: + // + // 0 1 2 X (new index = old index) + // 4 X (new index = old index - 1) + // 6 7 8 9 X (new index = old index - 2) + // B C (new index = old index - 3) + // @Override public ArrayList getMediaItem(int start, int count) { - if (!mDeletionInEffect || mDeletionIndex >= start + count) { - return mBaseSet.getMediaItem(start, count); + if (count <= 0) return new ArrayList(); + + int end = start + count - 1; + int n = mCurrent.size(); + // Find the segment that "start" falls into. Count the number of items + // not yet deleted until it reaches "start". + int i = 0; + for (i = 0; i < n; i++) { + Deletion d = mCurrent.get(i); + if (d.index - i > start) break; } - if (mDeletionIndex < start) { - return mBaseSet.getMediaItem(start + 1, count); + // Find the segment that "end" falls into. + int j = i; + for (; j < n; j++) { + Deletion d = mCurrent.get(j); + if (d.index - j > end) break; + } + + // Now get enough to cover deleted items in [start, end] + ArrayList base = mBaseSet.getMediaItem(start + i, count + (j - i)); + + // Remove the deleted items. + for (int m = j - 1; m >= i; m--) { + Deletion d = mCurrent.get(m); + int k = d.index - (start + i); + base.remove(k); } - ArrayList base = mBaseSet.getMediaItem(start, count + 1); - base.remove(mDeletionIndex - start); return base; } + // We apply the pending requests in the mRequests to construct mCurrent in reload(). @Override public long reload() { boolean newData = mBaseSet.reload() > mDataVersion; - if (!newData && !mNewDeletionSettingPending) return mDataVersion; - mNewDeletionSettingPending = false; - mDeletionInEffect = false; - if (mDeletionPath != null) { - // See if mDeletionPath can be found in the MediaSet. We don't want - // to search the whole mBaseSet, so we just search a small window - // that is close the the index hint. + synchronized (mRequests) { + if (!newData && mRequests.isEmpty()) { + return mDataVersion; + } + for (int i = 0; i < mRequests.size(); i++) { + Request r = mRequests.get(i); + switch (r.type) { + case REQUEST_ADD: { + // Add the path into mCurrent if there is no duplicate. + int n = mCurrent.size(); + int j; + for (j = 0; j < n; j++) { + if (mCurrent.get(j).path == r.path) break; + } + if (j == n) { + mCurrent.add(new Deletion(r.path, r.indexHint)); + } + break; + } + case REQUEST_REMOVE: { + // Remove the path from mCurrent. + int n = mCurrent.size(); + for (int j = 0; j < n; j++) { + if (mCurrent.get(j).path == r.path) { + mCurrent.remove(j); + break; + } + } + break; + } + case REQUEST_CLEAR: { + mCurrent.clear(); + break; + } + } + } + mRequests.clear(); + } + + if (!mCurrent.isEmpty()) { + // See if the elements in mCurrent can be found in the MediaSet. We + // don't want to search the whole mBaseSet, so we just search a + // small window that contains the index hints (plus some margin). + int minIndex = mCurrent.get(0).index; + int maxIndex = minIndex; + for (int i = 1; i < mCurrent.size(); i++) { + Deletion d = mCurrent.get(i); + minIndex = Math.min(d.index, minIndex); + maxIndex = Math.max(d.index, maxIndex); + } + int n = mBaseSet.getMediaItemCount(); - int from = Math.max(mDeletionIndexHint - 5, 0); - int to = Math.min(mDeletionIndexHint + 5, n); + int from = Math.max(minIndex - 5, 0); + int to = Math.min(maxIndex + 5, n); ArrayList items = mBaseSet.getMediaItem(from, to - from); + ArrayList result = new ArrayList(); for (int i = 0; i < items.size(); i++) { MediaItem item = items.get(i); - if (item != null && item.getPath() == mDeletionPath) { - mDeletionInEffect = true; - mDeletionIndex = i + from; + if (item == null) continue; + Path p = item.getPath(); + // Find the matching path in mCurrent, if found move it to result + for (int j = 0; j < mCurrent.size(); j++) { + Deletion d = mCurrent.get(j); + if (d.path == p) { + d.index = from + i; + result.add(d); + mCurrent.remove(j); + break; + } } } - // We cannot find this path. Set it to null to avoid further search. - if (!mDeletionInEffect) { - mDeletionPath = null; - } + mCurrent = result; } + + mMediaItemCount = mBaseSet.getMediaItemCount() - mCurrent.size(); mDataVersion = nextVersionNumber(); return mDataVersion; } + private void sendRequest(int type, Path path, int indexHint) { + Request r = new Request(type, path, indexHint); + synchronized (mRequests) { + mRequests.add(r); + } + notifyContentChanged(); + } + @Override public void onContentDirty() { notifyContentChanged(); } - public void setDeletion(Path path, int indexHint) { - mDeletionPath = path; - mDeletionIndexHint = indexHint; - mNewDeletionSettingPending = true; - notifyContentChanged(); + public void addDeletion(Path path, int indexHint) { + sendRequest(REQUEST_ADD, path, indexHint); + } + + public void removeDeletion(Path path) { + sendRequest(REQUEST_REMOVE, path, 0 /* unused */); + } + + public void clearDeletion() { + sendRequest(REQUEST_CLEAR, null /* unused */ , 0 /* unused */); } } diff --git a/src/com/android/gallery3d/ui/PhotoView.java b/src/com/android/gallery3d/ui/PhotoView.java index 496d34d05..e44905dbf 100644 --- a/src/com/android/gallery3d/ui/PhotoView.java +++ b/src/com/android/gallery3d/ui/PhotoView.java @@ -141,7 +141,8 @@ public class PhotoView extends GLView { private static final int MSG_CAPTURE_ANIMATION_DONE = 4; private static final int MSG_DELETE_ANIMATION_DONE = 5; private static final int MSG_DELETE_DONE = 6; - private static final int MSG_HIDE_UNDO_BAR = 7; + private static final int MSG_UNDO_BAR_TIMEOUT = 7; + private static final int MSG_UNDO_BAR_FULL_CAMERA = 8; private static final int MOVE_THRESHOLD = 256; private static final float SWIPE_THRESHOLD = 300f; @@ -240,6 +241,7 @@ public class PhotoView extends GLView { @Override public void onClick(GLView v) { mListener.onUndoDeleteImage(); + hideUndoBar(); } }); mLoadingText = StringTexture.newInstance( @@ -330,6 +332,15 @@ public class PhotoView extends GLView { mHandler.removeMessages(MSG_DELETE_DONE); Message m = mHandler.obtainMessage(MSG_DELETE_DONE); mHandler.sendMessageDelayed(m, 2000); + + int numberOfPictures = mNextBound - mPrevBound + 1; + if (numberOfPictures == 2) { + if (mModel.isCamera(mNextBound) + || mModel.isCamera(mPrevBound)) { + numberOfPictures--; + } + } + showUndoBar(numberOfPictures <= 1); break; } case MSG_DELETE_DONE: { @@ -339,10 +350,14 @@ public class PhotoView extends GLView { } break; } - case MSG_HIDE_UNDO_BAR: { + case MSG_UNDO_BAR_TIMEOUT: { checkHideUndoBar(UNDO_BAR_TIMEOUT); break; } + case MSG_UNDO_BAR_FULL_CAMERA: { + checkHideUndoBar(UNDO_BAR_FULL_CAMERA); + break; + } default: throw new AssertionError(message.what); } } @@ -593,7 +608,7 @@ public class PhotoView extends GLView { if ((mHolding & ~HOLD_TOUCH_DOWN) != 0) return; boolean isCenter = mPositionController.isCenter(); - boolean isCameraCenter = mIsCamera && isCenter; + boolean isCameraCenter = mIsCamera && isCenter && !canUndoLastPicture(); if (mWasCameraCenter && mIsCamera && !isCenter && !mFilmMode) { // Temporary disabled to de-emphasize filmstrip. @@ -1255,6 +1270,7 @@ public class PhotoView extends GLView { for (int i = -SCREEN_NAIL_MAX; i <= SCREEN_NAIL_MAX; i++) { mPictures.get(i).setScreenNail(null); } + hideUndoBar(); } public void resume() { @@ -1275,33 +1291,56 @@ public class PhotoView extends GLView { private static final int UNDO_BAR_SHOW = 1; private static final int UNDO_BAR_TIMEOUT = 2; private static final int UNDO_BAR_TOUCHED = 4; + private static final int UNDO_BAR_FULL_CAMERA = 8; + private static final int UNDO_BAR_DELETE_LAST = 16; - public void showUndoBar() { - mHandler.removeMessages(MSG_HIDE_UNDO_BAR); + // "deleteLast" means if the deletion is on the last remaining picture in + // the album. + private void showUndoBar(boolean deleteLast) { + mHandler.removeMessages(MSG_UNDO_BAR_TIMEOUT); mUndoBarState = UNDO_BAR_SHOW; + if(deleteLast) mUndoBarState |= UNDO_BAR_DELETE_LAST; mUndoBar.animateVisibility(GLView.VISIBLE); - mHandler.sendEmptyMessageDelayed(MSG_HIDE_UNDO_BAR, 3000); + mHandler.sendEmptyMessageDelayed(MSG_UNDO_BAR_TIMEOUT, 3000); } - public void hideUndoBar() { - mHandler.removeMessages(MSG_HIDE_UNDO_BAR); + private void hideUndoBar() { + mHandler.removeMessages(MSG_UNDO_BAR_TIMEOUT); mListener.onCommitDeleteImage(); mUndoBar.animateVisibility(GLView.INVISIBLE); mUndoBarState = 0; mUndoIndexHint = Integer.MAX_VALUE; } - // Check if the all conditions for hiding the undo bar have been met. The - // conditions are: it has been three seconds since last showing, and the - // user has touched. + // Check if the one of the conditions for hiding the undo bar has been + // met. The conditions are: + // + // 1. It has been three seconds since last showing, and (a) the user has + // touched, or (b) the deleted picture is the last remaining picture in the + // album. + // + // 2. The camera is shown in full screen. private void checkHideUndoBar(int addition) { mUndoBarState |= addition; - if (mUndoBarState == - (UNDO_BAR_SHOW | UNDO_BAR_TIMEOUT | UNDO_BAR_TOUCHED)) { + if ((mUndoBarState & UNDO_BAR_SHOW) == 0) return; + boolean timeout = (mUndoBarState & UNDO_BAR_TIMEOUT) != 0; + boolean touched = (mUndoBarState & UNDO_BAR_TOUCHED) != 0; + boolean fullCamera = (mUndoBarState & UNDO_BAR_FULL_CAMERA) != 0; + boolean deleteLast = (mUndoBarState & UNDO_BAR_DELETE_LAST) != 0; + if ((timeout && (touched || deleteLast)) || fullCamera) { hideUndoBar(); } } + // Returns true if the user can still undo the deletion of the last + // remaining picture in the album. We need to check this and delay making + // the camera preview full screen, otherwise the user won't have a chance to + // undo it. + private boolean canUndoLastPicture() { + if ((mUndoBarState & UNDO_BAR_SHOW) == 0) return false; + return (mUndoBarState & UNDO_BAR_DELETE_LAST) != 0; + } + //////////////////////////////////////////////////////////////////////////// // Rendering //////////////////////////////////////////////////////////////////////////// @@ -1315,6 +1354,7 @@ public class PhotoView extends GLView { if (full != mFullScreenCamera) { mFullScreenCamera = full; mListener.onFullScreenChanged(full); + if (full) mHandler.sendEmptyMessage(MSG_UNDO_BAR_FULL_CAMERA); } // Determine how many photos we need to draw in addition to the center -- cgit v1.2.3