No subject


Mon Jan 28 08:41:14 PST 2013


Path. Here you modify the path passed as a parameter, and modify the contex=
t.

As an alternative, why not 'strokeWavyTextDecoration(GraphicsContext* conte=
xt,
const FloatPoint& point1, const FloatPoint& point2)'?

+don't const the input float;

> Source/WebCore/rendering/InlineTextBox.cpp:965
> +    bool isVerticalLine =3D (p1.x() =3D=3D p2.x());

You should compute this after adjustLineToPixelBoundaries(). This cold coul=
d be
moved just before the if().

> Source/WebCore/rendering/InlineTextBox.cpp:971
> +    short signal =3D -1;

Why a short?
What is this anyway? "Signal" is a strange name in this context.

> Source/WebCore/rendering/InlineTextBox.cpp:994
> +	   x1 =3D x2 =3D p1.x();
> +
> +	   // Make sure (x1, y1) < (x2, y2)
> +	   if (p1.y() < p2.y()) {
> +	       y1 =3D p1.y();
> +	       y2 =3D p2.y();
> +	   } else {
> +	       y1 =3D p2.y();
> +	       y2 =3D p1.y();
> +	   }
> +
> +	   path.moveTo(FloatPoint(x1 + signal * step, y1));
> +	   float y =3D y1 + 2 * step;
> +
> +	   while (y <=3D y2) {
> +	       signal =3D -signal;
> +	       path.addLineTo(FloatPoint(x1 + signal * step, y));
> +	       path.addLineTo(FloatPoint(x1 + signal * step, y + flat)); //
Draw flat line between diagonal lines.
> +	       y +=3D 2 * step + flat;
> +	   }

Instinctively, I would have used bezier curve. Why do you use straight line=
s?

> Source/WebCore/rendering/InlineTextBox.cpp:1115
> +#if ENABLE(CSS3_TEXT)
> +	       if (decorationStyle =3D=3D TextDecorationStyleWavy) {
> +		   Path path;
> +		   createWavyPath(context, FloatPoint(localOrigin.x(),
localOrigin.y() + baseline + doubleOffset + 1), FloatPoint(localOrigin.x() +
width, localOrigin.y() + baseline + doubleOffset + 1), textDecorationThickn=
ess,
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 =3D=3D TextDecorationStyleDouble)
> -		   context->drawLineForText(FloatPoint(localOrigin.x(),
localOrigin.y() + baseline + 1 + doubleOffset), width, isPrinting);
> +		   if (decorationStyle =3D=3D 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?

> Source/WebCore/rendering/InlineTextBox.cpp:1130
> +	       if (decorationStyle =3D=3D TextDecorationStyleWavy) {
> +		   Path path;
> +		   createWavyPath(context, FloatPoint(localOrigin.x(),
localOrigin.y() - doubleOffset), FloatPoint(localOrigin.x() + width,
localOrigin.y() - doubleOffset), textDecorationThickness, path);
> +		   context->strokePath(path);
> +	       } else {
> +#endif // CSS3_TEXT
>	       context->drawLineForText(localOrigin, width, isPrinting);
>  #if ENABLE(CSS3_TEXT)
> -	       if (decorationStyle =3D=3D TextDecorationStyleDouble)
> -		   context->drawLineForText(FloatPoint(localOrigin.x(),
localOrigin.y() - doubleOffset), width, isPrinting);
> +		   if (decorationStyle =3D=3D TextDecorationStyleDouble)
> +		       context->drawLineForText(FloatPoint(localOrigin.x(),
localOrigin.y() - doubleOffset), width, isPrinting);
> +	       }

Ditto.=


More information about the webkit-reviews mailing list