Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Center debug indicators #2043

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ class ImageViewAction extends Action {
}

boolean indicatorsEnabled = picasso.indicatorsEnabled;
PicassoDrawable.setResult(target, picasso.context, result, noFade, indicatorsEnabled);
PicassoDrawable.setResult(
target, picasso.context, result, noFade, indicatorsEnabled, picasso.indicatorsCentered
);

if (callback != null) {
callback.onSuccess();
Expand Down
26 changes: 23 additions & 3 deletions picasso/src/main/java/com/squareup/picasso3/Picasso.java
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ public enum Priority {
@Nullable final Bitmap.Config defaultBitmapConfig;

boolean indicatorsEnabled;
boolean indicatorsCentered;
volatile boolean loggingEnabled;

boolean shutdown;
Expand All @@ -149,7 +150,7 @@ public enum Priority {
@Nullable okhttp3.Cache closeableCache, PlatformLruCache cache, @Nullable Listener listener,
List<RequestTransformer> requestTransformers, List<RequestHandler> extraRequestHandlers,
Stats stats, @Nullable Bitmap.Config defaultBitmapConfig, boolean indicatorsEnabled,
boolean loggingEnabled) {
boolean loggingEnabled, boolean indicatorsCentered) {
this.context = context;
this.dispatcher = dispatcher;
this.callFactory = callFactory;
Expand Down Expand Up @@ -182,6 +183,7 @@ public enum Priority {
this.targetToAction = new LinkedHashMap<>();
this.targetToDeferredRequestCreator = new LinkedHashMap<>();
this.indicatorsEnabled = indicatorsEnabled;
this.indicatorsCentered = indicatorsCentered;
this.loggingEnabled = loggingEnabled;
}

Expand Down Expand Up @@ -454,11 +456,21 @@ public void invalidate(@NonNull File file) {
indicatorsEnabled = enabled;
}

/** {@code true} if debug indicators should are displayed on images. */
/** {@code true} if debug indicators are displayed on images. */
@SuppressWarnings("UnusedDeclaration") public boolean getIndicatorsEnabled() {
return indicatorsEnabled;
}

/** Toggle whether debug indicators are centered on images. */
@SuppressWarnings("UnusedDeclaration") public void setIndicatorsCentered(boolean centered) {
indicatorsCentered = centered;
}

/** {@code true} if debug indicators are centered on images. */
@SuppressWarnings("UnusedDeclaration") public boolean getIndicatorsCentered() {
return indicatorsCentered;
}

/**
* Toggle whether debug logging is enabled.
* <p>
Expand Down Expand Up @@ -669,6 +681,7 @@ public static class Builder {
@Nullable private Bitmap.Config defaultBitmapConfig;

private boolean indicatorsEnabled;
private boolean indicatorsCentered;
private boolean loggingEnabled;

/** Start building a new {@link Picasso} instance. */
Expand Down Expand Up @@ -797,6 +810,13 @@ public Builder indicatorsEnabled(boolean enabled) {
return this;
}

/** Toggle whether indicators are centered on images. */
@NonNull
public Builder indicatorsCentered(boolean centered) {
this.indicatorsCentered = centered;
return this;
}

/**
* Toggle whether debug logging is enabled.
* <p>
Expand Down Expand Up @@ -836,7 +856,7 @@ public Picasso build() {

return new Picasso(context, dispatcher, callFactory, unsharedCache, cache, listener,
requestTransformers, requestHandlers, stats, defaultBitmapConfig, indicatorsEnabled,
loggingEnabled);
loggingEnabled, indicatorsCentered);
}
}

Expand Down
40 changes: 34 additions & 6 deletions picasso/src/main/java/com/squareup/picasso3/PicassoDrawable.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,14 @@ final class PicassoDrawable extends BitmapDrawable {
// Only accessed from main thread.
private static final Paint DEBUG_PAINT = new Paint();
private static final float FADE_DURATION = 200f; //ms
private static final int INDICATORS_WIDTH = 16;

/**
* Create or update the drawable on the target {@link ImageView} to display the supplied bitmap
* image.
*/
static void setResult(ImageView target, Context context, RequestHandler.Result result,
boolean noFade, boolean debugging) {
boolean noFade, boolean debugging, boolean indicatorsCentered) {
Drawable placeholder = target.getDrawable();
if (placeholder instanceof Animatable) {
((Animatable) placeholder).stop();
Expand All @@ -52,7 +53,8 @@ static void setResult(ImageView target, Context context, RequestHandler.Result r
if (bitmap != null) {
Picasso.LoadedFrom loadedFrom = result.getLoadedFrom();
PicassoDrawable drawable =
new PicassoDrawable(context, bitmap, placeholder, loadedFrom, noFade, debugging);
new PicassoDrawable(context, bitmap, placeholder, loadedFrom, noFade, debugging,
indicatorsCentered);
target.setImageDrawable(drawable);
return;
}
Expand All @@ -78,6 +80,7 @@ static void setPlaceholder(ImageView target, @Nullable Drawable placeholderDrawa
}

private final boolean debugging;
private final boolean indicatorsCentered;
private final float density;
private final Picasso.LoadedFrom loadedFrom;

Expand All @@ -88,10 +91,12 @@ static void setPlaceholder(ImageView target, @Nullable Drawable placeholderDrawa
int alpha = 0xFF;

PicassoDrawable(Context context, Bitmap bitmap, @Nullable Drawable placeholder,
Picasso.LoadedFrom loadedFrom, boolean noFade, boolean debugging) {
Picasso.LoadedFrom loadedFrom, boolean noFade, boolean debugging,
boolean indicatorsCentered) {
super(context.getResources(), bitmap);

this.debugging = debugging;
this.indicatorsCentered = indicatorsCentered;
this.density = context.getResources().getDisplayMetrics().density;

this.loadedFrom = loadedFrom;
Expand Down Expand Up @@ -127,7 +132,11 @@ static void setPlaceholder(ImageView target, @Nullable Drawable placeholderDrawa
}

if (debugging) {
drawDebugIndicator(canvas);
if (indicatorsCentered) {
drawCenteredDebugIndicator(canvas);
} else {
drawDebugIndicator(canvas);
}
}
}

Expand Down Expand Up @@ -155,11 +164,11 @@ static void setPlaceholder(ImageView target, @Nullable Drawable placeholderDrawa

private void drawDebugIndicator(Canvas canvas) {
DEBUG_PAINT.setColor(WHITE);
Path path = getTrianglePath(0, 0, (int) (16 * density));
Path path = getTrianglePath(0, 0, (int) (INDICATORS_WIDTH * density));
canvas.drawPath(path, DEBUG_PAINT);

DEBUG_PAINT.setColor(loadedFrom.debugColor);
path = getTrianglePath(0, 0, (int) (15 * density));
path = getTrianglePath(0, 0, (int) ((INDICATORS_WIDTH - 1) * density));
canvas.drawPath(path, DEBUG_PAINT);
}

Expand All @@ -171,4 +180,23 @@ private static Path getTrianglePath(int x1, int y1, int width) {

return path;
}

private void drawCenteredDebugIndicator(Canvas canvas) {
DEBUG_PAINT.setColor(WHITE);
Path path = getCirclePath(canvas.getWidth() / 2f, canvas.getHeight() / 2f,
(int) (INDICATORS_WIDTH * density));
canvas.drawPath(path, DEBUG_PAINT);

DEBUG_PAINT.setColor(loadedFrom.debugColor);
path = getCirclePath(canvas.getWidth() / 2f, canvas.getHeight() / 2f,
(int) ((INDICATORS_WIDTH - 1) * density));
canvas.drawPath(path, DEBUG_PAINT);
}

private static Path getCirclePath(float x, float y, float radius) {
final Path path = new Path();
path.addCircle(x, y, radius, Path.Direction.CW);

return path;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,8 @@ public void into(@NonNull ImageView target, @Nullable Callback callback) {
if (bitmap != null) {
picasso.cancelRequest(target);
RequestHandler.Result result = new RequestHandler.Result(bitmap, MEMORY);
setResult(target, picasso.context, result, noFade, picasso.indicatorsEnabled);
setResult(target, picasso.context, result, noFade, picasso.indicatorsEnabled,
picasso.indicatorsCentered);
if (picasso.loggingEnabled) {
log(OWNER_MAIN, VERB_COMPLETED, request.plainId(), "from " + MEMORY);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ public final class BitmapHunterTest {
// Must use non-mock constructor because that is where Picasso's list of handlers is created.
Picasso picasso =
new Picasso(context, dispatcher, UNUSED_CALL_FACTORY, null, cache, null, NO_TRANSFORMERS,
handlers, stats, ARGB_8888, false, false);
handlers, stats, ARGB_8888, false, false, false);
BitmapHunter hunter = forRequest(picasso, dispatcher, cache, stats, action);
assertThat(hunter.requestHandler).isEqualTo(handler);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public void invokesOnBitmapFailedIfTargetIsNotNullWithErrorResourceId() {
PlatformLruCache cache = new PlatformLruCache(0);
Picasso picasso =
new Picasso(context, dispatcher, UNUSED_CALL_FACTORY, null, cache, null, NO_TRANSFORMERS,
NO_HANDLERS, mock(Stats.class), ARGB_8888, false, false);
NO_HANDLERS, mock(Stats.class), ARGB_8888, false, false, false);
Resources res = mock(Resources.class);
BitmapTargetAction request = new BitmapTargetAction(picasso, target, null, null, RESOURCE_ID_1);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public void invokesTargetAndCallbackSuccessIfTargetIsNotNull() {
Picasso picasso =
new Picasso(RuntimeEnvironment.application, dispatcher, UNUSED_CALL_FACTORY, null, cache,
null, NO_TRANSFORMERS, NO_HANDLERS, mock(Stats.class), Bitmap.Config.ARGB_8888, false,
false);
false, false);
ImageView target = mockImageViewTarget();
Callback callback = mockCallback();
ImageViewAction request =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,21 @@ public class PicassoDrawableTest {
private final Bitmap bitmap = makeBitmap();

@Test public void createWithNoPlaceholderAnimation() {
PicassoDrawable pd = new PicassoDrawable(context, bitmap, null, DISK, false, false);
PicassoDrawable pd = new PicassoDrawable(context, bitmap, null, DISK, false, false, false);
assertThat(pd.getBitmap()).isSameAs(bitmap);
assertThat(pd.placeholder).isNull();
assertThat(pd.animating).isTrue();
}

@Test public void createWithPlaceholderAnimation() {
PicassoDrawable pd = new PicassoDrawable(context, bitmap, placeholder, DISK, false, false);
PicassoDrawable pd = new PicassoDrawable(context, bitmap, placeholder, DISK, false, false, false);
assertThat(pd.getBitmap()).isSameAs(bitmap);
assertThat(pd.placeholder).isSameAs(placeholder);
assertThat(pd.animating).isTrue();
}

@Test public void createWithBitmapCacheHit() {
PicassoDrawable pd = new PicassoDrawable(context, bitmap, placeholder, MEMORY, false, false);
PicassoDrawable pd = new PicassoDrawable(context, bitmap, placeholder, MEMORY, false, false, false);
assertThat(pd.getBitmap()).isSameAs(bitmap);
assertThat(pd.placeholder).isNull();
assertThat(pd.animating).isFalse();
Expand Down
19 changes: 14 additions & 5 deletions picasso/src/test/java/com/squareup/picasso3/PicassoTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public final class PicassoTest {
@Before public void setUp() {
initMocks(this);
picasso = new Picasso(context, dispatcher, UNUSED_CALL_FACTORY, null, cache, listener,
NO_TRANSFORMERS, NO_HANDLERS, stats, ARGB_8888, false, false);
NO_TRANSFORMERS, NO_HANDLERS, stats, ARGB_8888, false, false, false);
}

@Test public void submitWithTargetInvokesDispatcher() {
Expand Down Expand Up @@ -371,7 +371,7 @@ public final class PicassoTest {
okhttp3.Cache cache = new okhttp3.Cache(temporaryFolder.getRoot(), 100);
Picasso picasso =
new Picasso(context, dispatcher, UNUSED_CALL_FACTORY, cache, this.cache, listener,
NO_TRANSFORMERS, NO_HANDLERS, stats, ARGB_8888, false, false);
NO_TRANSFORMERS, NO_HANDLERS, stats, ARGB_8888, false, false, false);
picasso.shutdown();
assertThat(cache.isClosed()).isTrue();
}
Expand Down Expand Up @@ -403,7 +403,8 @@ public final class PicassoTest {
}
};
Picasso picasso = new Picasso(context, dispatcher, UNUSED_CALL_FACTORY, null, cache, listener,
Collections.singletonList(brokenTransformer), NO_HANDLERS, stats, ARGB_8888, false, false);
Collections.singletonList(brokenTransformer), NO_HANDLERS, stats, ARGB_8888, false, false,
false);
Request request = new Request.Builder(URI_1).build();
try {
picasso.transformRequest(request);
Expand All @@ -428,6 +429,12 @@ public final class PicassoTest {
assertThat(picasso.getIndicatorsEnabled()).isTrue();
}

@Test public void centerIndicators() {
assertThat(picasso.getIndicatorsCentered()).isFalse();
picasso.setIndicatorsCentered(true);
assertThat(picasso.getIndicatorsCentered()).isTrue();
}

@Test public void loadThrowsWithInvalidInput() {
try {
picasso.load("");
Expand Down Expand Up @@ -509,12 +516,14 @@ public final class PicassoTest {
}

@Test public void builderWithDebugIndicators() {
Picasso picasso = new Picasso.Builder(RuntimeEnvironment.application).indicatorsEnabled(true).build();
Picasso picasso =
new Picasso.Builder(RuntimeEnvironment.application).indicatorsEnabled(true).build();
assertThat(picasso.getIndicatorsEnabled()).isTrue();
}

@Test public void evictAll() {
Picasso picasso = new Picasso.Builder(RuntimeEnvironment.application).indicatorsEnabled(true).build();
Picasso picasso =
new Picasso.Builder(RuntimeEnvironment.application).indicatorsEnabled(true).build();
picasso.cache.set("key", Bitmap.createBitmap(1, 1, ALPHA_8));
assertThat(picasso.cache.size()).isEqualTo(1);
picasso.evictAll();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ private Picasso createPicasso() {
Dispatcher dispatcher = mock(Dispatcher.class);
PlatformLruCache cache = new PlatformLruCache(0);
return new Picasso(RuntimeEnvironment.application, dispatcher, UNUSED_CALL_FACTORY, null, cache,
null, NO_TRANSFORMERS, NO_HANDLERS, mock(Stats.class), ARGB_8888, false, false);
null, NO_TRANSFORMERS, NO_HANDLERS, mock(Stats.class), ARGB_8888, false, false, false);
}

static class TestableRemoteViewsAction extends RemoteViewsAction {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ public void intoImageViewWithQuickMemoryCacheCheckDoesNotSubmit() {
Picasso picasso =
spy(new Picasso(RuntimeEnvironment.application, mock(Dispatcher.class), UNUSED_CALL_FACTORY,
null, cache, null, NO_TRANSFORMERS, NO_HANDLERS, mock(Stats.class), ARGB_8888, false,
false));
false, false));
doReturn(bitmap).when(picasso).quickMemoryCacheCheck(URI_KEY_1);
ImageView target = mockImageViewTarget();
Callback callback = mockCallback();
Expand All @@ -291,7 +291,7 @@ public void intoImageViewSetsPlaceholderDrawable() {
Picasso picasso =
spy(new Picasso(RuntimeEnvironment.application, mock(Dispatcher.class), UNUSED_CALL_FACTORY,
null, cache, null, NO_TRANSFORMERS, NO_HANDLERS, mock(Stats.class), ARGB_8888, false,
false));
false, false));
ImageView target = mockImageViewTarget();
Drawable placeHolderDrawable = mock(Drawable.class);
new RequestCreator(picasso, URI_1, 0).placeholder(placeHolderDrawable).into(target);
Expand All @@ -306,7 +306,7 @@ public void intoImageViewNoPlaceholderDrawable() {
Picasso picasso =
spy(new Picasso(RuntimeEnvironment.application, mock(Dispatcher.class), UNUSED_CALL_FACTORY,
null, cache, null, NO_TRANSFORMERS, NO_HANDLERS, mock(Stats.class), ARGB_8888, false,
false));
false, false));
ImageView target = mockImageViewTarget();
new RequestCreator(picasso, URI_1, 0).noPlaceholder().into(target);
verifyNoMoreInteractions(target);
Expand All @@ -320,7 +320,7 @@ public void intoImageViewSetsPlaceholderWithResourceId() {
Picasso picasso =
spy(new Picasso(RuntimeEnvironment.application, mock(Dispatcher.class), UNUSED_CALL_FACTORY,
null, cache, null, NO_TRANSFORMERS, NO_HANDLERS, mock(Stats.class), ARGB_8888, false,
false));
false, false));
ImageView target = mockImageViewTarget();
new RequestCreator(picasso, URI_1, 0).placeholder(android.R.drawable.picture_frame)
.into(target);
Expand Down