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

[Skia] Stop using ScopedGLContextCurrent to make the Skia GL context current #25101

Merged

Conversation

carlosgcampos
Copy link
Contributor

@carlosgcampos carlosgcampos commented Feb 26, 2024

1a34924

[Skia] Stop using ScopedGLContextCurrent to make the Skia GL context current
https://bugs.webkit.org/show_bug.cgi?id=270082

Reviewed by Miguel Gomez.

In most of the cases we are setting anf unsetting the same context all
the time. If there's WebGL content, ANGLE graphics context will make the
ANGLE context current on every operation that requires it.

* Source/WebCore/platform/graphics/skia/GraphicsContextGLSkia.cpp:
(WebCore::GraphicsContextGLImageExtractor::extractImage):
* Source/WebCore/platform/graphics/skia/ImageBufferSkiaAcceleratedBackend.cpp:
(WebCore::ImageBufferSkiaAcceleratedBackend::create):
(WebCore::ImageBufferSkiaAcceleratedBackend::getPixelBuffer):
(WebCore::ImageBufferSkiaAcceleratedBackend::putPixelBuffer):
* Source/WebCore/platform/graphics/skia/ImageBufferUtilitiesSkia.cpp:
(WebCore::encodeAcceleratedImage):
* Source/WebCore/platform/graphics/skia/NativeImageSkia.cpp:
(WebCore::NativeImage::singlePixelSolidColor const):
* Source/WebCore/platform/graphics/skia/PlatformDisplaySkia.cpp:
(WebCore::PlatformDisplay::skiaGLContext):
* Source/WebCore/platform/graphics/skia/SkiaAcceleratedBufferPool.cpp:
(WebCore::SkiaAcceleratedBufferPool::~SkiaAcceleratedBufferPool):
(WebCore::SkiaAcceleratedBufferPool::releaseUnusedBuffersTimerFired):
* Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayerSkia.cpp:
(WebCore::CoordinatedGraphicsLayer::paintTile):

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

ada0d9e

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   πŸ§ͺ api-wpe
  πŸ§ͺ ios-wk2-wpt   πŸ§ͺ mac-wk1 βœ… πŸ›  gtk
  πŸ§ͺ api-ios   πŸ§ͺ mac-wk2   πŸ§ͺ gtk-wk2
βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2   πŸ§ͺ api-gtk
βœ… πŸ›  tv-sim
  πŸ›  watch
βœ… πŸ›  πŸ§ͺ unsafe-merge βœ… πŸ›  watch-sim

@carlosgcampos carlosgcampos self-assigned this Feb 26, 2024
@carlosgcampos carlosgcampos added the WPE WebKit WebKit WPE component label Feb 26, 2024
Copy link
Contributor

@nikolaszimmermann nikolaszimmermann left a comment

Choose a reason for hiding this comment

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

Since I have no experience with ANGLE, something for @magomez - just a few questions

@@ -86,9 +86,10 @@ bool GraphicsContextGLImageExtractor::extractImage(bool premultiplyAlpha, bool i

if (image->isTextureBacked()) {
auto data = SkData::MakeUninitialized(imageInfo.computeMinByteSize());
if (!PlatformDisplay::sharedDisplayForCompositing().skiaGLContext()->makeContextCurrent())
Copy link
Contributor

Choose a reason for hiding this comment

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

This would only return false, if the actual GL context switch fails, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, if the context is still current and nothing is done, true is returned.

@carlosgcampos carlosgcampos added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing labels Feb 27, 2024
@carlosgcampos carlosgcampos added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Feb 27, 2024
…current

https://bugs.webkit.org/show_bug.cgi?id=270082

Reviewed by Miguel Gomez.

In most of the cases we are setting anf unsetting the same context all
the time. If there's WebGL content, ANGLE graphics context will make the
ANGLE context current on every operation that requires it.

* Source/WebCore/platform/graphics/skia/GraphicsContextGLSkia.cpp:
(WebCore::GraphicsContextGLImageExtractor::extractImage):
* Source/WebCore/platform/graphics/skia/ImageBufferSkiaAcceleratedBackend.cpp:
(WebCore::ImageBufferSkiaAcceleratedBackend::create):
(WebCore::ImageBufferSkiaAcceleratedBackend::getPixelBuffer):
(WebCore::ImageBufferSkiaAcceleratedBackend::putPixelBuffer):
* Source/WebCore/platform/graphics/skia/ImageBufferUtilitiesSkia.cpp:
(WebCore::encodeAcceleratedImage):
* Source/WebCore/platform/graphics/skia/NativeImageSkia.cpp:
(WebCore::NativeImage::singlePixelSolidColor const):
* Source/WebCore/platform/graphics/skia/PlatformDisplaySkia.cpp:
(WebCore::PlatformDisplay::skiaGLContext):
* Source/WebCore/platform/graphics/skia/SkiaAcceleratedBufferPool.cpp:
(WebCore::SkiaAcceleratedBufferPool::~SkiaAcceleratedBufferPool):
(WebCore::SkiaAcceleratedBufferPool::releaseUnusedBuffersTimerFired):
* Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayerSkia.cpp:
(WebCore::CoordinatedGraphicsLayer::paintTile):

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

Committed 275379@main (1a34924): https://commits.webkit.org/275379@main

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

@webkit-commit-queue webkit-commit-queue merged commit 1a34924 into WebKit:main Feb 27, 2024
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Feb 27, 2024
@carlosgcampos carlosgcampos deleted the skia-no-scoped-gl-context branch February 27, 2024 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WPE WebKit WebKit WPE component
Projects
None yet
5 participants