Summary: | Add support for blendmode to WebKit1 platform code | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Rik Cabanier <cabanier> | ||||||||||||||||||||||||
Component: | CSS | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||||||||||
Status: | RESOLVED WONTFIX | ||||||||||||||||||||||||||
Severity: | Normal | CC: | dglazkov, dino, donggwan.kim, eric, gtk-ews, gustavo, noam, peter+ews, philn, simon.fraser, webkit.review.bot, xan.lopez | ||||||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||||||
Hardware: | All | ||||||||||||||||||||||||||
OS: | All | ||||||||||||||||||||||||||
Bug Depends on: | 98950 | ||||||||||||||||||||||||||
Bug Blocks: | 95614 | ||||||||||||||||||||||||||
Attachments: |
|
Description
Rik Cabanier
2012-10-04 13:50:58 PDT
Created attachment 167593 [details]
just seeing what the bots say. not ready for checkin yet
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 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 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 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 Created attachment 167602 [details]
made more code protected by flags. not ready
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 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 Created attachment 167622 [details]
more conditional code. Not ready for review
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 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 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 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 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 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 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 Created attachment 167705 [details]
fixes for QT & Chromium. Not ready for review
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 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 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 Created attachment 167709 [details]
this is the actual patch. Previous one was old. not for review
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.
Created attachment 167710 [details]
fix for strange webkit style problem
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 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 Created attachment 167712 [details]
silly QT typo
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 Comment on attachment 167712 [details] silly QT typo Attachment 167712 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14245018 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 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 Created attachment 167774 [details]
Type in non-compositing code
Created attachment 167794 [details]
previous patch failed to apply
Created attachment 167848 [details]
added test case and changelog
Created attachment 167862 [details]
Fixed a couple of typos
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. 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 |