Bug 96408 – [Skia] Support missing styles on text line rendering

• Status: RESOLVED
• Resolution: WONTFIX
• Component: Layout and Rendering

[Bruno Abinader (history only)]

The CSS3 property "text-decoration-style" accepts the following values: "solid | double | dotted | dashed | wavy" as seen on the link below:
http://dev.w3.org/csswg/css3-text/#text-decoration-style

All values except "wavy" were already implemented on Skia due to previous usage on "border-style" property:
http://www.w3.org/TR/css3-background/#the-border-style

This bug intends to provide Skia platform support for "dashed" and "dotted" values on GraphicsContext::drawLineForText. This bug is a subtask from bug 93509.

[Bruno Abinader (history only)]

Created attachment 163403 [details]
Patch

Proposed patch.

[Bruno Abinader (history only)]

Peter, can you please take a look at it? :)

[Peter Beverloo]

+Mike, Tom

I wouldn't know about the Skia code, adding Mike and Tom, hoping that either of them can take a look. Thanks for the patch!

[Tom Hudson]

This is actually in the WebKit-to-Skia glue layer, not inside Skia proper. senorblanco@ or junov@ might be (or know) a better subject-matter expert.

I'm surprised that the roundf() in drawLineForText() doesn't break anything?

Otherwise after reading over some of the related code this looks reasonable to me.

[Bruno Abinader (history only)]

Hi Tom,

You are right, I was trying to avoid using static_cast<int>, however the roundf()/lroundf() trick ~might~ not work on all compilers, as it relies on C99 standard. I'm going to revert this change on an updated patch.

(In reply to comment #4)
> This is actually in the WebKit-to-Skia glue layer, not inside Skia proper. senorblanco@ or junov@ might be (or know) a better subject-matter expert.
> 
> I'm surprised that the roundf() in drawLineForText() doesn't break anything?
> 
> Otherwise after reading over some of the related code this looks reasonable to me.

[Bruno Abinader (history only)]

Double checking the code, I've reminded myself that roundf() is actually already used to convert strokeThickness() to int (check http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp#L559 for details). I'm sorry I forgot to mention that previosuly. Said this, shall this gets a r+, then? :)

[Bruno Abinader (history only)]

Ping?

[Bruno Abinader (history only)]

Created attachment 178798 [details]
Patch

Updated version based on Tom's comments. Added 'double' and 'wavy' values under CSS3_TEXT feature flag.

[Darin Adler]

Comment on attachment 178798 [details]
Patch

Clearing flags on a Skia-specific patch.

[Darin Adler]

Skia-specific.