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

Improve performance of tint() #5471

Merged
merged 1 commit into from
Jul 30, 2022

Conversation

davepagurek
Copy link
Contributor

Addresses #3610

Changes:

  • Replaces the implementation of _getTintedImageCanvas to use globalCompositeOperation instead of modifying the pixels array
  • Removes a duplicated implementation of _getTintedImageCanvas
  • Adds unit tests to check that the multiplication result is correct
  • Adds a performance test for tint

PR Checklist

  • npm run lint passes
  • [Inline documentation] is included / updated
  • [Unit tests] are included / updated

Explanation

Benchmarks

I added test/manual-test-examples/tint-performance to demonstrate the issue and the effect of the fix. This is the same as test/manual-test-examples/tint, but uses larger source images to demonstrate the problem, shows the frame rate in the corner, and also translates the images over time so you can see the dropped frames.

The old tint implementation gets 8fps on my 2015 Macbook Pro. The new implementation gets 20fps (still not perfect, but much better.)

Here is an online version of this benchmark: https://editor.p5js.org/davepagurek/sketches/D_ehdpTjO Try commenting out the redefinition of _getTintedImageCanvas to see the performance of the original.

Why is the implementation so complicated?

At first I thought the whole tint operation could be replaced with a single multiply blend mode to apply the tint color, and then using globalAlpha to apply the alpha tint. However, the multiply blend mode destroys the alpha channel in the process and mixes the colors with white before applying the tint:

Left: an image with transparency, but after drawing a white rectangle on top with the `multiply` blend mode. Right: the same image drawn normally.

This is not a result of the top layer having full opacity. Even when the top layer has alpha values that match the bottom layer, it still blends the bottom layer with white before applying the tint and then finally applying the top layer's alpha. This would lead to semi-transparent parts of the tinted image getting progressively whiter as they fade out:

This time, the white rectangle has the same alpha as the original image. Note how the semitransparent section is whiter on the left (using `multiply`) compared to the right (the original image.)

I address this by reconstructing a fully opaque version of the original image before doing the color tint by rendering the image three times: first the original, then with the luminosity and chroma blend modes, which also wipe out the alpha channel, but restore the original color. (Unfortunately, due to the canvas storing colors with premultiplied alpha, we lose color precision by doing this.) Then we can tint the color without any white mixing in, and then bring the alpha back later with the destination-in blend mode.

Left: the four-pass render. It matches the original on the right finally!

So, yes, it takes four render passes to achieve what we did before by looping through pixels. I've tried to add a lot of comments explaining this since it's not the most intuitive thing to read. In practice, this method is still faster, showing a >2x speedup on my machine for color tints.

Since some people want to use tint just to apply opacity, I added a branch to optimize this by drawing just using globalAlpha. This branch is quite fast and hits 60fps for me!

Future work

The four-pass render is only necessary because the multiply blend mode method breaks images that have semitransparent sections. We could make this even faster still if we know images don't have any semitransparent pixels, but that would require having a fast way of checking for it, and looping through pixels is slow. If anyone has any ideas how to detect that, it would speed up what is probably the more common use case!

@Qianqianye Qianqianye requested a review from stalgiag April 2, 2022 20:46
@Qianqianye
Copy link
Contributor

Thanks @davepagurek! Adding the Core/Environment/Rendering Stewards @limzykenneth and @jeffawang to review this PR.

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