Summary: | [META][CSS Blending] Add support for blendmode to platform code for mac | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Rik Cabanier <cabanier> | ||||||||||||
Component: | CSS | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
Status: | NEW --- | ||||||||||||||
Severity: | Normal | CC: | achicu, dglazkov, dino, donggwan.kim, eric, krit, mihnea, simon.fraser, webkit.review.bot | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Bug Depends on: | 129661, 131354, 132675, 133178, 98450, 99119, 99200, 125804, 126158, 126160, 128805, 129154, 130331, 130337, 130582, 130664, 130891, 130892, 130922, 131153, 131355, 131436, 131824, 131889, 132325, 132578, 132600, 136563 | ||||||||||||||
Bug Blocks: | 91908 | ||||||||||||||
Attachments: |
|
Description
Rik Cabanier
2012-08-31 15:03:55 PDT
Created attachment 161789 [details]
First try
Comment on attachment 161789 [details] First try Attachment 161789 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13732113 New failing tests: css3/compositing/effect-blend-mode.html Comment on attachment 161789 [details] First try Attachment 161789 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13729202 New failing tests: css3/compositing/effect-blend-mode.html Created attachment 161837 [details]
updated expectations file for chromium
Comment on attachment 161837 [details] updated expectations file for chromium View in context: https://bugs.webkit.org/attachment.cgi?id=161837&action=review > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:1081 > +#if ENABLE(CSS_COMPOSITING) > + if (m_uncommittedChanges & CompositingChanged) > + updateLayersCompositing(); > +#endif I think it's very confusing to use the term "compositing" in code which is all about accelerated compositing. Maybe use "blending" instead? > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2782 > +void GraphicsLayerCA::updateLayersCompositing() This should become updateBlending() or updateBlendMode(). The 'layer' in the name is redundant. > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h:383 > + CompositingChanged = 1 << 26, blending. > Source/WebCore/rendering/RenderLayerBacking.cpp:253 > +void RenderLayerBacking::updateLayerBlendMode(const RenderStyle* style) 'layer' here is redundant. (In reply to comment #5) > (From update of attachment 161837 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=161837&action=review > > > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:1081 > > +#if ENABLE(CSS_COMPOSITING) > > + if (m_uncommittedChanges & CompositingChanged) > > + updateLayersCompositing(); > > +#endif > > I think it's very confusing to use the term "compositing" in code which is all about accelerated compositing. Maybe use "blending" instead? > > > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2782 > > +void GraphicsLayerCA::updateLayersCompositing() > > This should become updateBlending() or updateBlendMode(). The 'layer' in the name is redundant. > > > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h:383 > > + CompositingChanged = 1 << 26, > > blending. the blendmode is only the first item. The spec defines keywords for compositing as well and they will follow the same codepath. > > > Source/WebCore/rendering/RenderLayerBacking.cpp:253 > > +void RenderLayerBacking::updateLayerBlendMode(const RenderStyle* style) > > 'layer' here is redundant. I will fix this. > > Source/WebCore/rendering/RenderLayerBacking.cpp:253 > > +void RenderLayerBacking::updateLayerBlendMode(const RenderStyle* style) > > 'layer' here is redundant. It's the same as updateLayerFilters, updateLayerOpacity & updateLayerTransform. Should those functions change as well? > I think it's very confusing to use the term "compositing" in code which is all about accelerated compositing. Maybe use "blending" instead? To clarify, this function will also be used to set up the compositing modes and group isolation. (In reply to comment #7) > > > Source/WebCore/rendering/RenderLayerBacking.cpp:253 > > > +void RenderLayerBacking::updateLayerBlendMode(const RenderStyle* style) > > > > 'layer' here is redundant. > > It's the same as updateLayerFilters, updateLayerOpacity & updateLayerTransform. > Should those functions change as well? Yes, but you don't have to do that in this patch. > > I think it's very confusing to use the term "compositing" in code which is all about accelerated compositing. Maybe use "blending" instead? > > To clarify, this function will also be used to set up the compositing modes and group isolation. I still think using the term "compositing" unqualified in the context of GraphicsLayer is very confusing. I think a less confusing but not quite accurate name would be better. Created attachment 163315 [details]
renamed function per Simon Fraser's request
Comment on attachment 163315 [details] renamed function per Simon Fraser's request View in context: https://bugs.webkit.org/attachment.cgi?id=163315&action=review > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2794 > +void GraphicsLayerCA::updateBlending() > +{ > + primaryLayer()->setBlendMode(m_blendMode); > + > + if (LayerMap* layerCloneMap = primaryLayerClones()) { > + LayerMap::const_iterator end = layerCloneMap->end(); > + for (LayerMap::const_iterator it = layerCloneMap->begin(); it != end; ++it) { > + if (m_replicaLayer && isReplicatedRootClone(it->first)) > + continue; > + it->second->setBlendMode(m_blendMode); > + } > + } > +} Does this play nicely with -webkit-transform: preserve-3d? Before, we've had spew from Core Animation when setting filters on a CATransformLayer. (In reply to comment #10) > (From update of attachment 163315 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=163315&action=review > > > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2794 > > +void GraphicsLayerCA::updateBlending() > > +{ > > + primaryLayer()->setBlendMode(m_blendMode); > > + > > + if (LayerMap* layerCloneMap = primaryLayerClones()) { > > + LayerMap::const_iterator end = layerCloneMap->end(); > > + for (LayerMap::const_iterator it = layerCloneMap->begin(); it != end; ++it) { > > + if (m_replicaLayer && isReplicatedRootClone(it->first)) > > + continue; > > + it->second->setBlendMode(m_blendMode); > > + } > > + } > > +} > > Does this play nicely with -webkit-transform: preserve-3d? Before, we've had spew from Core Animation when setting filters on a CATransformLayer. It plays along fairly well. If you set the blend mode on an object that has a 3d transform, it will blend in 3d. I did not see any warnings. If you set a blend (or opacity or a filter) on an element, preserve-3d is supposed to be ignored but that doesn't happen today. That would be a new bug if you want me to look into that... > It plays along fairly well. If you set the blend mode on an object that has a 3d transform, it will blend in 3d.
preserve-3d doesn't create a 3D transform. It causes us to create a CATransformLayer, which may not be able to handle compositing filters.
(In reply to comment #12) > > It plays along fairly well. If you set the blend mode on an object that has a 3d transform, it will blend in 3d. > preserve-3d doesn't create a 3D transform. It causes us to create a CATransformLayer, which may not be able to handle compositing filters. I morphed the example from http://www.webkit.org/blog-files/3d-transforms/transform-style.html. What I see is that blending is limited to within the group that has the preserve-3d applied. If I apply it on the parent, it doesn't work. I don't think that that's a big deal because we should fix the bug that blending/opacity/filters removes the effect of the preserve-3d flag. Comment on attachment 163315 [details] renamed function per Simon Fraser's request View in context: https://bugs.webkit.org/attachment.cgi?id=163315&action=review > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:1080 > + if (m_uncommittedChanges & CompositingChanged) > + updateBlending(); The 'changed' enum name should match the updateFoo. > LayoutTests/css3/compositing/effect-blend-mode-expected.txt:3 > +layer at (0,0) size 800x600 > + RenderView at (0,0) size 800x600 > +layer at (0,0) size 800x600 Is this useful as render tree ouput? Also, this isn't going to work in WebKit1, where parts of the page content are rendered into the window, outside of Core Animation. Created attachment 167643 [details]
Updated patch because it became out-of-date
Created attachment 167644 [details]
Updated patch because previous version was out-of-date
Comment on attachment 167644 [details] Updated patch because previous version was out-of-date Attachment 167644 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14204899 New failing tests: css3/compositing/effect-blend-mode.html Comment on attachment 167644 [details] Updated patch because previous version was out-of-date Attachment 167644 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14211600 New failing tests: css3/compositing/effect-blend-mode.html |