Override placeholder/error/fallback equivalents when id or drawable i…

…s set. Glide will always check for a placeholder/error/fallback drawable first and only check the id equivalents if the drawable is null. Previously, when setting a placeholder/errror/fallback id on a RequestOptiosn object, we did not unset the drawable equivalents. As a result, it wasn't previously possible to replace a placeholder/error/fallback drawable by setting the equivalent id. After this change any calls to placeholder/error/fallback will always replace previous calls, regardless of whether and id or drawable is provided. Fixes #3068. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=195990546
bumptech · May 21, 2018 · 2dccb5c · 2dccb5c
1 parent 5ce19a6
commit 2dccb5c
Show file tree

Hide file tree

Showing 2 changed files with 357 additions and 0 deletions.
diff --git a/library/src/main/java/com/bumptech/glide/request/RequestOptions.java b/library/src/main/java/com/bumptech/glide/request/RequestOptions.java
@@ -561,6 +561,8 @@ public RequestOptions priority(@NonNull Priority priority) {
   /**
    * Sets an {@link Drawable} to display while a resource is loading.
    *
+   * <p>Replaces any previous calls to this method or {@link #placeholder(int)}.
+   *
    * @param drawable The drawable to display as a placeholder.
    * @return This request builder.
    */
@@ -574,13 +576,18 @@ public RequestOptions placeholder(@Nullable Drawable drawable) {
     this.placeholderDrawable = drawable;
     fields |= PLACEHOLDER;
 
+    placeholderId = 0;
+    fields &= ~PLACEHOLDER_ID;
+
     return selfOrThrowIfLocked();
   }
 
   /**
    * Sets an Android resource id for a {@link Drawable} resource to
    * display while a resource is loading.
    *
+   * <p>Replaces any previous calls to this method or {@link #placeholder(Drawable)}
+   *
    * @param resourceId The id of the resource to use as a placeholder
    * @return This request builder.
    */
@@ -594,6 +601,9 @@ public RequestOptions placeholder(@DrawableRes int resourceId) {
     this.placeholderId = resourceId;
     fields |= PLACEHOLDER_ID;
 
+    placeholderDrawable = null;
+    fields &= ~PLACEHOLDER;
+
     return selfOrThrowIfLocked();
   }
 
@@ -604,6 +614,8 @@ public RequestOptions placeholder(@DrawableRes int resourceId) {
    * <p> If a fallback is not set, null models will cause the error drawable to be displayed. If the
    * error drawable is not set, the placeholder will be displayed.
    *
+   * <p>Replaces any previous calls to this method or {@link #fallback(int)}.
+   *
    * @see #placeholder(Drawable)
    * @see #placeholder(int)
    *
@@ -620,6 +632,9 @@ public RequestOptions fallback(@Nullable Drawable drawable) {
     this.fallbackDrawable = drawable;
     fields |= FALLBACK;
 
+    fallbackId = 0;
+    fields &= ~FALLBACK_ID;
+
     return selfOrThrowIfLocked();
   }
 
@@ -630,6 +645,8 @@ public RequestOptions fallback(@Nullable Drawable drawable) {
    * <p> If a fallback is not set, null models will cause the error drawable to be displayed. If
    * the error drawable is not set, the placeholder will be displayed.
    *
+   * <p>Replaces any previous calls to this method or {@link #fallback(Drawable)}.
+   *
    * @see #placeholder(Drawable)
    * @see #placeholder(int)
    *
@@ -646,12 +663,17 @@ public RequestOptions fallback(@DrawableRes int resourceId) {
     this.fallbackId = resourceId;
     fields |= FALLBACK_ID;
 
+    fallbackDrawable = null;
+    fields &= ~FALLBACK;
+
     return selfOrThrowIfLocked();
   }
 
   /**
    * Sets a {@link Drawable} to display if a load fails.
    *
+   * <p>Replaces any previous calls to this method or {@link #error(int)}
+   *
    * @param drawable The drawable to display.
    * @return This request builder.
    */
@@ -665,12 +687,17 @@ public RequestOptions error(@Nullable Drawable drawable) {
     this.errorPlaceholder = drawable;
     fields |= ERROR_PLACEHOLDER;
 
+    this.errorId = 0;
+    fields &= ~ERROR_ID;
+
     return selfOrThrowIfLocked();
   }
 
   /**
    * Sets a resource to display if a load fails.
    *
+   * <p>Replaces any previous calls to this method or {@link #error(Drawable)}
+   *
    * @param resourceId The id of the resource to use as a placeholder.
    * @return This request builder.
    */
@@ -683,6 +710,9 @@ public RequestOptions error(@DrawableRes int resourceId) {
     this.errorId = resourceId;
     fields |= ERROR_ID;
 
+    this.errorPlaceholder = null;
+    fields &= ~ERROR_PLACEHOLDER;
+
     return selfOrThrowIfLocked();
   }
 
@@ -1391,15 +1421,23 @@ public RequestOptions apply(@NonNull RequestOptions other) {
     }
     if (isSet(other.fields, ERROR_PLACEHOLDER)) {
       errorPlaceholder = other.errorPlaceholder;
+      errorId = 0;
+      fields &= ~ERROR_ID;
     }
     if (isSet(other.fields, ERROR_ID)) {
       errorId = other.errorId;
+      errorPlaceholder = null;
+      fields &= ~ERROR_PLACEHOLDER;
     }
     if (isSet(other.fields, PLACEHOLDER)) {
       placeholderDrawable = other.placeholderDrawable;
+      placeholderId = 0;
+      fields &= ~PLACEHOLDER_ID;
     }
     if (isSet(other.fields, PLACEHOLDER_ID)) {
       placeholderId = other.placeholderId;
+      placeholderDrawable = null;
+      fields &= ~PLACEHOLDER;
     }
     if (isSet(other.fields, IS_CACHEABLE)) {
       isCacheable = other.isCacheable;
@@ -1416,9 +1454,13 @@ public RequestOptions apply(@NonNull RequestOptions other) {
     }
     if (isSet(other.fields, FALLBACK)) {
       fallbackDrawable = other.fallbackDrawable;
+      fallbackId = 0;
+      fields &= ~FALLBACK_ID;
     }
     if (isSet(other.fields, FALLBACK_ID)) {
       fallbackId = other.fallbackId;
+      fallbackDrawable = null;
+      fields &= ~FALLBACK;
     }
     if (isSet(other.fields, THEME)) {
       theme = other.theme;