Animated WebP support #605

garrettmoon · 2017-10-07T18:55:59Z

#117.

ghost · 2017-10-07T18:57:58Z

🚫 CI failed with log

djblake · 2017-10-08T13:45:57Z

@garrettmoon from my testing:

Fixed:

Transition behaviour now working properly again, button images are no longer being unloaded when a button is removed from the layer hierarchy.
No longer seeing flashing when removing an image from one parent and directly adding it to another parent.

Possible issues:

When moving a gif from one parent and immediately adding it to another I still see a flash (a double flash actually) as the image is presumably being unloaded when removeFromSupernode: triggers the didExitDisplayState which removes the cover image. Not sure if anything can be done about this as obviously still need the memory benefit of doing this when moving ASCellNodes outside of display range. Unfortunately displaysAsynchronously = YES seems to have no effect on the flashing like it did when the regular images had this problem. That would probably be the optimum fix for this if it worked. A hack for this is probably if I place the the gifs in a container ASDisplayNode and call removeFromSupernode: on that node instead, fooling the gifs into thinking they still belong to the same parent and never calling didExitDisplayState.
Not sure what can be done about this, but I can still crash (due to memory overload) that test example project I made with the 1000 gifs in a ASCollectionNode if I scroll as fast as I can to the bottom. Only happens when loading the gif through .URL =, presumably because the loading from the memory cache occurs after its already left the display range. It is worth noting however that the current master branch does not exhibit this behaviour, i.e. I can scroll as far as I can to the bottom with that and the memory stays completely level.

Other than those, all looking good!

garrettmoon · 2017-10-08T17:22:49Z

Thanks for the feedback @djblake! The second issue you brought up is something I want to look at. The first is a little harder to deal with. I could make displaysAsynchronously NO work, but it probably would really mess up the performance of any transitions. I'd recommend a workaround of copying the node's layer contents before removing it and setting them after adding it back in.

garrettmoon · 2017-10-08T17:56:58Z

@djblake Turns out #2 is actually a separate issue, probably only encountered more often on GIFs! Fix is here: #606

nguyenhuy · 2017-10-09T16:07:29Z

examples/AnimatedGIF/ASAnimatedImage/ViewController.m

@@ -32,7 +30,9 @@ - (void)viewDidLoad {
  // Do any additional setup after loading the view, typically from a nib.

  ASNetworkImageNode *imageNode = [[ASNetworkImageNode alloc] init];
-  imageNode.URL = [NSURL URLWithString:@"https://s-media-cache-ak0.pinimg.com/originals/07/44/38/074438e7c75034df2dcf37ba1057803e.gif"];
+//    imageNode.URL = [NSURL URLWithString:@"https://s-media-cache-ak0.pinimg.com/originals/07/44/38/074438e7c75034df2dcf37ba1057803e.gif"];
+//  imageNode.URL = [NSURL URLWithString:@"https://storage.googleapis.com/downloads.webmproject.org/webp/images/dancing_banana2.lossless.webp"];


Let's remove these lines.

I think I'll remove one and add a comment to the other one saying "uncomment to see animated webp support"?

ghost · 2017-10-09T17:43:19Z

🚫 CI failed with log

* Updating to support animated WebP * Fix a deadlock with display link * Fix playhead issue. * Fix up timing on iOS 10 and above * Don't redraw the same frame over and over * Clear out layer contents if we're an animated GIF on exit range * Clear out cover image on exit of visible range * Don't set cover image if we're no longer in display range. * Don't clear out image if we're not an animated image * Only set image if we're not already animating * Get rid of changes to podfile * Add CHANGELOG entry * Update license * Update PINRemoteImage * Remove commented out lines in example

@djblake

…nds changes. (#431) * [ASCollectionView] Improve performance and behavior of rotation / bounds changes. See #430 for details. * Edit CHANGELOG.md * [ASDataController] Implement -relayoutAllNodesWithInvalidationBlock:, to flush the ASMainSerialQueue before -invalidateLayout is called. * Don't set download results if no longer in preload range. (#606) Good catch by @djblake, if you scroll fast enough, you leave images set on the image node because didExitPreloadRange (which would have cleared it) was already called. * Animated WebP support (#605) * Updating to support animated WebP * Fix a deadlock with display link * Fix playhead issue. * Fix up timing on iOS 10 and above * Don't redraw the same frame over and over * Clear out layer contents if we're an animated GIF on exit range * Clear out cover image on exit of visible range * Don't set cover image if we're no longer in display range. * Don't clear out image if we're not an animated image * Only set image if we're not already animating * Get rid of changes to podfile * Add CHANGELOG entry * Update license * Update PINRemoteImage * Remove commented out lines in example * [ASDataController] Add nullable specifier to invalidationBlock for relayout of nodes.

* Updating to support animated WebP * Fix a deadlock with display link * Fix playhead issue. * Fix up timing on iOS 10 and above * Don't redraw the same frame over and over * Clear out layer contents if we're an animated GIF on exit range * Clear out cover image on exit of visible range * Don't set cover image if we're no longer in display range. * Don't clear out image if we're not an animated image * Only set image if we're not already animating * Get rid of changes to podfile * Add CHANGELOG entry * Update license * Update PINRemoteImage * Remove commented out lines in example

@djblake

…nds changes. (TextureGroup#431) * [ASCollectionView] Improve performance and behavior of rotation / bounds changes. See TextureGroup#430 for details. * Edit CHANGELOG.md * [ASDataController] Implement -relayoutAllNodesWithInvalidationBlock:, to flush the ASMainSerialQueue before -invalidateLayout is called. * Don't set download results if no longer in preload range. (TextureGroup#606) Good catch by @djblake, if you scroll fast enough, you leave images set on the image node because didExitPreloadRange (which would have cleared it) was already called. * Animated WebP support (TextureGroup#605) * Updating to support animated WebP * Fix a deadlock with display link * Fix playhead issue. * Fix up timing on iOS 10 and above * Don't redraw the same frame over and over * Clear out layer contents if we're an animated GIF on exit range * Clear out cover image on exit of visible range * Don't set cover image if we're no longer in display range. * Don't clear out image if we're not an animated image * Only set image if we're not already animating * Get rid of changes to podfile * Add CHANGELOG entry * Update license * Update PINRemoteImage * Remove commented out lines in example * [ASDataController] Add nullable specifier to invalidationBlock for relayout of nodes.

nguyenhuy approved these changes Oct 9, 2017

View reviewed changes

garrettmoon and others added 14 commits October 9, 2017 09:30

Updating to support animated WebP

01bb2af

Fix a deadlock with display link

f53ffef

Fix playhead issue.

67a7653

Fix up timing on iOS 10 and above

71e28f3

Don't redraw the same frame over and over

c7879b3

Clear out layer contents if we're an animated GIF on exit range

72146f6

Clear out cover image on exit of visible range

a8cc6ef

Don't set cover image if we're no longer in display range.

13066d1

Don't clear out image if we're not an animated image

5ce7e53

Only set image if we're not already animating

65152f0

Get rid of changes to podfile

bfc1242

Add CHANGELOG entry

127f853

Update license

bc0fbd9

Update PINRemoteImage

8448579

garrettmoon force-pushed the animatedWebPSupport branch from 96f0456 to 8448579 Compare October 9, 2017 17:13

Remove commented out lines in example

a5f12de

garrettmoon merged commit 4dbb644 into master Oct 9, 2017

nguyenhuy mentioned this pull request Jan 16, 2018

[ASImageNode / PINRemoteImage] Support WebP. #117

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Animated WebP support #605

Animated WebP support #605

garrettmoon commented Oct 7, 2017 •

edited by nguyenhuy

Loading

ghost commented Oct 7, 2017

djblake commented Oct 8, 2017 •

edited

Loading

garrettmoon commented Oct 8, 2017

garrettmoon commented Oct 8, 2017

nguyenhuy Oct 9, 2017

garrettmoon Oct 9, 2017

ghost commented Oct 9, 2017

Animated WebP support #605

Animated WebP support #605

Conversation

garrettmoon commented Oct 7, 2017 • edited by nguyenhuy Loading

ghost commented Oct 7, 2017

djblake commented Oct 8, 2017 • edited Loading

Fixed:

Possible issues:

garrettmoon commented Oct 8, 2017

garrettmoon commented Oct 8, 2017

nguyenhuy Oct 9, 2017

Choose a reason for hiding this comment

garrettmoon Oct 9, 2017

Choose a reason for hiding this comment

ghost commented Oct 9, 2017

garrettmoon commented Oct 7, 2017 •

edited by nguyenhuy

Loading

djblake commented Oct 8, 2017 •

edited

Loading