[webkit-reviews] review denied: [Bug 99200] Add support for blendmode to Core Animation layer : [Attachment 168574] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Oct 15 11:00:54 PDT 2012
Simon Fraser (smfr) <simon.fraser at apple.com> has denied Rik Cabanier
<cabanier at adobe.com>'s request for review:
Bug 99200: Add support for blendmode to Core Animation layer
https://bugs.webkit.org/show_bug.cgi?id=99200
Attachment 168574: Patch
https://bugs.webkit.org/attachment.cgi?id=168574&action=review
------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=168574&action=review
> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:1126
> + if (m_uncommittedChanges & CompositingChanged)
> + updateBlending();
Confusing that you called the flag CompositingChanged but then call
updateBlending().
> Source/WebCore/rendering/RenderLayerBacking.cpp:1389
> + if (m_graphicsLayer)
> + m_graphicsLayer->setBlendMode(blendMode);
No need to null-check m_graphicsLayer.
> Source/WebCore/rendering/RenderLayerCompositor.cpp:845
> + if (childState.m_subtreeHasBlending || layer->hasBlendMode())
> + compositingState.m_subtreeHasBlending = true;
I don't think you need to track this at all.
> Source/WebCore/rendering/RenderLayerCompositor.cpp:1805
> + bool HasBlendedDescendants, bool has3DTransformedDescendants,
RenderLayer::IndirectCompositingReason& reason) const
HasBlendedDescendants -> hasBlendedDescendants
> Source/WebCore/rendering/RenderLayerCompositor.cpp:1812
> - if (hasCompositedDescendants && (layer->transform() ||
renderer->createsGroup() || renderer->hasReflection())) {
> + if (hasCompositedDescendants
> + && (HasBlendedDescendants || layer->transform() ||
renderer->createsGroup() || renderer->hasReflection())) {
You have this backwards. A layer with blending needs to be composited if has
composited descendants, but not vice versa. It should be just like opacity and
filters, which are consulted in createsGroup().
> LayoutTests/css3/compositing/effect-blend-mode-expected.txt:38
> +layer at (0,0) size 800x600
> + RenderView at (0,0) size 800x600
> +layer at (0,0) size 800x600
> + RenderBlock {HTML} at (0,0) size 800x600
> + RenderBody {BODY} at (8,8) size 784x584
> + RenderBlock {UL} at (0,0) size 784x0
> + RenderBlock (floating) {LI} at (45,5) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> + RenderBlock (floating) {LI} at (185,5) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> + RenderBlock (floating) {LI} at (325,5) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> + RenderBlock (floating) {LI} at (465,5) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> + RenderBlock (floating) {LI} at (605,5) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> + RenderBlock (floating) {LI} at (45,145) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> + RenderBlock (floating) {LI} at (185,145) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> + RenderBlock (floating) {LI} at (325,145) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> + RenderBlock (floating) {LI} at (465,145) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> + RenderBlock (floating) {LI} at (605,145) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> + RenderBlock (floating) {LI} at (45,285) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> + RenderBlock (floating) {LI} at (185,285) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> + RenderBlock (floating) {LI} at (325,285) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> + RenderBlock (floating) {LI} at (465,285) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> + RenderBlock (floating) {LI} at (605,285) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> + RenderBlock (floating) {LI} at (45,425) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
This output is not useful. It should be a dumpAsText(true) test.
> LayoutTests/platform/mac/css3/compositing/effect-blend-mode-expected.txt:53
> +layer at (0,0) size 800x600
> + RenderView at (0,0) size 800x600
> +layer at (0,0) size 800x576
> + RenderBlock {HTML} at (0,0) size 800x576
> + RenderBody {BODY} at (8,16) size 784x0
> + RenderBlock {UL} at (0,0) size 784x0
> + RenderBlock (floating) {LI} at (45,5) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> + RenderBlock (floating) {LI} at (185,5) size 130x130
> + RenderBlock (floating) {LI} at (325,5) size 130x130
> + RenderBlock (floating) {LI} at (465,5) size 130x130
> + RenderBlock (floating) {LI} at (605,5) size 130x130
> + RenderBlock (floating) {LI} at (45,145) size 130x130
> + RenderBlock (floating) {LI} at (185,145) size 130x130
> + RenderBlock (floating) {LI} at (325,145) size 130x130
> + RenderBlock (floating) {LI} at (465,145) size 130x130
> + RenderBlock (floating) {LI} at (605,145) size 130x130
> + RenderBlock (floating) {LI} at (45,285) size 130x130
> + RenderBlock (floating) {LI} at (185,285) size 130x130
> + RenderBlock (floating) {LI} at (325,285) size 130x130
> + RenderBlock (floating) {LI} at (465,285) size 130x130
> + RenderBlock (floating) {LI} at (605,285) size 130x130
> + RenderBlock (floating) {LI} at (45,425) size 130x130
> +layer at (193,21) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> +layer at (333,21) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> +layer at (473,21) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> +layer at (613,21) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> +layer at (53,161) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> +layer at (193,161) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> +layer at (333,161) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> +layer at (473,161) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> +layer at (613,161) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> +layer at (53,301) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> +layer at (193,301) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> +layer at (333,301) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> +layer at (473,301) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> +layer at (613,301) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
> +layer at (53,441) size 130x130
> + RenderImage {IMG} at (0,0) size 130x130
Ditto.
>
LayoutTests/platform/mac/css3/compositing/should-have-compositing-layer-expecte
d.txt:1
> +Test to make sure a blend mode creates a compositing layer. Test is
successful of render tree shows compositing
You need a test that has blending on an element with composited children, and
dumps the layer tree too.
More information about the webkit-reviews
mailing list