Bug 96408 - [Skia] Support missing styles on text line rendering
Summary: [Skia] Support missing styles on text line rendering
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Bruno Abinader (history only)
URL: http://dev.w3.org/csswg/css-text-deco...
Keywords:
Depends on: 94094
Blocks: 58491 100546 93509 104682
  Show dependency treegraph
 
Reported: 2012-09-11 10:37 PDT by Bruno Abinader (history only)
Modified: 2013-04-09 09:37 PDT (History)
9 users (show)

See Also:


Attachments
Patch (2.98 KB, patch)
2012-09-11 10:56 PDT, Bruno Abinader (history only)
no flags Details | Formatted Diff | Diff
Patch (2.72 KB, patch)
2012-12-11 07:12 PST, Bruno Abinader (history only)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bruno Abinader (history only) 2012-09-11 10:37:40 PDT
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.
Comment 1 Bruno Abinader (history only) 2012-09-11 10:56:45 PDT
Created attachment 163403 [details]
Patch

Proposed patch.
Comment 2 Bruno Abinader (history only) 2012-10-29 09:28:57 PDT
Peter, can you please take a look at it? :)
Comment 3 Peter Beverloo 2012-10-29 09:32:21 PDT
+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!
Comment 4 Tom Hudson 2012-10-31 10:31:56 PDT
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.
Comment 5 Bruno Abinader (history only) 2012-11-12 07:43:27 PST
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.
Comment 6 Bruno Abinader (history only) 2012-11-12 07:55:58 PST
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? :)
Comment 7 Bruno Abinader (history only) 2012-12-06 19:52:49 PST
Ping?
Comment 8 Bruno Abinader (history only) 2012-12-11 07:12:44 PST
Created attachment 178798 [details]
Patch

Updated version based on Tom's comments. Added 'double' and 'wavy' values under CSS3_TEXT feature flag.
Comment 9 Darin Adler 2013-04-09 09:37:31 PDT
Comment on attachment 178798 [details]
Patch

Clearing flags on a Skia-specific patch.
Comment 10 Darin Adler 2013-04-09 09:37:46 PDT
Skia-specific.