Image Editor - Further crop improvements.

* Thumb accuracy improved.
* When out of bounds from drag, try to fix by adjusting translation.
* Update undo state when listener changes.

src/org/thoughtcrime/securesms/imageeditor/ThumbDragEditSession.java

@@ -2,6 +2,7 @@ package org.thoughtcrime.securesms.imageeditor;
 
 import android.graphics.Matrix;
 import android.graphics.PointF;
+import android.graphics.RectF;
 import android.support.annotation.NonNull;
 
 import org.thoughtcrime.securesms.imageeditor.model.EditorElement;
@@ -37,22 +38,32 @@ class ThumbDragEditSession extends ElementEditSession {
     float x = controlPoint.opposite().getX();
     float y = controlPoint.opposite().getY();
 
-    editorMatrix.postTranslate(-x, -y);
+    float dx = endPointElement[0].x - startPointElement[0].x;
+    float dy = endPointElement[0].y - startPointElement[0].y;
+
+    float xEnd = controlPoint.getX() + dx;
+    float yEnd = controlPoint.getY() + dy;
 
     boolean aspectLocked = selected.getFlags().isAspectLocked() && !controlPoint.isCenter();
 
     float defaultScale = aspectLocked ? 2 : 1;
 
-    float scaleX = controlPoint.isVerticalCenter()   ? defaultScale : (endPointElement[0].x - x) / (startPointElement[0].x - x);
+    float scaleX = controlPoint.isVerticalCenter()   ? defaultScale : (xEnd - x) / (controlPoint.getX() - x);
-    float scaleY = controlPoint.isHorizontalCenter() ? defaultScale : (endPointElement[0].y - y) / (startPointElement[0].y - y);
+    float scaleY = controlPoint.isHorizontalCenter() ? defaultScale : (yEnd - y) / (controlPoint.getY() - y);
 
+    scale(editorMatrix, aspectLocked, scaleX, scaleY, controlPoint.opposite());
+  }
+
+  private void scale(Matrix editorMatrix, boolean aspectLocked, float scaleX, float scaleY, ThumbRenderer.ControlPoint around) {
+    float x = around.getX();
+    float y = around.getY();
+    editorMatrix.postTranslate(-x, -y);
     if (aspectLocked) {
       float minScale = Math.min(scaleX, scaleY);
       editorMatrix.postScale(minScale, minScale);
     } else {
       editorMatrix.postScale(scaleX, scaleY);
     }
-
     editorMatrix.postTranslate(x, y);
   }
 
@@ -65,4 +76,4 @@ class ThumbDragEditSession extends ElementEditSession {
   public EditSession removePoint(@NonNull Matrix newInverse, int p) {
     return null;
   }
-}
+}

src/org/thoughtcrime/securesms/imageeditor/model/Bisect.java

@@ -0,0 +1,113 @@
+package org.thoughtcrime.securesms.imageeditor.model;
+
+import android.graphics.Matrix;
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
+
+final class Bisect {
+
+  static final float ACCURACY = 0.001f;
+
+  private static final int MAX_ITERATIONS = 16;
+
+  interface Predicate {
+     boolean test();
+  }
+
+  interface ModifyElement {
+    void applyFactor(@NonNull Matrix matrix, float factor);
+  }
+
+  /**
+   * Given a predicate function, attempts to finds the boundary between predicate true and predicate false.
+   * If it returns true, it will animate the element to the closest true value found to that boundary.
+   *
+   * @param element          The element to modify.
+   * @param outOfBoundsValue The current value, known to be out of bounds. 1 for a scale and 0 for a translate.
+   * @param atMost           A value believed to be in bounds.
+   * @param predicate        The out of bounds predicate.
+   * @param modifyElement    Apply the latest value to the element local matrix.
+   * @param invalidate       For animation if finds a result.
+   * @return true iff finds a result.
+   */
+ static boolean bisectToTest(@NonNull EditorElement element,
+                             float outOfBoundsValue,
+                             float atMost,
+                             @NonNull Predicate predicate,
+                             @NonNull ModifyElement modifyElement,
+                             @NonNull Runnable invalidate)
+ {
+   Matrix closestSuccesful = bisectToTest(element, outOfBoundsValue, atMost, predicate, modifyElement);
+
+   if (closestSuccesful != null) {
+     element.animateLocalTo(closestSuccesful, invalidate);
+     return true;
+   } else {
+     return false;
+   }
+  }
+
+  /**
+   * Given a predicate function, attempts to finds the boundary between predicate true and predicate false.
+   * Returns new local matrix for the element if a solution is found.
+   *
+   * @param element          The element to modify.
+   * @param outOfBoundsValue The current value, known to be out of bounds. 1 for a scale and 0 for a translate.
+   * @param atMost           A value believed to be in bounds.
+   * @param predicate        The out of bounds predicate.
+   * @param modifyElement    Apply the latest value to the element local matrix.
+   * @return matrix to replace local matrix iff finds a result, null otherwise.
+   */
+ static @Nullable Matrix bisectToTest(@NonNull EditorElement element,
+                                      float outOfBoundsValue,
+                                      float atMost,
+                                      @NonNull Predicate predicate,
+                                      @NonNull ModifyElement modifyElement)
+ {
+   Matrix  elementMatrix     = element.getLocalMatrix();
+   Matrix  original          = new Matrix(elementMatrix);
+   Matrix  closestSuccessful = new Matrix();
+   boolean haveResult        = false;
+   int     attempt           = 0;
+   float   successValue      = 0;
+   float   inBoundsValue     = atMost;
+   float   nextValueToTry    = inBoundsValue;
+
+   do {
+     attempt++;
+
+     modifyElement.applyFactor(elementMatrix, nextValueToTry);
+     try {
+
+       if (predicate.test()) {
+         inBoundsValue = nextValueToTry;
+
+         // if first success or closer to out of bounds than the current closest
+         if (!haveResult || Math.abs(nextValueToTry) < Math.abs(successValue)) {
+           haveResult = true;
+           successValue = nextValueToTry;
+           closestSuccessful.set(elementMatrix);
+         }
+       } else {
+         if (attempt == 1) {
+           // failure on first attempt means inBoundsValue is actually out of bounds and so no solution
+           return null;
+         }
+         outOfBoundsValue = nextValueToTry;
+       }
+     } finally {
+       // reset
+       elementMatrix.set(original);
+     }
+
+     nextValueToTry = (inBoundsValue + outOfBoundsValue) / 2f;
+
+   } while (attempt < MAX_ITERATIONS && Math.abs(inBoundsValue - outOfBoundsValue) > ACCURACY);
+
+   if (haveResult) {
+     return closestSuccessful;
+   }
+   return null;
+ }
+
+}

src/org/thoughtcrime/securesms/imageeditor/model/EditorModel.java

@@ -76,6 +76,8 @@ public final class EditorModel implements Parcelable, RendererContext.Ready {
 
   public void setUndoRedoStackListener(UndoRedoStackListener undoRedoStackListener) {
     this.undoRedoStackListener = undoRedoStackListener;
+
+    updateUndoRedoAvailableState(getActiveUndoRedoStacks(isCropping()));
   }
 
   /**
@@ -265,6 +267,8 @@ public final class EditorModel implements Parcelable, RendererContext.Ready {
         if (!tryToScaleToFit(cropEditorElement, 0.9f)) {
           tryToScaleToFit(mainImage, 2f);
         }
+      } else {
+        tryToFixTranslationOutOfBounds(mainImage, inBoundsMemory.getLastKnownGoodMainImageMatrix());
       }
 
       if (!currentCropIsAcceptable()) {
@@ -287,33 +291,104 @@ public final class EditorModel implements Parcelable, RendererContext.Ready {
    * @return true if successfully scaled the element. false if the element was left unchanged.
    */
   private boolean tryToScaleToFit(@NonNull EditorElement element, float scaleAtMost) {
-    Matrix  elementMatrix     = element.getLocalMatrix();
+    return Bisect.bisectToTest(element,
-    Matrix  original          = new Matrix(elementMatrix);
+                        1,
-    Matrix  lastSuccessful    = new Matrix();
+                        scaleAtMost,
-    boolean success           = false;
+                        this::cropIsWithinMainImageBounds,
-    float   unsuccessfulScale = 1;
+                        (matrix, scale) -> matrix.preScale(scale, scale),
-    int     attempt           = 0;
+                        invalidate);
+  }
 
-    do {
+  /**
-      float tryScale = (scaleAtMost + unsuccessfulScale) / 2f;
+   * Attempts to translate the supplied element such that {@link #cropIsWithinMainImageBounds} is true.
-      elementMatrix.set(original);
+   * If you supply both x and y, it will attempt to find a fit on the diagonal with vector x, y.
-      elementMatrix.preScale(tryScale, tryScale);
+   *
+   * @param element          The element to be translated. If successful, it will be animated to the correct position.
+   * @param translateXAtMost The maximum translation to apply in the x axis.
+   * @param translateYAtMost The maximum translation to apply in the y axis.
+   * @return a matrix if successfully translated the element. null if the element unable to be translated to fit.
+   */
+  private Matrix tryToTranslateToFit(@NonNull EditorElement element, float translateXAtMost, float translateYAtMost) {
+    return Bisect.bisectToTest(element,
+                               0,
+                               1,
+                               this::cropIsWithinMainImageBounds,
+                               (matrix, factor) -> matrix.postTranslate(factor * translateXAtMost, factor * translateYAtMost));
+  }
 
-      if (cropIsWithinMainImageBounds(editorElementHierarchy)) {
+  /**
-        scaleAtMost = tryScale;
+   * Tries to fix an element that is out of bounds by adjusting it's translation.
-        success = true;
+   *
-        lastSuccessful.set(elementMatrix);
+   * @param element               Element to move.
-      } else {
+   * @param lastKnownGoodPosition Last known good position of element.
-        unsuccessfulScale = tryScale;
+   * @return true iff fixed the element.
-      }
+   */
-      attempt++;
+  private boolean tryToFixTranslationOutOfBounds(@NonNull EditorElement element, @NonNull Matrix lastKnownGoodPosition) {
-    } while (attempt < 16 && Math.abs(scaleAtMost - unsuccessfulScale) > 0.001f);
+    final Matrix  elementMatrix = element.getLocalMatrix();
+    final Matrix  original      = new Matrix(elementMatrix);
+    final float[] current       = new float[9];
+    final float[] lastGood      = new float[9];
+    Matrix matrix;
 
-    elementMatrix.set(original);
+    elementMatrix.getValues(current);
-    if (success) {
+    lastKnownGoodPosition.getValues(lastGood);
-      element.animateLocalTo(lastSuccessful, invalidate);
+
+    final float xTranslate = current[2] - lastGood[2];
+    final float yTranslate = current[5] - lastGood[5];
+
+    if (Math.abs(xTranslate) < Bisect.ACCURACY && Math.abs(yTranslate) < Bisect.ACCURACY) {
+      return false;
     }
-    return success;
+
+    float pass1X;
+    float pass1Y;
+
+    float pass2X;
+    float pass2Y;
+
+    // try the fix by the smallest user translation first
+    if (Math.abs(xTranslate) < Math.abs(yTranslate)) {
+      // try to bisect along x
+      pass1X = -xTranslate;
+      pass1Y = 0;
+
+      // then y
+      pass2X = 0;
+      pass2Y = -yTranslate;
+    } else {
+      // try to bisect along y
+      pass1X = 0;
+      pass1Y = -yTranslate;
+
+      // then x
+      pass2X = -xTranslate;
+      pass2Y = 0;
+    }
+
+    matrix = tryToTranslateToFit(element, pass1X, pass1Y);
+    if (matrix != null) {
+      element.animateLocalTo(matrix, invalidate);
+      return true;
+    }
+
+    matrix = tryToTranslateToFit(element, pass2X, pass2Y);
+    if (matrix != null) {
+      element.animateLocalTo(matrix, invalidate);
+      return true;
+    }
+
+    // apply pass 1 fully
+    elementMatrix.postTranslate(pass1X, pass1Y);
+
+    matrix = tryToTranslateToFit(element, pass2X, pass2Y);
+    elementMatrix.set(original);
+
+    if (matrix != null) {
+      element.animateLocalTo(matrix, invalidate);
+      return true;
+    }
+
+    return false;
   }
 
   public void dragDropRelease() {
@@ -321,7 +396,7 @@ public final class EditorModel implements Parcelable, RendererContext.Ready {
   }
 
   /**
-   * Pixel count must be no smaller than {@link #MINIMUM_CROP_PIXEL_COUNT} (unless it's original size was less than that)
+   * Pixel count must be no smaller than {@link #MINIMUM_CROP_PIXEL_COUNT} (unless its original size was less than that)
    * and all points must be within the bounds.
    */
   private boolean currentCropIsAcceptable() {
@@ -338,7 +413,7 @@ public final class EditorModel implements Parcelable, RendererContext.Ready {
 
     return compareRatios(outputSize, thinnestRatio) >= 0 &&
            outputPixelCount >= minimumPixelCount &&
-           cropIsWithinMainImageBounds(editorElementHierarchy);
+           cropIsWithinMainImageBounds();
   }
 
   /**
@@ -359,8 +434,8 @@ public final class EditorModel implements Parcelable, RendererContext.Ready {
   /**
    * @return true if and only if the current crop rect is fully in the bounds.
    */
-  private static boolean cropIsWithinMainImageBounds(@NonNull EditorElementHierarchy hierarchy) {
+  private boolean cropIsWithinMainImageBounds() {
-    return Bounds.boundsRemainInBounds(hierarchy.imageMatrixRelativeToCrop());
+    return Bounds.boundsRemainInBounds(editorElementHierarchy.imageMatrixRelativeToCrop());
   }
 
   /**

src/org/thoughtcrime/securesms/imageeditor/model/InBoundsMemory.java

@@ -27,4 +27,8 @@ final class InBoundsMemory {
     }
     cropEditorElement.animateLocalTo(lastGoodUserCrop, invalidate);
   }
+
+  Matrix getLastKnownGoodMainImageMatrix() {
+    return new Matrix(lastGoodMainImage);
+  }
 }