[webkit-reviews] review denied: [Bug 46972] Add two new helper files for the new SVGTextLayoutEngine : [Attachment 69447] Patch v2

[bugzilla-daemon at webkit.org]

Dirk Schulze <krit at webkit.org> has denied Nikolas Zimmermann
<zimmermann at kde.org>'s request for review:
Bug 46972: Add two new helper files for the new SVGTextLayoutEngine
https://bugs.webkit.org/show_bug.cgi?id=46972

Attachment 69447: Patch v2
https://bugs.webkit.org/attachment.cgi?id=69447&action=review

------- Additional Comments from Dirk Schulze <krit at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=69447&action=review

> WebCore/rendering/svg/SVGTextLayoutEngineBaseline.cpp:146
> +	   return m_font.ascent() * 8.0f / 10.0f;
> +    case AB_MATHEMATICAL:
> +	   return m_font.ascent() / 2;
> +    default:

On one line you use / 2, on the other line / 10.0f. Please use a common style
here. I think it's more save to use / 2.f instead of / 2, but it's your
decision.

> WebCore/rendering/svg/SVGTextLayoutEngineBaseline.cpp:166
> +    case GO_AUTO:
> +	   {
> +	       // Spec: Fullwidth ideographic and fullwidth Latin text will be
set with a glyph-orientation of 0-degrees.
> +	       // Text which is not fullwidth will be set with a
glyph-orientation of 90-degrees.
> +	       unsigned int unicodeRange = findCharUnicodeRange(character);
> +	       if (unicodeRange == cRangeSetLatin || unicodeRange ==
cRangeArabic)
> +		   return 90;
> +
> +	       return 0;
> +	   }

The indention looks wrong here.