[ASNetworkImageNode] Issue with setting defaultImage as a resizable image

[garrettmoon]

From @tomizimobile on March 10, 2016 19:42

I have an ASNetworkImageNode that uses a resizable image as the defaultImage. The problem is that once the image is loaded, it will sometimes be shown with the wrong content mode. I dug around a little bit, and I think it's because in _ASCoreAnimationExtras.mm, ASDisplayNodeSetupLayerContentsWithResizableImage, the layer.contentsGravity is set to kCAGravityResize, and there's not another point where the contentsGravity/contentMode are reset when the image is loaded. Maybe if there's some kind of pending state that tracks contentMode changes until the image is loaded and then applied, this would fix it. For the time being, I have a workaround that avoids using resizable images as the defaultImage.

Copied from original issue: facebookarchive/AsyncDisplayKit#1362

[garrettmoon]

From @appleguy on April 7, 2016 7:11

@tomizimobile - this is a great find and I think your analysis is correct. Do you have any current ideas on how we might fix this? I believe it is probably affecting a few other areas as the function you mentioned is used elsewhere.

Note that there is very large memory cost to making even a single color image full-size rather than stretchable, so you might consider using -placeholderImage (you have to subclass a node and override this method to return the placeholderImage). This uses a separate placeholder layer and does wait for the image to load from the network. You probably want this behavior anyways as it will decode synchronously as defaultImage is asynchronous.

The main benefit of defaultImage is that it will remain visible even if no URL is set or if the image load fails. This is not the intended use of the placeholder, which will only exist during loading or decoding. But it should not be difficult to catch this case and show a default profile picture or whatever.

[garrettmoon]

From @tomizimobile on April 7, 2016 13:43

@appleguy The only idea to fix this that I have right now is what I mentioned above - to use another variable in ASImageNode to track changes in contentMode, and apply it in setImage:. I'm not sure what other things that might affect though, or what the performance impact would be - I'm sure there's optimizations to be made there. Unfortunately I don't have enough time to try out any fixes at the moment.

I was using a small, single color image along with the contentMode to stretch it rather than using a full-size image, although I'm not sure of the memory cost of that vs. stretchable images. I decided to use defaultImage so I wouldn't have to worry about what happens when the image load fails, but I definitely see the advantage now to using placeholderImage and handling the error case separately - thanks for the tip.

[garrettmoon]

From @appleguy on June 26, 2016 19:23

@tomizimobile: would you be willing to post a PR with your suggested fix?