Allow onlyScaleDown() with fit() method #1600
Conversation
|
It seems like an arbitrary restriction, I'd love this change! |
| @@ -500,6 +497,10 @@ public Request build() { | |||
| throw new IllegalStateException( | |||
| "Center inside requires calling resize with positive width and height."); | |||
| } | |||
| if (onlyScaleDown && (targetWidth == 0 && targetHeight == 0)) { | |||
There was a problem hiding this comment.
Should this be if(deferred || (targetWidth == 0 && targetHeight == 0))?
There was a problem hiding this comment.
And this check should be moved back to the onlyScaleDown method.
There was a problem hiding this comment.
@JakeWharton This method is in the Request class, which seems to be in line with the other checks such as for null lists, invalid properties, and center[Crop|Inside] checks.
There was a problem hiding this comment.
Ahh, the Request class has no knowledge of fitting, which is done in the RequestCreator class. I'm going to make my own pull request that will hopefully fix these issues cleanly.
| @@ -555,6 +565,11 @@ public void intoTargetNoResizeWithCenterInsideOrCenterCropThrows() { | |||
| fail("Center inside with unknown height should throw exception."); | |||
| } catch (IllegalStateException ignored) { | |||
| } | |||
| try { | |||
| new RequestCreator(picasso, URI_1, 0).onlyScaleDown().into(mockTarget()); | |||
| fail("onlyScaleDown with unknown height should throw exception."); | |||
There was a problem hiding this comment.
Shouldn't this fail when .onlyScaleDown() is called without a call to .fit() or .resize(int, int)?
| @@ -544,7 +554,7 @@ public void intoRemoteViewsNotificationWithFitThrows() { | |||
| } | |||
|
|
|||
| @Test | |||
| public void intoTargetNoResizeWithCenterInsideOrCenterCropThrows() { | |||
| public void intoTargetNoResizeWithCenterInsideOrCenterCropOrOnlyScaleDownThrows() { | |||
There was a problem hiding this comment.
This should probably also be named intoTargetNoResizeOrFitWithCenterInsideOrCenterCropOrOnlyScaleDownThrows.
|
@JakeWharton I have an alternative pull request, #1623, that I believe fixes the issues with this one. |
onlyScaleDown() method is so useful to avoid useless bitmap scale up for memory usage and performance.
This PR allows that onlyScaleDown() works with fit() method like centerCrop()/centerInside().
Refer to #941