[Webkit-unassigned] [Bug 98450] Add support for blendmode to WebKit1 platform code

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 9 16:49:31 PDT 2012


https://bugs.webkit.org/show_bug.cgi?id=98450


Simon Fraser (smfr) <simon.fraser at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #167862|review?                     |review-
               Flag|                            |




--- Comment #35 from Simon Fraser (smfr) <simon.fraser at apple.com>  2012-10-09 16:50:07 PST ---
(From update of attachment 167862)
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.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


More information about the webkit-unassigned mailing list