Fix Swift performance issues. Remove renderScale and contentsScale

[absolute-heike]

Related to #1325 #895 #1060 #1314 (and possibly others)

lottie-swift sets layer.contentsScale on every sublayer. ObjC didn't do that and this seems to explain the performance regression from 2.x to 3.x. Removing that code, I haven't found any regressions whatsoever while at the same time fixing the performance issues.

Profiling these changes gave the following insights:

• Swift contentsScale 3.0: ~70% CPU Utilization
• Swift contentScale 1.0: ~40% CPU Utilization
• ObjC (defaults to scale 1.0): ~37% CPU Utilization

[absolute-heike]

@buba447 Seems like @JoeSzymanski found the issue for the performance regression. We are unsure about the best way to tackle this.

I could not distinguish regular Lottie animations on a pixel level when removing contentsScale (Which is not what I would have expected, so I am wondering whether I overlooked something 😅 ). Only TextLayers become blurry, which is why I removed screenScale everywhere except for TextLayers.

Joes approach was to make renderScale publicly available. #1327
Which seems like a cleaner solution from a code point of view but has some downsides for usability. Most engineers probably won't know about this setting and you have to trade performance for blurry texts ¯\_(ツ)_/¯

[leberwurstsaft]

According to the docs, there is no need to change this value for layers backing views.

https://developer.apple.com/documentation/quartzcore/calayer/1410746-contentsscale

The default value of this property is 1.0. For layers attached to a view, the view changes the scale factor automatically to a value that is appropriate for the current screen. For layers you create and manage yourself, you must set the value of this property yourself based on the resolution of the screen and the content you are providing. Core Animation uses the value you specify as a cue to determine how to render your content.

[joeldrotleff]

This is working great for me. I had severe performance issues in loading and playback that went away completely after using this PR. Much thanks!

[buba447]

Nice catch here! Just a few comments to clean this up so we can get it merged.

The original purpose of the renderScale property was to provide every layer type the ability to set its own render scale accordingly. This pr removes the renderScale all together. Instead you should be able to remove the calls to self.contentsScale = self.renderScale inside of updateRenderScale() for CompositionLayer ShapeContainerLayer and ShapeRenderLayer. This should fix the render issues while keeping a clean way for lottie objects to know about the render scale (such as the text layer)

[absolute-heike]

@buba447 Great suggestions. Looks much cleaner now 😊

[marksands]

@buba447

👋 This commit introduces a visual regression and should be reverted.

I could not distinguish regular Lottie animations on a pixel level when removing contentsScale (Which is not what I would have expected, so I am wondering whether I overlooked something 😅 ). Only TextLayers become blurry, which is why I removed screenScale everywhere except for TextLayers.

@absolute-heike was on the right track initially. With retina assets on gradients we are definitely seeing scaling artifacts that makes smooth circles blurry.

I think the linked PR that allows clients to opt-out of the render scale is a much better solution: #1327 so devs can get the precise performance metrics they prefer and understand the tradeoffs.

The docs here are a little confusing, but it's differentiating between layer backed views and created sublayers. Only layer backed views are automatically set to the correct contentScale. Since lottie animations are all sublayers, then all layers are no longer scaled for retina devices.

This means all bitmaps are now 4x or 9x smaller on retina screens, so of course there will be less CPU utilization. For performance, we've seen success by setting some layers to drawsAsynchronously, but that's a hammer you should use with caution.

[ghost]

Any thoughts on @marksands comment? @buba447

[calda]

Hi all, I reverted this PR since it introduced some visual regressions (#1407). We now have snapshot tests that run on every PR (#1432) and should help catch these sorts of issues in the future.