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

[ASImageNode] Always dealloc images in a background queue #561

Merged
Merged
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
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

##2.4
- Fix an issue where inserting/deleting sections could lead to inconsistent supplementary element behavior. [Adlai Holler](https://github.com/Adlai-Holler)
Expand Down
17 changes: 14 additions & 3 deletions Source/ASImageNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@

#include <functional>

static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0};

typedef void (^ASImageNodeDrawParametersBlock)(ASWeakMapEntry *entry);

@interface ASImageNodeDrawParameters : NSObject {
Expand Down Expand Up @@ -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];
Expand All @@ -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
Expand Down
16 changes: 1 addition & 15 deletions Source/ASNetworkImageNode.mm
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@
#import <AsyncDisplayKit/ASPINRemoteImageDownloader.h>
#endif

static const CGSize kMinReleaseImageOnBackgroundSize = {20.0, 20.0};

@interface ASNetworkImageNode ()
{
// Only access any of these with __instanceLock__.
Expand Down Expand Up @@ -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;

Expand Down