From 5903949ad64fc7eb71427c382d93c1535fceab03 Mon Sep 17 00:00:00 2001 From: Janic Duplessis Date: Thu, 21 Jul 2016 07:45:54 -0700 Subject: [PATCH] Load local assets synchronously to prevent image flicker Summary: This uses `[UIImage imageNamed:]` to load local assets that are bundled using `require('../image/path.png')` and makes sure it is done synchronously on the main queue to prevent images from flickering. This improves user experience a lot when using large local images and prevents icon flickers to match the behaviour of most native apps. This adds to methods to the ImageLoader protocol, one to tell if the image loader must be executed on the url cache queue and one to tell if the result of the image loader should be cached. I then use these to make the LocalImageLoader bypass the url cache queue and avoid caching images twice. Note that this doesn't affect debug builds since images are loaded from the packager. I'm not sure if we want to still support async loading of local images as I'm not sure how much of a perf difference this will make. Maybe someone at fb can benchmark this see how it affects your apps but there wasn't a noticeable one in mine. Also I only enabled this for loading png and jpg im Closes https://github.com/facebook/react-native/pull/8102 Reviewed By: bnham Differential Revision: D3433647 Pulled By: javache fbshipit-source-id: 37bd6aff20c0465c163db3cdbcaeaedff55f7b1f --- Examples/UIExplorer/js/ImageExample.js | 14 +++++ .../Image/RCTImage.xcodeproj/project.pbxproj | 14 ++--- Libraries/Image/RCTImageLoader.h | 16 +++++ Libraries/Image/RCTImageLoader.m | 59 ++++++++++++------- ...ageLoader.h => RCTLocalAssetImageLoader.h} | 2 +- ...ageLoader.m => RCTLocalAssetImageLoader.m} | 24 ++++++-- Libraries/Network/RCTFileRequestHandler.m | 2 +- React/Base/RCTUtils.h | 4 +- React/Base/RCTUtils.m | 17 ++---- 9 files changed, 101 insertions(+), 51 deletions(-) rename Libraries/Image/{RCTXCAssetImageLoader.h => RCTLocalAssetImageLoader.h} (83%) rename Libraries/Image/{RCTXCAssetImageLoader.m => RCTLocalAssetImageLoader.m} (75%) diff --git a/Examples/UIExplorer/js/ImageExample.js b/Examples/UIExplorer/js/ImageExample.js index 34418ea67..df54d10f9 100644 --- a/Examples/UIExplorer/js/ImageExample.js +++ b/Examples/UIExplorer/js/ImageExample.js @@ -589,6 +589,20 @@ exports.examples = [ }, platform: 'android', }, + { + title: 'Legacy local image', + description: + 'Images shipped with the native bundle, but not managed ' + + 'by the JS packager', + render: function() { + return ( + + ); + }, + }, ]; var fullImage = {uri: 'http://facebook.github.io/react/img/logo_og.png'}; diff --git a/Libraries/Image/RCTImage.xcodeproj/project.pbxproj b/Libraries/Image/RCTImage.xcodeproj/project.pbxproj index 5e642b41f..e5b29308f 100644 --- a/Libraries/Image/RCTImage.xcodeproj/project.pbxproj +++ b/Libraries/Image/RCTImage.xcodeproj/project.pbxproj @@ -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 */; }; @@ -44,8 +42,8 @@ 134B00A11B54232B00EC8DFB /* RCTImageUtils.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTImageUtils.m; sourceTree = ""; }; 139A38821C4D57AD00862840 /* RCTResizeMode.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTResizeMode.h; sourceTree = ""; }; 139A38831C4D587C00862840 /* RCTResizeMode.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTResizeMode.m; sourceTree = ""; }; - 13EF7F7D1BC825B1003F47DD /* RCTXCAssetImageLoader.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTXCAssetImageLoader.h; sourceTree = ""; }; - 13EF7F7E1BC825B1003F47DD /* RCTXCAssetImageLoader.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTXCAssetImageLoader.m; sourceTree = ""; }; + 13EF7F7D1BC825B1003F47DD /* RCTLocalAssetImageLoader.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTLocalAssetImageLoader.h; sourceTree = ""; }; + 13EF7F7E1BC825B1003F47DD /* RCTLocalAssetImageLoader.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTLocalAssetImageLoader.m; sourceTree = ""; }; 143879361AAD32A300F088A5 /* RCTImageLoader.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTImageLoader.h; sourceTree = ""; }; 143879371AAD32A300F088A5 /* RCTImageLoader.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTImageLoader.m; sourceTree = ""; }; 35123E691B59C99D00EBAD80 /* RCTImageStoreManager.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTImageStoreManager.h; sourceTree = ""; }; @@ -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 */, @@ -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; diff --git a/Libraries/Image/RCTImageLoader.h b/Libraries/Image/RCTImageLoader.h index fadb2b5ca..095b7afa5 100644 --- a/Libraries/Image/RCTImageLoader.h +++ b/Libraries/Image/RCTImageLoader.h @@ -141,6 +141,22 @@ typedef dispatch_block_t RCTImageLoaderCancellationBlock; */ - (float)loaderPriority; +/** + * If the loader must be called on the serial url cache queue, and whether the completion + * block should be dispatched off the main thread. If this is NO, the loader will be + * called from the main queue. Defaults to YES. + * + * Use with care: disabling scheduling will reduce RCTImageLoader's ability to throttle + * network requests. + */ +- (BOOL)requiresScheduling; + +/** + * If images loaded by the loader should be cached in the decoded image cache. + * Defaults to YES. + */ +- (BOOL)shouldCacheLoadedImages; + @end /** diff --git a/Libraries/Image/RCTImageLoader.m b/Libraries/Image/RCTImageLoader.m index 78337bdff..1f5c70e86 100644 --- a/Libraries/Image/RCTImageLoader.m +++ b/Libraries/Image/RCTImageLoader.m @@ -287,31 +287,58 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, * path taken. This is useful if you want to skip decoding, e.g. when preloading * the image, or retrieving metadata. */ -- (RCTImageLoaderCancellationBlock)loadImageOrDataWithURLRequest:(NSURLRequest *)imageURLRequest +- (RCTImageLoaderCancellationBlock)loadImageOrDataWithURLRequest:(NSURLRequest *)request size:(CGSize)size 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 dispatch_block_t cancelLoad = nil; __weak RCTImageLoader *weakSelf = self; + // Add missing png extension + if (request.URL.fileURL && request.URL.pathExtension.length == 0) { + NSMutableURLRequest *mutableRequest = [request mutableCopy]; + mutableRequest.URL = [NSURL fileURLWithPath:[request.URL.path stringByAppendingPathExtension:@"png"]]; + request = mutableRequest; + } + + // Find suitable image URL loader + id loadHandler = [self imageURLLoaderForURL:request.URL]; + BOOL requiresScheduling = [loadHandler respondsToSelector:@selector(requiresScheduling)] ? + [loadHandler requiresScheduling] : YES; + void (^completionHandler)(NSError *error, id imageOrData) = ^(NSError *error, id imageOrData) { - if (RCTIsMainQueue()) { + BOOL cacheResult = [loadHandler respondsToSelector:@selector(shouldCacheLoadedImages)] ? + [loadHandler shouldCacheLoadedImages] : YES; + // If we've received an image, we should try to set it synchronously, + // if it's data, do decoding on a background thread. + if (RCTIsMainQueue() && ![imageOrData isKindOfClass:[UIImage class]]) { // 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:request.URL + size:size + scale:scale + resizeMode:resizeMode + progressHandler:progressHandler + completionHandler:completionHandler]; + } + // All access to URL cache must be serialized if (!_URLCacheQueue) { [self setUp]; @@ -323,25 +350,14 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, return; } - // Use a local variable so we can reassign it in this block - NSURLRequest *request = imageURLRequest; - - // Add missing png extension - if (request.URL.fileURL && request.URL.pathExtension.length == 0) { - NSMutableURLRequest *mutableRequest = [request mutableCopy]; - mutableRequest.URL = [NSURL fileURLWithPath:[request.URL.path stringByAppendingPathExtension:@"png"]]; - request = mutableRequest; - } - // Find suitable image URL loader - id loadHandler = [strongSelf imageURLLoaderForURL:request.URL]; if (loadHandler) { cancelLoad = [loadHandler loadImageForURL:request.URL size:size scale:scale resizeMode:resizeMode progressHandler:progressHandler - completionHandler:completionHandler] ?: ^{}; + completionHandler:completionHandler]; } else { // Use networking module to load image cancelLoad = [strongSelf _loadURLRequest:request @@ -530,17 +546,18 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *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]; } } }; @@ -678,7 +695,7 @@ static UIImage *RCTResizeImageIfNeeded(UIImage *image, 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); diff --git a/Libraries/Image/RCTXCAssetImageLoader.h b/Libraries/Image/RCTLocalAssetImageLoader.h similarity index 83% rename from Libraries/Image/RCTXCAssetImageLoader.h rename to Libraries/Image/RCTLocalAssetImageLoader.h index ca982bf03..bf6e5c675 100644 --- a/Libraries/Image/RCTXCAssetImageLoader.h +++ b/Libraries/Image/RCTLocalAssetImageLoader.h @@ -9,6 +9,6 @@ #import "RCTImageLoader.h" -@interface RCTXCAssetImageLoader : NSObject +@interface RCTLocalAssetImageLoader : NSObject @end diff --git a/Libraries/Image/RCTXCAssetImageLoader.m b/Libraries/Image/RCTLocalAssetImageLoader.m similarity index 75% rename from Libraries/Image/RCTXCAssetImageLoader.m rename to Libraries/Image/RCTLocalAssetImageLoader.m index 750a1f848..5eced6987 100644 --- a/Libraries/Image/RCTXCAssetImageLoader.m +++ b/Libraries/Image/RCTLocalAssetImageLoader.m @@ -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 #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 @@ -30,11 +44,11 @@ RCT_EXPORT_MODULE() 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) { diff --git a/Libraries/Network/RCTFileRequestHandler.m b/Libraries/Network/RCTFileRequestHandler.m index 348f46d97..214a6eb36 100644 --- a/Libraries/Network/RCTFileRequestHandler.m +++ b/Libraries/Network/RCTFileRequestHandler.m @@ -30,7 +30,7 @@ RCT_EXPORT_MODULE() { return [request.URL.scheme caseInsensitiveCompare:@"file"] == NSOrderedSame - && !RCTIsXCAssetURL(request.URL); + && !RCTIsLocalAssetURL(request.URL); } - (NSOperation *)sendRequest:(NSURLRequest *)request diff --git a/React/Base/RCTUtils.h b/React/Base/RCTUtils.h index 04705d383..af1aa8e05 100644 --- a/React/Base/RCTUtils.h +++ b/React/Base/RCTUtils.h @@ -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); diff --git a/React/Base/RCTUtils.m b/React/Base/RCTUtils.m index 398f137b0..b2d987729 100644 --- a/React/Base/RCTUtils.m +++ b/React/Base/RCTUtils.m @@ -612,24 +612,15 @@ NSString *__nullable RCTBundlePathForURL(NSURL *__nullable URL) 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) { 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 - return NO; - } - return YES; + return [extension isEqualToString:@"png"] || [extension isEqualToString:@"jpg"]; } RCT_EXTERN NSString *__nullable RCTTempFilePath(NSString *extension, NSError **error)