diff --git a/CHANGELOG.md b/CHANGELOG.md index 405beaaf..df676f8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +# 4.2.2-SNAPSHOT +* Fixed `AsyncDrawable` display when it has placeholder with empty bounds ([#189]) + +[#189]: https://github.com/noties/Markwon/issues/189 + # 4.2.1 * Fix SpannableBuilder `subSequence` method * Introduce Nougat check in `BulletListItemSpan` to position bullet (for bullets to be diff --git a/gradle.properties b/gradle.properties index b0b8871f..3e1e4325 100644 --- a/gradle.properties +++ b/gradle.properties @@ -8,7 +8,7 @@ android.enableJetifier=true android.enableBuildCache=true android.buildCacheDir=build/pre-dex-cache -VERSION_NAME=4.2.1 +VERSION_NAME=4.2.2-SNAPSHOT GROUP=io.noties.markwon POM_DESCRIPTION=Markwon markdown for Android diff --git a/markwon-core/src/main/java/io/noties/markwon/image/AsyncDrawable.java b/markwon-core/src/main/java/io/noties/markwon/image/AsyncDrawable.java index ce6f92d5..36b2c3f2 100644 --- a/markwon-core/src/main/java/io/noties/markwon/image/AsyncDrawable.java +++ b/markwon-core/src/main/java/io/noties/markwon/image/AsyncDrawable.java @@ -164,20 +164,44 @@ public class AsyncDrawable extends Drawable { @SuppressWarnings("WeakerAccess") protected void setPlaceholderResult(@NonNull Drawable placeholder) { // okay, if placeholder has bounds -> use it, otherwise use original imageSize + // it's important to NOT pass to imageSizeResolver when placeholder has bounds + // this is done, so actual result and placeholder can have _different_ + // bounds. Assume image is loaded with HTML and has ImageSize width=100%, + // so, even if placeholder has exact bounds, it will still be scaled up. + + // this condition should not be true for placeholder (at least for now) + // (right now this method is always called from constructor) + if (result != null) { + // but it is, unregister current result + result.setCallback(null); + } final Rect rect = placeholder.getBounds(); if (rect.isEmpty()) { - // if bounds are empty -> just use placeholder as a regular result - DrawableUtils.applyIntrinsicBounds(placeholder); + // check for intrinsic bounds + final Rect intrinsic = DrawableUtils.intrinsicBounds(placeholder); + if (intrinsic.isEmpty()) { + // @since 4.2.2.-SNAPSHOT + // if intrinsic bounds are empty, use _any_ non-empty bounds, + // they must be non-empty so when result is obtained - proper invalidation will occur + // (0, 0, 1, 0) is still considered empty + placeholder.setBounds(0, 0, 1, 1); + } else { + // use them + placeholder.setBounds(intrinsic); + } + + // it is very important (if we have a placeholder) to set own bounds to it (and they must not be empty + // otherwise result won't be rendered) + // @since 4.2.2-SNAPSHOT + setBounds(placeholder.getBounds()); setResult(placeholder); + } else { - // this condition should not be true for placeholder (at least for now) - if (result != null) { - // but it is, unregister current result - result.setCallback(null); - } + // this method is not the same as above, as we do not want to trigger image-size-resolver + // in case when placeholder has exact bounds // placeholder has bounds specified -> use them until we have real result this.result = placeholder; diff --git a/markwon-core/src/test/java/io/noties/markwon/image/AsyncDrawableTest.java b/markwon-core/src/test/java/io/noties/markwon/image/AsyncDrawableTest.java index 809cfd5f..d0a8f5c6 100644 --- a/markwon-core/src/test/java/io/noties/markwon/image/AsyncDrawableTest.java +++ b/markwon-core/src/test/java/io/noties/markwon/image/AsyncDrawableTest.java @@ -4,6 +4,7 @@ import android.graphics.Canvas; import android.graphics.ColorFilter; import android.graphics.Rect; import android.graphics.drawable.Drawable; + import androidx.annotation.NonNull; import androidx.annotation.Nullable; @@ -18,7 +19,12 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; @RunWith(RobolectricTestRunner.class) @Config(manifest = Config.NONE) @@ -78,6 +84,123 @@ public class AsyncDrawableTest { assertNotNull(result2.getCallback()); } + @Test + public void placeholder_no_bounds_no_intrinsic_bounds() { + // when there is a placeholder and its + // * bounds are empty + // * intrinsic bounds are empty + // AsyncDrawable.this must have any non-empty bounds (otherwise result won't be rendered, + // due to missing invalidation call) + + final Drawable placeholder = new AbstractDrawable() { + @Override + public int getIntrinsicWidth() { + return 0; + } + + @Override + public int getIntrinsicHeight() { + return 0; + } + }; + + assertTrue(placeholder.getBounds().isEmpty()); + + final AsyncDrawableLoader loader = mock(AsyncDrawableLoader.class); + when(loader.placeholder(any(AsyncDrawable.class))).thenReturn(placeholder); + + final AsyncDrawable drawable = new AsyncDrawable( + "", + loader, + mock(ImageSizeResolver.class), + null + ); + + final Rect bounds = drawable.getBounds(); + assertFalse(bounds.toShortString(), bounds.isEmpty()); + assertEquals(bounds.toShortString(), bounds, placeholder.getBounds()); + } + + @Test + public void placeholder_no_bounds_has_intrinsic() { + // placeholder has no bounds, but instead has intrinsic bounds + + final Drawable placeholder = new AbstractDrawable() { + @Override + public int getIntrinsicWidth() { + return 42; + } + + @Override + public int getIntrinsicHeight() { + return 24; + } + }; + + assertTrue(placeholder.getBounds().isEmpty()); + + final AsyncDrawableLoader loader = mock(AsyncDrawableLoader.class); + when(loader.placeholder(any(AsyncDrawable.class))).thenReturn(placeholder); + + final AsyncDrawable drawable = new AsyncDrawable( + "", + loader, + mock(ImageSizeResolver.class), + null + ); + + final Rect bounds = drawable.getBounds(); + assertFalse(bounds.isEmpty()); + assertEquals(0, bounds.left); + assertEquals(42, bounds.right); + assertEquals(0, bounds.top); + assertEquals(24, bounds.bottom); + + assertEquals(bounds, placeholder.getBounds()); + } + + @Test + public void placeholder_has_bounds() { + + final Rect rect = new Rect(0, 0, 12, 99); + final Drawable placeholder = mock(Drawable.class); + when(placeholder.getBounds()).thenReturn(rect); + + assertFalse(rect.isEmpty()); + + final AsyncDrawableLoader loader = mock(AsyncDrawableLoader.class); + when(loader.placeholder(any(AsyncDrawable.class))).thenReturn(placeholder); + + final AsyncDrawable drawable = new AsyncDrawable( + "", + loader, + mock(ImageSizeResolver.class), + null + ); + + final Rect bounds = drawable.getBounds(); + assertEquals(rect, bounds); + + verify(placeholder, times(1)).getBounds(); + verify(placeholder, never()).getIntrinsicWidth(); + verify(placeholder, never()).getIntrinsicHeight(); + verify(placeholder, never()).setBounds(any(Rect.class)); + } + + @Test + public void no_placeholder_empty_bounds() { + // when AsyncDrawable has no placeholder, then its bounds must be empty at the start + + final AsyncDrawable drawable = new AsyncDrawable( + "", + mock(AsyncDrawableLoader.class), + mock(ImageSizeResolver.class), + null + ); + + assertTrue(drawable.getBounds().isEmpty()); + } + private static class AbstractDrawable extends Drawable { @Override diff --git a/sample/src/main/java/io/noties/markwon/sample/recycler/RecyclerActivity.java b/sample/src/main/java/io/noties/markwon/sample/recycler/RecyclerActivity.java index 815e5b03..3d86c683 100644 --- a/sample/src/main/java/io/noties/markwon/sample/recycler/RecyclerActivity.java +++ b/sample/src/main/java/io/noties/markwon/sample/recycler/RecyclerActivity.java @@ -2,6 +2,8 @@ package io.noties.markwon.sample.recycler; import android.app.Activity; import android.content.Context; +import android.graphics.drawable.ColorDrawable; +import android.graphics.drawable.Drawable; import android.net.Uri; import android.os.Bundle; import android.text.TextUtils; @@ -27,7 +29,12 @@ import io.noties.markwon.MarkwonConfiguration; import io.noties.markwon.MarkwonVisitor; import io.noties.markwon.core.CorePlugin; import io.noties.markwon.html.HtmlPlugin; +import io.noties.markwon.image.AsyncDrawable; +import io.noties.markwon.image.ImagesPlugin; +import io.noties.markwon.image.file.FileSchemeHandler; +import io.noties.markwon.image.network.OkHttpNetworkSchemeHandler; import io.noties.markwon.image.picasso.PicassoImagesPlugin; +import io.noties.markwon.image.svg.SvgMediaDecoder; import io.noties.markwon.recycler.MarkwonAdapter; import io.noties.markwon.recycler.SimpleEntry; import io.noties.markwon.recycler.table.TableEntry; @@ -74,13 +81,14 @@ public class RecyclerActivity extends Activity { private static Markwon markwon(@NonNull Context context) { return Markwon.builder(context) .usePlugin(CorePlugin.create()) -// .usePlugin(ImagesPlugin.create(plugin -> { -// plugin -// .addSchemeHandler(FileSchemeHandler.createWithAssets(context)) -// .addSchemeHandler(OkHttpNetworkSchemeHandler.create()) -// .addMediaDecoder(SvgMediaDecoder.create()); -// })) - .usePlugin(PicassoImagesPlugin.create(context)) + .usePlugin(ImagesPlugin.create(plugin -> { + plugin + .addSchemeHandler(FileSchemeHandler.createWithAssets(context)) + .addSchemeHandler(OkHttpNetworkSchemeHandler.create()) + .addMediaDecoder(SvgMediaDecoder.create()) + .placeholderProvider(drawable -> new ColorDrawable(0xFFff0000)); + })) +// .usePlugin(PicassoImagesPlugin.create(context)) // .usePlugin(GlideImagesPlugin.create(context)) // .usePlugin(CoilImagesPlugin.create(context)) // important to use TableEntryPlugin instead of TablePlugin