Summary: | [Texmap] Remove the backing store after 'style.visibility' for an element sets 'hidden'. | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dongseong Hwang <dongseong.hwang> | ||||
Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | noam, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Dongseong Hwang
2012-07-27 04:54:02 PDT
Created attachment 154909 [details]
Patch
(In reply to comment #1) > Created an attachment (id=154909) [details] > Patch 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. 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.
(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. Comment on attachment 154909 [details]
Patch
Previous r- was based on a misunderstanding of the spec :)
Comment on attachment 154909 [details]
Patch
I forgot to set commit-queue to ?. Could you change +? :)
Comment on attachment 154909 [details] Patch Clearing flags on attachment: 154909 Committed r124178: <http://trac.webkit.org/changeset/124178> All reviewed patches have been landed. Closing bug. |