Bug 92492 – [Texmap] Remove the backing store after 'style.visibility' for an element sets 'hidden'.

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Dongseong Hwang]

This patch's purpose is to save vram memory.
When visibility of the element sets hidden, we do not need to draw the element,
so we do not need to keep a texture of the backing store.
Currently, Texmap does not draw the element with visibility:hidden because
RenderLayerBacking::paintIntoLayer does not draw anything.
This patch just removes unused textures.

[Dongseong Hwang]

Created attachment 154909 [details]
Patch

[Dongseong Hwang]

(In reply to comment #1)
> Created an attachment (id=154909) [details]
> Patch

[Dongseong Hwang]

Currently, chromium port handles m_contentsVisible like m_drawsContent.

void GraphicsLayerChromium::updateLayerIsDrawable()
{
    // For the rest of the accelerated compositor code, there is no reason to make a
    // distinction between drawsContent and contentsVisible. So, for m_layer, these two
    // flags are combined here. m_contentsLayer shouldn't receive the drawsContent flag
    // so it is only given contentsVisible.

    m_layer.setDrawsContent(m_drawsContent && m_contentsVisible);

    if (!m_contentsLayer.isNull())
        m_contentsLayer.setDrawsContent(m_contentsVisible);

    if (m_drawsContent)
        m_layer.invalidate();

    updateDebugIndicators();
}

Some web developers think visibility:hidden causes to save the texture memory.

[Noam Rosenthal]

Comment on attachment 154909 [details]
Patch

Actually, something is missing here. We should clear the backing stores not just for the layer that becomes invisible, but also for all of its descendants.

[Dongseong Hwang]

(In reply to comment #4)
> (From update of attachment 154909 [details])
> Actually, something is missing here. We should clear the backing stores not just for the layer that becomes invisible, but also for all of its descendants.

Thank you for your review.

I was concerned about what you said.

http://www.w3.org/TR/CSS21/visufx.html#propdef-visibility

The spec said
"hidden
The generated box is invisible (fully transparent, nothing is drawn), but still affects layout. Furthermore, descendants of the element will be visible if they have 'visibility: visible'."

If some descendants have 'visibility: visible', they need the backing stores.

RenderLayerBacking set contents visible of the graphics layer to false even if a graphics layer has visible compositing descendant layers.

void RenderLayerBacking::updateGraphicsLayerGeometry()
{
    ...
    // m_graphicsLayer is the corresponding GraphicsLayer for this RenderLayer and its non-compositing
    // descendants. So, the visibility flag for m_graphicsLayer should be true if there are any
    // non-compositing visible layers.
    m_graphicsLayer->setContentsVisible(m_owningLayer->hasVisibleContent() || hasVisibleNonCompositingDescendantLayers());
    ...
}

So, I removed the backing stores for only the layer that becomes invisible.

I think I may misunderstand, so I need your feedback.

[Noam Rosenthal]

Comment on attachment 154909 [details]
Patch

Previous r- was based on a misunderstanding of the spec :)

[Dongseong Hwang]

Comment on attachment 154909 [details]
Patch

I forgot to set commit-queue to ?. Could you change +? :)

[WebKit Review Bot]

Comment on attachment 154909 [details]
Patch

Clearing flags on attachment: 154909

Committed r124178: <http://trac.webkit.org/changeset/124178>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.