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

Drop shadow overhaul: improve correctness and performance #2548

Merged
merged 14 commits into from
Oct 19, 2024

Conversation

geomaster
Copy link
Contributor

@geomaster geomaster commented Sep 13, 2024

High-level summary

This PR introduces a large change to how drop shadows are rendered, introducing an applyShadowsToLayers flag which, by analogy to applyOpacitiesToLayers, allows layers to be treated as a whole for the purposes of drop shadows, improving the accuracy and bringing lottie-android in line with other renderers (lottie-web and lottie-ios).

Several different codepaths for different hardware/software combinations are introduced to ensure the fastest rendering available, even on legacy devices.

The calculation of shadow direction with respect to transforms is improved so that the output matches lottie-web and lottie-ios.

Image layers now cast shadows correctly thanks to a workaround to device-specific issues when combining Paint.setShadowLayer() and bitmap rendering.

Even in non-applyShadowsToLayers mode, correctness is improved by allowing the shadow-to-be-applied to propagate in a similar way as alpha. This allows some amount of visual fidelity to be recovered for animations or environments where enabling applyShadowsToLayers is not possible.

A number of issues that caused incorrect rendering in some other cases have been fixed.

Background

Drop shadows in Lottie

Lottie specifies drop shadows as a tuple of (angle, distance, radius, color, alpha), with each element being animatable.

The consensus behavior for the rendering of a layer with a drop shadow, which seems to be mostly respected in lottie-web and lottie-ios, seems to be:

  1. Evaluate the values at the current frame for angle (theta), distance (d), radius (r), color with alpha (C).
  2. Apply the layer transform and render the layer normally to a surface So (original layer).
  3. Copy So to new surface Ss (shadow).
  4. Apply a gaussian blur of radius r' = c * r to Ss, where c is some platform-specific constant intended to normalize blur implementations between platforms. (Ours is 0.33, lottie-web's is 0.25; see Apply scaling factor to drop shadow softness #2541).
  5. Tint Ss with the color and combine the alpha by applying the following for each pixel P: P.rgb = C.rgb * P.a; P.a = C.a * P.a.
  6. Now the shadow is ready on Ss, and needs to be drawn into its final position.
  7. Convert from polar coordinates theta and d into dx and dy, with the 0 position at 12 o'clock: dx = d * cos(theta - pi/2); dy = d*sin(theta - pi/2).
  8. Draw Ss onto Si (intermediate surface) with a translation of (dx, dy).
  9. Draw So (original layer) onto Si with identity transform.
  10. Compose Si into the framebuffer using the layer's alpha and blend mode.

Some non-obvious consequences of the definition above:

  • The angle, distance, and radius are relative to the layer post-transform, not pre-transform. That is, rotating the layer (via its transform) still keeps the same screen-space direction of the shadow, and scaling the layer (via its transform) still keeps the same screen-space shadow blur radius.
  • The drop shadow is not based on any derived outline, so a layer's drop shadow can be seen through its non-fully-opaque pixels. At the same time, reducing the alpha of a pixel in a layer reduces its alpha in the drop shadow.
  • A layer's shadow and the layer do not blend on top of each other on the final canvas in case the layer has a blend mode or alpha. Instead, the shadow and the layer are alpha-blended with each other, and the result is then composited onto the canvas.
    • In case the layer has a normal blend mode, this is equivalent to alpha-blending the layer's shadow and then the shadow onto the canvas separately.

Drop shadows in lottie-android currently

lottie-android's current implementation of drop shadows differs in important ways:

  1. Shadows are applied per-shape. This means that a case like a shape with both fill and stroke has incorrect shadows, since both the fill and the stroke render a separate shadow on top of each other.
  2. Precomp layer shadows are ignored. This means that a precomp cannot cause any of its child shapes to cast a shadow. This is a consequence of the current implementation of (1).
  3. Image layers do not render correct shadows, due to the minefield that is the support matrix (or in Android's case, a more apt name would be a support tensor) of Android's graphics stack - setShadowLayer() simply doesn't work for images consistently. (See the last image in Improve drop shadow effect accuracy #2523 (comment).)

Contributions of this PR

This PR introduces the following improvements and additions.

  1. Move the drop shadow model from individual content elements to layers, and add some missing keypath callbacks. This is a prerequisite for handling drop shadows on a layer level.
  2. An OffscreenLayer implementation, which serves as an abstraction that can replace canvas.saveLayer() for off-screen rendering and composition onto the final bitmap, but with the important distinction that it can also handle drop shadows, and possibly use hardware-accelerated RenderNodes and RenderEffects where available.
    • To use an OffscreenLayer, call its .start() method with a parent canvas and a ComposeOp, and draw on the returned canvas. Once finished, call OffscreenLayer.finish() to compose everything from the returned canvas to the parent canvas, applying alpha, blend mode, drop shadows, and color filters.
    • OffscreenLayer makes a dynamic decision on what to use for rendering - a no-op, forward to .saveLayer(), a HW-accelerated RenderNode, or a software bitmap, depending on the requested ComposeOp and hardware/SDK support.
    • The hope is that OffscreenLayer becomes a useful abstraction that can be extended to e.g. support hardware blurs, multiple drop shadows, or to support mattes in a hardware-accelerated fashion where possible.
  3. The applyShadowsToLayers flag which, by analogy to applyOpacityToLayers, turns on a more accurate mode that implements the drop shadow algorithm described above.
    • OffscreenLayer is used to apply alpha if applyOpacityToLayers is enabled, and to apply shadows if applyShadowsToLayers is enabled. The cost is paid only once if both alpha and drop shadows are present on a layer.
    • Not all saveLayer() calls in the code have been rewritten to use OffscreenLayer - the blast radius is minimized. OffscreenLayer is presently used only to apply alpha and drop shadows, and blend mode and color filters are still applied in BaseLayer using saveLayer() directly.
  4. More accurate shadow transformations. Previously, the angle and distance were pre-transform, and only the radius was post-transform (contrary to step (2) of the algorithm). We correct this to match other renderers.
  5. More complete shadow handling even when applyShadowsToLayers is false: we plumb the shadow through .draw() and drawLayer() calls similarly to alpha, and this allows us to render per-shape shadows on children of composition layers too.
  6. *Workaround for drop shadows on image layers.
    • The workaround relies on OffscreenLayer as well, and image layers now render shadows properly in all cases.
  7. Fixes to a few subtle issues causing incorrect rendering in other cases. (will be marked using PR comments, I might have forgotten some)

Open questions

  • Should applyShadowsToLayers be true by default? Some codepaths, such as when rendering purely via software, can be slow if shadow-casting layers are exceedingly large. But, the performance is still acceptable, and in the vast majority of cases everything is quite snappy.
  • Have I introduced any regressions? The snapshot tests should answer this.
  • How does this perform on older devices? applyShadowsToLayers plus an old device should trigger the purely-software shadow rendering mode. Simulating this in condition manually yields accurate results, and the performance seems surprisingly good, but it's unclear what will happen on a lower-end phone. There's also always the possibility of some device subtlety being missed. I don't have access to an older Android device.

Testcases

These files now match between lottie-web and lottie-android:

drop_shadow_comparator.json

simple_shadow_casters_ll2.json

The files from this earlier PR still all render the same: #2523, with the exception of the fix for image layer bug, which fixes the rendering of the Map icon as mentioned in the comment of that PR.

This file has been used as a perf stress test with many <255 opacity precomps, some stacked inside each other, that must all be blended separately: precomp_opacity_killer.json

paint.setAlpha(clamp(alpha, 0, 255));
float strokeAlpha = opacityAnimation.getValue() / 100f;
int alpha = (int) (parentAlpha * strokeAlpha);
alpha = clamp(alpha, 0, 255);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applying this clamp right away, here and elsewhere, fixes issues with >255 or <0 alpha values, which caused shadows to not appear.

Such alpha values are possible when interpolating using an easing that evaluates to <0 or >1 at some point along its curve.

getBounds(offScreenRectF, matrix, true);
offScreenPaint.setAlpha(layerAlpha);
Utils.saveLayerCompat(canvas, offScreenRectF, offScreenPaint);
getBounds(offScreenRectF, parentMatrix, true);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one got lost in the diff, but is very important - matrix was being passed here, causing incorrect transformations being applied when applyOpacitiesToLayers is true; see the precomp_opacity_killer.json testcase.

@@ -17,6 +17,8 @@
<attr name="lottie_imageAssetsFolder" format="string" />
<attr name="lottie_progress" format="float" />
<attr name="lottie_enableMergePathsForKitKatAndAbove" format="boolean" />
<attr name="lottie_applyOpacityToLayers" format="boolean" />
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was missing for applyOpacityToLayers

Copy link

Snapshot Tests
API 23: Report Diff
API 31: Report Diff

Copy link
Collaborator

@gpeal gpeal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, this is really amazing work. I really appreciate it and also how this can directly lead to further improvements for mattes and masks.

I left a few comments but a couple of broad themes:

  1. Take a look at the warnings in Android Studio and try to make them all go away. I try to keep the gutter "green" so that it's easy to spot new issues
  2. There are a bunch of new allocations in the hot path. Calls to getMatrix() are one example but there are also RectFs, float arrays, etc. It would be great to keep the hot path clean of allocations
  3. Run the Android Studio formatter on the new files to fix a few consistency issues

Looking forward to working with you to get this in!

@gpeal
Copy link
Collaborator

gpeal commented Sep 15, 2024

Also, I'm not sure why the snapshot diff against master is so big but if you compare it to 453b43c which is at
https://happo.io/a/27/p/27/compare/453b43cbf0003ce182e37a72d9fc43b882d39187-android31/ca1b2040a05692731c919a90ad0c066615f43797-android31?t=diffs

You can see a much smaller snapshot diff. I just scanned it and the diffs look good to me! Feel free to scan them yourself as well.

@geomaster
Copy link
Contributor Author

@gpeal Thanks for the review, Gabriel! Addressed all of the comments. The snapshot diffs look fine to me as well, but I'd also like to see another run with the latest round of changes, to ensure nothing was regressed accidentally.

Copy link
Collaborator

@gpeal gpeal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really great work! One small comment to clean up some code and then rebase and I'm excited to see the snapshot diffs.

Run ./gradlew apiDump and it'll update the .api file and CI will pass. Again, I need to figure out a sustainable way to maintain abi backwards compatibility but I don't want to block this PR on that.

Comment on lines +270 to +274
if (needNewBitmap(bitmap, scaledBounds)) {
if (bitmap != null) {
deallocateBitmap(bitmap);
}
bitmap = allocateBitmap(scaledBounds, Bitmap.Config.ARGB_8888);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (needNewBitmap(bitmap, scaledBounds)) {
if (bitmap != null) {
deallocateBitmap(bitmap);
}
bitmap = allocateBitmap(scaledBounds, Bitmap.Config.ARGB_8888);
if (needNewBitmap(bitmap, scaledBounds)) {
if (bitmap != null) {
deallocateBitmap(bitmap);
}
bitmap = allocateBitmap(scaledBounds, Bitmap.Config.ARGB_8888);

I think it would be great if you could encapsulate the needNew + deallocate + allocate into a single helper that returns the bitmap.

Then, in each of the functions that calls the new function (something like getBitmap(Rect), it would just use the return value as a local variable rather than using fields on the object

This commit introduces a large change to how drop shadows are rendered,
introducing an `applyShadowsToLayers` flag which, by analogy to
`applyOpacitiesToLayers`, allows layers to be treated as a whole for the
purposes of drop shadows, improving the accuracy and bringing
lottie-android in line with other renderers (lottie-web and lottie-ios).

Several different codepaths for different hardware/software combinations
are introduced to ensure the fastest rendering available, even on legacy
devices.

The calculation of shadow direction with respect to transforms is
improved so that the output matches lottie-web and lottie-ios.

Image layers now cast shadows correctly thanks to a workaround to
device-specific issues when combining `Paint.setShadowLayer()` and
bitmap rendering.

Even in non-`applyShadowsToLayers` mode, correctness is improved by
allowing the shadow-to-be-applied to propagate in a similar way as
alpha. This allows some amount of visual fidelity to be recovered for
animations or environments where enabling `applyShadowsToLayers` is not
possible.

A number of issues that caused incorrect rendering in some other cases
have been fixed.
Copy link

github-actions bot commented Oct 1, 2024

Snapshot Tests
API 23: Report Diff
API 31: Report Diff

When rendering a view, the Android graphics stack, by convention, gives
it a canvas where the transformation to put the view in the right
position is applied. It also allows the view to draw with its intrinsic
width and height in pixels, and the canvas has an inverse scaling
transform to draw to the final surface with the right size.

Thus, the transform matrices we work with during hardware rendering
exclude this inverse scaling.

In software rendering mode, we bake this scaling into the root matrix,
which causes different values to appear in software vs hardware
rendering, which:
* Has an impact on the final image due to e.g. integer rounding
* Makes debugging HW/SW differences trickier, as values are different.

To make the modes closer, "unbake" the pre-existing scale of the canvas
from the initial root matrix.

To mitigate risk, this is only done for scale, and not for the full
matrix itself, as there is code which might interact with other types of
transforms in unexpected ways. (Whereas scaling is a "safer" one.) This
can be done in the future to achieve 100% equal values between the two
modes.
`ImageLayer.getBounds()` works with a parent-to-world matrix called
`parentMatrix`, but `ImageLayer.drawLayer()` gets called with
layer-to-world as `parentMatrix`, despite the name. (See call to
`drawLayer() `in `BaseLayer.draw()`).

This caused `getBounds()` to apply the layer's own transform effectively
two times and the returned bounds to be incorrect.

With non-shadowed layers, this was masked away by the fact that
`Canvas.saveLayer()` appears to ignore the provided bounds Rect, and
allows anything drawn anywhere on the canvas to appear after a
corresponding `canvas.restore()`.

On top of this, we erroneously multiplied the returned `bounds` by the
density again, which meant that the final bitmap was "overprovisioned"
by a factor of 2-3 (depending on the device's pixel density). In most
cases, and combined with an erroneous order of operations when applying
matrices to the original and off-screen canvases, this was enough to
hide the problem even on shadowed layers.

Fix by:
* Not relying on `getBounds()`, as we don't have a correct
  `parentMatrix` to feed it.
* Correcting the order of operations, so that we respect the convention
  of not applying any matrix to the canvas before calling
  `OffscreenLayer.start()`, which simplifies the code and allows us to
  use the same pattern as other invocations of `OffscreenLayer`.
* Getting rid of the unnecessary scale by `density` and then
  normalization back, borne out of a misunderstanding of why `density`
  is a factor in this calculation. (It effectively gets applied to *all*
  values somewhere in the pipeline, and has nothing to do with images
  per se.)

Drive-by fix `getBounds()` itself to respect
`maintainOriginalImageBounds`.
Copy link

Snapshot Tests
API 23: Report Diff
API 31: Report Diff

In this case, we need to calculate the actual bounds of the children
layers, and pass that as the clip rectangle to both `OffscreenLayer` and
`Canvas.clipRect()`.
Commit 9527586 introduced an issue with transformation order, causing
View-wide translations to be misapplied in software rendering.
Copy link

Snapshot Tests
API 23: Report Diff
API 31: Report Diff

Copy link

Snapshot Tests
API 23: Report Diff
API 31: Report Diff

Second strike... third strike I just remove it
Copy link

Snapshot Tests
API 23: Report Diff
API 31: Report Diff

Mostly seen on small renders.
Copy link

Snapshot Tests
API 23: Report Diff
API 31: Report Diff

Copy link
Collaborator

@gpeal gpeal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is truly fantastic work. I genuinely thank you for the hours you put into this. This is, without any doubt, the largest external contribution to this repo ever and it looks great. I hope we can build on this to get render node wins for mattes and masks.

@gpeal gpeal merged commit 5deb2de into airbnb:master Oct 19, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants