RESOLVED WONTFIX238775
GraphicsContextGL subclasses should be able to supply a custom IOSurfacePool 
https://bugs.webkit.org/show_bug.cgi?id=238775
Summary GraphicsContextGL subclasses should be able to supply a custom IOSurfacePool
Simon Fraser (smfr)
Reported 2022-04-04 16:20:51 PDT
GraphicsContextGL subclasses should be able to supply a custom IOSurfacePool
Attachments
Patch (7.12 KB, patch)
2022-04-04 16:22 PDT, Simon Fraser (smfr) 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (7.11 KB, patch)
2022-04-04 17:54 PDT, Simon Fraser (smfr) 		kkinnunen: review-
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1 2022-04-04 16:22:45 PDT
Created attachment 456645 [details] Patch
Radar WebKit Bug Importer
Comment 2 2022-04-04 16:24:37 PDT
<rdar://problem/91267303>
Tim Horton
Comment 3 2022-04-04 17:54:12 PDT
Comment on attachment 456645 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=456645&action=review > Source/WebCore/html/canvas/WebGLRenderingContextBase.h:418 > + IOSurfacePool* ioSurfacePool() const override; Is this not a teensy bit of a layering violation?
Simon Fraser (smfr)
Comment 4 2022-04-04 17:54:38 PDT
Created attachment 456654 [details] Patch
Simon Fraser (smfr)
Comment 5 2022-04-04 22:40:22 PDT
(In reply to Tim Horton from comment #3) > Comment on attachment 456645 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=456645&action=review > > > Source/WebCore/html/canvas/WebGLRenderingContextBase.h:418 > > + IOSurfacePool* ioSurfacePool() const override; > > Is this not a teensy bit of a layering violation? Perhaps; ideally we'd have a way to allow all platforms to recycle their buffers, accelerated or not.
Kimmo Kinnunen
Comment 6 2022-04-04 23:49:54 PDT
Comment on attachment 456654 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=456654&action=review > Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:8180 > +IOSurfacePool* WebGLRenderingContextBase::ioSurfacePool() const I don't think this is a WebGLRenderingContextBase property, especially currently if all it can do is to have global pool. My mind should, the polymorphism in be provided at the level where the context is being created, e.g. it would be a GraphicsContextGLCocoa constructor parameter. > Source/WebCore/platform/graphics/cocoa/GraphicsContextGLCocoa.mm:578 > + auto backing = IOSurface::create(ioSurfacePool, getInternalFramebufferSize(), DestinationColorSpace::SRGB()); If preserving current behaviour is not an explicit goal, I think better first step commit would be to unilaterally just pass nullptr here. - WebGL-in-WP reuses surfaces internally - WebGL-in-GPUP does not have pool in this commit - IOSurfacePool is not entirely correct
Kimmo Kinnunen
Comment 7 2022-04-04 23:53:43 PDT
Comment on attachment 456654 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=456654&action=review >> Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:8180 >> +IOSurfacePool* WebGLRenderingContextBase::ioSurfacePool() const > > I don't think this is a WebGLRenderingContextBase property, especially currently if all it can do is to have global pool. > My mind should, the polymorphism in be provided at the level where the context is being created, e.g. it would be a GraphicsContextGLCocoa constructor parameter. And not a GraphicsContextGL::Client property, either)
Simon Fraser (smfr)
Comment 8 2022-04-05 09:18:37 PDT
I'll just leave the existing code alone.
Note You need to log in before you can comment on or make changes to this bug.