Skip to content

Commit

Permalink
Allow modifying RequestBuilders after calling into().
Browse files Browse the repository at this point in the history
Fixes an issue where setting options on the generated GlideRequest after
having used it to load one image would throw an exception because the
internal RequestOptions object was locked when the first load was 
started. Now we apply autoClone() to make sure that the options object
for the first load isn’t modified by subsequent changes to options, but
also allowing those changes to occur.
  • Loading branch information
sjudd committed Sep 14, 2017
1 parent c3479c4 commit d56e08c
Showing 1 changed file with 28 additions and 17 deletions.
45 changes: 28 additions & 17 deletions library/src/main/java/com/bumptech/glide/RequestBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -349,14 +349,18 @@ public RequestBuilder<TranscodeType> clone() {
* @see RequestManager#clear(Target)
*/
public <Y extends Target<TranscodeType>> Y into(@NonNull Y target) {
return into(target, getMutableOptions());
}

private <Y extends Target<TranscodeType>> Y into(@NonNull Y target, RequestOptions options) {
Util.assertMainThread();
Preconditions.checkNotNull(target);
if (!isModelSet) {
throw new IllegalArgumentException("You must call #load() before calling #into()");
}

requestOptions.lock();
Request request = buildRequest(target);
options = options.autoClone();
Request request = buildRequest(target, options);

Request previous = target.getRequest();
if (request.isEquivalentTo(previous)) {
Expand All @@ -378,6 +382,7 @@ public <Y extends Target<TranscodeType>> Y into(@NonNull Y target) {
return target;
}


/**
* Sets the {@link ImageView} the resource will be loaded into, cancels any existing loads into
* the view, and frees any resources Glide may have previously loaded into the view so they may be
Expand All @@ -393,26 +398,27 @@ public Target<TranscodeType> into(ImageView view) {
Util.assertMainThread();
Preconditions.checkNotNull(view);

RequestOptions requestOptions = this.requestOptions;
if (!requestOptions.isTransformationSet()
&& requestOptions.isTransformationAllowed()
&& view.getScaleType() != null) {
if (requestOptions.isLocked()) {
requestOptions = requestOptions.clone();
}
// Clone in this method so that if we use this RequestBuilder to load into a View and then
// into a different target, we don't retain the transformation applied based on the previous
// View's scale type.
switch (view.getScaleType()) {
case CENTER_CROP:
requestOptions.optionalCenterCrop();
requestOptions.clone().optionalCenterCrop();
break;
case CENTER_INSIDE:
requestOptions.optionalCenterInside();
requestOptions.clone().optionalCenterInside();
break;
case FIT_CENTER:
case FIT_START:
case FIT_END:
requestOptions.optionalFitCenter();
requestOptions.clone().optionalFitCenter();
break;
case FIT_XY:
requestOptions.optionalCenterInside();
requestOptions.clone().optionalCenterInside();
break;
case CENTER:
case MATRIX:
Expand All @@ -421,7 +427,7 @@ public Target<TranscodeType> into(ImageView view) {
}
}

return into(context.buildImageViewTarget(view, transcodeClass));
return into(context.buildImageViewTarget(view, transcodeClass), requestOptions);
}

/**
Expand Down Expand Up @@ -578,15 +584,15 @@ private Priority getThumbnailPriority(Priority current) {
}
}

private Request buildRequest(Target<TranscodeType> target) {
private Request buildRequest(Target<TranscodeType> target, RequestOptions requestOptions) {
return buildRequestRecursive(target, null, transitionOptions, requestOptions.getPriority(),
requestOptions.getOverrideWidth(), requestOptions.getOverrideHeight());
requestOptions.getOverrideWidth(), requestOptions.getOverrideHeight(), requestOptions);
}

private Request buildRequestRecursive(Target<TranscodeType> target,
@Nullable ThumbnailRequestCoordinator parentCoordinator,
TransitionOptions<?, ? super TranscodeType> transitionOptions,
Priority priority, int overrideWidth, int overrideHeight) {
Priority priority, int overrideWidth, int overrideHeight, RequestOptions requestOptions) {
if (thumbnailBuilder != null) {
// Recursive case: contains a potentially recursive thumbnail request builder.
if (isThumbnailBuilt) {
Expand Down Expand Up @@ -619,8 +625,15 @@ private Request buildRequestRecursive(Target<TranscodeType> target,
transitionOptions, priority, overrideWidth, overrideHeight);
isThumbnailBuilt = true;
// Recursively generate thumbnail requests.
Request thumbRequest = thumbnailBuilder.buildRequestRecursive(target, coordinator,
thumbTransitionOptions, thumbPriority, thumbOverrideWidth, thumbOverrideHeight);
Request thumbRequest =
thumbnailBuilder.buildRequestRecursive(
target,
coordinator,
thumbTransitionOptions,
thumbPriority,
thumbOverrideWidth,
thumbOverrideHeight,
requestOptions);
isThumbnailBuilt = false;
coordinator.setRequests(fullRequest, thumbRequest);
return coordinator;
Expand Down Expand Up @@ -648,8 +661,6 @@ private Request obtainRequest(Target<TranscodeType> target,
RequestOptions requestOptions, RequestCoordinator requestCoordinator,
TransitionOptions<?, ? super TranscodeType> transitionOptions, Priority priority,
int overrideWidth, int overrideHeight) {
requestOptions.lock();

return SingleRequest.obtain(
context,
model,
Expand Down

0 comments on commit d56e08c

Please sign in to comment.