From c121401e154f98cfa9ea6181c697c99b13efcae8 Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Mon, 11 Sep 2017 21:33:30 +0100 Subject: [PATCH] [ASImageNode] Always dealloc images in a background queue (#561) * ASImageNode to always dealloc its images in a background queue * Update CHANGELOG --- CHANGELOG.md | 4 +++- Source/ASImageNode.mm | 17 ++++++++++++++--- Source/ASNetworkImageNode.mm | 16 +--------------- 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16e657169..648ec0b0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,8 +19,10 @@ - [ASDisplayNode] Ensure `-displayWillStartAsynchronously:` and `-displayDidFinish` are invoked on rasterized subnodes. [Eric Scheers](https://github.com/smeis) [#532](https://github.com/TextureGroup/Texture/pull/532) - Fixed a memory corruption issue in the ASImageNode display system. [Adlai Holler](https://github.com/Adlai-Holler) [#555](https://github.com/TextureGroup/Texture/pull/555) - [Breaking] Rename ASCollectionGalleryLayoutSizeProviding to ASCollectionGalleryLayoutPropertiesProviding. Besides a fixed item size, it now can provide interitem and line spacings, as well as section inset [Huy Nguyen](https://github.com/nguyenhuy) [#496](https://github.com/TextureGroup/Texture/pull/496) [#533](https://github.com/TextureGroup/Texture/pull/533) -- Deprecate `-[ASDisplayNode displayWillStart]` in favor of `-displayWillStartAsynchronously:` [Huy Nguyen](https://github.com/nguyenhuy) [536](https://github.com/TextureGroup/Texture/pull/536) +- Deprecate `-[ASDisplayNode displayWillStart]` in favor of `-displayWillStartAsynchronously:` [Huy Nguyen](https://github.com/nguyenhuy) [#536](https:/ +/github.com/TextureGroup/Texture/pull/536) - Add support for URLs on ASNetworkImageNode. [Garrett Moon](https://github.com/garrettmoon) +- [ASImageNode] Always dealloc images in a background queue [Huy Nguyen](https://github.com/nguyenhuy) [#561](https://github.com/TextureGroup/Texture/pull/561) - Mark ASRunLoopQueue as drained if it contains only NULLs [Cesar Estebanez](https://github.com/cesteban) [#558](https://github.com/TextureGroup/Texture/pull/558) ##2.4 diff --git a/Source/ASImageNode.mm b/Source/ASImageNode.mm index dce952d2c..165d74535 100644 --- a/Source/ASImageNode.mm +++ b/Source/ASImageNode.mm @@ -41,6 +41,8 @@ #include +static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; + typedef void (^ASImageNodeDrawParametersBlock)(ASWeakMapEntry *entry); @interface ASImageNodeDrawParameters : NSObject { @@ -248,11 +250,11 @@ - (void)_locked_setImage:(UIImage *)image if (ASObjectIsEqual(_image, image)) { return; } - + + UIImage *oldImage = _image; _image = image; if (image != nil) { - // We explicitly call setNeedsDisplay in this case, although we know setNeedsDisplay will be called with lock held. // Therefore we have to be careful in methods that are involved with setNeedsDisplay to not run into a deadlock [self setNeedsDisplay]; @@ -265,10 +267,19 @@ - (void)_locked_setImage:(UIImage *)image [self addSubnode:_debugLabelNode]; }); } - } else { self.contents = nil; } + + // Destruction of bigger images on the main thread can be expensive + // and can take some time, so we dispatch onto a bg queue to + // actually dealloc. + CGSize oldImageSize = oldImage.size; + BOOL shouldReleaseImageOnBackgroundThread = oldImageSize.width > kMinReleaseImageOnBackgroundSize.width + || oldImageSize.height > kMinReleaseImageOnBackgroundSize.height; + if (shouldReleaseImageOnBackgroundThread) { + ASPerformBackgroundDeallocation(oldImage); + } } - (UIImage *)image diff --git a/Source/ASNetworkImageNode.mm b/Source/ASNetworkImageNode.mm index 0e9c1fb1f..d96932276 100755 --- a/Source/ASNetworkImageNode.mm +++ b/Source/ASNetworkImageNode.mm @@ -32,8 +32,6 @@ #import #endif -static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0}; - @interface ASNetworkImageNode () { // Only access any of these with __instanceLock__. @@ -521,21 +519,9 @@ - (void)_locked_cancelDownloadAndClearImageWithResumePossibility:(BOOL)storeResu { [self _locked_cancelImageDownloadWithResumePossibility:storeResume]; - // Destruction of bigger images on the main thread can be expensive - // and can take some time, so we dispatch onto a bg queue to - // actually dealloc. - UIImage *image = [self _locked_Image]; - UIImage *defaultImage = _defaultImage; - CGSize imageSize = image.size; - BOOL shouldReleaseImageOnBackgroundThread = imageSize.width > kMinReleaseImageOnBackgroundSize.width || - imageSize.height > kMinReleaseImageOnBackgroundSize.height; - if (shouldReleaseImageOnBackgroundThread) { - ASPerformBackgroundDeallocation(image); - } - [self _locked_setAnimatedImage:nil]; [self _locked_setCurrentImageQuality:0.0]; - [self _locked__setImage:defaultImage]; + [self _locked__setImage:_defaultImage]; _imageLoaded = NO;