Add SVGTextLayoutEngineBaseline & SVGTextLayoutEngineSpacing, which will be used & owned by the new SVGTextLayoutEngine.
Created attachment 69446 [details] Patch
Attachment 69446 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/rendering/svg/SVGTextLayoutEngineBaseline.cpp:158: Non-label code inside switch statements should be indented. [whitespace/indent] [4] WebCore/rendering/svg/SVGTextLayoutEngineBaseline.cpp:181: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] WebCore/rendering/svg/SVGTextLayoutEngineBaseline.cpp:199: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] WebCore/rendering/svg/SVGTextLayoutEngineBaseline.cpp:210: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] WebCore/rendering/svg/SVGTextLayoutEngineBaseline.cpp:226: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] WebCore/rendering/svg/SVGTextLayoutEngineSpacing.cpp:82: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 6 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 69447 [details] Patch v2 Fix style issues.
Comment on attachment 69447 [details] Patch v2 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.
(In reply to comment #4) > (From update of attachment 69447 [details]) > 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. Well the common style is to avoid .f where possible. But as 8 / 10 gives me an integer, I need to use these postfixes. > > > 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. If you check the last patch, I did that on purpose, otherwhise the style bot moans.
Created attachment 69448 [details] Patch v3 We agreed to ignore the style warning regarding indentation in a switch, it looks awkward, and the coding style guidelines don't say a word about it.
Comment on attachment 69448 [details] Patch v3 LGTM. r=me
Landed in r68878.