Skip to content

Commit 01da3cd

Browse files
kirwanFacebook Github Bot 7
kirwan
authored and
Facebook Github Bot 7
committed
Fallback to next thumbnail if not big enough
Reviewed By: massimocarli Differential Revision: D3059245 fb-gh-sync-id: 1d02dc89a4eb59bf124f93143709c176dc934bfe fbshipit-source-id: 1d02dc89a4eb59bf124f93143709c176dc934bfe
1 parent a175df7 commit 01da3cd

File tree

6 files changed

+143
-33
lines changed

6 files changed

+143
-33
lines changed

‎imagepipeline/src/main/java/com/facebook/imagepipeline/core/ProducerFactory.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ public LocalContentUriFetchProducer newLocalContentUriFetchProducer() {
212212
mDecodeFileDescriptorEnabled);
213213
}
214214

215-
public LocalContentUriThumbnailFetchProducer newLocalContentUriThumbnailFetchProducer() {
215+
public LocalContentUriThumbnailFetchProducer newLocalContentUriThumbnailFetchProducer() {
216216
return new LocalContentUriThumbnailFetchProducer(
217217
mExecutorSupplier.forLocalStorageRead(),
218218
mPooledByteBufferFactory,

‎imagepipeline/src/main/java/com/facebook/imagepipeline/producers/BranchOnSeparateImagesProducer.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ protected void onNewResultImpl(EncodedImage newResult, boolean isLast) {
5757
getConsumer().onNewResult(newResult, isLast && isGoodEnough);
5858
}
5959
if (isLast && !isGoodEnough) {
60+
EncodedImage.closeSafely(newResult);
61+
6062
mInputProducer2.produceResults(getConsumer(), mProducerContext);
6163
}
6264
}

‎imagepipeline/src/main/java/com/facebook/imagepipeline/producers/ThumbnailBranchProducer.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,20 +49,25 @@ private class ThumbnailConsumer extends DelegatingConsumer<EncodedImage, Encoded
4949

5050
private final ProducerContext mProducerContext;
5151
private final int mProducerIndex;
52+
private final ResizeOptions mResizeOptions;
5253

5354
public ThumbnailConsumer(
5455
final Consumer<EncodedImage> consumer,
5556
final ProducerContext producerContext, int producerIndex) {
5657
super(consumer);
5758
mProducerContext = producerContext;
5859
mProducerIndex = producerIndex;
60+
mResizeOptions = mProducerContext.getImageRequest().getResizeOptions();
5961
}
6062

6163
@Override
6264
protected void onNewResultImpl(EncodedImage newResult, boolean isLast) {
63-
if (newResult != null) {
65+
if (newResult != null &&
66+
(!isLast || ThumbnailSizeChecker.isImageBigEnough(newResult, mResizeOptions))) {
6467
getConsumer().onNewResult(newResult, isLast);
6568
} else if (isLast) {
69+
EncodedImage.closeSafely(newResult);
70+
6671
boolean fallback = produceResultsFromThumbnailProducer(
6772
mProducerIndex + 1,
6873
getConsumer(),
@@ -97,7 +102,7 @@ private boolean produceResultsFromThumbnailProducer(
97102
}
98103

99104
mThumbnailProducers[producerIndex]
100-
.produceResults(new ThumbnailConsumer(consumer, context, startIndex), context);
105+
.produceResults(new ThumbnailConsumer(consumer, context, producerIndex), context);
101106
return true;
102107
}
103108

‎imagepipeline/src/main/java/com/facebook/imagepipeline/producers/ThumbnailSizeChecker.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ public final class ThumbnailSizeChecker {
2626
*/
2727
public static final float ACCEPTABLE_REQUESTED_TO_ACTUAL_SIZE_RATIO = 4.0f/3;
2828

29+
private static final int ROTATED_90_DEGREES_CLOCKWISE = 90;
30+
private static final int ROTATED_90_DEGREES_COUNTER_CLOCKWISE = 270;
31+
2932
/**
3033
* Checks whether the producer may be able to produce images of the specified size. This makes no
3134
* promise about being able to produce images for a particular source, only generally being able
@@ -50,7 +53,14 @@ public static boolean isImageBigEnough(EncodedImage encodedImage, ResizeOptions
5053
return false;
5154
}
5255

53-
return isImageBigEnough(encodedImage.getWidth(), encodedImage.getHeight(), resizeOptions);
56+
switch (encodedImage.getRotationAngle()) {
57+
case ROTATED_90_DEGREES_CLOCKWISE:
58+
case ROTATED_90_DEGREES_COUNTER_CLOCKWISE:
59+
// Swap width and height when checking size as this will be rotated
60+
return isImageBigEnough(encodedImage.getHeight(), encodedImage.getWidth(), resizeOptions);
61+
default:
62+
return isImageBigEnough(encodedImage.getWidth(), encodedImage.getHeight(), resizeOptions);
63+
}
5464
}
5565

5666
public static int getAcceptableSize(int size) {

‎imagepipeline/src/test/java/com/facebook/imagepipeline/producers/ThumbnailBranchProducerTest.java

Lines changed: 57 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,14 @@ public void testNullReturnedIfNoResizeOptions() {
7171
mProducer.produceResults(mImageConsumer, mProducerContext);
7272

7373
verify(mImageConsumer).onNewResult(null, true);
74-
verifyZeroInteractions(mThumbnailProducers);
74+
verifyZeroInteractions((Object[]) mThumbnailProducers);
7575
}
7676

7777
@Test
7878
public void testFirstProducerUsedIfSufficientForResizeOptions() {
7979
mockRequestWithResizeOptions(THUMBNAIL_WIDTHS[0], THUMBNAIL_HEIGHTS[0]);
8080

81-
EncodedImage firstImage = mockEncodedImage(THUMBNAIL_WIDTHS[0], THUMBNAIL_HEIGHTS[0]);
81+
EncodedImage firstImage = mockEncodedImage(THUMBNAIL_WIDTHS[0], THUMBNAIL_HEIGHTS[0], 0);
8282
mockProducersToProduce(firstImage);
8383

8484
mProducer.produceResults(mImageConsumer, mProducerContext);
@@ -91,9 +91,9 @@ public void testFirstProducerUsedIfSufficientForResizeOptions() {
9191
public void testSecondProducerUsedIfSufficientForResizeOptions() {
9292
mockRequestWithResizeOptions(THUMBNAIL_WIDTHS[0] + 50, THUMBNAIL_HEIGHTS[0] + 50);
9393

94-
EncodedImage secondImage = mockEncodedImage(THUMBNAIL_WIDTHS[1], THUMBNAIL_HEIGHTS[1]);
94+
EncodedImage secondImage = mockEncodedImage(THUMBNAIL_WIDTHS[1], THUMBNAIL_HEIGHTS[1], 0);
9595
mockProducersToProduce(
96-
mockEncodedImage(THUMBNAIL_WIDTHS[0] + 50, THUMBNAIL_HEIGHTS[0] + 50),
96+
mockEncodedImage(THUMBNAIL_WIDTHS[0] + 50, THUMBNAIL_HEIGHTS[0] + 50, 0),
9797
secondImage);
9898

9999
mProducer.produceResults(mImageConsumer, mProducerContext);
@@ -106,7 +106,7 @@ public void testSecondProducerUsedIfSufficientForResizeOptions() {
106106
public void testFinalProducerUsedIfFirstTwoReturnNullOrFailure() {
107107
mockRequestWithResizeOptions(THUMBNAIL_WIDTHS[0] - 50, THUMBNAIL_HEIGHTS[0] - 50);
108108

109-
EncodedImage thirdImage = mockEncodedImage(THUMBNAIL_WIDTHS[2], THUMBNAIL_HEIGHTS[2]);
109+
EncodedImage thirdImage = mockEncodedImage(THUMBNAIL_WIDTHS[2], THUMBNAIL_HEIGHTS[2], 0);
110110
mockProducersToProduce(THROW_FAILURE, null, thirdImage);
111111

112112
mProducer.produceResults(mImageConsumer, mProducerContext);
@@ -128,7 +128,7 @@ public void testNullReturnedIfNoProducerSufficientForResizeOptions() {
128128
verify(mThumbnailProducers[0]).canProvideImageForSize(resizeOptions);
129129
verify(mThumbnailProducers[1]).canProvideImageForSize(resizeOptions);
130130
verify(mThumbnailProducers[2]).canProvideImageForSize(resizeOptions);
131-
verifyNoMoreInteractions(mThumbnailProducers);
131+
verifyNoMoreInteractions((Object[]) mThumbnailProducers);
132132
}
133133

134134
@Test
@@ -159,15 +159,65 @@ public void testFailureReturnedIfAllProducersFailOrReturnNullEndingWithFailure()
159159
verifyAllProducersRequestedForResults();
160160
}
161161

162+
@Test
163+
public void testFinalProducerUsedIfFirstTwoReturnTooSmallImages() {
164+
int desiredWidth = THUMBNAIL_WIDTHS[0] - 50;
165+
int desiredHeight = THUMBNAIL_HEIGHTS[0] - 50;
166+
mockRequestWithResizeOptions(desiredWidth, desiredHeight);
167+
168+
EncodedImage thirdImage = mockEncodedImage(THUMBNAIL_WIDTHS[2], THUMBNAIL_HEIGHTS[2], 0);
169+
mockProducersToProduce(
170+
mockEncodedImage(desiredWidth / 2, desiredHeight / 2, 0),
171+
mockEncodedImage(desiredWidth / 2, desiredHeight / 2, 0),
172+
thirdImage);
173+
174+
mProducer.produceResults(mImageConsumer, mProducerContext);
175+
176+
verify(mImageConsumer).onNewResult(thirdImage, true);
177+
verifyAllProducersRequestedForResults();
178+
}
179+
180+
@Test
181+
public void testSecondProducerUsedIfImageBigEnoughWhenRotated() {
182+
mockRequestWithResizeOptions(THUMBNAIL_WIDTHS[1], THUMBNAIL_HEIGHTS[1]);
183+
184+
EncodedImage secondImage =
185+
mockEncodedImage(THUMBNAIL_HEIGHTS[1] * 3 / 4, THUMBNAIL_WIDTHS[1] * 3 / 4, 90);
186+
mockProducersToProduce(
187+
mockEncodedImage(THUMBNAIL_WIDTHS[0], THUMBNAIL_HEIGHTS[0], 0),
188+
secondImage);
189+
190+
mProducer.produceResults(mImageConsumer, mProducerContext);
191+
192+
verify(mImageConsumer).onNewResult(secondImage, true);
193+
verifyZeroInteractions(mThumbnailProducers[2]);
194+
}
195+
196+
@Test
197+
public void testNullReturnedIfLastImageNotBigEnoughWhenRotated() {
198+
mockRequestWithResizeOptions(THUMBNAIL_WIDTHS[2], THUMBNAIL_HEIGHTS[2]);
199+
200+
mockProducersToProduce(
201+
mockEncodedImage(THUMBNAIL_WIDTHS[0], THUMBNAIL_HEIGHTS[0], 0),
202+
mockEncodedImage(THUMBNAIL_WIDTHS[1], THUMBNAIL_HEIGHTS[1], 0),
203+
mockEncodedImage(THUMBNAIL_HEIGHTS[2] / 2, THUMBNAIL_WIDTHS[2] / 2, 90));
204+
205+
mProducer.produceResults(mImageConsumer, mProducerContext);
206+
207+
verify(mImageConsumer).onNewResult(null, true);
208+
verify(mThumbnailProducers[2]).produceResults(any(Consumer.class), any(ProducerContext.class));
209+
}
210+
162211
private void mockRequestWithResizeOptions(int width, int height) {
163212
ResizeOptions resizeOptions = new ResizeOptions(width, height);
164213
when(mImageRequest.getResizeOptions()).thenReturn(resizeOptions);
165214
}
166215

167-
private EncodedImage mockEncodedImage(int width, int height) {
216+
private static EncodedImage mockEncodedImage(int width, int height, int rotationAngle) {
168217
EncodedImage mockImage = mock(EncodedImage.class);
169218
when(mockImage.getWidth()).thenReturn(width);
170219
when(mockImage.getHeight()).thenReturn(height);
220+
when(mockImage.getRotationAngle()).thenReturn(rotationAngle);
171221
return mockImage;
172222
}
173223

‎imagepipeline/src/test/java/com/facebook/imagepipeline/producers/ThumbnailSizeCheckerTest.java

Lines changed: 65 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -86,60 +86,103 @@ public void testWithInsufficientHeightWhenNoResizeOptions() {
8686

8787
@Test
8888
public void testWithImageAndResizeOptionsNotMoreThan133PercentOfActual() {
89-
for (int i = 0; i < TEST_COUNT; i++) {
90-
EncodedImage encodedImage = mockImage(IMAGE_WIDTHS[i], IMAGE_HEIGHTS[i]);
91-
ResizeOptions resizeOptions = new ResizeOptions(REQUEST_WIDTHS[i], REQUEST_HEIGHTS[i]);
92-
assertTrue(ThumbnailSizeChecker.isImageBigEnough(encodedImage, resizeOptions));
89+
testWithImageBigEnoughForResizeOptions(IMAGE_WIDTHS, IMAGE_HEIGHTS, 0);
90+
}
91+
92+
@Test
93+
public void testWithRotatedImageAndResizeOptionsNotMoreThan133PercentOfActual() {
94+
testWithImageBigEnoughForResizeOptions(IMAGE_HEIGHTS, IMAGE_WIDTHS, 90);
95+
}
96+
97+
private static void testWithImageBigEnoughForResizeOptions(
98+
int[] imageWidths,
99+
int[] imageHeights,
100+
int startRotation) {
101+
for (int rotation = startRotation; rotation < 360; rotation += 180) {
102+
for (int i = 0; i < TEST_COUNT; i++) {
103+
EncodedImage encodedImage = mockImage(imageWidths[i], imageHeights[i], rotation);
104+
ResizeOptions resizeOptions = new ResizeOptions(REQUEST_WIDTHS[i], REQUEST_HEIGHTS[i]);
105+
assertTrue(ThumbnailSizeChecker.isImageBigEnough(encodedImage, resizeOptions));
106+
}
93107
}
94108
}
95109

96110
@Test
97111
public void testWithImageAndResizeOptionsWithWidthMoreThan133PercentOfActual() {
98-
testWithImageNotBigEnoughForResizeOptions(1, 0);
112+
testWithImageNotBigEnoughForResizeOptions(IMAGE_WIDTHS, IMAGE_HEIGHTS, 0, 1, 0);
99113
}
100114

101115
@Test
102116
public void testWithImageAndResizeOptionsWithHeightMoreThan133PercentOfActual() {
103-
testWithImageNotBigEnoughForResizeOptions(0, 1);
117+
testWithImageNotBigEnoughForResizeOptions(IMAGE_WIDTHS, IMAGE_HEIGHTS, 0, 0, 1);
118+
}
119+
120+
@Test
121+
public void testWithRotatedImageAndResizeOptionsWithWidthMoreThan133PercentOfActual() {
122+
testWithImageNotBigEnoughForResizeOptions(IMAGE_HEIGHTS, IMAGE_WIDTHS, 90, 1, 0);
123+
}
124+
125+
@Test
126+
public void testWithRotatedImageAndResizeOptionsWithHeightMoreThan133PercentOfActual() {
127+
testWithImageNotBigEnoughForResizeOptions(IMAGE_HEIGHTS, IMAGE_WIDTHS, 90, 0, 1);
104128
}
105129

106130
private static void testWithImageNotBigEnoughForResizeOptions(
131+
int[] imageWidths,
132+
int[] imageHeights,
133+
int startRotation,
107134
int additionalRequestWidth,
108135
int additionalRequestHeight) {
109-
for (int i = 0; i < TEST_COUNT; i++) {
110-
ResizeOptions resizeOptions = new ResizeOptions(
111-
REQUEST_WIDTHS[i] + additionalRequestWidth,
112-
REQUEST_HEIGHTS[i] + additionalRequestHeight);
113-
EncodedImage encodedImage = mockImage(IMAGE_WIDTHS[i], IMAGE_HEIGHTS[i]);
114-
assertFalse(ThumbnailSizeChecker.isImageBigEnough(encodedImage, resizeOptions));
136+
for (int rotation = startRotation; rotation < 360; rotation += 180) {
137+
for (int i = 0; i < TEST_COUNT; i++) {
138+
ResizeOptions resizeOptions = new ResizeOptions(
139+
REQUEST_WIDTHS[i] + additionalRequestWidth,
140+
REQUEST_HEIGHTS[i] + additionalRequestHeight);
141+
EncodedImage encodedImage = mockImage(imageWidths[i], imageHeights[i], rotation);
142+
assertFalse(ThumbnailSizeChecker.isImageBigEnough(encodedImage, resizeOptions));
143+
}
115144
}
116145
}
117146

118147
@Test
119148
public void testWithLargeEnoughImageWhenNoResizeOptions() {
120-
assertTrue(ThumbnailSizeChecker.isImageBigEnough(
121-
mockImage(BIG_ENOUGH_SIZE_FOR_NO_RESIZE_OPTIONS, BIG_ENOUGH_SIZE_FOR_NO_RESIZE_OPTIONS),
122-
null));
149+
for (int rotation = 0; rotation < 360; rotation += 90) {
150+
assertTrue(ThumbnailSizeChecker.isImageBigEnough(
151+
mockImage(
152+
BIG_ENOUGH_SIZE_FOR_NO_RESIZE_OPTIONS,
153+
BIG_ENOUGH_SIZE_FOR_NO_RESIZE_OPTIONS,
154+
rotation),
155+
null));
156+
}
123157
}
124158

125159
@Test
126160
public void testImageWithInsufficientWidthWhenNoResizeOptions() {
127-
assertFalse(ThumbnailSizeChecker.isImageBigEnough(
128-
mockImage(BIG_ENOUGH_SIZE_FOR_NO_RESIZE_OPTIONS - 1, BIG_ENOUGH_SIZE_FOR_NO_RESIZE_OPTIONS),
129-
null));
161+
for (int rotation = 0; rotation < 360; rotation += 90) {
162+
EncodedImage mockImage = mockImage(
163+
BIG_ENOUGH_SIZE_FOR_NO_RESIZE_OPTIONS - 1,
164+
BIG_ENOUGH_SIZE_FOR_NO_RESIZE_OPTIONS,
165+
rotation);
166+
assertFalse(ThumbnailSizeChecker.isImageBigEnough(mockImage, null));
167+
}
130168
}
131169

132170
@Test
133171
public void testImageWithInsufficientHeightWhenNoResizeOptions() {
134-
assertFalse(ThumbnailSizeChecker.isImageBigEnough(
135-
mockImage(BIG_ENOUGH_SIZE_FOR_NO_RESIZE_OPTIONS, BIG_ENOUGH_SIZE_FOR_NO_RESIZE_OPTIONS - 1),
136-
null));
172+
for (int rotation = 0; rotation < 360; rotation += 90) {
173+
EncodedImage mockImage = mockImage(
174+
BIG_ENOUGH_SIZE_FOR_NO_RESIZE_OPTIONS,
175+
BIG_ENOUGH_SIZE_FOR_NO_RESIZE_OPTIONS - 1,
176+
rotation);
177+
assertFalse(ThumbnailSizeChecker.isImageBigEnough(mockImage, null));
178+
}
137179
}
138180

139-
private static EncodedImage mockImage(int width, int height) {
181+
private static EncodedImage mockImage(int width, int height, int rotation) {
140182
EncodedImage image = mock(EncodedImage.class);
141183
when(image.getWidth()).thenReturn(width);
142184
when(image.getHeight()).thenReturn(height);
185+
when(image.getRotationAngle()).thenReturn(rotation);
143186
return image;
144187
}
145188
}

0 commit comments

Comments
 (0)