[Webkit-unassigned] [Bug 77067] Parenthesis in RTL fonts are not mirrored with SVG fonts

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=77067





--- Comment #9 from Nikolas Zimmermann <zimmermann at kde.org>  2012-01-27 01:42:31 PST ---
(In reply to comment #7)
> (From update of attachment 124241 [details])
> Attachment 124241 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/11265580
> 
> New failing tests:
> svg/custom/glyph-selection-bidi-mirror.svg
> svg/custom/glyph-selection-arabic-forms.svg

You didn't update the cr expectations.

(In reply to comment #5)
> Good eye :) I added the glyph-selection-arabic-forms.svg test and I think everything still works as it did (except now with correct mirroring!) I'm new to Arabic forms but if I understand them correctly, the original code did not handle mirroring for Arabic forms. A mirror parameter is passed to charactersWithArabicForm but only for determining whether the text should be processed ltr or rtl. I think it would be best to change that bool parameter from "mirror" to "rtl" in charactersWithArabicForm to prevent confusion--do you agree?
Hah, good spot as well, yeah stupid that I named it mirror :-) Please rename to avoid the confusion.

> > It's also a PITA that we're allocating a remainingTextInRun String object several times :( This needs to be optimized, applySVGGlyphSelection is hot.
> > You shoulnd't need to build a whole new string, but instead iterate over the remainingTextInRun by looping over its const UChar* buffer, and call mirroredChar on the individual characters.
Meep, forgot that a String is immutable, ignore that comment..

> We definitely agree here but (per our IRC chat) I think it's best to leave this slightly slow function because the characters are immutable. If this ends up being a problem, a future optimization would be to do both mirroring and space normalization in a single pass.
... fuly agreed. It needs someone to create a perf testcase for this, and Instruments-it. If we ever see it in profiling, we can revisit this.

> Note: This patch will fail tests on most platforms at the moment. I need to add/update platform-specific test expectations when I get in the office tomorrow morning.
Ah okay, great.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.