[Webkit-unassigned] [Bug 92868] [css3-text] Add platform support for "wavy" text decoration style
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Feb 11 17:53:27 PST 2013
https://bugs.webkit.org/show_bug.cgi?id=92868
--- Comment #33 from Lamarque V. Souza <Lamarque.Souza at basyskom.com> 2013-02-11 17:55:38 PST ---
(In reply to comment #32)
> (From update of attachment 186394 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=186394&action=review
> > Source/WebCore/ChangeLog:14
> > + Tests are in
> > + fast/css3-text/css3-text-decoration/text-decoration-style.html already,
> > + just need to rebaseline them (see bug 100546).
>
> Then when not already rebaseline them for your platform in this patch?
Ok, I will do it.
> As an alternative, why not 'strokeWavyTextDecoration(GraphicsContext* context, const FloatPoint& point1, const FloatPoint& point2)'?
>
> +don't const the input float;
Why not const them?
> > Source/WebCore/rendering/InlineTextBox.cpp:971
> > + short signal = -1;
>
> Why a short?
> What is this anyway? "Signal" is a strange name in this context.
"sign" is the correct name here. I will fix it.
> > Source/WebCore/rendering/InlineTextBox.cpp:994
> > + x1 = x2 = p1.x();
> > +
> > + // Make sure (x1, y1) < (x2, y2)
> > + if (p1.y() < p2.y()) {
> > + y1 = p1.y();
> > + y2 = p2.y();
> > + } else {
> > + y1 = p2.y();
> > + y2 = p1.y();
> > + }
> > +
> > + path.moveTo(FloatPoint(x1 + signal * step, y1));
> > + float y = y1 + 2 * step;
> > +
> > + while (y <= y2) {
> > + signal = -signal;
> > + path.addLineTo(FloatPoint(x1 + signal * step, y));
> > + path.addLineTo(FloatPoint(x1 + signal * step, y + flat)); // Draw flat line between diagonal lines.
> > + y += 2 * step + flat;
> > + }
>
> Instinctively, I would have used bezier curve. Why do you use straight lines?
Well, I am not used to use bezier curves. Also, we deciced to make this curse to look similar to mozilla's implementation [1]. There is also some performance concerns about using strokPath to implement wavy stroke [2], would bezier curve improve performance here?
[1] https://bugs.webkit.org/show_bug.cgi?id=94110#c16
[2] https://bugs.webkit.org/show_bug.cgi?id=93507#c43
> > Source/WebCore/rendering/InlineTextBox.cpp:1115
> > +#if ENABLE(CSS3_TEXT)
> > + if (decorationStyle == TextDecorationStyleWavy) {
> > + Path path;
> > + createWavyPath(context, FloatPoint(localOrigin.x(), localOrigin.y() + baseline + doubleOffset + 1), FloatPoint(localOrigin.x() + width, localOrigin.y() + baseline + doubleOffset + 1), textDecorationThickness, path);
> > + context->strokePath(path);
> > + } else {
> > +#endif // CSS3_TEXT
> > // Leave one pixel of white between the baseline and the underline.
> > context->drawLineForText(FloatPoint(localOrigin.x(), localOrigin.y() + baseline + 1), width, isPrinting);
> > #if ENABLE(CSS3_TEXT)
> > - if (decorationStyle == TextDecorationStyleDouble)
> > - context->drawLineForText(FloatPoint(localOrigin.x(), localOrigin.y() + baseline + 1 + doubleOffset), width, isPrinting);
> > + if (decorationStyle == TextDecorationStyleDouble)
> > + context->drawLineForText(FloatPoint(localOrigin.x(), localOrigin.y() + baseline + 1 + doubleOffset), width, isPrinting);
> > + }
> > #endif // CSS3_TEXT
>
> Shouldn't this become a switch-case based on decorationStyle?
I can use change it to a switch-case, but it would look like this
switch (decorationStyle) {
case TextDecorationStyleWavy:
...
break;
default:
...
break;
}
That more verbose than
if (decorationStyle == TextDecorationStyleWavy) {
...
} else {
...
}
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list