Summary: | Direct pattern compositing breaks when no-repeat is set on a large layer | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Noam Rosenthal <noam> | ||||||||||||
Component: | New Bugs | Assignee: | Noam Rosenthal <noam> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | commit-queue, esprehn+autocc, glenn, simon.fraser | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Noam Rosenthal
2013-05-15 00:36:28 PDT
Created attachment 201797 [details]
Patch
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 Created attachment 201798 [details]
Patch
(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 :) 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. 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. > 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 :) Created attachment 202070 [details]
Patch
Created attachment 202829 [details]
Patch
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. Created attachment 202879 [details]
Patch for landing
Comment on attachment 202879 [details] Patch for landing Clearing flags on attachment: 202879 Committed r150685: <http://trac.webkit.org/changeset/150685> All reviewed patches have been landed. Closing bug. |