Bug 130664

Summary: [CSS Blending] Compositing requirements for blending are not computed correctly
Product: WebKit Reporter: Ion Rosca <rosca>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, glenn, kondapallykalyan, simon.fraser, WebkitBugTracker
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 95614    
Attachments:
Description Flags
use cases
none
patch v1
none
add ifdef to blending code, nits
none
more comments and ifdefs none

Description Ion Rosca 2014-03-24 03:50:34 PDT
Here is the requirement regarding layer acceleration for blend modes:
    If a layer having blending is accelerated for any reason then its stacking context layer needs to be accelerated too.

What happens now:
1) If a blending layer is accelerated, all its RenderLayer ascendants get accelerated.  
2) If a non-accelerated layer has an accelerated sibling, then all its ascendants are accelerated.
Comment 1 Ion Rosca 2014-03-24 03:51:22 PDT
Created attachment 227636 [details]
use cases
Comment 2 Ion Rosca 2014-03-24 12:08:22 PDT
Created attachment 227676 [details]
patch v1
Comment 3 Ion Rosca 2014-03-31 07:52:56 PDT
Created attachment 228178 [details]
add ifdef to blending code, nits
Comment 4 Mihnea Ovidenie 2014-03-31 10:34:02 PDT
Comment on attachment 228178 [details]
add ifdef to blending code, nits

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

> Source/WebCore/ChangeLog:13
> +        being the closest stacking context should be accelerated as well.

Probably this can be rewritten: "if a layer having blend mode is composited, its closest stacking context ancestor should be composited as well"

> Source/WebCore/ChangeLog:23
> +        * rendering/RenderLayerCompositor.cpp:

You should mention something about the renaming of m_subtreeHasBlending => m_hasUnisolatedCompositedBlendingDescendants in CompositingState.

> Source/WebCore/rendering/RenderLayer.h:789
> +    bool isolatesCompositedBlending() const

In general, i would like to see as much as possible code related to this feature guarded by ENABLE(CSS_COMPOSITING)

> Source/WebCore/rendering/RenderLayerCompositor.cpp:-208
> -        , m_subtreeHasBlending(other.m_subtreeHasBlending)

Maybe you can make these bool bit fields in a follow up patch.
Comment 5 Ion Rosca 2014-03-31 14:19:27 PDT
Created attachment 228203 [details]
more comments and ifdefs
Comment 6 WebKit Commit Bot 2014-04-02 00:31:19 PDT
Comment on attachment 228203 [details]
more comments and ifdefs

Clearing flags on attachment: 228203

Committed r166634: <http://trac.webkit.org/changeset/166634>
Comment 7 WebKit Commit Bot 2014-04-02 00:31:24 PDT
All reviewed patches have been landed.  Closing bug.