Don't set LottieDrawable bounds from within itself (#1713)
Drawables should provide intrinsic bounds to its consumers and should respect the bounds set on it. LottieDrawable was not conforming to this because it was settings its own bounds to the composition bounds. This caused issues like that reported in #1542.
Fixes #1542
diff --git a/lottie/src/main/java/com/airbnb/lottie/LottieAnimationView.java b/lottie/src/main/java/com/airbnb/lottie/LottieAnimationView.java
index 01757a5..df44788 100644
--- a/lottie/src/main/java/com/airbnb/lottie/LottieAnimationView.java
+++ b/lottie/src/main/java/com/airbnb/lottie/LottieAnimationView.java
@@ -214,9 +214,6 @@
setRenderMode(RenderMode.values()[renderModeOrdinal]);
}
- if (getScaleType() != null) {
- lottieDrawable.setScaleType(getScaleType());
- }
ta.recycle();
lottieDrawable.setSystemAnimationsAreEnabled(Utils.getAnimationScale(getContext()) != 0f);
@@ -965,13 +962,6 @@
return lottieDrawable.getScale();
}
- @Override public void setScaleType(ScaleType scaleType) {
- super.setScaleType(scaleType);
- if (lottieDrawable != null) {
- lottieDrawable.setScaleType(scaleType);
- }
- }
-
@MainThread
public void cancelAnimation() {
wasAnimatingWhenDetached = false;
diff --git a/lottie/src/main/java/com/airbnb/lottie/LottieDrawable.java b/lottie/src/main/java/com/airbnb/lottie/LottieDrawable.java
index d846b69..fc409bd 100644
--- a/lottie/src/main/java/com/airbnb/lottie/LottieDrawable.java
+++ b/lottie/src/main/java/com/airbnb/lottie/LottieDrawable.java
@@ -33,10 +33,8 @@
import java.lang.annotation.RetentionPolicy;
import java.util.ArrayList;
import java.util.Collections;
-import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
-import java.util.Set;
import androidx.annotation.FloatRange;
import androidx.annotation.IntDef;
@@ -51,10 +49,8 @@
*
* @see <a href="http://airbnb.io/lottie">Full Documentation</a>
*/
-@SuppressWarnings({"WeakerAccess", "unused"})
+@SuppressWarnings({"WeakerAccess"})
public class LottieDrawable extends Drawable implements Drawable.Callback, Animatable {
- private static final String TAG = LottieDrawable.class.getSimpleName();
-
private interface LazyCompositionTask {
void run(LottieComposition composition);
}
@@ -66,7 +62,6 @@
private boolean systemAnimationsEnabled = true;
private boolean safeMode = false;
- private final Set<ColorFilterData> colorFilterData = new HashSet<>();
private final ArrayList<LazyCompositionTask> lazyCompositionTasks = new ArrayList<>();
private final ValueAnimator.AnimatorUpdateListener progressUpdateListener = new ValueAnimator.AnimatorUpdateListener() {
@Override
@@ -77,8 +72,6 @@
}
};
@Nullable
- private ImageView.ScaleType scaleType;
- @Nullable
private ImageAssetManager imageAssetManager;
@Nullable
private String imageAssetsFolder;
@@ -183,10 +176,6 @@
* `setImageAssetsFolder("airbnb_loader/");`.
* <p>
* <p>
- * If you use LottieDrawable directly, you MUST call {@link #recycleBitmaps()} when you
- * are done. Calling {@link #recycleBitmaps()} doesn't have to be final and {@link LottieDrawable}
- * will recreate the bitmaps if needed but they will leak if you don't recycle them.
- * <p>
* Be wary if you are using many images, however. Lottie is designed to work with vector shapes
* from After Effects. If your images look like they could be represented with vector shapes,
* see if it is possible to convert them to shape layers and re-export your animation. Check
@@ -219,7 +208,6 @@
animator.setComposition(composition);
setProgress(animator.getAnimatedFraction());
setScale(scale);
- updateBounds();
// We copy the tasks to a new ArrayList so that if this method is called from multiple threads,
// then there won't be two iterators iterating and removing at the same time.
@@ -399,13 +387,25 @@
}
private void drawInternal(@NonNull Canvas canvas) {
- if (ImageView.ScaleType.FIT_XY == scaleType) {
+ if (!boundsMatchesCompositionAspectRatio()) {
drawWithNewAspectRatio(canvas);
} else {
drawWithOriginalAspectRatio(canvas);
}
}
+ private boolean boundsMatchesCompositionAspectRatio() {
+ LottieComposition composition = this.composition;
+ if (composition == null || getBounds().isEmpty()) {
+ return true;
+ }
+ return aspectRatio(getBounds()) == aspectRatio(composition.getBounds());
+ }
+
+ private float aspectRatio(Rect rect) {
+ return rect.width() / (float) rect.height();
+ }
+
// <editor-fold desc="animator">
@MainThread
@@ -655,8 +655,8 @@
}
int startFrame = (int) startMarker.startFrame;
- Marker endMarker = composition.getMarker(endMarkerName);
- if (endMarkerName == null) {
+ final Marker endMarker = composition.getMarker(endMarkerName);
+ if (endMarker == null) {
throw new IllegalArgumentException("Cannot find marker with name " + endMarkerName + ".");
}
int endFrame = (int) (endMarker.startFrame + (playEndMarkerStartFrame ? 1f : 0f));
@@ -859,6 +859,7 @@
public boolean isAnimating() {
// On some versions of Android, this is called from the LottieAnimationView constructor, before animator was created.
// https://github.com/airbnb/lottie-android/issues/1430
+ //noinspection ConstantConditions
if (animator == null) {
return false;
}
@@ -885,7 +886,6 @@
*/
public void setScale(float scale) {
this.scale = scale;
- updateBounds();
}
/**
@@ -899,8 +899,7 @@
* the documentation at http://airbnb.io/lottie for more information about importing shapes from
* Sketch or Illustrator to avoid this.
*/
- public void setImageAssetDelegate(
- @SuppressWarnings("NullableProblems") ImageAssetDelegate assetDelegate) {
+ public void setImageAssetDelegate(ImageAssetDelegate assetDelegate) {
this.imageAssetDelegate = assetDelegate;
if (imageAssetManager != null) {
imageAssetManager.setDelegate(assetDelegate);
@@ -910,8 +909,7 @@
/**
* Use this to manually set fonts.
*/
- public void setFontAssetDelegate(
- @SuppressWarnings("NullableProblems") FontAssetDelegate assetDelegate) {
+ public void setFontAssetDelegate(FontAssetDelegate assetDelegate) {
this.fontAssetDelegate = assetDelegate;
if (fontAssetManager != null) {
fontAssetManager.setDelegate(assetDelegate);
@@ -939,15 +937,6 @@
return composition;
}
- private void updateBounds() {
- if (composition == null) {
- return;
- }
- float scale = getScale();
- setBounds(0, 0, (int) (composition.getBounds().width() * scale),
- (int) (composition.getBounds().height() * scale));
- }
-
public void cancelAnimation() {
lazyCompositionTasks.clear();
animator.cancel();
@@ -993,7 +982,7 @@
/**
* Add an property callback for the specified {@link KeyPath}. This {@link KeyPath} can resolve
- * to multiple contents. In that case, the callbacks's value will apply to all of them.
+ * to multiple contents. In that case, the callback's value will apply to all of them.
* <p>
* Internally, this will check if the {@link KeyPath} has already been resolved with
* {@link #resolveKeyPath(KeyPath)} and will resolve it if it hasn't.
@@ -1164,10 +1153,6 @@
callback.unscheduleDrawable(this, what);
}
- void setScaleType(ImageView.ScaleType scaleType) {
- this.scaleType = scaleType;
- }
-
/**
* If the composition is larger than the canvas, we have to use a different method to scale it up.
* See the comments in {@link #draw(Canvas)} for more info.
@@ -1265,49 +1250,4 @@
canvas.restoreToCount(saveCount);
}
}
-
- private static class ColorFilterData {
-
- final String layerName;
- @Nullable
- final String contentName;
- @Nullable
- final ColorFilter colorFilter;
-
- ColorFilterData(@Nullable String layerName, @Nullable String contentName,
- @Nullable ColorFilter colorFilter) {
- this.layerName = layerName;
- this.contentName = contentName;
- this.colorFilter = colorFilter;
- }
-
- @Override
- public int hashCode() {
- int hashCode = 17;
- if (layerName != null) {
- hashCode = hashCode * 31 * layerName.hashCode();
- }
-
- if (contentName != null) {
- hashCode = hashCode * 31 * contentName.hashCode();
- }
- return hashCode;
- }
-
- @Override
- public boolean equals(Object obj) {
- if (this == obj) {
- return true;
- }
-
- if (!(obj instanceof ColorFilterData)) {
- return false;
- }
-
- final ColorFilterData other = (ColorFilterData) obj;
-
- return hashCode() == other.hashCode() && colorFilter == other.colorFilter;
-
- }
- }
}
diff --git a/sample/src/androidTest/java/com/airbnb/lottie/samples/HappoSnapshotter.kt b/sample/src/androidTest/java/com/airbnb/lottie/samples/HappoSnapshotter.kt
index 4d3d279..976668d 100644
--- a/sample/src/androidTest/java/com/airbnb/lottie/samples/HappoSnapshotter.kt
+++ b/sample/src/androidTest/java/com/airbnb/lottie/samples/HappoSnapshotter.kt
@@ -51,6 +51,8 @@
private val gitBranch = URLEncoder.encode((if (BC.BITRISE_GIT_BRANCH == "null") BC.GIT_BRANCH else BC.BITRISE_GIT_BRANCH).replace("/", "_"), "UTF-8")
private val androidVersion = "android${Build.VERSION.SDK_INT}"
private val reportNamePrefixes = listOf(BC.GIT_SHA, gitBranch, BC.VERSION_NAME).filter { it.isNotBlank() }
+ // Use this when running snapshots locally.
+ // private val reportNamePrefixes = listOf(System.currentTimeMillis().toString()).filter { it.isNotBlank() }
private val reportNames = reportNamePrefixes.map { "$it-$androidVersion" }
private val okhttp = OkHttpClient()
diff --git a/sample/src/androidTest/java/com/airbnb/lottie/samples/LottieTest.kt b/sample/src/androidTest/java/com/airbnb/lottie/samples/LottieTest.kt
index 4b6ccb6..1fd3424 100644
--- a/sample/src/androidTest/java/com/airbnb/lottie/samples/LottieTest.kt
+++ b/sample/src/androidTest/java/com/airbnb/lottie/samples/LottieTest.kt
@@ -101,6 +101,7 @@
@Test
fun testAll() = runBlocking {
withTimeout(TimeUnit.MINUTES.toMillis(45)) {
+ testCustomBounds()
testColorStateListColorFilter()
testFailure()
snapshotFrameBoundaries()
@@ -756,12 +757,13 @@
drawable.addValueCallback(KeyPath("Linear", "Rectangle", "Gradient Fill"), LottieProperty.OPACITY, value)
}
- withDrawable("Tests/MatteTimeStretchScan.json", "Mirror animation", "Mirror animation") {
- drawable ->
- drawable.addValueCallback(KeyPath.COMPOSITION, LottieProperty.TRANSFORM_ANCHOR_POINT,
- { PointF(drawable.bounds.width().toFloat(), 0f) })
- drawable.addValueCallback(KeyPath.COMPOSITION, LottieProperty.TRANSFORM_SCALE,
- { ScaleXY(-1.0f, 1.0f) })
+ withDrawable("Tests/MatteTimeStretchScan.json", "Mirror animation", "Mirror animation") { drawable ->
+ drawable.addValueCallback(KeyPath.COMPOSITION, LottieProperty.TRANSFORM_ANCHOR_POINT) {
+ PointF(drawable.composition.bounds.width().toFloat(), 0f)
+ }
+ drawable.addValueCallback(KeyPath.COMPOSITION, LottieProperty.TRANSFORM_SCALE) {
+ ScaleXY(-1.0f, 1.0f)
+ }
}
withDrawable("Tests/TrackMattes.json", "Matte", "Matte property") { drawable ->
@@ -1028,6 +1030,25 @@
}
}
+ private suspend fun testCustomBounds() {
+ val composition = LottieCompositionFactory.fromRawResSync(application, R.raw.heart).value!!
+ val bitmap = bitmapPool.acquire(50, 100)
+ val canvas = Canvas(bitmap)
+ val drawable = LottieDrawable()
+ drawable.composition = composition
+ drawable.repeatCount = Integer.MAX_VALUE
+ drawable.setBounds(0, 0, 25, 100)
+ withContext(Dispatchers.Main) {
+ drawable.draw(canvas)
+ }
+ LottieCompositionCache.getInstance().clear()
+ snapshotter.record(bitmap, "CustomBounds", "Heart-25x100")
+ snapshotActivityRule.scenario.onActivity { activity ->
+ activity.recordSnapshot("CustomBounds", "Heart-25x100")
+ }
+ bitmapPool.release(bitmap)
+ }
+
private suspend fun testColorStateListColorFilter() {
log("Testing color filter")
val context = ContextThemeWrapper(application, R.style.AppTheme)