Skip to content

Conversation

@litherum
Copy link
Contributor

@litherum litherum commented Jan 28, 2023

2f40848

[WebGPU] Simplify the lifetime of GPUPresentationContext within GPUCanvasContextCocoa
https://bugs.webkit.org/show_bug.cgi?id=251067
rdar://104784569

Reviewed by Tadeu Zagallo.

There's no reason to lazily create the GPUPresentationContext; we can just create
it eagerly, which allows us to use a Ref instead of a RefPtr for the object,
thereby eliminating one potential state in the class.

* Source/WebCore/html/canvas/GPUCanvasContextCocoa.cpp:
(WebCore::presentationContextDescriptor):
(WebCore::GPUCanvasContextCocoa::GPUCanvasContextCocoa):
(WebCore::GPUCanvasContextCocoa::reshape):
(WebCore::GPUCanvasContextCocoa::unconfigurePresentationContextIfNeeded):
(WebCore::GPUCanvasContextCocoa::configurePresentationContextIfNeeded):
(WebCore::GPUCanvasContextCocoa::getCurrentTexture):
(WebCore::GPUCanvasContextCocoa::createPresentationContextIfNeeded): Deleted.
* Source/WebCore/html/canvas/GPUCanvasContextCocoa.h:

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

4b1cabe

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk2
✅ 🛠 tv-sim ❌ 🧪 mac-AS-debug-wk2
✅ 🛠 watch ✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge ✅ 🛠 watch-sim

@litherum litherum requested review from cdumez and rniwa as code owners January 28, 2023 20:34
@litherum litherum self-assigned this Jan 28, 2023
@litherum litherum added the WebGPU For bugs in WebGPU label Jan 28, 2023
@litherum litherum force-pushed the eng/WebGPU-Simplify-the-lifetime-of-GPUPresentationContext-within-GPUCanvasContextCocoa branch 2 times, most recently from e8b2a93 to 4b1cabe Compare January 28, 2023 20:41
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 29, 2023
Comment on lines +123 to +124
Copy link
Contributor

Choose a reason for hiding this comment

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

when requesting to configure and there is no configuration, is that an error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, though I'm going to end up rewriting this function entirely (as in #8963) so I didn't want to spend a lot of time polishing up the existing code in this function.

Copy link
Contributor

@djg djg left a comment

Choose a reason for hiding this comment

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

:shipit:

@litherum litherum added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Jan 30, 2023
…nvasContextCocoa

https://bugs.webkit.org/show_bug.cgi?id=251067
rdar://104784569

Reviewed by Tadeu Zagallo.

There's no reason to lazily create the GPUPresentationContext; we can just create
it eagerly, which allows us to use a Ref instead of a RefPtr for the object,
thereby eliminating one potential state in the class.

* Source/WebCore/html/canvas/GPUCanvasContextCocoa.cpp:
(WebCore::presentationContextDescriptor):
(WebCore::GPUCanvasContextCocoa::GPUCanvasContextCocoa):
(WebCore::GPUCanvasContextCocoa::reshape):
(WebCore::GPUCanvasContextCocoa::unconfigurePresentationContextIfNeeded):
(WebCore::GPUCanvasContextCocoa::configurePresentationContextIfNeeded):
(WebCore::GPUCanvasContextCocoa::getCurrentTexture):
(WebCore::GPUCanvasContextCocoa::createPresentationContextIfNeeded): Deleted.
* Source/WebCore/html/canvas/GPUCanvasContextCocoa.h:

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

Committed 259564@main (2f40848): https://commits.webkit.org/259564@main

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

@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/WebGPU-Simplify-the-lifetime-of-GPUPresentationContext-within-GPUCanvasContextCocoa branch from 4b1cabe to 2f40848 Compare January 30, 2023 17:55
@webkit-early-warning-system webkit-early-warning-system merged commit 2f40848 into WebKit:main Jan 30, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jan 30, 2023
@litherum litherum deleted the eng/WebGPU-Simplify-the-lifetime-of-GPUPresentationContext-within-GPUCanvasContextCocoa branch January 30, 2023 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WebGPU For bugs in WebGPU

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants