Bug 217377 – Implement ANGLE version of WebGL layer snapshot copyImageSnapshotWithColorSpace

• Component: WebGL
• Status: RESOLVED FIXED

[Kimmo Kinnunen]

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

[Radar WebKit Bug Importer]

<rdar://problem/70248151>

[Kenneth Russell]

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]

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]

Sorry - I must have been tired - you're right, and I meant that it looked like it was snapshotting the back buffer.

[Kenneth Russell]

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]

Function introduced in trunk@50067
    2009-10-26  Simon Fraser  <[e-mail]>
    
            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]

Created attachment 436876 [details]
Patch

[EWS]

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].