[Webkit-unassigned] [Bug 50833] [chromium] Add support to compositor to composite to offscreen texture.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=50833





--- Comment #8 from Vangelis Kokkevis <vangelis at chromium.org>  2010-12-14 00:44:47 PST ---
(From update of attachment 76232)
View in context: https://bugs.webkit.org/attachment.cgi?id=76232&action=review

Generally looks fine. I think if this were a real implementation you would eliminate the copyOffscreenTextureToDisplay call and you would allocate the additional texture for the default render surface only when the compositor is told to render off screen. Otherwise you would just render straight to the display backbuffer as it's done today.

I also think there might be some additional merging that needs to happen with jamesr's texture manager CL.

> WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:201
> +        if (m_rootLayer->m_renderSurface.get())

you don't have to include the .get().  It's enough to test m_rootLayer->m_renderSurface .

> WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:294
> +    if (!m_rootLayer->m_renderSurface.get())

remove .get()

> WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:666
> +    m_renderOffscreen = compositeOffscreen;

nit: Ideally the member name and the variable name should match.  I think I prefer compositeOffscreen to renderOffscreen

> WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:696
> +        GLC(m_context, m_context->colorMask(true, true, true, false));

I don't know that masking is necessary as it should have been done when you drew into the default render surface so hopefully the alpha channel is ok.

> WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:702
> +                                        contentLayerValues->shaderMatrixLocation(), contentLayerValues->shaderAlphaLocation());

You need to use m_textureLayerShaderMatrixLocation and m_textureLayerShaderAlphaLocation here. Also, after Jamesr patch landed, RenderSurfaceChromium has its own draw method I believe.

> WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:796
> +    // But, if rendering to offscreen texture, we reverse our sense of 'upside down'.

This seems a little strange.  I can see how maybe you don't need to render upside down when rendering to the default render surface but I don't see why you need to flip all the other render surfaces too.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.