Bug 18031

Summary:

SVGTextElement.getStartPositionOfChar is wrong in various cases (affects Acid3 test 79)

Product:

WebKit

Reporter:

Maciej Stachowiak <mjs>

Component:

SVG

Assignee:

Maciej Stachowiak <mjs>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

eric, ian, webkit, zimmermann

Priority:

Version:

528+ (Nightly build)

Hardware:

Mac

OS:

OS X 10.5

Bug Depends on:

18046

Bug Blocks:

17064

Attachments:

Description	Flags
preliminary patch	none

Maciej Stachowiak

Reported 2008-03-23 19:10:23 PDT

With my fix for 17062, Acid3 test 79 continues to fail because it gets the wrong start position for nearly every character in the string. I modified Acid3 to print all errors in each test, and to look at the position delta for each character, to learn more about the errors. It appears that the most common error is counting the glyph advance instead of 0 for positions between characters of a multi-character glyph, another likely one is failing to take kerning into account, as well as failing to render altGlyph properly: Test 79 failed: character advance + kerning 2, which is between the first character of a two-character glyph and the second character of a two-character glyph, is 300 but should be 0. character advance + kerning 3, which is between the second character of a two-character glyph and a normal character, which shouldn't be the first character of a two-character glyph, is 500 but should be 200. character advance + kerning 6, which is between a normal character, which has some kerning after it and the first character of a two-character glyph, which has some kerning before it, is 10000 but should be 10200. character advance + kerning 7, which is between the first character of a two-character glyph, which has some kerning before it and the second character of a two-character glyph, which has some kerning before it, is 10000 but should be 0. character advance + kerning 8, which is between the second character of a two-character glyph, which has some kerning before it and the first character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning after it, but this glyph does not, is 10000 but should be 1300. character advance + kerning 9, which is between the first character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning after it, but this glyph does not and the second character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning after it, but this glyph does not, is 10000 but should be 0. character advance + kerning 10, which is between the second character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning after it, but this glyph does not and the third character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning after it, but this glyph does not, is 10000 but should be 0. character advance + kerning 11, which is between the third character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning after it, but this glyph does not and the fourth character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning after it, but this glyph does not, is 10000 but should be 0. character advance + kerning 12, which is between the fourth character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning after it, but this glyph does not and the fifth character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning after it, but this glyph does not, is 10000 but should be 0. character advance + kerning 13, which is between the fifth character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning after it, but this glyph does not and the sixth character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning after it, but this glyph does not, is 10000 but should be 0. character advance + kerning 14, which is between the sixth character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning after it, but this glyph does not and the first character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning before it, but this glyph does not, is 10000 but should be 1700. character advance + kerning 15, which is between the first character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning before it, but this glyph does not and the second character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning before it, but this glyph does not, is 10000 but should be 0. character advance + kerning 16, which is between the second character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning before it, but this glyph does not and the third character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning before it, but this glyph does not, is 10000 but should be 0. character advance + kerning 17, which is between the third character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning before it, but this glyph does not and the fourth character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning before it, but this glyph does not, is 10000 but should be 0. character advance + kerning 18, which is between the fourth character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning before it, but this glyph does not and the fifth character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning before it, but this glyph does not, is 10000 but should be 0. character advance + kerning 19, which is between the fifth character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning before it, but this glyph does not and the sixth character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning before it, but this glyph does not, is 10000 but should be 0. character advance + kerning 20, which is between the sixth character of a six-character glyph, which happens to look like a Unicode range, where the range-specified glyph has kerning before it, but this glyph does not and a normal character, which has some kerning after it that is specified by glyph name, is 10000 but should be 1700. character advance + kerning 21, which is between a normal character, which has some kerning after it that is specified by glyph name and a normal character, which has some kerning before it that is specified by glyph name, is 2300 but should be 2500. character advance + kerning 25, which is between a normal character, whose glyph should match the one with a lang="" attribute on it and a normal character, which has some kerning after it that is equal to the advance of the character, is 4300 but should be 4100. character advance + kerning 26, which is between a normal character, which has some kerning after it that is equal to the advance of the character and a normal character, which has some kerning before it that is equal to the advance of the previous character, is 4700 but should be 0. character advance + kerning 31, which is between a normal character, whose glyph is chosen to be another by using <altGlyph> and a character not in Plane 0 (high surrogate pair), is 10000 but should be 2900. character advance + kerning 32, which is between a character not in Plane 0 (high surrogate pair) and a character not in Plane 0 (low surrogate pair), is 10000 but should be 2900. character advance + kerning 33, which is between a character not in Plane 0 (low surrogate pair) and a normal character, is 10000 but should be 0.

Attachments
preliminary patch (15.74 KB, patch) 2008-03-24 02:20 PDT, Maciej Stachowiak	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Maciej Stachowiak

Comment 1 2008-03-24 02:20:46 PDT

Created attachment 20004 [details] preliminary patch With this patch we get much fewer of the advances wrong. It fixes measurement of start position for multichar glyphs, making sure to count the glyph's advance only after its last char. Still needs test case (and probably code cleanup).

Maciej Stachowiak

Comment 2 2008-03-24 02:23:21 PDT

The following 9 incorrect advances remain after the patch above (down from 23): character advance + kerning 4, which is between a normal character, which shouldn't be the first character of a two-character glyph and a normal character, which shouldn't be the second character of a two-character glyph, is 0 but should be 300. character advance + kerning 5, which is between a normal character, which shouldn't be the second character of a two-character glyph and a normal character, which has some kerning after it, is 700 but should be 1100. character advance + kerning 6, which is between a normal character, which has some kerning after it and the first character of a two-character glyph, which has some kerning before it, is 10000 but should be 10200. character advance + kerning 21, which is between a normal character, which has some kerning after it that is specified by glyph name and a normal character, which has some kerning before it that is specified by glyph name, is 2300 but should be 2500. character advance + kerning 25, which is between a normal character, whose glyph should match the one with a lang="" attribute on it and a normal character, which has some kerning after it that is equal to the advance of the character, is 4300 but should be 4100. character advance + kerning 26, which is between a normal character, which has some kerning after it that is equal to the advance of the character and a normal character, which has some kerning before it that is equal to the advance of the previous character, is 4700 but should be 0. character advance + kerning 31, which is between a normal character, whose glyph is chosen to be another by using <altGlyph> and a character not in Plane 0 (high surrogate pair), is 10000 but should be 2900. character advance + kerning 32, which is between a character not in Plane 0 (high surrogate pair) and a character not in Plane 0 (low surrogate pair), is 0 but should be 2900. character advance + kerning 33, which is between a character not in Plane 0 (low surrogate pair) and a normal character, is 2900 but should be 0.

Maciej Stachowiak

Comment 3 2008-03-24 02:41:13 PDT

Yeah, I am pretty sure the test is wrong. These advances are given: 2900, // 𐂅 high surrogate pair 0, // 𐂅 low surrogate pair But <http://www.w3.org/TR/SVG/text.html#InterfaceSVGTextContentElement> says: "If multiple consecutive characters are rendered inseparably (e.g., as a single glyph or a sequence of glyphs), then each of the inseparable characters will return the start position for the first glyph." The two halves of a surrogate pair are certainly rendered inseparably, so they must have the same start position and so there cannot be an advance between them.

Maciej Stachowiak

Comment 4 2008-03-24 02:48:24 PDT

That light comment might seem a little confusing out of context of my analaysis of remaining failures, which I meant to paste in but somehow lost: Advances 4 & 5: SVG font glyph matching is incorrectly picking the longest match instead of the first, when selecting glyphs. Advances 6, 21 & 26: missing support for kerning pairs and hkern element. Advance 25: we don't consider language during glyph selection Advance 31: altGlyph renders the contents instead of the substitute glyph Advances 32 & 33: apparent bug in the test; it seems to expect the advance of a surrogate pair to happen on the first code point instead of the second (this doesn't seem to match the SVG spec).

Ian 'Hixie' Hickson

Comment 5 2008-03-24 12:45:17 PDT

Fixed the surrogate thing. Sorry. That was my bad, when I split out the surrogate into two as per the recent WG decision, I assumed the advance would go on the first codepoint and not the second. I see now that that is inconsistent with the rest of the test. :-)

Maciej Stachowiak

Comment 6 2008-03-24 13:31:00 PDT

Results with Hixie's fix to the test: character advance + kerning 4, which is between a normal character, which shouldn't be the first character of a two-character glyph and a normal character, which shouldn't be the second character of a two-character glyph, is 0 but should be 300. character advance + kerning 5, which is between a normal character, which shouldn't be the second character of a two-character glyph and a normal character, which has some kerning after it, is 700 but should be 1100. character advance + kerning 6, which is between a normal character, which has some kerning after it and the first character of a two-character glyph, which has some kerning before it, is 10000 but should be 10200. character advance + kerning 21, which is between a normal character, which has some kerning after it that is specified by glyph name and a normal character, which has some kerning before it that is specified by glyph name, is 2300 but should be 2500. character advance + kerning 25, which is between a normal character, whose glyph should match the one with a lang="" attribute on it and a normal character, which has some kerning after it that is equal to the advance of the character, is 4300 but should be 4100. character advance + kerning 26, which is between a normal character, which has some kerning after it that is equal to the advance of the character and a normal character, which has some kerning before it that is equal to the advance of the previous character, is 4700 but should be 0. character advance + kerning 31, which is between a normal character, whose glyph is chosen to be another by using <altGlyph> and a character not in Plane 0 (high surrogate pair), is 10000 but should be 2900.

Darin Adler

Comment 7 2008-03-26 19:47:52 PDT

Maciej and Hyatt finished the last bits of this in r31342.

Note You need to log in before you can comment on or make changes to this bug.