New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 229166 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Email to this user bounced
Closed: Apr 2015
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 229615



Sign in to add a comment

[META][CSS Blending] Add support for -webkit-background-blend-mode to chromium

Project Member Reported by schenney@chromium.org, Apr 8 2013

Issue description

Migrated from WebKit Bugzilla: https://bugs.webkit.org/show_bug.cgi?id=108550
Originally reported 2013-01-31 16:53 PST by Rik Cabanier (cabanier@adobe.com).


Description:
Add support for -webkit-background-blend-mode to chromium


Blocks:
[http://wkb.ug/108546] NEW - Add support for blending to background images
Depends On:
[http://wkb.ug/108547] RESOLVED FIXED - Add support for parsing of -webkit-background-blend-mode
[http://wkb.ug/111818] RESOLVED FIXED - create runtime flags for CSS Compositing
[http://wkb.ug/113394] NEW - Add support for runtime flags for experimental blending to chromium

Attachments:
2013-04-02 02:37 PST: Patch [https://bugs.webkit.org/attachment.cgi?id=196107&action=prettypatch]



Comments:
================================

Comment #1
Posted on 2013-04-02 02:37:09 PST by Horia Olaru (olaru@adobe.com)

Created an attachment (id=196107) [https://bugs.webkit.org/attachment.cgi?id=196107] [details] [https://bugs.webkit.org/attachment.cgi?id=196107&action=edit]
Patch

================================

Comment #2
Posted on 2013-04-02 06:20:00 PST by Horia Olaru (olaru@adobe.com)

I have taken the Skia blend mode enabling code from the latest patches in  bug 100071  [https://bugs.webkit.org/show_bug.cgi?id=100071], so one could say they are partially reviewed. Therefore, I'm not sure how we should go about this code.

What is added on top of that is changes to ImageSkia to take into account the blend mode, and a new test that goes through Image::drawPattern (which did not get called otherwise).

I also have a question. Skia relies on the SkPaint object to pass and read the blend/composite mode (setXfermodeMode). However, I noticed that the Image::draw* functions also receive a GraphicsContext object, which also has a reference to a PlatformGraphicsContext (returned by platformContext). For Skia, this is a PlatformContextSkia object. The GraphicsContext object has a setCompositeOperation function, which ultimately ends up calling the PlatformContextSkia::setXfermodeMode. However, I'm not sure if this xfer mode is ever actually read or used from somewhere, since the SkPaint object seems to be enough to pass in this mode to Skia.

I feel like I should call setCompositeOperation, so that the Skia graphics context it is in sync with the SkPaint mode, in case someone reads it in the future, but I would like another opinion on it. Is this API only there to conform to the GraphicsContext API? Because setCompositeOperation seems to be redundant (at least for Skia), so I might be missing something.

================================

Comment #3
Posted on 2013-04-02 10:55:39 PST by Rik Cabanier (cabanier@adobe.com)

(In reply to comment #2 [https://bugs.webkit.org/show_bug.cgi?id=108550#c2])
> I have taken the Skia blend mode enabling code from the latest patches in  bug 100071  [https://bugs.webkit.org/show_bug.cgi?id=100071], so one could say they are partially reviewed. Therefore, I'm not sure how we should go about this code.

Since this is a patch for chromium and is blocked by 113394, you should add test results for that platform too.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 20 2013

Labels: -WebKit-ID-108550 WebKit-Rev-142168 WebKit-ID-108550-RESOLVED WebKit-Rev-145784
https://bugs.webkit.org/show_bug.cgi?id=108550
http://trac.webkit.org/changeset/142168
http://trac.webkit.org/changeset/145784

Project Member

Comment 2 by bugdroid1@chromium.org, May 20 2013

The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=150710

------------------------------------------------------------------------
r150710 | olaru@adobe.com | 2013-05-20T22:50:04.570832Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/platform/graphics/skia/ImageSkia.cpp?r1=150710&r2=150709&pathrev=150710
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/css3/compositing/effect-background-blend-mode-tiled-expected.txt?r1=150710&r2=150709&pathrev=150710
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/css3/compositing/effect-background-blend-mode-expected.txt?r1=150710&r2=150709&pathrev=150710
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/css3/compositing/effect-background-blend-mode-stacking-expected.txt?r1=150710&r2=150709&pathrev=150710
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/css3/compositing/effect-background-blend-mode-tiled.html?r1=150710&r2=150709&pathrev=150710
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/TestExpectations?r1=150710&r2=150709&pathrev=150710
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/css3/compositing/effect-background-blend-mode.html?r1=150710&r2=150709&pathrev=150710
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/css3/compositing/effect-background-blend-mode-stacking.html?r1=150710&r2=150709&pathrev=150710

Addding support for the background-blend-mode CSS property.

The patch forwards the background blend modes from WebCore to Skia, enabling background images to blend between themeselves.

Added 3 pixel tests. The first two, effect-background-blend-mode and effect-background-blend-mode-stacking are just readded after being removed in CL 149989. They were removed because they failed, since background blend mode was not implemented, and TestExpectations entries were ignored.

The effect-background-blend-mode-tiled test is new, and causes Image::drawPattern to be called (since the other background blending tests did not go through that function).

Add logic to override the acceleratedCompositingEnabled setting on testRunner through the WebKitAcceleratedCompositingEnabled key. This was necessary because blending was rendered in software in Skia until not long ago, and there was no hardware path available.

R=eric@chromium.org
BUG= 229166 

Review URL: https://chromiumcodereview.appspot.com/14208013
------------------------------------------------------------------------
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 23 2014

The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=165628

------------------------------------------------------------------------
r165628 | mitica@adobe.com | 2014-01-23T10:27:36.704921Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/TestExpectations?r1=165628&r2=165627&pathrev=165628
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/css3/compositing/background-blend-mode-svg-expected.html?r1=165628&r2=165627&pathrev=165628
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/css3/compositing/background-blend-mode-svg.html?r1=165628&r2=165627&pathrev=165628
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/svg/graphics/SVGImage.cpp?r1=165628&r2=165627&pathrev=165628

[CSS Blending] background-blend-mode fails for certain SVG files.
Porting webkit fix from https://bugs.webkit.org/show_bug.cgi?id=127350:
The graphics context of the SVG inherits the blend mode set on the
background layer. Fix consists in drawing the SVG in a transparency
layer.

BUG= 229166 

Review URL: https://codereview.chromium.org/145063003
------------------------------------------------------------------------
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 24 2014

The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=165790

------------------------------------------------------------------------
r165790 | ojan@chromium.org | 2014-01-24T23:09:37.225667Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/win/css3/compositing/background-blend-mode-svg-color-expected.png?r1=165790&r2=165789&pathrev=165790
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/TestExpectations?r1=165790&r2=165789&pathrev=165790
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/linux/css3/compositing/background-blend-mode-svg-color-expected.png?r1=165790&r2=165789&pathrev=165790
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/android/css3/compositing/background-blend-mode-svg-color-expected.png?r1=165790&r2=165789&pathrev=165790
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/css3/compositing/background-blend-mode-svg-color-expected.png?r1=165790&r2=165789&pathrev=165790
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/win/css3/compositing/background-blend-mode-crossfade-image-gradient-expected.png?r1=165790&r2=165789&pathrev=165790
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/linux/css3/compositing/background-blend-mode-crossfade-image-gradient-expected.png?r1=165790&r2=165789&pathrev=165790
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/android/css3/compositing/background-blend-mode-crossfade-image-gradient-expected.png?r1=165790&r2=165789&pathrev=165790
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/platform/mac/css3/compositing/background-blend-mode-crossfade-image-gradient-expected.png?r1=165790&r2=165789&pathrev=165790

Auto-rebaseline for r165628

http://src.chromium.org/viewvc/blink?view=revision&revision=165628

BUG= 229166 
TBR=mitica@adobe.com

Review URL: https://codereview.chromium.org/139853027
------------------------------------------------------------------------
It's under experimental feature flag for a while. any blocker prob for unflagging?
Cc: eseidel@chromium.org abarth@chromium.org
At a minimum I believe we need must remove the vendor prefix. Chrome will not ship any more new vendor prefixed flags.

Has there been an "intent to ship" for this feature? I seem to recall one.

Comment 7 by rcaban...@gmail.com, Feb 25 2014

Yes, we had a lgtm on shipping this last week and it was landed without a prefix a couple of days ago. 
Thanks, I could see the demo working on canary w/o flag.
http://codepen.io/adobe/pen/FeiCp

Comment 9 by caban...@gmail.com, Mar 2 2014

great!
See also http://codepen.io/bennettfeely/full/rxoAc for the typical use case
This fix looks like a simple fix but makes so many senses. (Wish to see css filter effects/shaders as well, in not so distance future :)
Cc: mihnea.a...@gmail.com ro...@adobe.com
Summary: [META][CSS Blending] Add support for -webkit-background-blend-mode to chromium (was: Add support for -webkit-background-blend-mode to chromium)
Blocking: chromium:229615
Labels: -WebKit-CSS Cr-Blink-CSS
Owner: caban...@adobe.com
Status: Fixed
Seems fixed per comment #7.

Sign in to add a comment