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

Big drop in canvas performance above 3840px #18080

Conversation

kkinnunen-apple
Copy link
Contributor

@kkinnunen-apple kkinnunen-apple commented Sep 22, 2023

45446c5

Big drop in canvas performance above 3840px
https://bugs.webkit.org/show_bug.cgi?id=236173
rdar://88536159

Reviewed by Simon Fraser.

Limiting the accelerted canvas 2d context to arbitrary 5120*2880 is
not useful. The performance difference gets more beneficial to
hardware rasterization the higher the resolution.

The limit did not protect against hitting hardware rasterization
limits, since the limits are always per dimension, not per area.

As per above, capping based on any upper limit is not useful. Thus
remove the upper limit.

Reuse the unused lower limit as the trigger. The lower limit is
justified, as very small areas might be useful to rasterize with CPU.
Start the limit at 0, to match the previous behavior.

The hardware acceleration limits are checked in the respective
ImageBufferBackends, e.g. ImageBufferIOSurfaceBackend::calculateSafeSize.
This is done in WP for both GPUP mode and in-process mode. Both will
fall back to Unaccelerated.

Changes the RemoteRenderingBackend::createImageBuffer() to use
buffer options instead of rendering mode. The the buffer
options may be honored, but rendering mode would imply that it must
be honored. Later on when the rendering mode is more policy based on
rendering purpose, this is more clear.

* LayoutTests/compositing/canvas/accelerated-canvas-compositing-size-limit-expected.txt:
* LayoutTests/compositing/canvas/accelerated-canvas-compositing-size-limit.html:
* LayoutTests/compositing/canvas/accelerated-small-canvas-compositing-expected.txt: Removed.
* LayoutTests/compositing/canvas/accelerated-small-canvas-compositing.html: Removed.
* LayoutTests/fast/canvas/image-buffer-backend-variants-expected.txt:
* LayoutTests/platform/ios/compositing/canvas/accelerated-canvas-compositing-size-limit-expected.txt: Removed.
* LayoutTests/platform/ios/fast/canvas/image-buffer-backend-variants-expected.txt:
* LayoutTests/platform/mac-wk1/compositing/canvas/accelerated-canvas-compositing-size-limit-expected.txt: Added.
* LayoutTests/platform/mac-wk1/fast/canvas/image-buffer-backend-variants-expected.txt:
* LayoutTests/platform/mac-wk2/TestExpectations:
* LayoutTests/platform/mac-wk2/compositing/canvas/accelerated-canvas-compositing-size-limit-expected.txt: Removed.
* LayoutTests/platform/wincairo/TestExpectations:
* Source/WebCore/html/CanvasBase.cpp:
(WebCore::CanvasBase::shouldAccelerate const):
(WebCore::CanvasBase::allocateImageBuffer const):
* Source/WebCore/html/CanvasBase.h:
* Source/WebCore/page/Settings.yaml:

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

c87002e

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

@kkinnunen-apple kkinnunen-apple self-assigned this Sep 22, 2023
@kkinnunen-apple kkinnunen-apple added the Canvas Bugs related to the canvas element. label Sep 22, 2023
@webkit-ews-buildbot webkit-ews-buildbot added merging-blocked Applied to prevent a change from being merged labels Sep 22, 2023
@kkinnunen-apple kkinnunen-apple removed the merging-blocked Applied to prevent a change from being merged label Sep 22, 2023
@kkinnunen-apple kkinnunen-apple force-pushed the imagebuffer-accelerated-no-limits-1 branch from c0fde81 to 28c6985 Compare September 22, 2023 16:02
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 22, 2023
@kkinnunen-apple kkinnunen-apple removed the merging-blocked Applied to prevent a change from being merged label Sep 22, 2023
@kkinnunen-apple kkinnunen-apple force-pushed the imagebuffer-accelerated-no-limits-1 branch from 28c6985 to 1a56d1f Compare September 22, 2023 16:52
@kkinnunen-apple kkinnunen-apple force-pushed the imagebuffer-accelerated-no-limits-1 branch from 1a56d1f to 56ee661 Compare September 22, 2023 17:01
@kkinnunen-apple kkinnunen-apple requested review from smfr, litherum and shallawa and removed request for rniwa and cdumez September 22, 2023 17:05
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 22, 2023
@kkinnunen-apple kkinnunen-apple removed the merging-blocked Applied to prevent a change from being merged label Sep 25, 2023
@kkinnunen-apple kkinnunen-apple force-pushed the imagebuffer-accelerated-no-limits-1 branch from 56ee661 to 88ea72a Compare September 25, 2023 09:05
@kkinnunen-apple kkinnunen-apple force-pushed the imagebuffer-accelerated-no-limits-1 branch from 88ea72a to 97dc78e Compare September 25, 2023 10:14
@kkinnunen-apple kkinnunen-apple force-pushed the imagebuffer-accelerated-no-limits-1 branch from 97dc78e to 42f06ac Compare September 25, 2023 10:16
@kkinnunen-apple kkinnunen-apple force-pushed the imagebuffer-accelerated-no-limits-1 branch from 42f06ac to 0a563c7 Compare September 25, 2023 11:45
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 25, 2023
@kkinnunen-apple kkinnunen-apple removed the merging-blocked Applied to prevent a change from being merged label Sep 25, 2023
@kkinnunen-apple kkinnunen-apple force-pushed the imagebuffer-accelerated-no-limits-1 branch from 0a563c7 to 5d63817 Compare September 25, 2023 18:50
@kkinnunen-apple kkinnunen-apple force-pushed the imagebuffer-accelerated-no-limits-1 branch from 5d63817 to c87002e Compare September 25, 2023 18:55
@kkinnunen-apple kkinnunen-apple added the merge-queue Applied to send a pull request to merge-queue label Sep 26, 2023
https://bugs.webkit.org/show_bug.cgi?id=236173
rdar://88536159

Reviewed by Simon Fraser.

Limiting the accelerted canvas 2d context to arbitrary 5120*2880 is
not useful. The performance difference gets more beneficial to
hardware rasterization the higher the resolution.

The limit did not protect against hitting hardware rasterization
limits, since the limits are always per dimension, not per area.

As per above, capping based on any upper limit is not useful. Thus
remove the upper limit.

Reuse the unused lower limit as the trigger. The lower limit is
justified, as very small areas might be useful to rasterize with CPU.
Start the limit at 0, to match the previous behavior.

The hardware acceleration limits are checked in the respective
ImageBufferBackends, e.g. ImageBufferIOSurfaceBackend::calculateSafeSize.
This is done in WP for both GPUP mode and in-process mode. Both will
fall back to Unaccelerated.

Changes the RemoteRenderingBackend::createImageBuffer() to use
buffer options instead of rendering mode. The the buffer
options may be honored, but rendering mode would imply that it must
be honored. Later on when the rendering mode is more policy based on
rendering purpose, this is more clear.

* LayoutTests/compositing/canvas/accelerated-canvas-compositing-size-limit-expected.txt:
* LayoutTests/compositing/canvas/accelerated-canvas-compositing-size-limit.html:
* LayoutTests/compositing/canvas/accelerated-small-canvas-compositing-expected.txt: Removed.
* LayoutTests/compositing/canvas/accelerated-small-canvas-compositing.html: Removed.
* LayoutTests/fast/canvas/image-buffer-backend-variants-expected.txt:
* LayoutTests/platform/ios/compositing/canvas/accelerated-canvas-compositing-size-limit-expected.txt: Removed.
* LayoutTests/platform/ios/fast/canvas/image-buffer-backend-variants-expected.txt:
* LayoutTests/platform/mac-wk1/compositing/canvas/accelerated-canvas-compositing-size-limit-expected.txt: Added.
* LayoutTests/platform/mac-wk1/fast/canvas/image-buffer-backend-variants-expected.txt:
* LayoutTests/platform/mac-wk2/TestExpectations:
* LayoutTests/platform/mac-wk2/compositing/canvas/accelerated-canvas-compositing-size-limit-expected.txt: Removed.
* LayoutTests/platform/wincairo/TestExpectations:
* Source/WebCore/html/CanvasBase.cpp:
(WebCore::CanvasBase::shouldAccelerate const):
(WebCore::CanvasBase::allocateImageBuffer const):
* Source/WebCore/html/CanvasBase.h:
* Source/WebCore/page/Settings.yaml:

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

Committed 268440@main (45446c5): https://commits.webkit.org/268440@main

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

@webkit-commit-queue webkit-commit-queue merged commit 45446c5 into WebKit:main Sep 26, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Sep 26, 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