-
Notifications
You must be signed in to change notification settings - Fork 133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unclear step in Text Layout Algorithm (section 11.5) #617
Comments
While it's still open...
|
Actually I think there's a little more to this than I'd hoped in my first point. Take this example:
this positions the A at 30 and B at 50 in Inkscape, Webkit and Firefox, so I presume that's the intention. But I don't see how this arises from the spec. Here is section 4 which deals with "dx,dy", modified as I suggested in my original post
and here's section 6, which deals with "x" and "y".
First, there is the initial line in section 4: I presume the description of shift (and it is only a description, not an instruction) is incorrect, it's the cumulative x and y shifts due to "dx" and "dy", not "x" and "y". Second, I try to run this algorithm for the example above. The first and second glyphs are shifted by 20 and 40 from their natural positions, which looks correct. But then in section 6 the "x" value overwrites this adjustment for the first glyph:
To sum up: section 6 will ensure that any x/y values replace any previous modifications to x or y, but this doesn't match any implementation I have tested. So I suspect it is section 6 that is at fault, but I'm not familiar enough with the intention to be able to suggest a solution, sorry. |
This is related to #271 |
I believe the purpose of 6.3.1 and 6.3.2 is to remove the previous shift due to 'dx' and 'dy' (shift get's zeroed). The example y="20 40 60" dy="-20 -40" should position the first character at y=0, the second at y=0 and the third at y=60. Inkscape, Firefox, and Chrome do this. |
I've had time to look at this more closely. The problem seems to be that the algorithm assumes that 'dx' is not applied when 'x' is set for a given code point. Chrome/Firefox/Inkscape do apply the corresponding 'dx' when 'x' is set (ignoring previous 'dx' values). Changing the calculation in 6.3.3 to include 'dx' seems to fix the problem. I find the variable names to be confusing and inconsistent. I propose renaming all in the form of variable[i].x. I propose also using shift.dx and result[i].dx in section 4 and shift.x and result[i].x in section 6. With these changes in names: Section 4.2.2 should be: Similar changes apply to 'y'. |
The SVG Working Group just discussed
The full IRC log of that discussion<AmeliaBR> Topic: Unclear step in Text Layout Algorithm (section 11.5)<AmeliaBR> github: https://github.com//issues/617 <AmeliaBR> Tav: This is a rather confusing algorithm, but I think I've figured out where the issue is. According to the prose, if a given character has both `x` and `dx` values, they get combined (set `x`, and then add `dx`), but according to the algorithm `dx` gets dropped in that case. And that's not what Inkscape or browsers do. <AmeliaBR> Chris: Are implementations consistent? Can we just adjust the algorithm to match? <AmeliaBR> Tav: I think everyone's consistent. <AmeliaBR> Chris: Have you figured out the suggested changes, in your last comment Tav? <AmeliaBR> Tav: I think so. Needs a review. <AmeliaBR> Dirk: Can you make it a proper PR? <AmeliaBR> Tav: Sure. <chris> https://bfo.com/ <AmeliaBR> Chris: And tag the original poster for review, who seems to be working on an implementation. <AmeliaBR> RESOLUTION: Fix algorithm to be consistent with SVG 1 and implementations (dx/dy add to x/y on same character). |
Thank you everyone and @Tavmjong in particular for your time on this, the proposed change to the algorithm has fixed the issue. I'm aware there's a PR for this change to the spec outstanding, but I'm satisfied so closing. Thanks again. |
Reopening only because the proposals in #617 (comment) haven't been applied to the spec yet. |
There's a minor typo in the text layout algorithm as described in https://www.w3.org/TR/SVG/text.html#TextLayoutAlgorithm
The text was updated successfully, but these errors were encountered: