Bug 98450 – Add support for blendmode to WebKit1 platform code

• Status: RESOLVED
• Resolution: WONTFIX
• Component: CSS

[Rik Cabanier]

Bug 95614 will add support for WebKit2 code.
This patch will add it to the WebKit1 codebase.

[Rik Cabanier]

Created attachment 167593 [details]
just seeing what the bots say. not ready for checkin yet

[Early Warning System Bot]

Comment on attachment 167593 [details]
just seeing what the bots say. not ready for checkin yet

Attachment 167593 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14209584

[Early Warning System Bot]

Comment on attachment 167593 [details]
just seeing what the bots say. not ready for checkin yet

Attachment 167593 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/14211484

[Gyuyoung Kim]

Comment on attachment 167593 [details]
just seeing what the bots say. not ready for checkin yet

Attachment 167593 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14209588

[Build Bot]

Comment on attachment 167593 [details]
just seeing what the bots say. not ready for checkin yet

Attachment 167593 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14205768

[Rik Cabanier]

Created attachment 167602 [details]
made more code protected by flags. not ready

[kov's GTK+ EWS bot]

Comment on attachment 167602 [details]
made more code protected by flags. not ready

Attachment 167602 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/14215450

[Gyuyoung Kim]

Comment on attachment 167602 [details]
made more code protected by flags. not ready

Attachment 167602 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14204787

[Rik Cabanier]

Created attachment 167622 [details]
more conditional code. Not ready for review

[Gyuyoung Kim]

Comment on attachment 167622 [details]
more conditional code. Not ready for review

Attachment 167622 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14215470

[Early Warning System Bot]

Comment on attachment 167622 [details]
more conditional code. Not ready for review

Attachment 167622 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14207644

[Early Warning System Bot]

Comment on attachment 167622 [details]
more conditional code. Not ready for review

Attachment 167622 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/14204818

[WebKit Review Bot]

Comment on attachment 167622 [details]
more conditional code. Not ready for review

Attachment 167622 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14228366

[Peter Beverloo (cr-android ews)]

Comment on attachment 167622 [details]
more conditional code. Not ready for review

Attachment 167622 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/14229287

[Build Bot]

Comment on attachment 167622 [details]
more conditional code. Not ready for review

Attachment 167622 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14229301

[Build Bot]

Comment on attachment 167622 [details]
more conditional code. Not ready for review

Attachment 167622 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14229368

New failing tests:
css3/compositing/should-have-compositing-layer.html

[Rik Cabanier]

Created attachment 167705 [details]
fixes for QT & Chromium. Not ready for review

[Gyuyoung Kim]

Comment on attachment 167705 [details]
fixes for QT & Chromium. Not ready for review

Attachment 167705 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14209803

[Early Warning System Bot]

Comment on attachment 167705 [details]
fixes for QT & Chromium. Not ready for review

Attachment 167705 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14238195

[Early Warning System Bot]

Comment on attachment 167705 [details]
fixes for QT & Chromium. Not ready for review

Attachment 167705 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/14207815

[Rik Cabanier]

Created attachment 167709 [details]
this is the actual patch. Previous one was old. not for review

[WebKit Review Bot]

Attachment 167709 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/platform/graphics/GraphicsC..." exit_code: 1
Source/WebCore/platform/graphics/qt/ImageBufferQt.cpp:129:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Rik Cabanier]

Created attachment 167710 [details]
fix for strange webkit style problem

[Early Warning System Bot]

Comment on attachment 167709 [details]
this is the actual patch. Previous one was old. not for review

Attachment 167709 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14238207

[Early Warning System Bot]

Comment on attachment 167709 [details]
this is the actual patch. Previous one was old. not for review

Attachment 167709 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/14228553

[Rik Cabanier]

Created attachment 167712 [details]
silly QT typo

[Build Bot]

Comment on attachment 167709 [details]
this is the actual patch. Previous one was old. not for review

Attachment 167709 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14229470

[Build Bot]

Comment on attachment 167712 [details]
silly QT typo

Attachment 167712 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14245018

[Build Bot]

Comment on attachment 167709 [details]
this is the actual patch. Previous one was old. not for review

Attachment 167709 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14223633

New failing tests:
css3/compositing/should-have-compositing-layer.html

[Build Bot]

Comment on attachment 167712 [details]
silly QT typo

Attachment 167712 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14214692

New failing tests:
css3/compositing/should-have-compositing-layer.html

[Rik Cabanier]

Created attachment 167774 [details]
Type in non-compositing code

[Rik Cabanier]

Created attachment 167794 [details]
previous patch failed to apply

[Rik Cabanier]

Created attachment 167848 [details]
added test case and changelog

[Rik Cabanier]

Created attachment 167862 [details]
Fixed a couple of typos

[Simon Fraser (smfr)]

Comment on attachment 167862 [details]
Fixed a couple of typos

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

> Source/WebCore/platform/graphics/GraphicsContext.cpp:595
> +#if ENABLE(CSS_COMPOSITING)
> +        image->draw(this, styleColorSpace, FloatRect(dest.location(), FloatSize(tw, th)), FloatRect(src.location(), FloatSize(tsw, tsh)), op, useLowQualityScale, blendMode);
> +#else        
>          image->draw(this, styleColorSpace, FloatRect(dest.location(), FloatSize(tw, th)), FloatRect(src.location(), FloatSize(tsw, tsh)), op, useLowQualityScale);
> +        UNUSED_PARAM(blendMode);
> +#endif
>          setImageInterpolationQuality(previousInterpolationQuality);
>      } else
> +#if ENABLE(CSS_COMPOSITING)
> +        image->draw(this, styleColorSpace, FloatRect(dest.location(), FloatSize(tw, th)), FloatRect(src.location(), FloatSize(tsw, tsh)), op, useLowQualityScale, blendMode);
> +#else
>          image->draw(this, styleColorSpace, FloatRect(dest.location(), FloatSize(tw, th)), FloatRect(src.location(), FloatSize(tsw, tsh)), op, useLowQualityScale);
> +#endif

Why not just pass the blendMode in? I see no advantage of using #if ENABLE(CSS_COMPOSITING) down here; it should just be used to not expose new functionality to CSS.

> Source/WebCore/platform/graphics/GraphicsContext.cpp:704
> +void GraphicsContext::setCompositeOperation(CompositeOperator compositeOperation )

Extra space

> Source/WebCore/platform/graphics/GraphicsContext.h:555
> +#if ENABLE(CSS_COMPOSITING)
> +        void setPlatformCompositeOperation(CompositeOperator, BlendMode = BlendModeNormal);
> +#else
>          void setPlatformCompositeOperation(CompositeOperator);
> +#endif

No need for the #ifdef.

> Source/WebCore/platform/graphics/GraphicsLayer.cpp:74
> +#if ENABLE(CSS_COMPOSITING)
> +    , m_blendMode(BlendModeNormal)
> +#endif

Why is this in the CG patch? GraphicsLayers are just about accelerated compositing.

> Source/WebCore/platform/graphics/GraphicsLayer.h:38
>  #endif
> +#if ENABLE(CSS_COMPOSITING)
> +#include "GraphicsTypes.h"
> +#endif

This file shouldn't change at all.

> Source/WebCore/platform/graphics/ImageBuffer.h:134
> +#if ENABLE(CSS_COMPOSITING)
> +        void draw(GraphicsContext*, ColorSpace, const FloatRect& destRect, const FloatRect& srcRect = FloatRect(0, 0, -1, -1), CompositeOperator = CompositeSourceOver, bool useLowQualityScale = false, BlendMode = BlendModeNormal);
> +#else
>          void draw(GraphicsContext*, ColorSpace, const FloatRect& destRect, const FloatRect& srcRect = FloatRect(0, 0, -1, -1), CompositeOperator = CompositeSourceOver, bool useLowQualityScale = false);
> +#endif

Dont' #ifdef.

> Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:273
> +#if ENABLE(CSS_COMPOSITING)
> +void GraphicsContext::drawNativeImage(NativeImagePtr imagePtr, const FloatSize& imageSize, ColorSpace styleColorSpace, const FloatRect& destRect, const FloatRect& srcRect, CompositeOperator op, ImageOrientation orientation, BlendMode blendMode)
> +#else
>  void GraphicsContext::drawNativeImage(NativeImagePtr imagePtr, const FloatSize& imageSize, ColorSpace styleColorSpace, const FloatRect& destRect, const FloatRect& srcRect, CompositeOperator op, ImageOrientation orientation)
> +#endif
>  {

Don't #ifdef. Also put the BlendMode after the CompositeOperator.

> Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:336
> +#if ENABLE(CSS_COMPOSITING)
> +    setPlatformCompositeOperation(op, blendMode);
> +#else
>      setPlatformCompositeOperation(op);
> +#endif
>  

Ditto.

> Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:1757
> +#if ENABLE(CSS_COMPOSITING)   
> +    if (blendMode != BlendModeNormal)
> +        switch (blendMode) {
> +        case BlendModeMultiply:
> +            target = kCGBlendModeMultiply;
> +            break;
> +        case BlendModeScreen:
> +            target = kCGBlendModeScreen;
> +            break;
> +        case BlendModeOverlay:
> +            target = kCGBlendModeOverlay;
> +            break;
> +        case BlendModeDarken:
> +            target = kCGBlendModeDarken;
> +            break;
> +        case BlendModeLighten:
> +            target = kCGBlendModeLighten;
> +            break;
> +        case BlendModeColorDodge:
> +            target = kCGBlendModeColorDodge;
> +            break;
> +        case BlendModeColorBurn:
> +            target = kCGBlendModeColorBurn;
> +            break;
> +        case BlendModeHardLight:
> +            target = kCGBlendModeHardLight;
> +            break;
> +        case BlendModeSoftLight:
> +            target = kCGBlendModeSoftLight;
> +            break;
> +        case BlendModeDifference:
> +            target = kCGBlendModeDifference;
> +            break;
> +        case BlendModeExclusion:
> +            target = kCGBlendModeExclusion;
> +            break;
> +        case BlendModeHue:
> +            target = kCGBlendModeHue;
> +            break;
> +        case BlendModeSaturation:
> +            target = kCGBlendModeSaturation;
> +            break;
> +        case BlendModeColor:
> +            target = kCGBlendModeColor;
> +            break;
> +        case BlendModeLuminosity:
> +            target = kCGBlendModeLuminosity;
> +        default:
> +            break;
> +        } else
> +#endif

Don't #ifdef.

> Source/WebCore/platform/graphics/cg/ImageBufferCG.cpp:266
> +#if ENABLE(CSS_COMPOSITING)
> +void ImageBuffer::draw(GraphicsContext* destContext, ColorSpace styleColorSpace, const FloatRect& destRect, const FloatRect& srcRect, CompositeOperator op, bool useLowQualityScale, BlendMode blendMode)
> +#else
>  void ImageBuffer::draw(GraphicsContext* destContext, ColorSpace styleColorSpace, const FloatRect& destRect, const FloatRect& srcRect, CompositeOperator op, bool useLowQualityScale)
> +#endif

Ditto.

> Source/WebCore/platform/graphics/cg/ImageBufferCG.cpp:283
> +#if ENABLE(CSS_COMPOSITING)
> +    destContext->drawNativeImage(image.get(), internalSize(), colorSpace, destRect, adjustedSrcRect, op, DefaultImageOrientation, blendMode);
> +#else
> +    destContext->drawNativeImage(image.get(), internalSize(), colorSpace, destRect, adjustedSrcRect, op, DefaultImageOrientation);
> +#endif

Ditto.

> Source/WebCore/rendering/FilterEffectRenderer.cpp:476
> +void FilterEffectRenderer::createPassthroughFilter()
> +{
> +#if ENABLE(CSS_SHADERS)
> +    m_hasCustomShaderFilter = false;
> +#endif
> +
> +    RefPtr<FilterEffect> previousEffect = m_sourceGraphic;
> +    
> +    ComponentTransferFunction nullFunction;
> +    ComponentTransferFunction transferFunction;
> +    transferFunction.type = FECOMPONENTTRANSFER_TYPE_TABLE;
> +    Vector<float> transferParameters;
> +    transferParameters.append(0);
> +    transferParameters.append(1.0f);
> +    transferFunction.tableValues = transferParameters;
> +
> +    RefPtr<FilterEffect>  effect = FEComponentTransfer::create(this, nullFunction, nullFunction, nullFunction, transferFunction);
> +    effect->setClipsToBounds(false);
> +    effect->setColorSpace(ColorSpaceDeviceRGB);
> +    effect->inputEffects().append(previousEffect);
> +
> +    m_effects.clear();
> +    m_effects.append(effect);
> +    
> +    previousEffect = effect.release();
> +
> +    setMaxEffectRects(m_sourceDrawingRegion);
> +}
> +

Not sure if this is related.

> Source/WebCore/rendering/FilterEffectRenderer.h:120
> +    void createPassthroughFilter();

Unrelated?

> Source/WebCore/rendering/RenderLayer.cpp:1193
> -    return renderer()->isTransparent() || renderer()->hasMask();
> +    return renderer()->isTransparent() || renderer()->hasMask() || renderer()->hasBlendMode();

This method needs renaming. It's no longer just about opacity.

> Source/WebCore/rendering/RenderLayer.cpp:3171
> +    FilterEffectRendererHelper filterPainter(filterRenderer() && (paintsWithFilters() || hasBlendMode()));

Doesn't paintsWithFilters() include blend mode?

> Source/WebCore/rendering/RenderLayer.h:618
> +    bool hasBlendMode() const { return m_blendMode != BlendModeNormal; }
> +    BlendMode blendMode() const { return m_blendMode; }

I don't see m_blendMode being updated in styleDidChange.

> 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 should be dumpAsText() test.

[Build Bot]

Comment on attachment 167794 [details]
previous patch failed to apply

Attachment 167794 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14228900

New failing tests:
css3/compositing/should-have-compositing-layer.html