Bug 217377 - Implement ANGLE version of WebGL layer snapshot copyImageSnapshotWithColorSpace
Summary: Implement ANGLE version of WebGL layer snapshot copyImageSnapshotWithColorSpace
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: WebKit Local Build
Hardware: Mac Unspecified
: P2 Normal
Assignee: Kimmo Kinnunen
URL:
Keywords: InRadar
Depends on:
Blocks: webglangle
  Show dependency treegraph
 
Reported: 2020-10-06 04:47 PDT by Kimmo Kinnunen
Modified: 2021-09-01 22:47 PDT (History)
6 users (show)

See Also:


Attachments
Patch (2.24 KB, patch)
2021-08-31 05:01 PDT, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kimmo Kinnunen 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
Comment 1 Radar WebKit Bug Importer 2020-10-13 04:48:16 PDT
<rdar://problem/70248151>
Comment 2 Kenneth Russell 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.
Comment 3 Kimmo Kinnunen 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.
Comment 4 Kenneth Russell 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.
Comment 5 Kenneth Russell 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.
Comment 6 Kimmo Kinnunen 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.
Comment 7 Kimmo Kinnunen 2021-08-31 05:01:29 PDT
Created attachment 436876 [details]
Patch
Comment 8 EWS 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].