[webkit-reviews] review denied: [Bug 77067] Parenthesis in RTL fonts are not mirrored with SVG fonts : [Attachment 124053] First-pass at fixing mirroring with SVG fonts

[bugzilla-daemon at webkit.org]

Nikolas Zimmermann <zimmermann at kde.org> has denied  review:
Bug 77067: Parenthesis in RTL fonts are not mirrored with SVG fonts
https://bugs.webkit.org/show_bug.cgi?id=77067

Attachment 124053: First-pass at fixing mirroring with SVG fonts
https://bugs.webkit.org/attachment.cgi?id=124053&action=review

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=124053&action=review


Nice first stab, needs more tests and some speedups:

> Source/WebCore/svg/SVGFontData.cpp:142
> +    if (mirror)
> +	   remainingTextInRun =
replaceMirroredCharacters(remainingTextInRun.characters(),
remainingTextInRun.length());
>      if (!currentCharacter && arabicForms.isEmpty())
>	   arabicForms = charactersWithArabicForm(remainingTextInRun, mirror);

Danger Will Robinson :-) This can potentially break the arabic-form support -
which already handles mirroring. Please add another testcase (you can grep for
arabic-form and duplicate an existing) to see how it plays together with your
testcase text data.

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.