Summary: | Land new files in WebCore/rendering related to accelerated compositing | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||||
Component: | Layout and Rendering | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cmarrin, dino, hyatt | ||||||||
Priority: | P1 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Mac | ||||||||||
OS: | OS X 10.5 | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 23359 | ||||||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2009-01-15 14:53:24 PST
Created attachment 26775 [details]
Patch, changelog
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
I'll fix all these issues. Comment on attachment 26775 [details]
Patch, changelog
Will attach new patch with stylistic changes addressed.
Created attachment 26986 [details]
Patch, changelog
Created attachment 26988 [details]
Patch, changelog
Comment on attachment 26988 [details]
Patch, changelog
Looks great. I'll be much more picky about GraphicsLayer when I see it. :)
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 |