|Summary:||[Chromium] Improve glyph positioning of HarfBuzzShaper|
|Product:||WebKit||Reporter:||Kenichi Ishibashi <bashi>|
|Component:||Platform||Assignee:||Kenichi Ishibashi <bashi>|
|Version:||528+ (Nightly build)|
Description Kenichi Ishibashi 2012-09-19 03:08:57 PDT
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.
Comment 2 Kenichi Ishibashi 2012-09-19 03:13:06 PDT
Hi Tony, Could you take a look?
Comment 3 Tony Chang 2012-09-19 10:27:30 PDT
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.
Comment 4 Kenichi Ishibashi 2012-09-19 15:25:28 PDT
Created attachment 164786 [details] Patch for landing
Comment 5 Kenichi Ishibashi 2012-09-19 15:26:37 PDT
(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.
Comment 6 WebKit Review Bot 2012-09-19 15:47:15 PDT
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
Comment 7 Kenichi Ishibashi 2012-09-19 15:54:47 PDT
Created attachment 164791 [details] Patch for landing
Comment 8 WebKit Review Bot 2012-09-19 17:00:24 PDT
Comment on attachment 164791 [details] Patch for landing Clearing flags on attachment: 164791 Committed r129074: <http://trac.webkit.org/changeset/129074>
Comment 9 WebKit Review Bot 2012-09-19 17:00:28 PDT
All reviewed patches have been landed. Closing bug.