Skip to content

Commit

Permalink
Address review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
@sumita12
sumita12 committed Dec 18, 2023
1 parent 6d23472 commit b48b690
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 36 deletions.
31 changes: 16 additions & 15 deletions library/src/main/java/com/bumptech/glide/load/resource/gif/GifDrawable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ public class GifDrawable extends Drawable

private static final int GRAVITY = Gravity.FILL;

private static final PorterDuff.Mode DEFAULT_TINT_MODE = PorterDuff.Mode.SRC_IN;

private final GifState state;
/** True if the drawable is currently animating. */
private boolean isRunning;
Expand All @@ -70,8 +72,9 @@ public class GifDrawable extends Drawable

private boolean applyGravity;
private Paint paint;
private ColorFilter colorFilter;
private ColorStateList tint;
private PorterDuff.Mode tintMode;
private PorterDuff.Mode tintMode = DEFAULT_TINT_MODE;
private ColorFilter tintFilter;
private Rect destRect;

Expand Down Expand Up @@ -296,16 +299,15 @@ public void draw(@NonNull Canvas canvas) {

Bitmap currentFrame = state.frameLoader.getCurrentFrame();
Paint paint = getPaint();
ColorFilter colorFilter = paint.getColorFilter();
if (colorFilter != null || tintFilter == null) {
if (colorFilter != null) {
// ColorFilter disables tint list. See Drawable#setColorFilter().
canvas.drawBitmap(currentFrame, null, getDestRect(), paint);
} else {
// Temporary set a tint filter then restore.
paint.setColorFilter(tintFilter);
canvas.drawBitmap(currentFrame, null, getDestRect(), paint);
paint.setColorFilter(colorFilter);
} else if (tintFilter != null) {
paint.setColorFilter(tintFilter);
} else {
paint.setColorFilter(null);
}
canvas.drawBitmap(currentFrame, null, getDestRect(), paint);
}

@Override
Expand All @@ -315,10 +317,8 @@ public void setAlpha(int i) {

@Override
public void setColorFilter(ColorFilter colorFilter) {
if (getColorFilter() != colorFilter) {
getPaint().setColorFilter(colorFilter);
invalidateSelf();
}
this.colorFilter = colorFilter;
invalidateSelf();
}

@Override
Expand All @@ -342,17 +342,18 @@ public void setTintMode(PorterDuff.Mode tintMode) {

@Override
protected boolean onStateChange(int[] stateSet) {
if (tint != null && tintMode != null) {
if (tint != null) {
updateTintFilter();
return true;
}
return false;
}

private void updateTintFilter() {
if (tint != null && tintMode != null) {
if (tint != null) {
int color = tint.getColorForState(getState(), Color.TRANSPARENT);
tintFilter = new PorterDuffColorFilter(color, tintMode);
PorterDuff.Mode mode = tintMode != null ? tintMode : DEFAULT_TINT_MODE;
tintFilter = new PorterDuffColorFilter(color, mode);
} else {
tintFilter = null;
}
Expand Down
101 changes: 80 additions & 21 deletions library/test/src/test/java/com/bumptech/glide/load/resource/gif/GifDrawableTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.robolectric.RobolectricTestRunner;
Expand Down Expand Up @@ -567,55 +568,113 @@ public void testSetAlphaSetsAlphaOnPaint() {

@Test
public void testSetColorFilterSetsColorFilterOnPaint() {
// Use a real Paint object, as this test depends on Paint#(get|set)ColorFilter.
drawable = new GifDrawable(frameLoader, new Paint());
ColorFilter colorFilter = new PorterDuffColorFilter(Color.RED, Mode.ADD);
drawable.setColorFilter(colorFilter);
verify(paint).setColorFilter(eq(colorFilter));
Canvas canvas = mock(Canvas.class);
drawable.draw(canvas);

ArgumentCaptor<Paint> captor = ArgumentCaptor.forClass(Paint.class);
verify(canvas).drawBitmap(isA(Bitmap.class), isNull(), isA(Rect.class), captor.capture());
assertThat(captor.getValue().getColorFilter()).isEqualTo(colorFilter);
}

@Config(sdk = Build.VERSION_CODES.LOLLIPOP)
@Test
public void testDrawSetsTintListColorFilterOnPaint() {
public void testSetTintListSetsColorFilterOnPaint() {
// Use a real Paint object, as this test depends on Paint#(get|set)ColorFilter.
Paint paint = new Paint();
drawable = new GifDrawable(frameLoader, paint);
ColorStateList tint =
new ColorStateList(
new int[][] {new int[] {android.R.attr.state_pressed}, new int[0]},
new int[] {Color.RED, Color.GREEN});
drawable.setTintList(tint);
drawable.setTintMode(Mode.ADD);
when(paint.getColorFilter()).thenReturn(null);
drawable.draw(new Canvas());
Canvas canvas = mock(Canvas.class);
drawable.draw(canvas);

// draw() temporary sets tint filter then restore.
verify(paint).setColorFilter(eq(new PorterDuffColorFilter(Color.GREEN, Mode.ADD)));
verify(paint).setColorFilter(null);
ArgumentCaptor<Paint> captor = ArgumentCaptor.forClass(Paint.class);
verify(canvas).drawBitmap(isA(Bitmap.class), isNull(), isA(Rect.class), captor.capture());
assertThat(captor.getValue().getColorFilter()).isEqualTo(new PorterDuffColorFilter(Color.GREEN, Mode.SRC_IN));
}

@Config(sdk = Build.VERSION_CODES.LOLLIPOP)
@Test
public void testSetTintListSetsColorFilterForPressedStateOnPaint() {
// Use a real Paint object, as this test depends on Paint#(get|set)ColorFilter.
drawable = new GifDrawable(frameLoader, new Paint());
ColorStateList tint =
new ColorStateList(
new int[][] {new int[] {android.R.attr.state_pressed}, new int[0]},
new int[] {Color.RED, Color.GREEN});
drawable.setTintList(tint);
assertThat(drawable.setState(new int[] {android.R.attr.state_pressed})).isTrue();
drawable.draw(new Canvas());
Canvas canvas = mock(Canvas.class);
drawable.draw(canvas);

// Pressed state. draw() temporary sets a red color filter.
verify(paint).setColorFilter(eq(new PorterDuffColorFilter(Color.RED, Mode.ADD)));
verify(paint, times(2)).setColorFilter(null);
ArgumentCaptor<Paint> captor = ArgumentCaptor.forClass(Paint.class);
verify(canvas).drawBitmap(isA(Bitmap.class), isNull(), isA(Rect.class), captor.capture());
assertThat(captor.getValue().getColorFilter()).isEqualTo(new PorterDuffColorFilter(Color.RED, Mode.SRC_IN));
}

@Config(sdk = Build.VERSION_CODES.LOLLIPOP)
@Test
public void testDrawUsesColorFilterInsteadOfTintList() {
public void testSetTintModeSetsColorFilterOnPaint() {
// Use a real Paint object, as this test depends on Paint#(get|set)ColorFilter.
drawable = new GifDrawable(frameLoader, new Paint());
ColorStateList tint =
new ColorStateList(
new int[][] {new int[] {android.R.attr.state_pressed}, new int[0]},
new int[] {Color.RED, Color.GREEN});
drawable.setTintList(tint);
drawable.setTintMode(Mode.ADD);
Canvas canvas = mock(Canvas.class);
drawable.draw(canvas);

ArgumentCaptor<Paint> captor = ArgumentCaptor.forClass(Paint.class);
verify(canvas).drawBitmap(isA(Bitmap.class), isNull(), isA(Rect.class), captor.capture());
assertThat(captor.getValue().getColorFilter()).isEqualTo(new PorterDuffColorFilter(Color.GREEN, Mode.ADD));
}

@Config(sdk = Build.VERSION_CODES.LOLLIPOP)
@Test
public void testNullTintModeFallsBackToDefaultTintMode() {
// Use a real Paint object, as this test depends on Paint#(get|set)ColorFilter.
drawable = new GifDrawable(frameLoader, new Paint());
ColorStateList tint =
new ColorStateList(
new int[][] {new int[] {android.R.attr.state_pressed}, new int[0]},
new int[] {Color.RED, Color.GREEN});
drawable.setTintList(tint);
drawable.setTintMode(null);
Canvas canvas = mock(Canvas.class);
drawable.draw(canvas);

ArgumentCaptor<Paint> captor = ArgumentCaptor.forClass(Paint.class);
verify(canvas).drawBitmap(isA(Bitmap.class), isNull(), isA(Rect.class), captor.capture());
assertThat(captor.getValue().getColorFilter()).isEqualTo(new PorterDuffColorFilter(Color.GREEN, Mode.SRC_IN));
}

@Config(sdk = Build.VERSION_CODES.LOLLIPOP)
@Test
public void testSetColorFilterIsPrioritizedThanSetTintList() {
// Use a real Paint object, as this test depends on Paint#(get|set)ColorFilter.
drawable = new GifDrawable(frameLoader, new Paint());
ColorFilter colorFilter = new PorterDuffColorFilter(Color.BLUE, Mode.ADD);
drawable.setColorFilter(colorFilter);
verify(paint).setColorFilter(eq(colorFilter));
when(paint.getColorFilter()).thenReturn(colorFilter);

drawable.draw(new Canvas());
drawable.onStateChange(new int[] {android.R.attr.state_pressed});
drawable.draw(new Canvas());
ColorStateList tint =
new ColorStateList(
new int[][] {new int[] {android.R.attr.state_pressed}, new int[0]},
new int[] {Color.RED, Color.GREEN});
drawable.setTintList(tint);
drawable.setTintMode(Mode.ADD);
Canvas canvas = mock(Canvas.class);
drawable.draw(canvas);

// ColorFilter disables tint list, so draw() should not invoke setColorFilter() any more.
verify(paint).setColorFilter(any());
ArgumentCaptor<Paint> captor = ArgumentCaptor.forClass(Paint.class);
verify(canvas).drawBitmap(isA(Bitmap.class), isNull(), isA(Rect.class), captor.capture());
assertThat(captor.getValue().getColorFilter()).isEqualTo(colorFilter);
}

@Test
Expand Down

0 comments on commit b48b690

Please sign in to comment.