Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(539)

Unified Diff: third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp

Issue 2392443009: reflow comments in core/paint (Closed)
Patch Set: Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
diff --git a/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp b/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
index 464633567631c7cd7608ee6d1ecee373e3be25f5..4765fd8d4e364994778af3321db57bf8f13ab943 100644
--- a/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
+++ b/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
@@ -48,9 +48,10 @@ template <typename LayoutObjectTraversalFunctor>
void traverseNonCompositingDescendantsBelongingToAncestorPaintInvalidationContainer(
const LayoutObject& object,
const LayoutObjectTraversalFunctor& functor) {
- // |object| is a paint invalidation container but is not a stacking context, so the paint
- // invalidation container of stacked descendants don't belong to |object| but belong to
- // an ancestor. This function traverses all such descendants.
+ // |object| is a paint invalidation container but is not a stacking context,
+ // so the paint invalidation container of stacked descendants don't belong to
+ // |object| but belong to an ancestor. This function traverses all such
+ // descendants.
DCHECK(object.isPaintInvalidationContainer() &&
!object.styleRef().isStackingContext());
@@ -58,9 +59,10 @@ void traverseNonCompositingDescendantsBelongingToAncestorPaintInvalidationContai
while (descendant) {
if (!descendant->hasLayer() || !descendant->styleRef().isStacked()) {
// Case 1: The descendant is not stacked (or is stacked but has not been
- // allocated a layer yet during style change), so either it's a paint invalidation
- // container in the same situation as |object|, or its paint invalidation
- // container is in such situation. Keep searching until a stacked layer is found.
+ // allocated a layer yet during style change), so either it's a paint
+ // invalidation container in the same situation as |object|, or its paint
+ // invalidation container is in such situation. Keep searching until a
+ // stacked layer is found.
descendant = descendant->nextInPreOrder(&object);
} else if (!descendant->isPaintInvalidationContainer()) {
// Case 2: The descendant is stacked and is not composited.
@@ -69,13 +71,13 @@ void traverseNonCompositingDescendantsBelongingToAncestorPaintInvalidationContai
traverseNonCompositingDescendantsInPaintOrder(*descendant, functor);
descendant = descendant->nextInPreOrderAfterChildren(&object);
} else if (descendant->styleRef().isStackingContext()) {
- // Case 3: The descendant is an invalidation container and is a stacking context.
- // No objects in the subtree can have invalidation container outside of it,
- // thus skip the whole subtree.
+ // Case 3: The descendant is an invalidation container and is a stacking
+ // context. No objects in the subtree can have invalidation container
+ // outside of it, thus skip the whole subtree.
descendant = descendant->nextInPreOrderAfterChildren(&object);
} else {
- // Case 4: The descendant is an invalidation container but not a stacking context.
- // This is the same situation as |object|, thus keep searching.
+ // Case 4: The descendant is an invalidation container but not a stacking
+ // context. This is the same situation as |object|, thus keep searching.
descendant = descendant->nextInPreOrder(&object);
}
}
@@ -93,8 +95,8 @@ void traverseNonCompositingDescendantsInPaintOrder(
descendant = descendant->nextInPreOrder(&object);
} else if (descendant->styleRef().isStackingContext()) {
// The descendant is an invalidation container and is a stacking context.
- // No objects in the subtree can have invalidation container outside of it,
- // thus skip the whole subtree.
+ // No objects in the subtree can have invalidation container outside of
+ // it, thus skip the whole subtree.
descendant = descendant->nextInPreOrderAfterChildren(&object);
} else {
// If a paint invalidation container is not a stacking context,
@@ -109,7 +111,8 @@ void traverseNonCompositingDescendantsInPaintOrder(
void ObjectPaintInvalidator::
invalidateDisplayItemClientsIncludingNonCompositingDescendants(
PaintInvalidationReason reason) {
- // This is valid because we want to invalidate the client in the display item list of the current backing.
+ // This is valid because we want to invalidate the client in the display item
+ // list of the current backing.
DisableCompositingQueryAsserts disabler;
slowSetPaintingLayerNeedsRepaint();
@@ -126,12 +129,14 @@ DISABLE_CFI_PERF
void ObjectPaintInvalidator::invalidatePaintOfPreviousPaintInvalidationRect(
const LayoutBoxModelObject& paintInvalidationContainer,
PaintInvalidationReason reason) {
- // It's caller's responsibility to ensure enclosingSelfPaintingLayer's needsRepaint is set.
- // Don't set the flag here because getting enclosingSelfPaintLayer has cost and the caller can use
- // various ways (e.g. PaintInvalidatinState::enclosingSelfPaintingLayer()) to reduce the cost.
+ // It's caller's responsibility to ensure enclosingSelfPaintingLayer's
+ // needsRepaint is set. Don't set the flag here because getting
+ // enclosingSelfPaintLayer has cost and the caller can use various ways (e.g.
+ // PaintInvalidatinState::enclosingSelfPaintingLayer()) to reduce the cost.
DCHECK(!m_object.paintingLayer() || m_object.paintingLayer()->needsRepaint());
- // These disablers are valid because we want to use the current compositing/invalidation status.
+ // These disablers are valid because we want to use the current
+ // compositing/invalidation status.
DisablePaintInvalidationStateAsserts invalidationDisabler;
DisableCompositingQueryAsserts compositingDisabler;
@@ -140,16 +145,18 @@ void ObjectPaintInvalidator::invalidatePaintOfPreviousPaintInvalidationRect(
reason);
m_object.invalidateDisplayItemClients(reason);
- // This method may be used to invalidate paint of an object changing paint invalidation container.
- // Clear previous paint invalidation rect on the original paint invalidation container to avoid
- // under-invalidation if the new paint invalidation rect on the new paint invalidation container
- // happens to be the same as the old one.
+ // This method may be used to invalidate paint of an object changing paint
+ // invalidation container. Clear previous paint invalidation rect on the
+ // original paint invalidation container to avoid under-invalidation if the
+ // new paint invalidation rect on the new paint invalidation container happens
+ // to be the same as the old one.
m_object.getMutableForPainting().clearPreviousPaintInvalidationRects();
}
void ObjectPaintInvalidator::
invalidatePaintIncludingNonCompositingDescendants() {
- // Since we're only painting non-composited layers, we know that they all share the same paintInvalidationContainer.
+ // Since we're only painting non-composited layers, we know that they all
+ // share the same paintInvalidationContainer.
const LayoutBoxModelObject& paintInvalidationContainer =
m_object.containerForPaintInvalidation();
traverseNonCompositingDescendantsInPaintOrder(
@@ -188,9 +195,10 @@ void ObjectPaintInvalidator::
void ObjectPaintInvalidator::invalidateDisplayItemClient(
const DisplayItemClient& client,
PaintInvalidationReason reason) {
- // It's caller's responsibility to ensure enclosingSelfPaintingLayer's needsRepaint is set.
- // Don't set the flag here because getting enclosingSelfPaintLayer has cost and the caller can use
- // various ways (e.g. PaintInvalidatinState::enclosingSelfPaintingLayer()) to reduce the cost.
+ // It's caller's responsibility to ensure enclosingSelfPaintingLayer's
+ // needsRepaint is set. Don't set the flag here because getting
+ // enclosingSelfPaintLayer has cost and the caller can use various ways (e.g.
+ // PaintInvalidatinState::enclosingSelfPaintingLayer()) to reduce the cost.
DCHECK(!m_object.paintingLayer() || m_object.paintingLayer()->needsRepaint());
client.setDisplayItemsUncached(reason);
@@ -243,15 +251,16 @@ void ObjectPaintInvalidator::setBackingNeedsPaintInvalidationInRect(
const LayoutBoxModelObject& paintInvalidationContainer,
const LayoutRect& rect,
PaintInvalidationReason reason) {
- // https://bugs.webkit.org/show_bug.cgi?id=61159 describes an unreproducible crash here,
- // so assert but check that the layer is composited.
+ // https://bugs.webkit.org/show_bug.cgi?id=61159 describes an unreproducible
+ // crash here, so assert but check that the layer is composited.
DCHECK(paintInvalidationContainer.compositingState() != NotComposited);
PaintLayer& layer = *paintInvalidationContainer.layer();
if (layer.groupedMapping()) {
if (GraphicsLayer* squashingLayer =
layer.groupedMapping()->squashingLayer()) {
- // Note: the subpixel accumulation of layer() does not need to be added here. It is already taken into account.
+ // Note: the subpixel accumulation of layer() does not need to be added
+ // here. It is already taken into account.
squashingLayer->setNeedsDisplayInRect(enclosingIntRect(rect), reason,
m_object);
}
@@ -262,8 +271,8 @@ void ObjectPaintInvalidator::setBackingNeedsPaintInvalidationInRect(
} else if (paintInvalidationContainer.usesCompositedScrolling()) {
if (layer.compositedLayerMapping()
->backgroundPaintsOntoScrollingContentsLayer()) {
- // TODO(flackr): Get a correct rect in the context of the scrolling contents layer to update
- // rather than updating the entire rect.
+ // TODO(flackr): Get a correct rect in the context of the scrolling
+ // contents layer to update rather than updating the entire rect.
const LayoutRect& scrollingContentsRect =
toLayoutBox(m_object).layoutOverflowRect();
layer.compositedLayerMapping()->setScrollingContentsNeedDisplayInRect(
@@ -285,7 +294,8 @@ void ObjectPaintInvalidator::invalidatePaintUsingContainer(
const LayoutBoxModelObject& paintInvalidationContainer,
const LayoutRect& dirtyRect,
PaintInvalidationReason invalidationReason) {
- // TODO(wangxianzhu): Enable the following assert after paint invalidation for spv2 is ready.
+ // TODO(wangxianzhu): Enable the following assert after paint invalidation for
+ // spv2 is ready.
// ASSERT(!RuntimeEnabledFeatures::slimmingPaintV2Enabled());
if (paintInvalidationContainer.frameView()->shouldThrottleRendering())
@@ -302,7 +312,8 @@ void ObjectPaintInvalidator::invalidatePaintUsingContainer(
RELEASE_ASSERT(m_object.isRooted());
- // FIXME: Unify "devtools.timeline.invalidationTracking" and "blink.invalidation". crbug.com/413527.
+ // FIXME: Unify "devtools.timeline.invalidationTracking" and
+ // "blink.invalidation". crbug.com/413527.
TRACE_EVENT_INSTANT1(
TRACE_DISABLED_BY_DEFAULT("devtools.timeline.invalidationTracking"),
"PaintInvalidationTracking", TRACE_EVENT_SCOPE_THREAD, "data",
@@ -315,8 +326,8 @@ void ObjectPaintInvalidator::invalidatePaintUsingContainer(
jsonObjectForPaintInvalidationInfo(
dirtyRect, paintInvalidationReasonToString(invalidationReason)));
- // This conditional handles situations where non-rooted (and hence non-composited) frames are
- // painted, such as SVG images.
+ // This conditional handles situations where non-rooted (and hence
+ // non-composited) frames are painted, such as SVG images.
if (!paintInvalidationContainer.isPaintInvalidationContainer())
invalidatePaintRectangleOnWindow(paintInvalidationContainer,
enclosingIntRect(dirtyRect));
@@ -408,7 +419,8 @@ void ObjectPaintInvalidatorWithContext::fullyInvalidatePaint(
PaintInvalidationReason reason,
const LayoutRect& oldBounds,
const LayoutRect& newBounds) {
- // The following logic avoids invalidating twice if one set of bounds contains the other.
+ // The following logic avoids invalidating twice if one set of bounds contains
+ // the other.
if (!newBounds.contains(oldBounds)) {
LayoutRect invalidationRect = oldBounds;
invalidatePaintUsingContainer(*m_context.paintInvalidationContainer,
@@ -424,7 +436,8 @@ void ObjectPaintInvalidatorWithContext::fullyInvalidatePaint(
PaintInvalidationReason
ObjectPaintInvalidatorWithContext::computePaintInvalidationReason() {
- // This is before any early return to ensure the background obscuration status is saved.
+ // This is before any early return to ensure the background obscuration status
+ // is saved.
bool backgroundObscurationChanged = false;
bool backgroundObscured = m_object.backgroundIsKnownToBeObscured();
if (backgroundObscured != m_object.previousBackgroundObscured()) {
@@ -451,9 +464,9 @@ ObjectPaintInvalidatorWithContext::computePaintInvalidationReason() {
const ComputedStyle& style = m_object.styleRef();
- // The outline may change shape because of position change of descendants. For simplicity,
- // just force full paint invalidation if this object is marked for checking paint invalidation
- // for any reason.
+ // The outline may change shape because of position change of descendants. For
+ // simplicity, just force full paint invalidation if this object is marked for
+ // checking paint invalidation for any reason.
// TODO(wangxianzhu): Optimize this.
if (style.hasOutline())
return PaintInvalidationOutline;
@@ -461,10 +474,12 @@ ObjectPaintInvalidatorWithContext::computePaintInvalidationReason() {
bool locationChanged = m_context.newLocation != m_context.oldLocation;
// If the bounds are the same then we know that none of the statements below
- // can match, so we can early out. However, we can't return PaintInvalidationNone even if
- // !locationChagned, but conservatively return PaintInvalidationIncremental because we are
- // not sure whether paint invalidation is actually needed just based on information known
- // to LayoutObject. For example, a LayoutBox may need paint invalidation if border box changes.
+ // can match, so we can early out. However, we can't return
+ // PaintInvalidationNone even if !locationChagned, but conservatively return
+ // PaintInvalidationIncremental because we are not sure whether paint
+ // invalidation is actually needed just based on information known to
+ // LayoutObject. For example, a LayoutBox may need paint invalidation if
+ // border box changes.
if (m_context.oldBounds == m_context.newBounds)
return locationChanged ? PaintInvalidationLocationChange
: PaintInvalidationIncremental;
@@ -476,8 +491,10 @@ ObjectPaintInvalidatorWithContext::computePaintInvalidationReason() {
if (m_context.newBounds.isEmpty())
return PaintInvalidationBecameInvisible;
- // If we shifted, we don't know the exact reason so we are conservative and trigger a full invalidation. Shifting could
- // be caused by some layout property (left / top) or some in-flow layoutObject inserted / removed before us in the tree.
+ // If we shifted, we don't know the exact reason so we are conservative and
+ // trigger a full invalidation. Shifting could be caused by some layout
+ // property (left / top) or some in-flow layoutObject inserted / removed
+ // before us in the tree.
if (m_context.newBounds.location() != m_context.oldBounds.location())
return PaintInvalidationBoundsChange;
@@ -489,9 +506,10 @@ ObjectPaintInvalidatorWithContext::computePaintInvalidationReason() {
void ObjectPaintInvalidatorWithContext::invalidateSelectionIfNeeded(
PaintInvalidationReason reason) {
- // Update selection rect when we are doing full invalidation (in case that the object is moved,
- // composite status changed, etc.) or shouldInvalidationSelection is set (in case that the
- // selection itself changed).
+ // Update selection rect when we are doing full invalidation (in case that the
+ // object is moved, composite status changed, etc.) or
+ // shouldInvalidationSelection is set (in case that the selection itself
+ // changed).
bool fullInvalidation = isImmediateFullPaintInvalidationReason(reason);
if (!fullInvalidation && !m_object.shouldInvalidateSelection())
return;
@@ -518,8 +536,9 @@ void ObjectPaintInvalidatorWithContext::invalidateSelectionIfNeeded(
PaintInvalidationReason
ObjectPaintInvalidatorWithContext::invalidatePaintIfNeededWithComputedReason(
PaintInvalidationReason reason) {
- // We need to invalidate the selection before checking for whether we are doing a full invalidation.
- // This is because we need to update the previous selection rect regardless.
+ // We need to invalidate the selection before checking for whether we are
+ // doing a full invalidation. This is because we need to update the previous
+ // selection rect regardless.
invalidateSelectionIfNeeded(reason);
if (reason == PaintInvalidationIncremental && !incrementallyInvalidatePaint())
@@ -527,11 +546,12 @@ ObjectPaintInvalidatorWithContext::invalidatePaintIfNeededWithComputedReason(
switch (reason) {
case PaintInvalidationNone:
- // TODO(trchen): Currently we don't keep track of paint offset of layout objects.
- // There are corner cases that the display items need to be invalidated for paint offset
- // mutation, but incurs no pixel difference (i.e. bounds stay the same) so no rect-based
- // invalidation is issued. See crbug.com/508383 and crbug.com/515977.
- // This is a workaround to force display items to update paint offset.
+ // TODO(trchen): Currently we don't keep track of paint offset of layout
+ // objects. There are corner cases that the display items need to be
+ // invalidated for paint offset mutation, but incurs no pixel difference
+ // (i.e. bounds stay the same) so no rect-based invalidation is issued.
+ // See crbug.com/508383 and crbug.com/515977. This is a workaround to
+ // force display items to update paint offset.
if (m_context.forcedSubtreeInvalidationFlags &
PaintInvalidatorContext::ForcedSubtreeInvalidationChecking) {
reason = PaintInvalidationLocationChange;

Powered by Google App Engine
This is Rietveld 408576698