[Chromium] Improve glyph positioning of HarfBuzzShaper

• Component: Platform
• Status: RESOLVED FIXED

[Kenichi Ishibashi]

There are two problems in glyph positioning of HarfBuzzShaper:
- If the shaper produces multiple HarfBuzzRuns, the position of first glyph of second and following runs could be wrong if the offsets of the precedence glyph (that is the last glyph of the previous run) aren't zero.
- Letter spacing and word spacing are added at the right side of characters/words even for RTL run. Spacing should be added at the left side of characters/words.

We should fix these problems.

[Kenichi Ishibashi]

Created attachment 164705 [details]
Patch

[Kenichi Ishibashi]

Hi Tony,

Could you take a look?

[Tony Chang]

Comment on attachment 164705 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=164705&action=review

FWIW, if you uploaded before and after png results for the tests that change as a separate attachment to the bug, it would help me understand the change.

> Source/WebCore/platform/graphics/harfbuzz/ng/HarfBuzzShaper.cpp:319
> +    // HarfBuzz returns the shaping result in visual order. We need not to flip for RTL

Nit: Missing period at the end of the sentence.

[Kenichi Ishibashi]

Created attachment 164786 [details]
Patch for landing

[Kenichi Ishibashi]

(In reply to comment #3)
> (From update of attachment 164705 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=164705&action=review
> 
> FWIW, if you uploaded before and after png results for the tests that change as a separate attachment to the bug, it would help me understand the change.
> 

I should have done so. I'll do it next time.

> > Source/WebCore/platform/graphics/harfbuzz/ng/HarfBuzzShaper.cpp:319
> > +    // HarfBuzz returns the shaping result in visual order. We need not to flip for RTL
> 
> Nit: Missing period at the end of the sentence.

Done.

[WebKit Review Bot]

Comment on attachment 164786 [details]
Patch for landing

Rejecting attachment 164786 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
kitpy/layout_tests/models/test_expectations_unittest.py
Failed to merge in the changes.
Patch failed at 0001 implement first part of support for the new TestExpectations syntax

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.

Full output: http://queues.webkit.org/results/13905607

[Kenichi Ishibashi]

Created attachment 164791 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 164791 [details]
Patch for landing

Clearing flags on attachment: 164791

Committed r129074: <http://trac.webkit.org/changeset/129074>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.