Bug 23357 – Land new files in WebCore/rendering related to accelerated compositing

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

Bug to track the landing of new files in WebCore/rendering related to accelerate
compositing.

[Simon Fraser (smfr)]

Created attachment 26775 [details]
Patch, changelog

[Sam Weinig]

Comment on attachment 26775 [details]
Patch, changelog

Issues:
  Should use the 2-clause bsd license.
  Should avoid the "in" prefix for parameters
  Multiline if-statements need braces.



+#include "RenderLayerBacking.h"

This include should be the first include after "config.h"

+struct LayerHitTestData {
+    LayerHitTestData(RenderLayer* inRootLayer, const HitTestRequest& inRequest, HitTestResult& inResult, const IntRect& hitTestRect)
+    : m_rootLayer(inRootLayer)
+    , m_request(inRequest)
+    , m_hitTestRect(hitTestRect)
+    , m_result(inResult)

The initialization list should be indented further and the "in" prefixes are not necessary.

+        if (renderer()->node()->isHTMLElement() && renderer()->node()->hasID())
+            m_graphicsLayer->setName(renderer()->renderName() + String(" ") + ((HTMLElement*)renderer()->node())->id());

Please use a c++ style cast


+float RenderLayerBacking::compositingOpacity(float rendererOpacity) const
+{
+    float finalOpacity = rendererOpacity;
+    
+    for (RenderLayer* curr = m_owningLayer->parent(); curr != 0; curr = curr->parent()) {

No need for curr != 0.  It can just be curr.


+// A simple background is either none or a solid color.
+static bool hasSimpleBackground(RenderStyle* inStyle)
+{
+    return !inStyle->hasBackgroundImage();
+}
+
+static bool hasBorderOutlineOrShadow(RenderStyle* inStyle)
+{
+    return (inStyle->hasBorder() || inStyle->hasBorderRadius() || inStyle->hasOutline() || (inStyle->boxShadow() != 0));
+}

The "in" prefix is not necessary.

+        Vector<RenderLayer*>* posZOrderList = m_owningLayer->posZOrderList();
+        if (posZOrderList && posZOrderList->size() > 0)
+            for (Vector<RenderLayer*>::const_iterator it = posZOrderList->begin(); it != posZOrderList->end(); ++it) {
+                RenderLayer* curLayer = (*it);
+                if (!curLayer->isComposited())
+                    return true;
+            }

The if-statement should have braces.

+    }
+
+    Vector<RenderLayer*>* overflowList = m_owningLayer->overflowList();
+    if (overflowList && overflowList->size() > 0)
+        for (Vector<RenderLayer*>::const_iterator it = overflowList->begin(); it != overflowList->end(); ++it) {
+            RenderLayer* curLayer = (*it);
+            if (!curLayer->isComposited())
+                return true;
+        }

Same here.

+
+    IntSize  contentOffset = contentOffsetInCompostingLayer();

Extra space.

+        // Paint any child layers that have overflow.
+        Vector<RenderLayer*>* overflowList = m_owningLayer->overflowList();
+        if (overflowList)
+            for (Vector<RenderLayer*>::iterator it = overflowList->begin(); it != overflowList->end(); ++it)
+                it[0]->paintLayer(rootLayer, context, paintDirtyRect, haveTransparency, paintRestriction, paintingRoot);
+        
+        // Now walk the sorted list of children with positive z-indices.
+        Vector<RenderLayer*>* posZOrderList = m_owningLayer->posZOrderList();
+        if (posZOrderList)
+            for (Vector<RenderLayer*>::iterator it = posZOrderList->begin(); it != posZOrderList->end(); ++it)
+                it[0]->paintLayer(rootLayer, context, paintDirtyRect, haveTransparency, paintRestriction, paintingRoot);

If statements need braces.

+        // FIXME: move into a helper function
+        FloatQuad   hitTestQuad(hitTestRect);

Extra space.

+    // Layer to clip children
+    bool hasClippingLayer() const   { return m_clippingLayer != 0; }
+    GraphicsLayer* clippingLayer() const  { return m_clippingLayer; }
+
+    // Layer to get clipped by ancestor
+    bool hasAncestorClippingLayer() const   { return m_ancestorClippingLayer != 0; }
+    GraphicsLayer* ancestorClippingLayer() const  { return m_ancestorClippingLayer; }
+
+    bool hasContentsLayer() const   { return m_contentsLayer != 0; }
+    GraphicsLayer* contentsLayer() const   { return m_contentsLayer; }

Extra spaces before the opening brace.

+    RenderLayer* findHitCompositingLayer(RenderLayer* rootLayer, const HitTestRequest&, HitTestResult&, const IntRect& hitTestRect, const IntPoint& hitTestPoint);
+
+private:

This private: is redundant

+
+#if ENABLE(3D_TRANSFORMS)
+// This symbol is used to determine from a script whether 3D transforms are enabled (via 'nm').
+bool WebCoreHas3DTransforms = true;

This seems a little odd. How is this used and why?

+#include "SystemTime.h"

I think this is now in WTF

+
+#ifndef NDEBUG
+#include "CString.h"
+#include "RenderTreeAsText.h"
+#endif
+
+namespace WebCore {
+
+struct CompositingState {
+    CompositingState(RenderLayer* inCompAncestor)
+    : m_subtreeIsCompositing(false)
+    , m_compositingAncestor(inCompAncestor)
+    , m_depth(0)

Initialization list should be indented and the "in" prefix is not necessary

+    {
+    }
+    
+    bool m_subtreeIsCompositing;
+    RenderLayer* m_compositingAncestor;
+    int m_depth;        // just used for debugging

If this is just for debugging, shoudld it be in #ifndef NDEBUG?
+void RenderLayerCompositor::enableCompositingMode(bool enable /* = true */)
+{
+    if (enable != m_compositing) {
+        m_compositing = enable;

Early return might be cleaner here.

+    
+    {
+        // If you uncomment this, be sure to comment the layout() and updateCompositingLayers() lines in
+        // RenderTreeAsText.cpp, externalRepresentation(), to avoid recursion.
+        //String dump = externalRepresentation(m_renderView);
+        //fprintf(stderr, "%s", dump.utf8().data());

We really don't like including commented out code.  If you would like to keep this in, you should surround it with a #define that
is usually off.

+    Vector<RenderLayer*>* negZOrderList = layer->negZOrderList();
+    if (negZOrderList)
+        for (Vector<RenderLayer*>::iterator it = negZOrderList->begin(); it != negZOrderList->end(); ++it) {
+            RenderLayer* curLayer = (*it);
+            if (!curLayer->isComposited()) {
+                IntRect childUnionBounds = calculateCompositedBounds(curLayer, layer);
+                unionBounds.unite(childUnionBounds);
+            }
+        }
+
+    Vector<RenderLayer*>* posZOrderList = layer->posZOrderList();
+    if (posZOrderList)
+        for (Vector<RenderLayer*>::iterator it = posZOrderList->begin(); it != posZOrderList->end(); ++it) {
+            RenderLayer* curLayer = (*it);
+            if (!curLayer->isComposited()) {
+                IntRect childUnionBounds = calculateCompositedBounds(curLayer, layer);
+                unionBounds.unite(childUnionBounds);
+            }
+        }
+
+    Vector<RenderLayer*>* overflowList = layer->overflowList();
+    if (overflowList)
+        for (Vector<RenderLayer*>::iterator it = overflowList->begin(); it != overflowList->end(); ++it) {
+            RenderLayer* curLayer = (*it);
+            if (!curLayer->isComposited()) {
+                IntRect curAbsBounds = calculateCompositedBounds(curLayer, layer);
+                unionBounds.unite(curAbsBounds);
+            }
+        }

If statements need braces.


+    bool childOverflowOnly = layer->isOverflowOnly();
+    RenderLayer* curr = layer->parent();
+    
+    for ( ; curr != 0; curr = curr->parent()) {

The RenderLayer* curr = layer->parent(); can be inside the for-loop header and there is no need for the != 0.

+RenderLayer* RenderLayerCompositor::enclosingNonStackingClippingLayer(const RenderLayer* layer) const
+{
+    RenderLayer* curr = layer->parent();
+    for ( ; curr != 0; curr = curr->parent()) {

Same here.

+    ASSERT(!layer->m_overflowListDirty);
+    Vector<RenderLayer*>* overflowList = layer->overflowList();
+    if (overflowList && overflowList->size() > 0)
+        for (Vector<RenderLayer*>::const_iterator it = overflowList->begin(); it != overflowList->end(); ++it) {
+            RenderLayer* curLayer = (*it);
+            computeCompositingRequirements(curLayer, childState);
+        }
+
+    if (layer->isStackingContext()) {
+        Vector<RenderLayer*>* posZOrderList = layer->posZOrderList();
+        if (posZOrderList && posZOrderList->size() > 0)
+            for (Vector<RenderLayer*>::const_iterator it = posZOrderList->begin(); it != posZOrderList->end(); ++it) {
+                RenderLayer* curLayer = (*it);
+                computeCompositingRequirements(curLayer, childState);
+            }
+    }

Braces around multiline if-statements.



+void RenderLayerCompositor::rebuildCompositingLayerTree(RenderLayer* layer, struct CompositingState& ioCompState)
+{
+#if 0
+    bool internalNeedToComposite = requiresCompositingLayer(layer);
+
+    for (int i = 0; i < ioCompState.m_depth; ++i)
+        fprintf(stderr, "  ");
+
+    fprintf(stderr, "rebuildCompositingLayerTree %p: has layer %d, requires layer %d, force layer %d, stacking %d\n",
+                    this, layer->isComposited(), internalNeedToComposite, ioCompState.m_subtreeIsCompositing, layer->isStackingContext());
+
+    for (int i = 0; i < ioCompState.m_depth; ++i)
+        fprintf(stderr, "  ");
+    fprintf(stderr, "forceCompositingLayer(%d) %p\n", ioCompState.m_subtreeIsCompositing, this);
+#endif

This is pretty much the same thing as commented out code.  If we want to keep this, we should a named macro.

+    ASSERT(!layer->m_overflowListDirty);
+    Vector<RenderLayer*>* overflowList = layer->overflowList();
+    if (overflowList && overflowList->size() > 0)
+        for (Vector<RenderLayer*>::iterator it = overflowList->begin(); it != overflowList->end(); ++it) {
+            RenderLayer* curLayer = (*it);
+            rebuildCompositingLayerTree(curLayer, childState);
+            if (curLayer->isComposited())
+                setCompositingParent(curLayer, childState.m_compositingAncestor);
+        }
+    
+    if (layer->isStackingContext()) {
+        Vector<RenderLayer*>* posZOrderList = layer->posZOrderList();
+        if (posZOrderList && posZOrderList->size() > 0)
+            for (Vector<RenderLayer*>::const_iterator it = posZOrderList->begin(); it != posZOrderList->end(); ++it) {
+                RenderLayer* curLayer = (*it);
+                rebuildCompositingLayerTree(curLayer, childState);
+                if (curLayer->isComposited())
+                    setCompositingParent(curLayer, childState.m_compositingAncestor);
+            }
+    }

Multiline if-statment.

+        Vector<RenderLayer*>* negZOrderList = layer->negZOrderList();
+        if (negZOrderList)
+            for (Vector<RenderLayer*>::iterator it = negZOrderList->begin(); it != negZOrderList->end(); ++it) {
+                RenderLayer* curLayer = (*it);
+                int x = 0, y = 0;
+                curLayer->convertToLayerCoords(layer, x, y);
+                IntRect childRect(inRect);
+                childRect.move(-x, -y);
+                repaintCompositingLayerRect(curLayer, childRect, true);
+            }
+
+        Vector<RenderLayer*>* posZOrderList = layer->posZOrderList();
+        if (posZOrderList)
+            for (Vector<RenderLayer*>::iterator it = posZOrderList->begin(); it != posZOrderList->end(); ++it) {
+                RenderLayer* curLayer = (*it);
+                int x = 0, y = 0;
+                curLayer->convertToLayerCoords(layer, x, y);
+                IntRect childRect(inRect);
+                childRect.move(-x, -y);
+                repaintCompositingLayerRect(curLayer, childRect, true);
+            }
+
+        Vector<RenderLayer*>* overflowList = layer->overflowList();
+        if (overflowList)
+            for (Vector<RenderLayer*>::iterator it = overflowList->begin(); it != overflowList->end(); ++it) {
+                RenderLayer* curLayer = (*it);
+                int x = 0, y = 0;
+                curLayer->convertToLayerCoords(layer, x, y);
+                IntRect childRect(inRect);
+                childRect.move(-x, -y);
+                repaintCompositingLayerRect(curLayer, childRect, true);
+            }

Same here,

+void RenderLayerCompositor::willBeDetached()
+{
+    if (m_rootPlatformLayer && m_rootLayerAttached) {
+        Frame* frame = m_renderView->frameView()->frame();
+        if (!frame)
+            return;
+        Page* page = frame->page();
+        if (!page)
+            return;
+        page->chrome()->client()->attachRootGraphicsLayer(frame, 0);
+        m_rootLayerAttached = false;
+    }
+}
+
+void RenderLayerCompositor::wasAttached()
+{
+    if (m_rootPlatformLayer) {
+        Frame* frame = m_renderView->frameView()->frame();
+        if (!frame)
+            return;
+        Page* page = frame->page();
+        if (!page)
+            return;
+        page->chrome()->client()->attachRootGraphicsLayer(frame, m_rootPlatformLayer);
+        m_rootLayerAttached = true;
+    }
+}

These might both look cleaner with early returns.

+
+#define VERBOSE_COMPOSITINGLAYER    0

We usually put these at the top of the file.

+    void computeCompositingRequirements(RenderLayer*, struct CompositingState& compState);
+    void rebuildCompositingLayerTree(RenderLayer* layer, struct CompositingState& compState);
+

+    void removeCompositedChildren(RenderLayer* layer);
+
+    void parentInRootLayer(RenderLayer* layer);

The "layer" and "compState" parameter names are not necessary

[Simon Fraser (smfr)]

I'll fix all these issues.

[Simon Fraser (smfr)]

Comment on attachment 26775 [details]
Patch, changelog

Will attach new patch with stylistic changes addressed.

[Simon Fraser (smfr)]

Created attachment 26986 [details]
Patch, changelog

[Simon Fraser (smfr)]

Created attachment 26988 [details]
Patch, changelog

[Dave Hyatt]

Comment on attachment 26988 [details]
Patch, changelog

Looks great.  I'll be much more picky about GraphicsLayer when I see it. :)

[Simon Fraser (smfr)]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/WebCore.xcodeproj/project.pbxproj
	A	WebCore/rendering/RenderLayerBacking.cpp
	A	WebCore/rendering/RenderLayerBacking.h
	A	WebCore/rendering/RenderLayerCompositor.cpp
	A	WebCore/rendering/RenderLayerCompositor.h
Committed r40434