Bug 116140 – Direct pattern compositing breaks when no-repeat is set on a large layer

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Noam Rosenthal]

Direct pattern compositing breaks when no-repeat is set on a large layer

[Noam Rosenthal]

Created attachment 201797 [details]
Patch

[Csaba Osztrogonác]

Comment on attachment 201797 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=201797&action=review

> LayoutTests/compositing/patterns/direct-pattern-compositing-contain-no-repeat-expected.html:8
> +      background-image: url(http://achicu.github.io/css-presentation/images/webkit-icon.png);

Do we really want to depend on an external image?

> LayoutTests/compositing/patterns/direct-pattern-compositing-contain-no-repeat.html:8
> +      background-image: url(http://achicu.github.io/css-presentation/images/webkit-icon.png);

ditto

[Noam Rosenthal]

Created attachment 201798 [details]
Patch

[Noam Rosenthal]

(In reply to comment #2)
> (From update of attachment 201797 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=201797&action=review
> 
> > LayoutTests/compositing/patterns/direct-pattern-compositing-contain-no-repeat-expected.html:8
> > +      background-image: url(http://achicu.github.io/css-presentation/images/webkit-icon.png);
> 
> Do we really want to depend on an external image?
> 
> > LayoutTests/compositing/patterns/direct-pattern-compositing-contain-no-repeat.html:8
> > +      background-image: url(http://achicu.github.io/css-presentation/images/webkit-icon.png);
> 
> ditto

Oops, of course not :)

[Benjamin Poulain]

Comment on attachment 201798 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=201798&action=review

> LayoutTests/compositing/patterns/direct-pattern-compositing-contain-no-repeat-expected.html:4
> +    .composited { -webkit-transform: matrix3d(1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 3, 1); }

I'd remove this from the reference.

[Simon Fraser (smfr)]

Comment on attachment 201798 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=201798&action=review

> Source/WebCore/ChangeLog:10
> +        Making sure that the solid color logic only overrides the cotnentsRect if the background

Typo cotnentsRect

> Source/WebCore/rendering/RenderLayerBacking.cpp:894
>      updateDirectlyCompositedBackgroundImage(isSimpleContainer, didUpdateContentsRect);
> +    if (didUpdateContentsRect)
> +        return;
>      updateDirectlyCompositedBackgroundColor(isSimpleContainer, didUpdateContentsRect);

It's not clear why you should bail if didUpdateContentsRect is true. Does that mean you chose to use the background image path? Why does didUpdateContentsRect indicate that, rather than the return value?

>> LayoutTests/compositing/patterns/direct-pattern-compositing-contain-no-repeat-expected.html:4
>> +    .composited { -webkit-transform: matrix3d(1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 3, 1); }
> 
> I'd remove this from the reference.

Composited vs. non-composted refs break on Mac, alas.

But this should use translateZ() or something less mysterious.

[Noam Rosenthal]

> It's not clear why you should bail if didUpdateContentsRect is true. Does that mean you chose to use the background image path? Why does didUpdateContentsRect indicate that, rather than the return value?
> 
Right, I'll change it to be a return value.

> >> LayoutTests/compositing/patterns/direct-pattern-compositing-contain-no-repeat
> 
> Composited vs. non-composted refs break on Mac, alas.
Is that going to be fixed as we discussed at the summit, or should I make this into a regular pixel test? Ref-tests seem like the right choice here given that the platform supports them.

> But this should use translateZ() or something less mysterious.
Sure :)

[Noam Rosenthal]

Created attachment 202070 [details]
Patch

[Noam Rosenthal]

Created attachment 202829 [details]
Patch

[Antti Koivisto]

Comment on attachment 202829 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=202829&action=review

> Source/WebCore/rendering/RenderLayerBacking.cpp:879
> -    updateDirectlyCompositedBackgroundImage(isSimpleContainer, didUpdateContentsRect);
>      updateDirectlyCompositedBackgroundColor(isSimpleContainer, didUpdateContentsRect);
> +    updateDirectlyCompositedBackgroundImage(isSimpleContainer, didUpdateContentsRect);

Would be good to add a comment mentioning why the order matters.

[Noam Rosenthal]

Created attachment 202879 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 202879 [details]
Patch for landing

Clearing flags on attachment: 202879

Committed r150685: <http://trac.webkit.org/changeset/150685>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.