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

Load local assets synchronously to prevent image flicker #8102

Closed
Closed
Show file tree
Hide file tree
Changes from 2 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
14 changes: 6 additions & 8 deletions Libraries/Image/RCTImage.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@
1304D5B21AA8C50D0002E2BE /* RCTGIFImageDecoder.m in Sources */ = {isa = PBXBuildFile; fileRef = 1304D5B11AA8C50D0002E2BE /* RCTGIFImageDecoder.m */; };
134B00A21B54232B00EC8DFB /* RCTImageUtils.m in Sources */ = {isa = PBXBuildFile; fileRef = 134B00A11B54232B00EC8DFB /* RCTImageUtils.m */; };
139A38841C4D587C00862840 /* RCTResizeMode.m in Sources */ = {isa = PBXBuildFile; fileRef = 139A38831C4D587C00862840 /* RCTResizeMode.m */; };
13EF7F0B1BC42D4E003F47DD /* RCTShadowVirtualImage.m in Sources */ = {isa = PBXBuildFile; fileRef = 13EF7F081BC42D4E003F47DD /* RCTShadowVirtualImage.m */; };
13EF7F0C1BC42D4E003F47DD /* RCTVirtualImageManager.m in Sources */ = {isa = PBXBuildFile; fileRef = 13EF7F0A1BC42D4E003F47DD /* RCTVirtualImageManager.m */; };
13EF7F7F1BC825B1003F47DD /* RCTXCAssetImageLoader.m in Sources */ = {isa = PBXBuildFile; fileRef = 13EF7F7E1BC825B1003F47DD /* RCTXCAssetImageLoader.m */; };
13EF7F7F1BC825B1003F47DD /* RCTLocalAssetImageLoader.m in Sources */ = {isa = PBXBuildFile; fileRef = 13EF7F7E1BC825B1003F47DD /* RCTLocalAssetImageLoader.m */; };
143879381AAD32A300F088A5 /* RCTImageLoader.m in Sources */ = {isa = PBXBuildFile; fileRef = 143879371AAD32A300F088A5 /* RCTImageLoader.m */; };
35123E6B1B59C99D00EBAD80 /* RCTImageStoreManager.m in Sources */ = {isa = PBXBuildFile; fileRef = 35123E6A1B59C99D00EBAD80 /* RCTImageStoreManager.m */; };
354631681B69857700AA0B86 /* RCTImageEditingManager.m in Sources */ = {isa = PBXBuildFile; fileRef = 354631671B69857700AA0B86 /* RCTImageEditingManager.m */; };
Expand Down Expand Up @@ -44,8 +42,8 @@
134B00A11B54232B00EC8DFB /* RCTImageUtils.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTImageUtils.m; sourceTree = "<group>"; };
139A38821C4D57AD00862840 /* RCTResizeMode.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTResizeMode.h; sourceTree = "<group>"; };
139A38831C4D587C00862840 /* RCTResizeMode.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTResizeMode.m; sourceTree = "<group>"; };
13EF7F7D1BC825B1003F47DD /* RCTXCAssetImageLoader.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTXCAssetImageLoader.h; sourceTree = "<group>"; };
13EF7F7E1BC825B1003F47DD /* RCTXCAssetImageLoader.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTXCAssetImageLoader.m; sourceTree = "<group>"; };
13EF7F7D1BC825B1003F47DD /* RCTLocalAssetImageLoader.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTLocalAssetImageLoader.h; sourceTree = "<group>"; };
13EF7F7E1BC825B1003F47DD /* RCTLocalAssetImageLoader.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTLocalAssetImageLoader.m; sourceTree = "<group>"; };
143879361AAD32A300F088A5 /* RCTImageLoader.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTImageLoader.h; sourceTree = "<group>"; };
143879371AAD32A300F088A5 /* RCTImageLoader.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTImageLoader.m; sourceTree = "<group>"; };
35123E691B59C99D00EBAD80 /* RCTImageStoreManager.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTImageStoreManager.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -75,8 +73,8 @@
EEF314711C9B0DD30049118E /* RCTImageBlurUtils.m */,
139A38821C4D57AD00862840 /* RCTResizeMode.h */,
139A38831C4D587C00862840 /* RCTResizeMode.m */,
13EF7F7D1BC825B1003F47DD /* RCTXCAssetImageLoader.h */,
13EF7F7E1BC825B1003F47DD /* RCTXCAssetImageLoader.m */,
13EF7F7D1BC825B1003F47DD /* RCTLocalAssetImageLoader.h */,
13EF7F7E1BC825B1003F47DD /* RCTLocalAssetImageLoader.m */,
1304D5B01AA8C50D0002E2BE /* RCTGIFImageDecoder.h */,
1304D5B11AA8C50D0002E2BE /* RCTGIFImageDecoder.m */,
354631661B69857700AA0B86 /* RCTImageEditingManager.h */,
Expand Down Expand Up @@ -169,7 +167,7 @@
139A38841C4D587C00862840 /* RCTResizeMode.m in Sources */,
1304D5AB1AA8C4A30002E2BE /* RCTImageView.m in Sources */,
EEF314721C9B0DD30049118E /* RCTImageBlurUtils.m in Sources */,
13EF7F7F1BC825B1003F47DD /* RCTXCAssetImageLoader.m in Sources */,
13EF7F7F1BC825B1003F47DD /* RCTLocalAssetImageLoader.m in Sources */,
134B00A21B54232B00EC8DFB /* RCTImageUtils.m in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
Expand Down
12 changes: 12 additions & 0 deletions Libraries/Image/RCTImageLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,18 @@ __deprecated_msg("Use getImageSizeWithURLRequest:block: instead");
*/
- (float)loaderPriority;

/**
* If the loader must be called on the serial url cache queue. If this is NO,
* the loader will be called from the main queue. Defaults to YES.
*/
- (BOOL)requiresScheduling;

/**
* If images loaded by the loader should be cached in the decoded image cache.
* Defaults to YES.
*/
- (BOOL)shouldCacheLoadedImages;

@end

/**
Expand Down
51 changes: 34 additions & 17 deletions Libraries/Image/RCTImageLoader.m
Original file line number Diff line number Diff line change
Expand Up @@ -290,27 +290,44 @@ - (RCTImageLoaderCancellationBlock)loadImageOrDataWithURLRequest:(NSURLRequest *
scale:(CGFloat)scale
resizeMode:(RCTResizeMode)resizeMode
progressBlock:(RCTImageLoaderProgressBlock)progressHandler
completionBlock:(void (^)(NSError *error, id imageOrData))completionBlock
completionBlock:(void (^)(NSError *error, id imageOrData, BOOL cacheResult))completionBlock
{
__block volatile uint32_t cancelled = 0;
__block void(^cancelLoad)(void) = nil;
__weak RCTImageLoader *weakSelf = self;
// Find suitable image URL loader
__block id<RCTImageURLLoader> loadHandler = [self imageURLLoaderForURL:imageURLRequest.URL];
__block BOOL cacheResult = [loadHandler respondsToSelector:@selector(shouldCacheLoadedImages)] ?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You only need to mark variables as __block if you plan to re-assign them from inside a block. That doesn't seem to be the case here.

[loadHandler shouldCacheLoadedImages] : YES;
BOOL requiresScheduling = [loadHandler respondsToSelector:@selector(requiresScheduling)] ?
[loadHandler requiresScheduling] : YES;

void (^completionHandler)(NSError *error, id imageOrData) = ^(NSError *error, id imageOrData) {
if (RCTIsMainQueue()) {
if (RCTIsMainQueue() && requiresScheduling) {

// Most loaders do not return on the main thread, so caller is probably not
// expecting it, and may do expensive post-processing in the callback
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
if (!cancelled) {
completionBlock(error, imageOrData);
completionBlock(error, imageOrData, cacheResult);
}
});
} else if (!cancelled) {
completionBlock(error, imageOrData);
completionBlock(error, imageOrData, cacheResult);
}
};

// If the loader doesn't require scheduling we call it directly on
// the main queue.
if (loadHandler && !requiresScheduling) {
return [loadHandler loadImageForURL:imageURLRequest.URL
size:size
scale:scale
resizeMode:resizeMode
progressHandler:progressHandler
completionHandler:completionHandler] ?: ^{};
}

// All access to URL cache must be serialized
if (!_URLCacheQueue) {
[self setUp];
Expand All @@ -328,16 +345,15 @@ - (RCTImageLoaderCancellationBlock)loadImageOrDataWithURLRequest:(NSURLRequest *
return;
}

// Find suitable image URL loader
NSURLRequest *request = imageURLRequest; // Use a local variable so we can reassign it in this block
id<RCTImageURLLoader> loadHandler = [strongSelf imageURLLoaderForURL:request.URL];
if (loadHandler) {
cancelLoad = [loadHandler loadImageForURL:request.URL
size:size
scale:scale
resizeMode:resizeMode
progressHandler:progressHandler
completionHandler:completionHandler] ?: ^{};

if (loadHandler && requiresScheduling) {
cancelLoad = [loadHandler loadImageForURL:imageURLRequest.URL
size:size
scale:scale
resizeMode:resizeMode
progressHandler:progressHandler
completionHandler:completionHandler] ?: ^{};
return;
}

Expand Down Expand Up @@ -504,17 +520,18 @@ - (RCTImageLoaderCancellationBlock)loadImageWithURLRequest:(NSURLRequest *)image
completionBlock(error, image);
};

void (^completionHandler)(NSError *, id) = ^(NSError *error, id imageOrData) {
void (^completionHandler)(NSError *, id, BOOL) = ^(NSError *error, id imageOrData, BOOL cacheResult) {
if (!cancelled) {
RCTImageLoaderCompletionBlock resultHandler = cacheResult ? cacheResultHandler : completionBlock;
if (!imageOrData || [imageOrData isKindOfClass:[UIImage class]]) {
cacheResultHandler(error, imageOrData);
resultHandler(error, imageOrData);
} else {
cancelLoad = [weakSelf decodeImageData:imageOrData
size:size
scale:scale
clipped:clipped
resizeMode:resizeMode
completionBlock:cacheResultHandler];
completionBlock:resultHandler];
}
}
};
Expand Down Expand Up @@ -652,7 +669,7 @@ - (RCTImageLoaderCancellationBlock)getImageSizeForURLRequest:(NSURLRequest *)ima
scale:1
resizeMode:RCTResizeModeStretch
progressBlock:nil
completionBlock:^(NSError *error, id imageOrData) {
completionBlock:^(NSError *error, id imageOrData, __unused BOOL cacheResult) {
CGSize size;
if ([imageOrData isKindOfClass:[NSData class]]) {
NSDictionary *meta = RCTGetImageMetadata(imageOrData);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@

#import "RCTImageLoader.h"

@interface RCTXCAssetImageLoader : NSObject <RCTImageURLLoader>
@interface RCTLocalAssetImageLoader : NSObject <RCTImageURLLoader>

@end
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,33 @@
* of patent rights can be found in the PATENTS file in the same directory.
*/

#import "RCTXCAssetImageLoader.h"
#import "RCTLocalAssetImageLoader.h"

#import <libkern/OSAtomic.h>

#import "RCTUtils.h"

@implementation RCTXCAssetImageLoader
@implementation RCTLocalAssetImageLoader

RCT_EXPORT_MODULE()

- (BOOL)canLoadImageURL:(NSURL *)requestURL
{
return RCTIsXCAssetURL(requestURL);
return RCTIsLocalAssetURL(requestURL);
}

- (BOOL)requiresScheduling
{
// Don't schedule this loader on the URL queue so we can load the
// local assets synchronously to avoid flickers.
return NO;
}

- (BOOL)shouldCacheLoadedImages
{
// UIImage imageNamed handles the caching automatically so we don't want
// to add it to the image cache.
return NO;
}

- (RCTImageLoaderCancellationBlock)loadImageForURL:(NSURL *)imageURL
Expand All @@ -30,11 +44,12 @@ - (RCTImageLoaderCancellationBlock)loadImageForURL:(NSURL *)imageURL
completionHandler:(RCTImageLoaderCompletionBlock)completionHandler
{
__block volatile uint32_t cancelled = 0;
dispatch_async(dispatch_get_main_queue(), ^{

RCTExecuteOnMainQueue(^{
if (cancelled) {
return;
}

NSString *imageName = RCTBundlePathForURL(imageURL);
UIImage *image = [UIImage imageNamed:imageName];
if (image) {
Expand Down
2 changes: 1 addition & 1 deletion Libraries/Network/RCTFileRequestHandler.m
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ - (BOOL)canHandleRequest:(NSURLRequest *)request
{
return
[request.URL.scheme caseInsensitiveCompare:@"file"] == NSOrderedSame
&& !RCTIsXCAssetURL(request.URL);
&& !RCTIsLocalAssetURL(request.URL);
}

- (NSOperation *)sendRequest:(NSURLRequest *)request
Expand Down
4 changes: 2 additions & 2 deletions React/Base/RCTUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ RCT_EXTERN NSData *__nullable RCTGzipData(NSData *__nullable data, float level);
// (or nil, if the URL does not specify a path within the main bundle)
RCT_EXTERN NSString *__nullable RCTBundlePathForURL(NSURL *__nullable URL);

// Determines if a given image URL actually refers to an XCAsset
RCT_EXTERN BOOL RCTIsXCAssetURL(NSURL *__nullable imageURL);
// Determines if a given image URL refers to a local image
RCT_EXTERN BOOL RCTIsLocalAssetURL(NSURL *__nullable imageURL);

// Creates a new, unique temporary file path with the specified extension
RCT_EXTERN NSString *__nullable RCTTempFilePath(NSString *__nullable extension, NSError **error);
Expand Down
16 changes: 6 additions & 10 deletions React/Base/RCTUtils.m
Original file line number Diff line number Diff line change
Expand Up @@ -612,21 +612,17 @@ BOOL RCTIsGzippedData(NSData *__nullable data)
return path;
}

BOOL RCTIsXCAssetURL(NSURL *__nullable imageURL)
BOOL RCTIsLocalAssetURL(NSURL *__nullable imageURL)
{
NSString *name = RCTBundlePathForURL(imageURL);
if (name.pathComponents.count != 1) {
// URL is invalid, or is a file path, not an XCAsset identifier
if (name == nil) {
return NO;
}
NSString *extension = [name pathExtension];
if (extension.length && ![extension isEqualToString:@"png"]) {
// Not a png
return NO;
}
extension = extension.length ? nil : @"png";
if ([[NSBundle mainBundle] pathForResource:name ofType:extension]) {
// File actually exists in bundle, so is not an XCAsset
if (extension.length &&
![extension isEqualToString:@"png"] &&
![extension isEqualToString:@"jpg"]) {
// Not a supported image.
return NO;
}
return YES;
Expand Down