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

Avoid going back to the GPU process for canvas ImageData when possible #16028

Merged
merged 1 commit into from
Aug 7, 2023

Conversation

heycam
Copy link
Contributor

@heycam heycam commented Jul 24, 2023

5466cd2

Avoid going back to the GPU process for canvas ImageData when possible
https://bugs.webkit.org/show_bug.cgi?id=259436
rdar://problem/112746729

Reviewed by Cameron McCormack and Myles C. Maxfield.

On a 2D canvas, if a recent putImageData() was made with no other
intervening drawing commands, we can use its data to fulfill a
subsequent getImageData(). Start with some simple conditions for
ImageData size and drawing dimensions, which can be extended later if it
proves useful.

* LayoutTests/fast/canvas/canvas-ImageData-cache-expected.txt: Added.
* LayoutTests/fast/canvas/canvas-ImageData-cache.html: Added.
* Source/WebCore/html/ImageData.cpp:
(WebCore::ImageData::clone const):
* Source/WebCore/html/ImageData.h:
* Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:
(WebCore::CanvasRenderingContext2DBase::reset):
(WebCore::CanvasRenderingContext2DBase::didDraw):
(WebCore::CanvasRenderingContext2DBase::cacheImageDataIfPossible):
(WebCore::CanvasRenderingContext2DBase::takeCachedImageDataIfPossible const):
(WebCore::CanvasRenderingContext2DBase::getImageData const):
(WebCore::CanvasRenderingContext2DBase::putImageData):
* Source/WebCore/html/canvas/CanvasRenderingContext2DBase.h:

Canonical link: https://commits.webkit.org/266632@main

be9ac84

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug   πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ›  gtk
βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1   πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-wk2-stress
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@heycam heycam requested review from cdumez and rniwa as code owners July 24, 2023 01:18
@heycam heycam self-assigned this Jul 24, 2023
@heycam heycam added the Canvas Bugs related to the canvas element. label Jul 24, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jul 24, 2023
@litherum litherum removed the merging-blocked Applied to prevent a change from being merged label Aug 4, 2023
@litherum litherum self-assigned this Aug 4, 2023
@litherum litherum marked this pull request as draft August 4, 2023 18:04
@litherum
Copy link
Contributor

litherum commented Aug 4, 2023

I think there's still a bit more to do here, even if EWS is green:

  1. Only take this codepath for small canvases
  2. Make an eviction policy, so we don't keep the data cached forever

@litherum litherum marked this pull request as ready for review August 4, 2023 18:35
@heycam
Copy link
Contributor Author

heycam commented Aug 4, 2023

And the conversion to premultiplied alpha and back.

@litherum
Copy link
Contributor

litherum commented Aug 4, 2023

This is not ready yet. When you get/put image data, the data is presented as unpremultiplied, but needs to round-trip through premultiplied, back to unpremultiplied. This isn't actually the identity function, because it's slightly lossy (consider round-tripping the color [0, 131, 0, 32] -> it ends up becoming [0, 128, 0, 32])

@litherum litherum marked this pull request as draft August 4, 2023 22:29
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 5, 2023
@litherum litherum removed the merging-blocked Applied to prevent a change from being merged label Aug 5, 2023
@litherum litherum marked this pull request as ready for review August 5, 2023 07:25
@litherum
Copy link
Contributor

litherum commented Aug 5, 2023

I guess I can review (and approve) @heycam's part of this. Someone else (maybe @heycam?) will have to review my part of this.

@litherum
Copy link
Contributor

litherum commented Aug 5, 2023

This leads to a 4.53% performance improvement on MotionMark.

@heycam
Copy link
Contributor Author

heycam commented Aug 5, 2023

(Since GitHub thinks I am still the PR owner I'm unable to r+ or r- the PR.)

@litherum litherum added merge-queue Applied to send a pull request to merge-queue and removed merge-queue Applied to send a pull request to merge-queue labels Aug 7, 2023
@litherum litherum added the merge-queue Applied to send a pull request to merge-queue label Aug 7, 2023
https://bugs.webkit.org/show_bug.cgi?id=259436
rdar://problem/112746729

Reviewed by Cameron McCormack and Myles C. Maxfield.

On a 2D canvas, if a recent putImageData() was made with no other
intervening drawing commands, we can use its data to fulfill a
subsequent getImageData(). Start with some simple conditions for
ImageData size and drawing dimensions, which can be extended later if it
proves useful.

* LayoutTests/fast/canvas/canvas-ImageData-cache-expected.txt: Added.
* LayoutTests/fast/canvas/canvas-ImageData-cache.html: Added.
* Source/WebCore/html/ImageData.cpp:
(WebCore::ImageData::clone const):
* Source/WebCore/html/ImageData.h:
* Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:
(WebCore::CanvasRenderingContext2DBase::reset):
(WebCore::CanvasRenderingContext2DBase::didDraw):
(WebCore::CanvasRenderingContext2DBase::cacheImageDataIfPossible):
(WebCore::CanvasRenderingContext2DBase::takeCachedImageDataIfPossible const):
(WebCore::CanvasRenderingContext2DBase::getImageData const):
(WebCore::CanvasRenderingContext2DBase::putImageData):
* Source/WebCore/html/canvas/CanvasRenderingContext2DBase.h:

Canonical link: https://commits.webkit.org/266632@main
@webkit-commit-queue
Copy link
Collaborator

Committed 266632@main (5466cd2): https://commits.webkit.org/266632@main

Reviewed commits have been landed. Closing PR #16028 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Aug 7, 2023
@webkit-commit-queue webkit-commit-queue merged commit 5466cd2 into WebKit:main Aug 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Canvas Bugs related to the canvas element.
Projects
None yet
5 participants