Paint local attachment backgrounds into composited scrolling contents layer

third_party/WebKit/Source/core/layout/LayoutBox.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2005 Allan Sandfeld Jensen (kde@carewolf.com)
  *           (C) 2005, 2006 Samuel Weinig (sam.weinig@gmail.com)
  * Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2013 Adobe Systems Incorporated. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 686 matching lines @@
     offset.move(absPos.x(), absPos.y());
     return toIntSize(offset);
 }
 
 FloatQuad LayoutBox::absoluteContentQuad() const
 {
     LayoutRect rect = contentBoxRect();
     return localToAbsoluteQuad(FloatRect(rect));
 }
 
-LayoutRect LayoutBox::backgroundClipRect() const
+LayoutRect LayoutBox::backgroundRect(BackgroundRectOpacity rectOpacity) const

[trchen, 2016/08/13 00:14:08]

I'm worried about the semantic of this function being vague.

Does it return a cull rect so it is guaranteed nothing outside of this rect will
paint (i.e. allow false positive, no false negative)? However in the case of
OpaqueBackgroundRect it reports a rect that's absolutely opaque, but there can
still be opaque pixels outside of the rect (i.e. allow false negative, no false
positive).

[flackr, 2016/08/16 17:52:51]

When called with OpaqueBackgroundRect from backgroundIsOpaqueInRect we are okay
with false negatives, as this will not result in visual artifacts. If we have
false positives though it would result in creating an opaque layer and seeing
unpainted parts of the picture.

When called with TranlucentBackgroundRect from compositedLayerMapping we want
the cull / clip rect such that nothing outside this rect will paint.

Perhaps I should rephrase the values in the enum to reflect this, what do you
think of enum BackgroundRectType { BackgroundClipRect, BackgroundKnownOpaqueRect
};?

[trchen, 2016/08/17 01:22:55]

Great idea. It totally sounds clearer!

 {
-    // TODO(flackr): Check for the maximum background clip rect.
-    switch (style()->backgroundClip()) {
+    EFillBox backgroundBox = TextFillBox;
+    // Find the largest background rect of the given opaqueness.
+    if (const FillLayer* current = &(style()->backgroundLayers())) {
+        do {
+            const FillLayer* cur = current;
+            current = current->next();
+            if (rectOpacity == OpaqueBackgroundRect) {
+                if (cur->blendMode() != WebBlendModeNormal || cur->composite() != CompositeSourceOver)
+                continue;

[chrishtr, 2016/08/12 17:03:00]

Indentation.

[flackr, 2016/08/16 17:52:51]

Done.

+                // Check if the image or color has alpha.
+                if (const StyleImage* image = cur->image()) {
+                    if (!image->knownToBeOpaque(*this))
+                        continue;
+                } else {

[trchen, 2016/08/13 00:06:42]

}
if (!current->next()) {

1. A layer with translucent image can still have opaque color below.
2. Background color is only applicable on bottom layer.

Also I recall schenney@ may have wrote a helper function to determine whether a
fill layer is opaque. We should reuse it if it's there.

[flackr, 2016/08/16 17:52:51]

Yes, that is why we continue to the next layer if we encounter a translucent
image.
Hmm, I had mistakenly assumed that !cur->image() implies the background color,
but at least "none" can be specified in a background list which has no image -
I've added a test for this..
The only related patch I can find recently is
https://codereview.chromium.org/1826013002 which only checked the background
color property. I've checked FillLayer but it doesn't seem to have any such
property, but perhaps the contents of this loop should become a helper on
FillLayer to this end.

[trchen, 2016/08/17 01:22:55]

The background color is always applicable even if the bottom layer has an image,
so this line should be 'if' instead of 'else if'. See
BoxPainter::paintFillLayer().

[flackr, 2016/08/17 18:20:55]

Thanks for the clarification. Done, but needed to rework this to be a neither
condition. If neither the image or color is opaque, then continue.

+                    Color backgroundColor = resolveColor(CSSPropertyBackgroundColor);
+                    if (backgroundColor.hasAlpha())
+                        continue;
+                }
+            }
+            EFillBox currentClip = cur->clip();
+            // Adjust clip if attachment is local.
+            if (currentClip == BorderFillBox && cur->attachment() == LocalBackgroundAttachment)
+                currentClip = PaddingFillBox;
+            backgroundBox = enclosingFillBox(backgroundBox, currentClip);

[trchen, 2016/08/13 00:06:42]

I'm worried about this line, because the unscrolled content box and the local
content box are different boxes.

http://jsbin.com/dujuzazeva/ demonstrates the difference.

[flackr, 2016/08/16 17:52:51]

Right, I think we should treat background-attachment: local content-box as the
padding box when we're asking for the "translucent" / clip / cull rect (i.e.
over-estimate is okay) but the content-box when we're asking for the opaque
background rect (i.e. under-estimate is okay). Perhaps this will be more clear
with the new enum values.

[trchen, 2016/08/17 01:22:55]

Agreed. We can do that plus a comment to explain.

[flackr, 2016/08/17 18:20:55]

Done, and added a test for this case.

+        } while (current);
+    }
+    switch (backgroundBox) {
     case BorderFillBox:
-        // A 'border-box' clip on scrollable elements local attachment is treated as 'padding-box'.
-        // https://www.w3.org/TR/css3-background/#the-background-attachment
-        if (!style()->isOverflowVisible() && style()->backgroundLayers().attachment() == LocalBackgroundAttachment)

[chrishtr, 2016/08/12 17:03:00]

Why can you remove this?

[trchen, 2016/08/13 00:06:42]

The matching logic is in line 730 on the right side.

-            return paddingBoxRect();
         return borderBoxRect();
         break;
     case PaddingFillBox:
         return paddingBoxRect();
         break;
     case ContentFillBox:
         return contentBoxRect();
         break;
     default:
         break;
@@ skipping 643 matching lines @@
         return false;
     paintedExtent = LayoutRect(geometry.destRect());
     return true;
 }
 
 bool LayoutBox::backgroundIsKnownToBeOpaqueInRect(const LayoutRect& localRect) const
 {
     if (isDocumentElement() || backgroundStolenForBeingBody())
         return false;
 
-    Color backgroundColor = resolveColor(CSSPropertyBackgroundColor);
-    if (backgroundColor.hasAlpha())
-        return false;
-
     // If the element has appearance, it might be painted by theme.
     // We cannot be sure if theme paints the background opaque.
     // In this case it is safe to not assume opaqueness.
     // FIXME: May be ask theme if it paints opaque.
     if (style()->hasAppearance())
         return false;
     // FIXME: Check the opaqueness of background images.
 
     // FIXME: Use rounded rect if border radius is present.
     if (style()->hasBorderRadius())
         return false;
     if (hasClipPath())
         return false;
-    // FIXME: The background color clip is defined by the last layer.
-    if (style()->backgroundLayers().next())
+    if (style()->hasBlendMode())
         return false;
-    return backgroundClipRect().contains(localRect);
+    return backgroundRect(OpaqueBackgroundRect).contains(localRect);
 }
 
 static bool isCandidateForOpaquenessTest(const LayoutBox& childBox)
 {
     const ComputedStyle& childStyle = childBox.styleRef();
     if (childStyle.position() != StaticPosition && childBox.containingBlock() != childBox.parent())
         return false;
     if (childStyle.visibility() != EVisibility::Visible || childStyle.shapeOutside())
         return false;
     if (childBox.size().isZero())
@@ skipping 352 matching lines @@
     LayoutBoxModelObject::mapLocalToAncestor(ancestor, transformState, mode);
 }
 
 void LayoutBox::mapAncestorToLocal(const LayoutBoxModelObject* ancestor, TransformState& transformState, MapCoordinatesFlags mode) const
 {
     if (this == ancestor)
         return;
 
     bool isFixedPos = style()->position() == FixedPosition;
 
-    if (style()->canContainFixedPositionObjects() && !isFixedPos) {
-        // If this box has a transform or contains paint, it acts as a fixed position container for fixed descendants,
-        // and may itself also be fixed position. So propagate 'fixed' up only if this box is fixed position.
+    // If this box has a transform or contains paint, it acts as a fixed position container for fixed descendants,
+    // and may itself also be fixed position. So propagate 'fixed' up only if this box is fixed position.
+    if (style()->canContainFixedPositionObjects() && !isFixedPos)
         mode &= ~IsFixed;
-    } else if (isFixedPos) {
+    else if (isFixedPos)
         mode |= IsFixed;
-    }
 
     LayoutBoxModelObject::mapAncestorToLocal(ancestor, transformState, mode);
 }
 
 LayoutSize LayoutBox::offsetFromContainer(const LayoutObject* o) const
 {
     ASSERT(o == container());
 
     LayoutSize offset;
     if (isInFlowPositioned())
@@ skipping 3205 matching lines @@
         m_rareData->m_snapAreas->remove(&snapArea);
     }
 }
 
 SnapAreaSet* LayoutBox::snapAreas() const
 {
     return m_rareData ? m_rareData->m_snapAreas.get() : nullptr;
 }
 
 } // namespace blink