diff --git a/library/src/main/java/com/bumptech/glide/GlideBuilder.java b/library/src/main/java/com/bumptech/glide/GlideBuilder.java index e77742fcb9..1276daea8e 100644 --- a/library/src/main/java/com/bumptech/glide/GlideBuilder.java +++ b/library/src/main/java/com/bumptech/glide/GlideBuilder.java @@ -329,8 +329,14 @@ public Glide build(Context context) { } if (engine == null) { - engine = new Engine(memoryCache, diskCacheFactory, diskCacheExecutor, sourceExecutor, - GlideExecutor.newUnlimitedSourceExecutor()); + engine = + new Engine( + memoryCache, + diskCacheFactory, + diskCacheExecutor, + sourceExecutor, + GlideExecutor.newUnlimitedSourceExecutor(), + GlideExecutor.newAnimationExecutor()); } RequestManagerRetriever requestManagerRetriever = new RequestManagerRetriever( diff --git a/library/src/main/java/com/bumptech/glide/load/engine/Engine.java b/library/src/main/java/com/bumptech/glide/load/engine/Engine.java index 315d4cf113..ed0ad6a585 100644 --- a/library/src/main/java/com/bumptech/glide/load/engine/Engine.java +++ b/library/src/main/java/com/bumptech/glide/load/engine/Engine.java @@ -66,9 +66,21 @@ public Engine(MemoryCache memoryCache, DiskCache.Factory diskCacheFactory, GlideExecutor diskCacheExecutor, GlideExecutor sourceExecutor, - GlideExecutor sourceUnlimitedExecutor) { - this(memoryCache, diskCacheFactory, diskCacheExecutor, sourceExecutor, sourceUnlimitedExecutor, - null, null, null, null, null, null); + GlideExecutor sourceUnlimitedExecutor, + GlideExecutor animationExecutor) { + this( + memoryCache, + diskCacheFactory, + diskCacheExecutor, + sourceExecutor, + sourceUnlimitedExecutor, + animationExecutor, + /*jobs=*/ null, + /*keyFactory=*/ null, + /*activeResources=*/ null, + /*engineJobFactory=*/ null, + /*decodeJobFactory=*/ null, + /*resourceRecycler=*/ null); } // Visible for testing. @@ -77,6 +89,7 @@ public Engine(MemoryCache memoryCache, GlideExecutor diskCacheExecutor, GlideExecutor sourceExecutor, GlideExecutor sourceUnlimitedExecutor, + GlideExecutor animationExecutor, Map> jobs, EngineKeyFactory keyFactory, Map>> activeResources, @@ -102,8 +115,9 @@ public Engine(MemoryCache memoryCache, this.jobs = jobs; if (engineJobFactory == null) { - engineJobFactory = new EngineJobFactory(diskCacheExecutor, sourceExecutor, - sourceUnlimitedExecutor, this); + engineJobFactory = + new EngineJobFactory( + diskCacheExecutor, sourceExecutor, sourceUnlimitedExecutor, animationExecutor, this); } this.engineJobFactory = engineJobFactory; @@ -155,6 +169,7 @@ public LoadStatus load( Options options, boolean isMemoryCacheable, boolean useUnlimitedSourceExecutorPool, + boolean useAnimationPool, boolean onlyRetrieveFromCache, ResourceCallback cb) { Util.assertMainThread(); @@ -191,7 +206,7 @@ public LoadStatus load( } EngineJob engineJob = engineJobFactory.build(key, isMemoryCacheable, - useUnlimitedSourceExecutorPool); + useUnlimitedSourceExecutorPool, useAnimationPool); DecodeJob decodeJob = decodeJobFactory.build( glideContext, model, @@ -453,29 +468,40 @@ static class EngineJobFactory { @Synthetic final GlideExecutor diskCacheExecutor; @Synthetic final GlideExecutor sourceExecutor; @Synthetic final GlideExecutor sourceUnlimitedExecutor; + @Synthetic final GlideExecutor animationExecutor; @Synthetic final EngineJobListener listener; @Synthetic final Pools.Pool> pool = FactoryPools.simple(JOB_POOL_SIZE, new FactoryPools.Factory>() { @Override public EngineJob create() { - return new EngineJob(diskCacheExecutor, sourceExecutor, sourceUnlimitedExecutor, - listener, pool); + return new EngineJob( + diskCacheExecutor, + sourceExecutor, + sourceUnlimitedExecutor, + animationExecutor, + listener, + pool); } }); - EngineJobFactory(GlideExecutor diskCacheExecutor, GlideExecutor sourceExecutor, - GlideExecutor sourceUnlimitedExecutor, EngineJobListener listener) { + EngineJobFactory( + GlideExecutor diskCacheExecutor, + GlideExecutor sourceExecutor, + GlideExecutor sourceUnlimitedExecutor, + GlideExecutor animationExecutor, + EngineJobListener listener) { this.diskCacheExecutor = diskCacheExecutor; this.sourceExecutor = sourceExecutor; this.sourceUnlimitedExecutor = sourceUnlimitedExecutor; + this.animationExecutor = animationExecutor; this.listener = listener; } @SuppressWarnings("unchecked") EngineJob build(Key key, boolean isMemoryCacheable, - boolean useUnlimitedSourceGeneratorPool) { + boolean useUnlimitedSourceGeneratorPool, boolean isAnimation) { EngineJob result = (EngineJob) pool.acquire(); - return result.init(key, isMemoryCacheable, useUnlimitedSourceGeneratorPool); + return result.init(key, isMemoryCacheable, useUnlimitedSourceGeneratorPool, isAnimation); } } } diff --git a/library/src/main/java/com/bumptech/glide/load/engine/EngineJob.java b/library/src/main/java/com/bumptech/glide/load/engine/EngineJob.java index 3517092aca..38957f2808 100644 --- a/library/src/main/java/com/bumptech/glide/load/engine/EngineJob.java +++ b/library/src/main/java/com/bumptech/glide/load/engine/EngineJob.java @@ -39,10 +39,12 @@ class EngineJob implements DecodeJob.Callback, private final GlideExecutor diskCacheExecutor; private final GlideExecutor sourceExecutor; private final GlideExecutor sourceUnlimitedExecutor; + private final GlideExecutor animationExecutor; private Key key; private boolean isCacheable; private boolean useUnlimitedSourceGeneratorPool; + private boolean isAnimation; private Resource resource; private DataSource dataSource; private boolean hasResource; @@ -57,31 +59,51 @@ class EngineJob implements DecodeJob.Callback, // Checked primarily on the main thread, but also on other threads in reschedule. private volatile boolean isCancelled; - EngineJob(GlideExecutor diskCacheExecutor, GlideExecutor sourceExecutor, + EngineJob( + GlideExecutor diskCacheExecutor, + GlideExecutor sourceExecutor, GlideExecutor sourceUnlimitedExecutor, - EngineJobListener listener, Pools.Pool> pool) { - this(diskCacheExecutor, sourceExecutor, sourceUnlimitedExecutor, listener, pool, + GlideExecutor animationExecutor, + EngineJobListener listener, + Pools.Pool> pool) { + this( + diskCacheExecutor, + sourceExecutor, + sourceUnlimitedExecutor, + animationExecutor, + listener, + pool, DEFAULT_FACTORY); } // Visible for testing. - EngineJob(GlideExecutor diskCacheExecutor, GlideExecutor sourceExecutor, + EngineJob( + GlideExecutor diskCacheExecutor, + GlideExecutor sourceExecutor, GlideExecutor sourceUnlimitedExecutor, - EngineJobListener listener, Pools.Pool> pool, + GlideExecutor animationExecutor, + EngineJobListener listener, + Pools.Pool> pool, EngineResourceFactory engineResourceFactory) { this.diskCacheExecutor = diskCacheExecutor; this.sourceExecutor = sourceExecutor; this.sourceUnlimitedExecutor = sourceUnlimitedExecutor; + this.animationExecutor = animationExecutor; this.listener = listener; this.pool = pool; this.engineResourceFactory = engineResourceFactory; } // Visible for testing. - EngineJob init(Key key, boolean isCacheable, boolean useUnlimitedSourceGeneratorPool) { + EngineJob init( + Key key, + boolean isCacheable, + boolean useUnlimitedSourceGeneratorPool, + boolean isAnimation) { this.key = key; this.isCacheable = isCacheable; this.useUnlimitedSourceGeneratorPool = useUnlimitedSourceGeneratorPool; + this.isAnimation = isAnimation; return this; } @@ -119,6 +141,9 @@ public void removeCallback(ResourceCallback cb) { } private GlideExecutor getActiveSourceExecutor() { + if (isAnimation) { + return animationExecutor; + } return useUnlimitedSourceGeneratorPool ? sourceUnlimitedExecutor : sourceExecutor; } diff --git a/library/src/main/java/com/bumptech/glide/load/engine/executor/GlideExecutor.java b/library/src/main/java/com/bumptech/glide/load/engine/executor/GlideExecutor.java index d80ff20598..b66014a041 100644 --- a/library/src/main/java/com/bumptech/glide/load/engine/executor/GlideExecutor.java +++ b/library/src/main/java/com/bumptech/glide/load/engine/executor/GlideExecutor.java @@ -44,6 +44,9 @@ public final class GlideExecutor extends ThreadPoolExecutor { private static final String CPU_LOCATION = "/sys/devices/system/cpu/"; // Don't use more than four threads when automatically determining thread count.. private static final int MAXIMUM_AUTOMATIC_THREAD_COUNT = 4; + // May be accessed on other threads, but this is an optimization only so it's ok if we set its + // value more than once. + private static volatile int bestThreadCount; private final boolean executeSynchronously; /** @@ -52,10 +55,11 @@ public final class GlideExecutor extends ThreadPoolExecutor { */ private static final String SOURCE_UNLIMITED_EXECUTOR_NAME = "source-unlimited"; /** - * The default keep alive time for threads in source unlimited executor pool in milliseconds. + * The default keep alive time for threads in our cached thread pools in milliseconds. */ - private static final long SOURCE_UNLIMITED_EXECUTOR_KEEP_ALIVE_TIME_MS = - TimeUnit.SECONDS.toMillis(10); + private static final long KEEP_ALIVE_TIME_MS = TimeUnit.SECONDS.toMillis(10); + + private static final String ANIMATION_EXECUTOR_NAME = "animation"; /** * Returns a new fixed thread pool with the default thread count returned from @@ -160,7 +164,7 @@ public static GlideExecutor newSourceExecutor(int threadCount, String name, /** * Returns a new unlimited thread pool with zero core thread count to make sure no threads are - * created by default, {@link #SOURCE_UNLIMITED_EXECUTOR_KEEP_ALIVE_TIME_MS} keep alive + * created by default, {@link #KEEP_ALIVE_TIME_MS} keep alive * time, the {@link #SOURCE_UNLIMITED_EXECUTOR_NAME} thread name prefix, the * {@link com.bumptech.glide.load.engine.executor.GlideExecutor.UncaughtThrowableStrategy#DEFAULT} * uncaught throwable strategy, and the {@link SynchronousQueue} since using default unbounded @@ -175,7 +179,7 @@ public static GlideExecutor newSourceExecutor(int threadCount, String name, public static GlideExecutor newUnlimitedSourceExecutor() { return new GlideExecutor(0 /* corePoolSize */, Integer.MAX_VALUE /* maximumPoolSize */, - SOURCE_UNLIMITED_EXECUTOR_KEEP_ALIVE_TIME_MS, + KEEP_ALIVE_TIME_MS, SOURCE_UNLIMITED_EXECUTOR_NAME, UncaughtThrowableStrategy.DEFAULT, false /*preventNetworkOperations*/, @@ -183,6 +187,25 @@ public static GlideExecutor newUnlimitedSourceExecutor() { new SynchronousQueue()); } + public static GlideExecutor newAnimationExecutor() { + int bestThreadCount = calculateBestThreadCount(); + // We don't want to add a ton of threads running animations in parallel with our source and + // disk cache executors. Doing so adds unnecessary CPU load and can also dramatically increase + // our maximum memory usage. Typically one thread is sufficient here, but for higher end devices + // with more cores, two threads can provide better performance if lots of GIFs are showing at + // once. + int maximumPoolSize = bestThreadCount >= 4 ? 2 : 1; + return new GlideExecutor( + /*corePoolSize=*/ 0, + maximumPoolSize, + KEEP_ALIVE_TIME_MS, + ANIMATION_EXECUTOR_NAME, + UncaughtThrowableStrategy.DEFAULT, + /*preventNetworkOperations=*/ true, + /*executeSynchronously=*/ false, + new PriorityBlockingQueue()); + } + // Visible for testing. GlideExecutor(int poolSize, String name, UncaughtThrowableStrategy uncaughtThrowableStrategy, boolean preventNetworkOperations, @@ -280,31 +303,35 @@ public Future submit(Callable task) { * http://goo.gl/8H670N. */ public static int calculateBestThreadCount() { - // We override the current ThreadPolicy to allow disk reads. - // This shouldn't actually do disk-IO and accesses a device file. - // See: https://github.com/bumptech/glide/issues/1170 - ThreadPolicy originalPolicy = StrictMode.allowThreadDiskReads(); - File[] cpus = null; - try { - File cpuInfo = new File(CPU_LOCATION); - final Pattern cpuNamePattern = Pattern.compile(CPU_NAME_REGEX); - cpus = cpuInfo.listFiles(new FilenameFilter() { - @Override - public boolean accept(File file, String s) { - return cpuNamePattern.matcher(s).matches(); + if (bestThreadCount == 0) { + // We override the current ThreadPolicy to allow disk reads. + // This shouldn't actually do disk-IO and accesses a device file. + // See: https://github.com/bumptech/glide/issues/1170 + ThreadPolicy originalPolicy = StrictMode.allowThreadDiskReads(); + File[] cpus = null; + try { + File cpuInfo = new File(CPU_LOCATION); + final Pattern cpuNamePattern = Pattern.compile(CPU_NAME_REGEX); + cpus = cpuInfo.listFiles(new FilenameFilter() { + @Override + public boolean accept(File file, String s) { + return cpuNamePattern.matcher(s).matches(); + } + }); + } catch (Throwable t) { + if (Log.isLoggable(TAG, Log.ERROR)) { + Log.e(TAG, "Failed to calculate accurate cpu count", t); } - }); - } catch (Throwable t) { - if (Log.isLoggable(TAG, Log.ERROR)) { - Log.e(TAG, "Failed to calculate accurate cpu count", t); + } finally { + StrictMode.setThreadPolicy(originalPolicy); } - } finally { - StrictMode.setThreadPolicy(originalPolicy); - } - int cpuCount = cpus != null ? cpus.length : 0; - int availableProcessors = Math.max(1, Runtime.getRuntime().availableProcessors()); - return Math.min(MAXIMUM_AUTOMATIC_THREAD_COUNT, Math.max(availableProcessors, cpuCount)); + int cpuCount = cpus != null ? cpus.length : 0; + int availableProcessors = Math.max(1, Runtime.getRuntime().availableProcessors()); + bestThreadCount = + Math.min(MAXIMUM_AUTOMATIC_THREAD_COUNT, Math.max(availableProcessors, cpuCount)); + } + return bestThreadCount; } /** diff --git a/library/src/main/java/com/bumptech/glide/load/resource/gif/ByteBufferGifDecoder.java b/library/src/main/java/com/bumptech/glide/load/resource/gif/ByteBufferGifDecoder.java index 2b56d271e1..bb04fc8357 100644 --- a/library/src/main/java/com/bumptech/glide/load/resource/gif/ByteBufferGifDecoder.java +++ b/library/src/main/java/com/bumptech/glide/load/resource/gif/ByteBufferGifDecoder.java @@ -131,7 +131,7 @@ private static int getSampleSize(GifHeader gifHeader, int targetWidth, int targe // Although functionally equivalent to 0 for BitmapFactory, 1 is a safer default for our code // than 0. int sampleSize = Math.max(1, powerOfTwoSampleSize); - if (Log.isLoggable(TAG, Log.VERBOSE)) { + if (Log.isLoggable(TAG, Log.VERBOSE) && sampleSize > 1) { Log.v(TAG, "Downsampling GIF" + ", sampleSize: " + sampleSize + ", target dimens: [" + targetWidth + "x" + targetHeight + "]" diff --git a/library/src/main/java/com/bumptech/glide/load/resource/gif/GifFrameLoader.java b/library/src/main/java/com/bumptech/glide/load/resource/gif/GifFrameLoader.java index 9c51ba3edb..bda033ce9b 100644 --- a/library/src/main/java/com/bumptech/glide/load/resource/gif/GifFrameLoader.java +++ b/library/src/main/java/com/bumptech/glide/load/resource/gif/GifFrameLoader.java @@ -205,7 +205,7 @@ private void loadNextFrame() { gifDecoder.advance(); next = new DelayTarget(handler, gifDecoder.getCurrentFrameIndex(), targetTime); - requestBuilder.clone().apply(signatureOf(getFrameSignature())).load(gifDecoder).into(next); + requestBuilder.apply(signatureOf(getFrameSignature())).load(gifDecoder).into(next); } private void recycleFirstFrame() { @@ -298,11 +298,12 @@ private static RequestBuilder getRequestBuilder( .asBitmap() .apply( diskCacheStrategyOf(DiskCacheStrategy.NONE) + .useAnimationPool(true) .skipMemoryCache(true) .override(width, height)); } - static Key getFrameSignature() { + private static Key getFrameSignature() { // Some devices seem to have crypto bugs that throw exceptions when you create a new UUID. // See #1510. return new ObjectKey(Math.random()); diff --git a/library/src/main/java/com/bumptech/glide/request/RequestOptions.java b/library/src/main/java/com/bumptech/glide/request/RequestOptions.java index 6ad5e0d50d..277a7b1001 100644 --- a/library/src/main/java/com/bumptech/glide/request/RequestOptions.java +++ b/library/src/main/java/com/bumptech/glide/request/RequestOptions.java @@ -63,6 +63,7 @@ public class RequestOptions implements Cloneable { private static final int TRANSFORMATION_REQUIRED = 1 << 17; private static final int USE_UNLIMITED_SOURCE_GENERATORS_POOL = 1 << 18; private static final int ONLY_RETRIEVE_FROM_CACHE = 1 << 19; + private static final int USE_ANIMATION_POOL = 1 << 20; @Nullable private static RequestOptions skipMemoryCacheTrueOptions; @@ -116,6 +117,7 @@ public class RequestOptions implements Cloneable { private boolean useUnlimitedSourceGeneratorsPool; private boolean onlyRetrieveFromCache; private boolean isScaleOnlyOrNoTransform = true; + private boolean useAnimationPool; /** * Returns a {@link RequestOptions} object with {@link #sizeMultiplier(float)} set. @@ -417,6 +419,15 @@ public RequestOptions sizeMultiplier(@FloatRange(from = 0, to = 1) float sizeMul return selfOrThrowIfLocked(); } + /** + * If set to {@code true}, uses a cached unlimited {@link java.util.concurrent.Executor} to run + * the request. + * + *

This method should ONLY be used when a Glide load is started recursively on one + * of Glide's threads as part of another request. Using this method in other scenarios can lead + * to excessive memory usage and OOMs and/or a significant decrease in performance across an + * application. + */ @CheckResult public RequestOptions useUnlimitedSourceGeneratorsPool(boolean flag) { if (isAutoCloneEnabled) { @@ -429,6 +440,27 @@ public RequestOptions useUnlimitedSourceGeneratorsPool(boolean flag) { return selfOrThrowIfLocked(); } + /** + * If set to {@code true}, uses a special {@link java.util.concurrent.Executor} that is used + * exclusively for decoding frames of animated resources, like GIFs. + * + *

The animation executor disallows network operations and must not be used for loads that + * may load remote data. The animation executor has fewer threads available to it than Glide's + * normal executors and is only useful as a way of avoiding blocking on longer and more expensive + * reads for critical requests like those in an animating GIF. + */ + @CheckResult + public RequestOptions useAnimationPool(boolean flag) { + if (isAutoCloneEnabled) { + return clone().useAnimationPool(flag); + } + + useAnimationPool = flag; + fields |= USE_ANIMATION_POOL; + + return selfOrThrowIfLocked(); + } + /** * If set to true, will only load an item if found in the cache, and will not fetch from source. */ @@ -1233,6 +1265,9 @@ public RequestOptions apply(@NonNull RequestOptions other) { if (isSet(other.fields, USE_UNLIMITED_SOURCE_GENERATORS_POOL)) { useUnlimitedSourceGeneratorsPool = other.useUnlimitedSourceGeneratorsPool; } + if (isSet(other.fields, USE_ANIMATION_POOL)) { + useAnimationPool = other.useAnimationPool; + } if (isSet(other.fields, DISK_CACHE_STRATEGY)) { diskCacheStrategy = other.diskCacheStrategy; } @@ -1509,6 +1544,10 @@ public final boolean getUseUnlimitedSourceGeneratorsPool() { return useUnlimitedSourceGeneratorsPool; } + public final boolean getUseAnimationPool() { + return useAnimationPool; + } + public final boolean getOnlyRetrieveFromCache() { return onlyRetrieveFromCache; } diff --git a/library/src/main/java/com/bumptech/glide/request/SingleRequest.java b/library/src/main/java/com/bumptech/glide/request/SingleRequest.java index 2d79513513..e543cc66b9 100644 --- a/library/src/main/java/com/bumptech/glide/request/SingleRequest.java +++ b/library/src/main/java/com/bumptech/glide/request/SingleRequest.java @@ -439,6 +439,7 @@ public void onSizeReady(int width, int height) { requestOptions.getOptions(), requestOptions.isMemoryCacheable(), requestOptions.getUseUnlimitedSourceGeneratorsPool(), + requestOptions.getUseAnimationPool(), requestOptions.getOnlyRetrieveFromCache(), this); if (Log.isLoggable(TAG, Log.VERBOSE)) { diff --git a/library/src/test/java/com/bumptech/glide/load/engine/EngineJobTest.java b/library/src/test/java/com/bumptech/glide/load/engine/EngineJobTest.java index 6a740e6387..de3766e317 100644 --- a/library/src/test/java/com/bumptech/glide/load/engine/EngineJobTest.java +++ b/library/src/test/java/com/bumptech/glide/load/engine/EngineJobTest.java @@ -459,6 +459,7 @@ private static class MultiCbHarness { EngineJobListener listener = mock(EngineJobListener.class); boolean isCacheable = true; boolean useUnlimitedSourceGeneratorPool = false; + boolean isAnimation = false; int numCbs = 10; List cbs = new ArrayList<>(); EngineJob.EngineResourceFactory factory = mock(EngineJob.EngineResourceFactory.class); @@ -466,14 +467,22 @@ private static class MultiCbHarness { GlideExecutor diskCacheService = MockGlideExecutor.newMainThreadExecutor(); GlideExecutor sourceService = MockGlideExecutor.newMainThreadExecutor(); GlideExecutor sourceUnlimitedService = MockGlideExecutor.newMainThreadUnlimitedExecutor(); + GlideExecutor animationService = MockGlideExecutor.newMainThreadUnlimitedExecutor(); Pools.Pool> pool = new Pools.SimplePool<>(1); DecodeJob decodeJob = mock(DecodeJob.class); DataSource dataSource = DataSource.LOCAL; public MultiCbHarness() { when(factory.build(eq(resource), eq(isCacheable))).thenReturn(engineResource); - job = new EngineJob<>(diskCacheService, sourceService, sourceUnlimitedService, listener, pool, - factory).init(key, isCacheable, useUnlimitedSourceGeneratorPool); + job = + new EngineJob<>( + diskCacheService, + sourceService, + sourceUnlimitedService, + animationService, + listener, + pool, + factory).init(key, isCacheable, useUnlimitedSourceGeneratorPool, isAnimation); for (int i = 0; i < numCbs; i++) { cbs.add(mock(ResourceCallback.class)); } @@ -495,17 +504,26 @@ private static class EngineJobHarness { GlideExecutor diskCacheService = MockGlideExecutor.newMainThreadExecutor(); GlideExecutor sourceService = MockGlideExecutor.newMainThreadExecutor(); GlideExecutor sourceUnlimitedService = MockGlideExecutor.newMainThreadUnlimitedExecutor(); + GlideExecutor animationService = MockGlideExecutor.newMainThreadUnlimitedExecutor(); boolean isCacheable = true; boolean useUnlimitedSourceGeneratorPool = false; + boolean isAnimation = false; DecodeJob decodeJob = mock(DecodeJob.class); Pools.Pool> pool = new Pools.SimplePool<>(1); DataSource dataSource = DataSource.DATA_DISK_CACHE; public EngineJob getJob() { when(factory.build(eq(resource), eq(isCacheable))).thenReturn(engineResource); - EngineJob result = new EngineJob<>( - diskCacheService, sourceService, sourceUnlimitedService, listener, pool, factory) - .init(key, isCacheable, useUnlimitedSourceGeneratorPool); + EngineJob result = + new EngineJob<>( + diskCacheService, + sourceService, + sourceUnlimitedService, + animationService, + listener, + pool, + factory) + .init(key, isCacheable, useUnlimitedSourceGeneratorPool, isAnimation); result.addCallback(cb); return result; } diff --git a/library/src/test/java/com/bumptech/glide/load/engine/EngineTest.java b/library/src/test/java/com/bumptech/glide/load/engine/EngineTest.java index f442d96840..b226e3aa3a 100644 --- a/library/src/test/java/com/bumptech/glide/load/engine/EngineTest.java +++ b/library/src/test/java/com/bumptech/glide/load/engine/EngineTest.java @@ -422,7 +422,8 @@ public void testFactoryIsGivenNecessaryArguments() { verify(harness.engineJobFactory).build( eq(harness.cacheKey), eq(true) /*isMemoryCacheable*/, - eq(false) /*useUnlimitedSourceGeneratorPool*/); + eq(false) /*useUnlimitedSourceGeneratorPool*/, + /*isAnimation=*/ eq(false)); } @Test @@ -433,7 +434,8 @@ public void testFactoryIsGivenNecessaryArgumentsWithUnlimitedPool() { verify(harness.engineJobFactory).build( eq(harness.cacheKey), eq(true) /*isMemoryCacheable*/, - eq(true) /*useUnlimitedSourceGeneratorPool*/); + eq(true) /*useUnlimitedSourceGeneratorPool*/, + /*isAnimation=*/ eq(false)); } @Test @@ -492,16 +494,24 @@ public EngineTestHarness() { job = mock(EngineJob.class); - engine = new Engine(cache, mock(DiskCache.Factory.class), - GlideExecutor.newDiskCacheExecutor(), - MockGlideExecutor.newMainThreadExecutor(), - MockGlideExecutor.newMainThreadUnlimitedExecutor(), - jobs, keyFactory, activeResources, - engineJobFactory, decodeJobFactory, resourceRecycler); + engine = + new Engine( + cache, + mock(DiskCache.Factory.class), + GlideExecutor.newDiskCacheExecutor(), + MockGlideExecutor.newMainThreadExecutor(), + MockGlideExecutor.newMainThreadUnlimitedExecutor(), + MockGlideExecutor.newMainThreadUnlimitedExecutor(), + jobs, + keyFactory, + activeResources, + engineJobFactory, + decodeJobFactory, + resourceRecycler); } public Engine.LoadStatus doLoad() { - when(engineJobFactory.build(eq(cacheKey), anyBoolean(), anyBoolean())) + when(engineJobFactory.build(eq(cacheKey), anyBoolean(), anyBoolean(), anyBoolean())) .thenReturn((EngineJob) job); return engine.load(glideContext, model, @@ -514,10 +524,11 @@ public Engine.LoadStatus doLoad() { DiskCacheStrategy.ALL, transformations, false /*isTransformationRequired*/, - true, + isScaleOnlyOrNoTransform, options, isMemoryCacheable, useUnlimitedSourceGeneratorPool, + /*useAnimationPool=*/ false, onlyRetrieveFromCache, cb); } diff --git a/library/src/test/java/com/bumptech/glide/request/SingleRequestTest.java b/library/src/test/java/com/bumptech/glide/request/SingleRequestTest.java index fc06b4087f..aabefcffa7 100644 --- a/library/src/test/java/com/bumptech/glide/request/SingleRequestTest.java +++ b/library/src/test/java/com/bumptech/glide/request/SingleRequestTest.java @@ -277,10 +277,25 @@ public void testIgnoresOnSizeReadyIfNotWaitingForSize() { request.onSizeReady(100, 100); verify(harness.engine, times(1)) - .load(eq(harness.glideContext), eq(harness.model), eq(harness.signature), eq(100), eq(100), - eq(Object.class), eq(List.class), any(Priority.class), any(DiskCacheStrategy.class), - eq(harness.transformations), anyBoolean(), anyBoolean(), any(Options.class), - anyBoolean(), anyBoolean(), anyBoolean(), any(ResourceCallback.class)); + .load( + eq(harness.glideContext), + eq(harness.model), + eq(harness.signature), + eq(100), + eq(100), + eq(Object.class), + eq(List.class), + any(Priority.class), + any(DiskCacheStrategy.class), + eq(harness.transformations), + anyBoolean(), + anyBoolean(), + any(Options.class), + anyBoolean(), + /*useAnimationPool=*/ anyBoolean(), + anyBoolean(), + anyBoolean(), + any(ResourceCallback.class)); } @Test @@ -296,10 +311,25 @@ public void testEngineLoadCancelledOnCancel() { Engine.LoadStatus loadStatus = mock(Engine.LoadStatus.class); when(harness.engine - .load(eq(harness.glideContext), eq(harness.model), eq(harness.signature), anyInt(), anyInt(), - eq(Object.class), eq(List.class), any(Priority.class), any(DiskCacheStrategy.class), - eq(harness.transformations), anyBoolean(), anyBoolean(), any(Options.class), - anyBoolean(), anyBoolean(), anyBoolean(), any(ResourceCallback.class))) + .load( + eq(harness.glideContext), + eq(harness.model), + eq(harness.signature), + anyInt(), + anyInt(), + eq(Object.class), + eq(List.class), + any(Priority.class), + any(DiskCacheStrategy.class), + eq(harness.transformations), + anyBoolean(), + anyBoolean(), + any(Options.class), + anyBoolean(), + anyBoolean(), + anyBoolean(), + anyBoolean(), + any(ResourceCallback.class))) .thenReturn(loadStatus); SingleRequest request = harness.getRequest(); @@ -540,10 +570,24 @@ public void testRequestListenerIsCalledWithLoadedFromMemoryIfLoadCompletesSynchr final SingleRequest request = harness.getRequest(); when(harness.engine - .load(eq(harness.glideContext), eq(harness.model), eq(harness.signature), anyInt(), - anyInt(), eq(Object.class), eq(List.class), any(Priority.class), - any(DiskCacheStrategy.class), eq(harness.transformations), anyBoolean(), - anyBoolean(), any(Options.class), anyBoolean(), anyBoolean(), anyBoolean(), + .load( + eq(harness.glideContext), + eq(harness.model), + eq(harness.signature), + anyInt(), + anyInt(), + eq(Object.class), + eq(List.class), + any(Priority.class), + any(DiskCacheStrategy.class), + eq(harness.transformations), + anyBoolean(), + anyBoolean(), + any(Options.class), + anyBoolean(), + anyBoolean(), + /*useAnimationPool=*/ anyBoolean(), + anyBoolean(), any(ResourceCallback.class))) .thenAnswer(new Answer() { @Override @@ -655,10 +699,24 @@ public void testCallsEngineWithOverrideWidthAndHeightIfSet() { request.begin(); verify(harness.engine) - .load(eq(harness.glideContext), eq(harness.model), eq(harness.signature), anyInt(), - anyInt(), eq(Object.class), eq(List.class), any(Priority.class), - any(DiskCacheStrategy.class), eq(harness.transformations), anyBoolean(), - anyBoolean(), any(Options.class), anyBoolean(), anyBoolean(), anyBoolean(), + .load( + eq(harness.glideContext), + eq(harness.model), + eq(harness.signature), + anyInt(), + anyInt(), + eq(Object.class), + eq(List.class), + any(Priority.class), + any(DiskCacheStrategy.class), + eq(harness.transformations), + anyBoolean(), + anyBoolean(), + any(Options.class), + anyBoolean(), + anyBoolean(), + /*useAnimationPool*/ anyBoolean(), + anyBoolean(), any(ResourceCallback.class)); } @@ -678,10 +736,25 @@ public void testCanReRunCancelledRequests() { .getSize(any(SizeReadyCallback.class)); when(harness.engine - .load(eq(harness.glideContext), eq(harness.model), eq(harness.signature), eq(100), eq(100), - eq(Object.class), eq(List.class), any(Priority.class), any(DiskCacheStrategy.class), - eq(harness.transformations), anyBoolean(), anyBoolean(), any(Options.class), - anyBoolean(), anyBoolean(), anyBoolean(), any(ResourceCallback.class))) + .load( + eq(harness.glideContext), + eq(harness.model), + eq(harness.signature), + eq(100), + eq(100), + eq(Object.class), + eq(List.class), + any(Priority.class), + any(DiskCacheStrategy.class), + eq(harness.transformations), + anyBoolean(), + anyBoolean(), + any(Options.class), + anyBoolean(), + anyBoolean(), + /*useAnimationPool=*/ anyBoolean(), + anyBoolean(), + any(ResourceCallback.class))) .thenAnswer(new CallResourceCallback(harness.resource)); SingleRequest request = harness.getRequest(); @@ -707,10 +780,24 @@ public void testDoesNotStartALoadIfOnSizeReadyIsCalledAfterCancel() { request.onSizeReady(100, 100); verify(harness.engine, never()) - .load(eq(harness.glideContext), eq(harness.model), eq(harness.signature), anyInt(), - anyInt(), eq(Object.class), eq(List.class), any(Priority.class), - any(DiskCacheStrategy.class), eq(harness.transformations), anyBoolean(), - anyBoolean(), any(Options.class), anyBoolean(), anyBoolean(), anyBoolean(), + .load( + eq(harness.glideContext), + eq(harness.model), + eq(harness.signature), + anyInt(), + anyInt(), + eq(Object.class), + eq(List.class), + any(Priority.class), + any(DiskCacheStrategy.class), + eq(harness.transformations), + anyBoolean(), + anyBoolean(), + any(Options.class), + anyBoolean(), + anyBoolean(), + /*useAnimationPool=*/ anyBoolean(), + anyBoolean(), any(ResourceCallback.class)); } @@ -726,10 +813,24 @@ public void testCallsSourceUnlimitedExecutorEngineIfOptionsIsSet() { request.begin(); verify(harness.engine) - .load(eq(harness.glideContext), eq(harness.model), eq(harness.signature), anyInt(), - anyInt(), eq(Object.class), eq(List.class), any(Priority.class), - any(DiskCacheStrategy.class), eq(harness.transformations), anyBoolean(), - anyBoolean(), any(Options.class), anyBoolean(), eq(Boolean.TRUE), anyBoolean(), + .load( + eq(harness.glideContext), + eq(harness.model), + eq(harness.signature), + anyInt(), + anyInt(), + eq(Object.class), + eq(List.class), + any(Priority.class), + any(DiskCacheStrategy.class), + eq(harness.transformations), + anyBoolean(), + anyBoolean(), + any(Options.class), + anyBoolean(), + eq(true), + /*useAnimationPool=*/ anyBoolean(), + anyBoolean(), any(ResourceCallback.class)); } @@ -744,10 +845,24 @@ public void testCallsSourceExecutorEngineIfOptionsIsSet() { request.begin(); verify(harness.engine) - .load(eq(harness.glideContext), eq(harness.model), eq(harness.signature), anyInt(), - anyInt(), eq(Object.class), eq(List.class), any(Priority.class), - any(DiskCacheStrategy.class), eq(harness.transformations), anyBoolean(), - anyBoolean(), any(Options.class), anyBoolean(), eq(Boolean.FALSE), anyBoolean(), + .load( + eq(harness.glideContext), + eq(harness.model), + eq(harness.signature), + anyInt(), + anyInt(), + eq(Object.class), + eq(List.class), + any(Priority.class), + any(DiskCacheStrategy.class), + eq(harness.transformations), + anyBoolean(), + anyBoolean(), + any(Options.class), + anyBoolean(), + eq(false), + /*useAnimationPool=*/ anyBoolean(), + anyBoolean(), any(ResourceCallback.class)); }