1106138 - WebGL texture uploads: unpremultiplication is now handled as a separate pass (perf regression)

Status: RESOLVED FIXED

(Core :: Canvas: WebGL, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

I was skimming the WebGL implementation to double check a WebGL book chapter that I'm writing.

Found this:

http://dxr.mozilla.org/mozilla-central/source/dom/canvas/WebGLContextGL.cpp#2681

nsresult
WebGLContext::SurfaceFromElementResultToImageSurface(nsLayoutUtils::SurfaceFromElementResult& res,
                                                     RefPtr<DataSourceSurface>& imageOut,
                                                     WebGLTexelFormat* format)
{
   *format = WebGLTexelFormat::None;

    if (!res.mSourceSurface)
        return NS_OK;
    RefPtr<DataSourceSurface> data = res.mSourceSurface->GetDataSurface();
    if (!data) {
        // SurfaceFromElement lied!
        return NS_OK;
    }

    if (!mPixelStorePremultiplyAlpha && res.mIsPremultiplied) {
        switch (data->GetFormat()) {
        case SurfaceFormat::B8G8R8X8:
            // No alpha, so de-facto premult'd.
            break;
        case SurfaceFormat::B8G8R8A8:
            data = gfxUtils::CreateUnpremultipliedDataSurface(data);
            break;
        default:
            MOZ_ASSERT(false, "Format unsupported.");
            break;
        }
    }

It makes no sense to call CreateUnpremultipliedDataSurface here: our WebGL texel conversions code is already able to handle the case of unpremultiplication, and it is better to let it handle that, as that allows doing everything in one pass --- fewer cycles and much better on memory bandwidth.

The above would be fine by itself for a quick and dirty first implementation, but the thing is, we used to have the better way implemented already!

Talking of better here - unpremultiplication sucks anyway, especially as it only imprecisely recovers the original image, so if we want the most correct results, we would have to implement it by re-decoding the image, similar to what we do for the no-colorspace-conversion case.

Comment 1

[Kyle Fung]

Attachment: remove-early-premultiply.patch

Let texel conversion code handle unpremultiplying the texture.

Comment 2

[Kyle Fung]

Attachment: fix-pack-op.patch

Made it so that when an unimplemented pack function is called, a crash will occur. Also implemented pack for RGBA32F which is needed to pass https://www.khronos.org/registry/webgl/conformance-suites/1.0.2/conformance/extensions/oes-texture-float-with-canvas.html

Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=510a834e41c4

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing.

Comment 4

[Kyle Fung]

Attachment: remove-early-premultiply-rebase.patch

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1ce930b301cb
https://hg.mozilla.org/integration/mozilla-inbound/rev/571b4e0fdf53

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1ce930b301cb
https://hg.mozilla.org/mozilla-central/rev/571b4e0fdf53