Bug 146123 – Provide a way for web developers to draw a Theme-specific Wireless Playback icon

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dean Jackson]

Provide a way for web developers to draw a Theme-specific Wireless Playback icon

[Dean Jackson]

<rdar://problem/21119287>

[Dean Jackson]

We're going to expose a new <image> type to CSS via the function "named-icon(name)". This will render a system icon, and can be used to get access to platform-specific artwork like Airplay on Cocoa.

e.g. background-image: named-icon(wireless-playback);

Since it is an image generator, a site can adjust it to their theme using normal CSS techniques such as masking.

[Dean Jackson]

Created attachment 255133 [details]
Patch

[Dean Jackson]

Created attachment 255141 [details]
Patch

[Dean Jackson]

Created attachment 255145 [details]
Patch

[Dean Jackson]

Created attachment 255153 [details]
Patch

[Dean Jackson]

Created attachment 255156 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 255153 [details]
Patch

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

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:8414
> +		314BE3A01B30F6B700141982 /* CSSNamedImageValue.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; lineEnding = 0; path = CSSNamedImageValue.h; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.objcpp; };
> +		314BE3A21B30F6D100141982 /* CSSNamedImageValue.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; lineEnding = 0; path = CSSNamedImageValue.cpp; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.cpp; };
> +		314BE3A41B3103FB00141982 /* NamedImageGeneratedImage.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = NamedImageGeneratedImage.cpp; sourceTree = "<group>"; };
> +		314BE3A51B3103FB00141982 /* NamedImageGeneratedImage.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = NamedImageGeneratedImage.h; sourceTree = "<group>"; };

Please edit out the xcLanguageSpecificationIdentifier.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:12724
> +		BC23E76B0DAE88A9009FDC91 /* CSSImageGeneratorValue.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; lineEnding = 0; path = CSSImageGeneratorValue.cpp; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.cpp; };

I don't think the xcLanguageSpecificationIdentifier should be there.

> Source/WebCore/css/CSSNamedImageValue.h:39
> +    static Ref<CSSNamedImageValue> create(const String& name) { return adoptRef(*new CSSNamedImageValue(name)); }

I think you should unwrap this.

> Source/WebCore/css/CSSNamedImageValue.h:43
> +    bool isFixedSize() const { return false; }

Do named images have implicit size? We should probably say they do.

> Source/WebCore/platform/Theme.cpp:71
> +    context->scale(FloatSize(rect.size().width() / 100, rect.size().height() / 100));

The beziers below are designed to fit in 100x100?

> Source/WebCore/platform/Theme.cpp:111
> +    Path outline;
> +    outline.moveTo(FloatPoint(59, 58.7));
> +    outline.addBezierCurveTo(FloatPoint(58.1, 58.7), FloatPoint(57.2, 58.4), FloatPoint(56.4, 57.7));
> +    outline.addLineTo(FloatPoint(42, 45.5));
> +    outline.addLineTo(FloatPoint(27.6, 57.8));
> +    outline.addBezierCurveTo(FloatPoint(25.9, 59.2), FloatPoint(23.4, 59), FloatPoint(22, 57.3));
> +    outline.addBezierCurveTo(FloatPoint(20.6, 55.6), FloatPoint(20.8, 53.1), FloatPoint(22.5, 51.7));
> +    outline.addLineTo(FloatPoint(39.5, 37.3));
> +    outline.addBezierCurveTo(FloatPoint(41, 36), FloatPoint(43.2, 36), FloatPoint(44.7, 37.3));
> +    outline.addLineTo(FloatPoint(61.7, 51.7));
> +    outline.addBezierCurveTo(FloatPoint(63.4, 53.1), FloatPoint(63.6, 55.7), FloatPoint(62.2, 57.3));
> +    outline.addBezierCurveTo(FloatPoint(61.3, 58.2), FloatPoint(60.1, 58.7), FloatPoint(59, 58.7));
> +    outline.addLineTo(FloatPoint(59, 58.7));
> +    outline.closeSubpath();
> +
> +    outline.moveTo(FloatPoint(42, 98));
> +    outline.addBezierCurveTo(FloatPoint(39.8, 98), FloatPoint(38, 96.3), FloatPoint(38, 94.2));
> +    outline.addLineTo(FloatPoint(38, 43.6));
> +    outline.addBezierCurveTo(FloatPoint(38, 41.5), FloatPoint(39.8, 39.8), FloatPoint(42, 39.8));
> +    outline.addBezierCurveTo(FloatPoint(44.2, 39.8), FloatPoint(46, 41.5), FloatPoint(46, 43.6));
> +    outline.addLineTo(FloatPoint(46, 94.2));
> +    outline.addBezierCurveTo(FloatPoint(46, 96.3), FloatPoint(44.2, 98), FloatPoint(42, 98));
> +    outline.addLineTo(FloatPoint(42, 98));
> +    outline.closeSubpath();
> +
> +    outline.moveTo(FloatPoint(83.6, 41.6));
> +    outline.addBezierCurveTo(FloatPoint(83.6, 18.6), FloatPoint(65, 0), FloatPoint(42, 0));
> +    outline.addBezierCurveTo(FloatPoint(19, 0), FloatPoint(0.4, 18.6), FloatPoint(0.4, 41.6));
> +    outline.addBezierCurveTo(FloatPoint(0.4, 62.2), FloatPoint(15, 79.2), FloatPoint(35, 82.6));
> +    outline.addLineTo(FloatPoint(35, 74.5));
> +    outline.addBezierCurveTo(FloatPoint(20, 71.2), FloatPoint(8.4, 57.7), FloatPoint(8.4, 41.6));
> +    outline.addBezierCurveTo(FloatPoint(8.4, 23.1), FloatPoint(23.5, 8), FloatPoint(42, 8));
> +    outline.addBezierCurveTo(FloatPoint(60.5, 8), FloatPoint(75.5, 23.1), FloatPoint(75.5, 41.6));
> +    outline.addBezierCurveTo(FloatPoint(75.6, 57.7), FloatPoint(64, 71.2), FloatPoint(49, 74.5));
> +    outline.addLineTo(FloatPoint(49, 82.6));
> +    outline.addBezierCurveTo(FloatPoint(69, 79.3), FloatPoint(83.6, 62.2), FloatPoint(83.6, 41.6));
> +    outline.addLineTo(FloatPoint(83.6, 41.6));
> +    outline.closeSubpath();

Icky. Would be nicer to put a .svg in the bundle.

> Source/WebCore/platform/cocoa/ThemeCocoa.cpp:63
> +    FloatSize wirelessPlaybackSrcSize(32, 24.016);

24.016, really?

> Source/WebCore/platform/graphics/NamedImageGeneratedImage.cpp:57
> +    std::unique_ptr<ImageBuffer> imageBuffer = ImageBuffer::create(size(), 1, ColorSpaceDeviceRGB, context->isAcceleratedContext() ? Accelerated : Unaccelerated);

Doesn't createCompatibleBuffer do the right thing?

[Dean Jackson]

Comment on attachment 255153 [details]
Patch

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

>> Source/WebCore/css/CSSNamedImageValue.h:43
>> +    bool isFixedSize() const { return false; }
> 
> Do named images have implicit size? We should probably say they do.

No. They draw into whatever region they have been given by CSS. At least for now :)

>> Source/WebCore/platform/Theme.cpp:71
>> +    context->scale(FloatSize(rect.size().width() / 100, rect.size().height() / 100));
> 
> The beziers below are designed to fit in 100x100?

Yes.

>> Source/WebCore/platform/Theme.cpp:111
>> +    outline.closeSubpath();
> 
> Icky. Would be nicer to put a .svg in the bundle.

Then we'd have to load from the bundle, parse the SVG, wait for the Image to be ready, get a RenderImage and paint it. This is probably what we'll end up doing, but for now I think it is ok to hardcode this single outline.

>> Source/WebCore/platform/cocoa/ThemeCocoa.cpp:63
>> +    FloatSize wirelessPlaybackSrcSize(32, 24.016);
> 
> 24.016, really?

That's the artwork we were given. See below for the extent of the lines.

>> Source/WebCore/platform/graphics/NamedImageGeneratedImage.cpp:57
>> +    std::unique_ptr<ImageBuffer> imageBuffer = ImageBuffer::create(size(), 1, ColorSpaceDeviceRGB, context->isAcceleratedContext() ? Accelerated : Unaccelerated);
> 
> Doesn't createCompatibleBuffer do the right thing?

Maybe. I'll try. I copied this from crossfades.

[Dean Jackson]

Created attachment 255158 [details]
Patch

[Dean Jackson]

Committed r185731: <http://trac.webkit.org/changeset/185731>

[Joseph Pecoraro]

Comment on attachment 255158 [details]
Patch

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

> Source/WebCore/css/CSSParser.cpp:9157
> +        || equalIgnoringCase(value.function->name, "-webkit-named-image(");

Should we add this to Web Inspector's autocompletion?