Bug 58567 – invert is treated like currentColor

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

[Lea Verou]

The CSS 2.1 specification defines the initial value of outline-color as invert: http://www.w3.org/TR/CSS21/ui.html#dynamic-outlines
Same with the CSS3 Basic UI module: http://www.w3.org/TR/css3-ui/#outline-color

However, Webkit seems to treat it like currentColor, which makes support detection impossible.

Opera and IE9 already support this.

[David Barr]

Quoth the CSS21 TR:
"Conformant UAs may ignore the 'invert' value on platforms that do not support color inversion of the pixels on the screen. If the UA does not support the 'invert' value then the initial value of the 'outline-color' property is the value of the 'color' property, similar to the initial value of the 'border-top-color' property."

So in absence of color inversion support, the current color behavior is compliant.

[Lea Verou]

Maybe my understanding of the spec is incorrect, but how does the platform not support color inversion if IE9 and Opera are able to support it on the same platform?

[David Barr]

The graphics abstraction that WebCore uses to wrap CoreGraphics/Skia/Qt presently hides the necessary composite operation for color inversion. CoreGraphics has CGBlendMode::kCGBlendModeDifference, Skia has SkXfermode::Mode:: kDifference_Mode and Qt has QPainter::CompositionMode_Difference but there is not a corresponding value in WebCore::CompositeOperator as yet.

TL;DR: The primitives necessary for this feature are not yet exposed by the graphics layer in WebCore.

With the view that this abstraction forms part of the platform, the fallback behavior is compliant.

[David Barr]

I've written a quick proof-of-concept patch that demonstrates how this feature might be implemented.
I propose that this bug be broken down into smaller tasks, the first of which is to expose the necessary blend mode for consumption by the rendering classes. Then each outline-drawing method needs to be adapted to be aware of invert, (inline, box, image, etc.) There will likely be some interaction with any existing overdraw bugs.

[David Barr]

Created attachment 95311 [details]
RFC

[David Barr]

Comment on attachment 95311 [details]
RFC

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

OpenVG appears to be the only drawing API that doesn't have the required blend mode.

> Source/WebCore/platform/graphics/gpu/SharedGraphicsContext3D.cpp:348
> +        m_context->disable(GraphicsContext3D::BLEND);

Alternatively, something like:
m_context->enable(GraphicsContext3D::BLEND);
m_context->blendEquation(GraphicsContext3D::FUNC_SUBTRACT);
m_context->blendFunc(GraphicsContext3D::ONE, GraphicsContext3D::ONE);

> Source/WebCore/platform/graphics/openvg/PainterOpenVG.cpp:289
> +            notImplemented();

As best I can tell, there is no subtraction filter available in the OpenVG API.

[Dirk Schulze]

Comment on attachment 95311 [details]
RFC

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

If you change the rendering part you need a test case. But it looks like this patch is not intended to land as is, right?

>> Source/WebCore/platform/graphics/gpu/SharedGraphicsContext3D.cpp:348
>> +        // unsupported
>> +        m_context->disable(GraphicsContext3D::BLEND);
> 
> Alternatively, something like:
> m_context->enable(GraphicsContext3D::BLEND);
> m_context->blendEquation(GraphicsContext3D::FUNC_SUBTRACT);
> m_context->blendFunc(GraphicsContext3D::ONE, GraphicsContext3D::ONE);

If you have an alternative, you should use it (if the result looks the same).

>> Source/WebCore/platform/graphics/openvg/PainterOpenVG.cpp:289
>> +            notImplemented();
> 
> As best I can tell, there is no subtraction filter available in the OpenVG API.

But is notImplemented() a good idea? Can't you use another blend operation and add a FIXME ?

> Source/WebCore/rendering/RenderInline.cpp:1410
> +    bool invertColor = true;

If invertColor is always true, why do you check for it?

[David Barr]

Created attachment 109736 [details]
Patch

[David Barr]

New patch, https://bugs.webkit.org/attachment.cgi?id=109736, addresses the concerns of the last review and pushes the 'invert' value all the way from the 'outline-color' property through to the rendering code. With this patch, Lea's original test case passes. No tests quite yet, looking for feedback on the approach.

[David Barr]

Created attachment 109918 [details]
Patch

[David Barr]

Comment on attachment 109918 [details]
Patch

Added pixel test covering both block and inline elements.
Reviewed results on Opera, Firefox 4, and WebKit.
Firefox renders the outline black.
WebKit without this patch renders it red.
Opera inverts the outline.

[David Barr]

Comment on attachment 109918 [details]
Patch

Sigh, file list in ChangeLog is wrong. Aside from that, any reviewer comments?

[David Barr]

http://acid0.org/tests/acid0-28.html (CSS 2.1: invert outline) fails with the third patch because getComputedStyle returns #00FFFFFF rather than 'invert'.

[David Barr]

Also relevant is http://test.csswg.org/suites/css2.1/20110323/html4/outline-color-174.htm (CSS Test: Outline color set to 'invert') which passes with this patch but neglects to test the inline element case.

[David Barr]

(In reply to comment #13)
> http://acid0.org/tests/acid0-28.html (CSS 2.1: invert outline) fails with the third patch because getComputedStyle returns #00FFFFFF rather than 'invert'.

Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1555
case CSSPropertyWebkitColumnRuleColor:
return m_allowVisitedStyle ? primitiveValueCache->createColorValue(style->visitedDependentColor(CSSPropertyOutlineColor).rgb()) : currentColorOrValidColor(style.get(), style->columnRuleColor());

I think in either case, 'invert' ought to be a possible result.

[David Barr]

Created attachment 110328 [details]
Patch

[David Barr]

(In reply to comment #13)
> http://acid0.org/tests/acid0-28.html (CSS 2.1: invert outline) fails with the third patch because getComputedStyle returns #00FFFFFF rather than 'invert'.

With the fourth patch, the acid test also passes. Maybe the layout test could be extended to also check the computed style. Feedback on the general approach is welcome.

[Darin Adler]

To me this "invert" color does not seem like a desirable feature.

[Simon Fraser (smfr)]

I don't think 'invert' can play nicely with compositing.

[David Barr]

(In reply to comment #18)
> To me this "invert" color does not seem like a desirable feature.

Is it that it is a composite operation masquerading as a color that bothers you or that it is subtractive?

[David Barr]

(In reply to comment #19)
> I don't think 'invert' can play nicely with compositing.

Please expand on this a little. As demonstrated by the attached patch, it would be implemented using the same compositing APIs we use for transparency.

[Simon Fraser (smfr)]

By "compositing" mean accelerated compositing. Imagine a CSS 'invert' border that overlaps two other elements; one which is rendering into the window, and another which is rendered into a GraphicsLayer. There's no way invert can work there.

[David Barr]

Contrasting with a similar scenario that I would expect to work already: Imagine a CSS semi-transparent outline that overlaps two other elements; one which is rendering into the window, and another which is rendered into a GraphicsLayer. How does this work?

[Darin Adler]

(In reply to comment #23)
> Contrasting with a similar scenario that I would expect to work already: Imagine a CSS semi-transparent outline that overlaps two other elements; one which is rendering into the window, and another which is rendered into a GraphicsLayer. How does this work?

Accelerated compositing supports alpha; the GPU compositor understands that concept. It doesn’t support other kinds of transformations of the underlying pixels such inverting them.

[Simon Fraser (smfr)]

To expand on Darin's comment:
The (In reply to comment #23)
> Contrasting with a similar scenario that I would expect to work already: Imagine a CSS semi-transparent outline that overlaps two other elements; one which is rendering into the window, and another which is rendered into a GraphicsLayer. How does this work?

The outline itself is also in a graphics layer (because it overlaps another one), and those two graphics layers are composited by the GPU on top of the window background.

The problem with invert is that you'd need to use a different compositing operation for just that one part of the drawing, so you'd have to break it into its own GPU layer and composite on the GPU with the invert operation.

[David Barr]

(In reply to comment #25)
> The problem with invert is that you'd need to use a different compositing operation for just that one part of the drawing, so you'd have to break it into its own GPU layer and composite on the GPU with the invert operation.

How is this different from say, an SVG with a filter other than BlendNormal overlapping the elements?

[Simon Fraser (smfr)]

We currently don't create compositing layers in SVG. When we do, this will be a problem.

[David Barr]

(In reply to comment #27)
> We currently don't create compositing layers in SVG. When we do, this will be a problem.

Would you therefore be in favour of deferring this feature until that is a solved problem?
It seems to be a larger one than this.

[Simon Fraser (smfr)]

I would suggest just never implementing 'invert'. Support is not required by the spec.

[Eric Seidel (no email)]

Comment on attachment 110328 [details]
Patch

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

> Source/WebCore/platform/graphics/Color.h:157
> +    // FIXME: invert is a very special color value that also conveys composite mode
> +    static const RGBA32 invert = 0x00FFFFFF;

Really?  Do we really want this in-band data... seems like a bad idea.

[David Barr]

> > Source/WebCore/platform/graphics/Color.h:157
> > +    // FIXME: invert is a very special color value that also conveys composite mode
> > +    static const RGBA32 invert = 0x00FFFFFF;
> 
> Really?  Do we really want this in-band data... seems like a bad idea.

I think not. It would be better to make use of one of the unused bits in Color to carry this information. However, doing so would require checking every place that color values are reconstructed to ensure that the additional information is carried through. Yet another approach would be to decouple invert from Color but that would require duplication of the color propagation logic.

[Simon Fraser (smfr)]

Comment on attachment 110328 [details]
Patch

r- based on comments. I really don't think we should implement invert. Think of an outline that passes over half a compositing layer.

[David Barr]

In the absence of reviewer consensus in favor of this change, it is unlikely to land in the current or an improved form in the foreseeable future.