From b854dc1ebf3534ffda40b441e0a4e49720385b74 Mon Sep 17 00:00:00 2001 From: Ojan Vafai Date: Mon, 26 Jan 2015 18:55:09 -0800 Subject: [PATCH] Remove negative z-index. There's no good use-case for putting descendants behind their ancestors. R=esprehn@chromium.org Review URL: https://codereview.chromium.org/873983007 --- engine/core/css/CSSProperties.in | 2 +- .../css/resolver/AnimatedStyleBuilder.cpp | 2 +- engine/core/rendering/RenderLayer.cpp | 13 +----- .../rendering/RenderLayerStackingNode.cpp | 39 +++++----------- .../core/rendering/RenderLayerStackingNode.h | 30 ++++-------- .../RenderLayerStackingNodeIterator.cpp | 46 ++++--------------- .../RenderLayerStackingNodeIterator.h | 9 ++-- engine/core/rendering/RenderTreeAsText.cpp | 20 +------- engine/core/rendering/style/RenderStyle.h | 4 +- engine/core/rendering/style/StyleBoxData.cpp | 2 +- engine/core/rendering/style/StyleBoxData.h | 4 +- 11 files changed, 44 insertions(+), 127 deletions(-) diff --git a/engine/core/css/CSSProperties.in b/engine/core/css/CSSProperties.in index 19ec8ad1dbb..5a1bd9db509 100644 --- a/engine/core/css/CSSProperties.in +++ b/engine/core/css/CSSProperties.in @@ -283,7 +283,7 @@ word-break inherited word-spacing animatable, inherited, initial=initialLetterWordSpacing, converter=convertSpacing // UAs must treat 'word-wrap' as an alternate name for the 'overflow-wrap' property. So using the same handlers. word-wrap inherited, name_for_methods=OverflowWrap -z-index animatable, type_name=int, custom_all +z-index animatable, type_name=unsigned, custom_all // Non-standard direction aware properties diff --git a/engine/core/css/resolver/AnimatedStyleBuilder.cpp b/engine/core/css/resolver/AnimatedStyleBuilder.cpp index d7b6930d91d..017d56860ff 100644 --- a/engine/core/css/resolver/AnimatedStyleBuilder.cpp +++ b/engine/core/css/resolver/AnimatedStyleBuilder.cpp @@ -509,7 +509,7 @@ void AnimatedStyleBuilder::applyProperty(CSSPropertyID property, StyleResolverSt style->setVerticalAlignLength(animatableValueToLength(value, state)); return; case CSSPropertyZIndex: - style->setZIndex(animatableValueRoundClampTo(value)); + style->setZIndex(animatableValueRoundClampTo(value)); return; default: ASSERT_NOT_REACHED(); diff --git a/engine/core/rendering/RenderLayer.cpp b/engine/core/rendering/RenderLayer.cpp index cd60e82e415..da5e00a870b 100644 --- a/engine/core/rendering/RenderLayer.cpp +++ b/engine/core/rendering/RenderLayer.cpp @@ -305,7 +305,7 @@ bool RenderLayer::update3DTransformedDescendantStatus() // Transformed or preserve-3d descendants can only be in the z-order lists, not // in the normal flow list, so we only need to check those. - RenderLayerStackingNodeIterator iterator(*m_stackingNode.get(), PositiveZOrderChildren | NegativeZOrderChildren); + RenderLayerStackingNodeIterator iterator(*m_stackingNode.get(), PositiveZOrderChildren); while (RenderLayerStackingNode* node = iterator.next()) m_has3DTransformedDescendant |= node->layer()->update3DTransformedDescendantStatus(); @@ -1043,8 +1043,6 @@ void RenderLayer::paintLayerContents(GraphicsContext* context, const LayerPainti LayoutPoint layerLocation = toPoint(layerBounds.location() - renderBoxLocation() + localPaintingInfo.subPixelAccumulation); - paintChildren(NegativeZOrderChildren, context, paintingInfo, paintFlags); - if (shouldPaintContent) { paintForeground(context, transparencyLayerContext, paintingInfo.paintDirtyRect, haveTransparency, localPaintingInfo, paintingRootForRenderer, layerLocation, foregroundRect); @@ -1441,15 +1439,6 @@ RenderLayer* RenderLayer::hitTestLayer(RenderLayer* rootLayer, RenderLayer* cont } } - // Now check our negative z-index children. - hitLayer = hitTestChildren(NegativeZOrderChildren, rootLayer, request, result, hitTestRect, hitTestLocation, - localTransformState.get(), zOffsetForDescendantsPtr, zOffset, unflattenedTransformState.get(), depthSortDescendants); - if (hitLayer) { - if (!depthSortDescendants) - return hitLayer; - candidateLayer = hitLayer; - } - // If we found a layer, return. Child layers, and foreground always render in front of background. if (candidateLayer) return candidateLayer; diff --git a/engine/core/rendering/RenderLayerStackingNode.cpp b/engine/core/rendering/RenderLayerStackingNode.cpp index 45c45209643..aa77021df49 100644 --- a/engine/core/rendering/RenderLayerStackingNode.cpp +++ b/engine/core/rendering/RenderLayerStackingNode.cpp @@ -96,10 +96,8 @@ void RenderLayerStackingNode::dirtyZOrderLists() updateStackingParentForZOrderLists(0); #endif - if (m_posZOrderList) - m_posZOrderList->clear(); - if (m_negZOrderList) - m_negZOrderList->clear(); + if (m_zOrderList) + m_zOrderList->clear(); m_zOrderListsDirty = true; } @@ -128,14 +126,10 @@ void RenderLayerStackingNode::rebuildZOrderLists() ASSERT(isDirtyStackingContext()); for (RenderLayer* child = layer()->firstChild(); child; child = child->nextSibling()) - child->stackingNode()->collectLayers(m_posZOrderList, m_negZOrderList); + child->stackingNode()->collectLayers(m_zOrderList); - // Sort the two lists. - if (m_posZOrderList) - std::stable_sort(m_posZOrderList->begin(), m_posZOrderList->end(), compareZIndex); - - if (m_negZOrderList) - std::stable_sort(m_negZOrderList->begin(), m_negZOrderList->end(), compareZIndex); + if (m_zOrderList) + std::stable_sort(m_zOrderList->begin(), m_zOrderList->end(), compareZIndex); #if ENABLE(ASSERT) updateStackingParentForZOrderLists(this); @@ -166,10 +160,9 @@ void RenderLayerStackingNode::updateNormalFlowList() m_normalFlowListDirty = false; } -void RenderLayerStackingNode::collectLayers(OwnPtr >& posBuffer, OwnPtr >& negBuffer) +void RenderLayerStackingNode::collectLayers(OwnPtr >& buffer) { if (!isNormalFlowOnly()) { - OwnPtr >& buffer = (zIndex() >= 0) ? posBuffer : negBuffer; if (!buffer) buffer = adoptPtr(new Vector); buffer->append(this); @@ -177,7 +170,7 @@ void RenderLayerStackingNode::collectLayers(OwnPtrfirstChild(); child; child = child->nextSibling()) - child->stackingNode()->collectLayers(posBuffer, negBuffer); + child->stackingNode()->collectLayers(buffer); } } @@ -187,10 +180,7 @@ bool RenderLayerStackingNode::isInStackingParentZOrderLists() const if (!m_stackingParent || m_stackingParent->zOrderListsDirty()) return false; - if (m_stackingParent->posZOrderList() && m_stackingParent->posZOrderList()->find(this) != kNotFound) - return true; - - if (m_stackingParent->negZOrderList() && m_stackingParent->negZOrderList()->find(this) != kNotFound) + if (m_stackingParent->zOrderList() && m_stackingParent->zOrderList()->find(this) != kNotFound) return true; return false; @@ -206,14 +196,9 @@ bool RenderLayerStackingNode::isInStackingParentNormalFlowList() const void RenderLayerStackingNode::updateStackingParentForZOrderLists(RenderLayerStackingNode* stackingParent) { - if (m_posZOrderList) { - for (size_t i = 0; i < m_posZOrderList->size(); ++i) - m_posZOrderList->at(i)->setStackingParent(stackingParent); - } - - if (m_negZOrderList) { - for (size_t i = 0; i < m_negZOrderList->size(); ++i) - m_negZOrderList->at(i)->setStackingParent(stackingParent); + if (m_zOrderList) { + for (size_t i = 0; i < m_zOrderList->size(); ++i) + m_zOrderList->at(i)->setStackingParent(stackingParent); } } @@ -235,7 +220,7 @@ void RenderLayerStackingNode::updateLayerListsIfNeeded() void RenderLayerStackingNode::updateStackingNodesAfterStyleChange(const RenderStyle* oldStyle) { bool wasStackingContext = oldStyle ? !oldStyle->hasAutoZIndex() : false; - int oldZIndex = oldStyle ? oldStyle->zIndex() : 0; + unsigned oldZIndex = oldStyle ? oldStyle->zIndex() : 0; bool isStackingContext = this->isStackingContext(); if (isStackingContext == wasStackingContext && oldZIndex == zIndex()) diff --git a/engine/core/rendering/RenderLayerStackingNode.h b/engine/core/rendering/RenderLayerStackingNode.h index 2b0a7b17230..449d398d040 100644 --- a/engine/core/rendering/RenderLayerStackingNode.h +++ b/engine/core/rendering/RenderLayerStackingNode.h @@ -61,7 +61,7 @@ public: explicit RenderLayerStackingNode(RenderLayer*); ~RenderLayerStackingNode(); - int zIndex() const { return renderer()->style()->zIndex(); } + unsigned zIndex() const { return renderer()->style()->zIndex(); } // A stacking context is a layer that has a non-auto z-index. bool isStackingContext() const { return !renderer()->style()->hasAutoZIndex(); } @@ -75,8 +75,7 @@ public: void clearZOrderLists(); void dirtyStackingContextZOrderLists(); - bool hasPositiveZOrderList() const { return posZOrderList() && posZOrderList()->size(); } - bool hasNegativeZOrderList() const { return negZOrderList() && negZOrderList()->size(); } + bool hasPositiveZOrderList() const { return zOrderList() && zOrderList()->size(); } // FIXME: should check for dirtiness here? bool isNormalFlowOnly() const { return m_isNormalFlowOnly; } @@ -104,11 +103,11 @@ private: friend class RenderLayerStackingNodeReverseIterator; friend class RenderTreeAsText; - Vector* posZOrderList() const + Vector* zOrderList() const { ASSERT(!m_zOrderListsDirty); - ASSERT(isStackingContext() || !m_posZOrderList); - return m_posZOrderList.get(); + ASSERT(isStackingContext() || !m_zOrderList); + return m_zOrderList.get(); } Vector* normalFlowList() const @@ -117,15 +116,8 @@ private: return m_normalFlowList.get(); } - Vector* negZOrderList() const - { - ASSERT(!m_zOrderListsDirty); - ASSERT(isStackingContext() || !m_negZOrderList); - return m_negZOrderList.get(); - } - void rebuildZOrderLists(); - void collectLayers(OwnPtr >& posZOrderList, OwnPtr >& negZOrderList); + void collectLayers(OwnPtr >& zOrderList); #if ENABLE(ASSERT) bool isInStackingParentZOrderLists() const; @@ -146,12 +138,9 @@ private: RenderLayer* m_layer; - // m_posZOrderList holds a sorted list of all the descendant nodes within + // m_zOrderList holds a sorted list of all the descendant nodes within // that have z-indices of 0 or greater (auto will count as 0). - // m_negZOrderList holds descendants within our stacking context with - // negative z-indices. - OwnPtr > m_posZOrderList; - OwnPtr > m_negZOrderList; + OwnPtr > m_zOrderList; // This list contains child nodes that cannot create stacking contexts. OwnPtr > m_normalFlowList; @@ -174,8 +163,7 @@ inline void RenderLayerStackingNode::clearZOrderLists() updateStackingParentForZOrderLists(0); #endif - m_posZOrderList.clear(); - m_negZOrderList.clear(); + m_zOrderList.clear(); } inline void RenderLayerStackingNode::updateZOrderLists() diff --git a/engine/core/rendering/RenderLayerStackingNodeIterator.cpp b/engine/core/rendering/RenderLayerStackingNodeIterator.cpp index 2c54cfcb992..e4e7d367418 100644 --- a/engine/core/rendering/RenderLayerStackingNodeIterator.cpp +++ b/engine/core/rendering/RenderLayerStackingNodeIterator.cpp @@ -38,15 +38,6 @@ namespace blink { RenderLayerStackingNode* RenderLayerStackingNodeIterator::next() { - if (m_remainingChildren & NegativeZOrderChildren) { - Vector* negZOrderList = m_root.negZOrderList(); - if (negZOrderList && m_index < negZOrderList->size()) - return negZOrderList->at(m_index++); - - m_index = 0; - m_remainingChildren &= ~NegativeZOrderChildren; - } - if (m_remainingChildren & NormalFlowChildren) { Vector* normalFlowList = m_root.normalFlowList(); if (normalFlowList && m_index < normalFlowList->size()) @@ -57,9 +48,9 @@ RenderLayerStackingNode* RenderLayerStackingNodeIterator::next() } if (m_remainingChildren & PositiveZOrderChildren) { - Vector* posZOrderList = m_root.posZOrderList(); - if (posZOrderList && m_index < posZOrderList->size()) - return posZOrderList->at(m_index++); + Vector* zOrderList = m_root.zOrderList(); + if (zOrderList && m_index < zOrderList->size()) + return zOrderList->at(m_index++); m_index = 0; m_remainingChildren &= ~PositiveZOrderChildren; @@ -70,15 +61,6 @@ RenderLayerStackingNode* RenderLayerStackingNodeIterator::next() RenderLayerStackingNode* RenderLayerStackingNodeReverseIterator::next() { - if (m_remainingChildren & NegativeZOrderChildren) { - Vector* negZOrderList = m_root.negZOrderList(); - if (negZOrderList && m_index >= 0) - return negZOrderList->at(m_index--); - - m_remainingChildren &= ~NegativeZOrderChildren; - setIndexToLastItem(); - } - if (m_remainingChildren & NormalFlowChildren) { Vector* normalFlowList = m_root.normalFlowList(); if (normalFlowList && m_index >= 0) @@ -89,9 +71,9 @@ RenderLayerStackingNode* RenderLayerStackingNodeReverseIterator::next() } if (m_remainingChildren & PositiveZOrderChildren) { - Vector* posZOrderList = m_root.posZOrderList(); - if (posZOrderList && m_index >= 0) - return posZOrderList->at(m_index--); + Vector* zOrderList = m_root.zOrderList(); + if (zOrderList && m_index >= 0) + return zOrderList->at(m_index--); m_remainingChildren &= ~PositiveZOrderChildren; setIndexToLastItem(); @@ -102,16 +84,6 @@ RenderLayerStackingNode* RenderLayerStackingNodeReverseIterator::next() void RenderLayerStackingNodeReverseIterator::setIndexToLastItem() { - if (m_remainingChildren & NegativeZOrderChildren) { - Vector* negZOrderList = m_root.negZOrderList(); - if (negZOrderList) { - m_index = negZOrderList->size() - 1; - return; - } - - m_remainingChildren &= ~NegativeZOrderChildren; - } - if (m_remainingChildren & NormalFlowChildren) { Vector* normalFlowList = m_root.normalFlowList(); if (normalFlowList) { @@ -123,9 +95,9 @@ void RenderLayerStackingNodeReverseIterator::setIndexToLastItem() } if (m_remainingChildren & PositiveZOrderChildren) { - Vector* posZOrderList = m_root.posZOrderList(); - if (posZOrderList) { - m_index = posZOrderList->size() - 1; + Vector* zOrderList = m_root.zOrderList(); + if (zOrderList) { + m_index = zOrderList->size() - 1; return; } diff --git a/engine/core/rendering/RenderLayerStackingNodeIterator.h b/engine/core/rendering/RenderLayerStackingNodeIterator.h index fce1aeee2d4..c40b81e08e0 100644 --- a/engine/core/rendering/RenderLayerStackingNodeIterator.h +++ b/engine/core/rendering/RenderLayerStackingNodeIterator.h @@ -36,16 +36,15 @@ namespace blink { enum ChildrenIteration { - NegativeZOrderChildren = 1, - NormalFlowChildren = 1 << 1, - PositiveZOrderChildren = 1 << 2, - AllChildren = NegativeZOrderChildren | NormalFlowChildren | PositiveZOrderChildren + NormalFlowChildren = 1, + PositiveZOrderChildren = 1 << 1, + AllChildren = NormalFlowChildren | PositiveZOrderChildren }; class RenderLayerStackingNode; // This iterator walks the RenderLayerStackingNode lists in the following order: -// NegativeZOrderChildren -> NormalFlowChildren -> PositiveZOrderChildren. +// NormalFlowChildren -> PositiveZOrderChildren. class RenderLayerStackingNodeIterator { WTF_MAKE_NONCOPYABLE(RenderLayerStackingNodeIterator); public: diff --git a/engine/core/rendering/RenderTreeAsText.cpp b/engine/core/rendering/RenderTreeAsText.cpp index c527e1d6305..e1798e67ce6 100644 --- a/engine/core/rendering/RenderTreeAsText.cpp +++ b/engine/core/rendering/RenderTreeAsText.cpp @@ -414,24 +414,8 @@ void RenderTreeAsText::writeLayers(TextStream& ts, const RenderLayer* rootLayer, bool shouldPaint = (behavior & RenderAsTextShowAllLayers) ? true : layer->intersectsDamageRect(layerBounds, damageRect.rect(), rootLayer); - Vector* negList = layer->stackingNode()->negZOrderList(); - bool paintsBackgroundSeparately = negList && negList->size() > 0; - if (shouldPaint && paintsBackgroundSeparately) - write(ts, *layer, layerBounds, damageRect.rect(), clipRectToApply.rect(), outlineRect.rect(), LayerPaintPhaseBackground, indent, behavior); - - if (negList) { - int currIndent = indent; - if (behavior & RenderAsTextShowLayerNesting) { - writeIndent(ts, indent); - ts << " negative z-order list(" << negList->size() << ")\n"; - ++currIndent; - } - for (unsigned i = 0; i != negList->size(); ++i) - writeLayers(ts, rootLayer, negList->at(i)->layer(), paintDirtyRect, currIndent, behavior); - } - if (shouldPaint) - write(ts, *layer, layerBounds, damageRect.rect(), clipRectToApply.rect(), outlineRect.rect(), paintsBackgroundSeparately ? LayerPaintPhaseForeground : LayerPaintPhaseAll, indent, behavior); + write(ts, *layer, layerBounds, damageRect.rect(), clipRectToApply.rect(), outlineRect.rect(), LayerPaintPhaseAll, indent, behavior); if (Vector* normalFlowList = layer->stackingNode()->normalFlowList()) { int currIndent = indent; @@ -444,7 +428,7 @@ void RenderTreeAsText::writeLayers(TextStream& ts, const RenderLayer* rootLayer, writeLayers(ts, rootLayer, normalFlowList->at(i)->layer(), paintDirtyRect, currIndent, behavior); } - if (Vector* posList = layer->stackingNode()->posZOrderList()) { + if (Vector* posList = layer->stackingNode()->zOrderList()) { int currIndent = indent; if (behavior & RenderAsTextShowLayerNesting) { writeIndent(ts, indent); diff --git a/engine/core/rendering/style/RenderStyle.h b/engine/core/rendering/style/RenderStyle.h index 6332853b7be..2b28097c524 100644 --- a/engine/core/rendering/style/RenderStyle.h +++ b/engine/core/rendering/style/RenderStyle.h @@ -1006,8 +1006,8 @@ public: bool hasAutoZIndex() const { return m_box->hasAutoZIndex(); } void setHasAutoZIndex() { SET_VAR(m_box, m_hasAutoZIndex, true); SET_VAR(m_box, m_zIndex, 0); } - int zIndex() const { return m_box->zIndex(); } - void setZIndex(int v) { SET_VAR(m_box, m_hasAutoZIndex, false); SET_VAR(m_box, m_zIndex, v); } + unsigned zIndex() const { return m_box->zIndex(); } + void setZIndex(unsigned v) { SET_VAR(m_box, m_hasAutoZIndex, false); SET_VAR(m_box, m_zIndex, v); } void setHasAutoWidows() { SET_VAR(rareInheritedData, m_hasAutoWidows, true); SET_VAR(rareInheritedData, widows, initialWidows()); } void setWidows(short w) { SET_VAR(rareInheritedData, m_hasAutoWidows, false); SET_VAR(rareInheritedData, widows, w); } diff --git a/engine/core/rendering/style/StyleBoxData.cpp b/engine/core/rendering/style/StyleBoxData.cpp index 10d16863bc0..b38ea21c3f1 100644 --- a/engine/core/rendering/style/StyleBoxData.cpp +++ b/engine/core/rendering/style/StyleBoxData.cpp @@ -28,7 +28,7 @@ namespace blink { struct SameSizeAsStyleBoxData : public RefCounted { Length length[7]; - int m_zIndex; + unsigned m_zIndex; uint32_t bitfields; }; diff --git a/engine/core/rendering/style/StyleBoxData.h b/engine/core/rendering/style/StyleBoxData.h index d3f44a8d518..70923647639 100644 --- a/engine/core/rendering/style/StyleBoxData.h +++ b/engine/core/rendering/style/StyleBoxData.h @@ -54,7 +54,7 @@ public: const Length& verticalAlign() const { return m_verticalAlign; } - int zIndex() const { return m_zIndex; } + unsigned zIndex() const { return m_zIndex; } bool hasAutoZIndex() const { return m_hasAutoZIndex; } EBoxSizing boxSizing() const { return static_cast(m_boxSizing); } @@ -77,7 +77,7 @@ private: Length m_verticalAlign; - int m_zIndex; + unsigned m_zIndex; unsigned m_hasAutoZIndex : 1; unsigned m_boxSizing : 1; // EBoxSizing unsigned m_boxDecorationBreak : 1; // EBoxDecorationBreak