Bug 217377

Summary: Implement ANGLE version of WebGL layer snapshot copyImageSnapshotWithColorSpace
Product: WebKit Reporter: Kimmo Kinnunen <kkinnunen>
Component: WebGLAssignee: Kimmo Kinnunen <kkinnunen>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, jdarpinian, kbr, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Mac   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 198948    
Attachments:
Description Flags
Patch none

Kimmo Kinnunen
Reported 2020-10-06 04:47:43 PDT
To understand: Should the function snapshot the back buffer or the front buffer? Old CGL implementation (not sure if this is correct implementation) #if USE(OPENGL) CGLContextObj cglContext = static_cast<CGLContextObj>(_context->platformGraphicsContextGL()); CGLSetCurrentContext(cglContext); RetainPtr<CGColorSpaceRef> imageColorSpace = colorSpace; if (!imageColorSpace) imageColorSpace = WebCore::sRGBColorSpaceRef(); CGRect layerBounds = CGRectIntegral([self bounds]); size_t width = layerBounds.size.width * _devicePixelRatio; size_t height = layerBounds.size.height * _devicePixelRatio; size_t rowBytes = (width * 4 + 15) & ~15; size_t dataSize = rowBytes * height; void* data = fastMalloc(dataSize); if (!data) return nullptr; glPixelStorei(GL_PACK_ROW_LENGTH, rowBytes / 4); glReadPixels(0, 0, width, height, GL_BGRA, GL_UNSIGNED_INT_8_8_8_8_REV, data); WebCore::verifyImageBufferIsBigEnough((uint8_t*)data, dataSize); CGDataProviderRef provider = CGDataProviderCreateWithData(0, data, dataSize, freeData); CGImageRef image = CGImageCreate(width, height, 8, 32, rowBytes, imageColorSpace.get(), kCGImageAlphaPremultipliedFirst | kCGBitmapByteOrder32Host, provider, 0, true, kCGRenderingIntentDefault); CGDataProviderRelease(provider); return image; #else
Attachments
Patch (2.24 KB, patch)
2021-08-31 05:01 PDT, Kimmo Kinnunen 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2020-10-13 04:48:16 PDT
<rdar://problem/70248151>
Kenneth Russell
Comment 2 2020-10-26 14:05:09 PDT
Which code calls this method? The old implementation used the front buffer and perhaps that's the correct semantic, but it's not clear because there doesn't seem to be any documentation for it.
Kimmo Kinnunen
Comment 3 2020-10-26 23:33:02 PDT
As per my understanding, the old implementation used the back buffer, not the front buffer. This is on the grounds that there's no bind of any kind, so it expects to read what is bound. The front buffer is not bound ever. In fact, the old code did not have any concept of front buffer conceptually "inside OpenGL", so it can not be read by that read pixels. Front buffer it a CA layer holding a IOSurface. The only way it can be reading front buffer if it was, back in the day before prepareForDisplay, called during CA commit but before buffer swap. E.g. it would read the back buffer just before it became front buffer. However, even in this case it will either have read wrong frame buffers if client had bound something or alternatively left gl state wrong (by a bind that is not seen in the code). I couldn't find the call site, so I'd assume it is either dead code or CALayer Obj-C vfunc override which I'm not that familiar with at the moment.
Kenneth Russell
Comment 4 2020-10-28 12:56:46 PDT
Sorry - I must have been tired - you're right, and I meant that it looked like it was snapshotting the back buffer.
Kenneth Russell
Comment 5 2020-10-28 12:57:21 PDT
If it looks like this function's unused then perhaps the best thing to do is to remove it along with the old implementations.
Kimmo Kinnunen
Comment 6 2021-08-31 03:44:16 PDT
Function introduced in trunk@50067 2009-10-26 Simon Fraser <simon.fraser@apple.com> Reviewed by Sam Weinig. <rdar://problem/6988966> Hardware layers do not show up in page snapshots * WebView/WebHTMLViewPrivate.h: * WebView/WebHTMLView.mm: (-[WebHTMLView _compositingLayersHostingView]): Add a private method that returns the NSView used to host compositing layers. * platform/graphics/mac/Canvas3DLayer.h: * platform/graphics/mac/Canvas3DLayer.mm: (-[Canvas3DLayer copyImageSnapshotWithColorSpace:]): Add a method that gets called when snapshotting Canvas3DLayers for page snapshots, that allows the layer to return a CGImageRef of its contents. Unclear if it's still used.
Kimmo Kinnunen
Comment 7 2021-08-31 05:01:29 PDT
Created attachment 436876 [details] Patch
EWS
Comment 8 2021-09-01 22:47:29 PDT
Committed r281909 (241221@main): <https://commits.webkit.org/241221@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 436876 [details].
Note You need to log in before you can comment on or make changes to this bug.