Summary: | Render "simple" Hebrew using the CG codepath | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | mitz | ||||||||||
Component: | Layout and Rendering | Assignee: | mitz | ||||||||||
Status: | VERIFIED FIXED | ||||||||||||
Severity: | Normal | ||||||||||||
Priority: | P3 | ||||||||||||
Version: | 420+ | ||||||||||||
Hardware: | Mac | ||||||||||||
OS: | OS X 10.4 | ||||||||||||
Bug Depends on: | 4940, 5166 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
mitz
2005-09-04 10:23:06 PDT
Created attachment 3768 [details]
Exclude Hebrew letters and maqaf from ATSUI rendering
Comment on attachment 3768 [details]
Exclude Hebrew letters and maqaf from ATSUI rendering
This patch causes 4 bidi layout tests to fail with benign horizontal shifting
of text.
Comment on attachment 3768 [details]
Exclude Hebrew letters and maqaf from ATSUI rendering
Seems fine. r=me
Can we add some more Hebrew test cases to layout tests?
Created attachment 3772 [details]
patch expected layout test results
Created attachment 3773 [details]
updated expected pixel test results
Comment on attachment 3768 [details]
Exclude Hebrew letters and maqaf from ATSUI rendering
Patch causes a problem where shouldUseATSU returns YES for a run (when passed
from=0, to=length) but NO for some of its subruns (typically words), because CG
and ATSU don't give the same width even for CG-safe runs.
Of the Hebrew fonts I have, I'm seeing differences between CG and ATSU in width of Hebrew text only in Lucida Grande. Comment on attachment 3768 [details]
Exclude Hebrew letters and maqaf from ATSUI rendering
Now that ATSU does the rounding hack it's safe to make this change. I'm not
sure the updated test results are still valid, though.
Comment on attachment 3768 [details]
Exclude Hebrew letters and maqaf from ATSUI rendering
r=me -- we're assuming this won't affect any layout tests now
(In reply to comment #9) > r=me -- we're assuming this won't affect any layout tests now Unfortunately, it will, for two reasons: 1. There can still be a <1px difference in the position of individual glyphs between CG and ATSU (since the CG code path applies the rounding in logical order and ATSU in left-to-right order), even though the total width remains the same. 2. The fix for bug 4940 didn't address fallback fonts that don't have the same rendering mode as the original font. This happens in tests that use Times (since the fallback is Lucida Grande). Comment on attachment 3768 [details]
Exclude Hebrew letters and maqaf from ATSUI rendering
OK. Issue (1) is not a concern, but issue (2) is. I think we should put this on
hold again until that's dealt with.
Comment on attachment 3768 [details] Exclude Hebrew letters and maqaf from ATSUI rendering > OK. Issue (1) is not a concern, but issue (2) is. I think we should put this on > hold again until that's dealt with. I think the exact opposite in this case. The first time I pulled the patch was because it made things worse for lines mixing "simple" Hebrew with non-"simple" Hebrew (say, one word with niqqud), since they went from all-ATSU lines to mixed ATSU/CG, and there was a problem with mixed ATSU/CG. Issue (2) currently affects (certain) mixed Latin/Hebrew lines, since they currently mix ATSU with CG, and there is (still) a problem with mixed ATSU/CG, but this time, changing Hebrew to CG will actually make things better for these lines. Comment on attachment 3768 [details]
Exclude Hebrew letters and maqaf from ATSUI rendering
OK.
Comment on attachment 3768 [details] Exclude Hebrew letters and maqaf from ATSUI rendering I changed my mind (again). The phenomenon described in comment #6 does remain a problem in too many cases, due to issue (2). Need to figure out a solution for that. Sorry about the mess. Created attachment 5240 [details]
Exclude Hebrew letters and maqaf from ATSUI rendering
At last, making this change really affects only rendering. Pixel results are
expected to change for the following tests:
fast/text/international/bidi-AN-after-L
fast/text/international/bidi-L2-run-reordering
fast/text/international/bidi-LDB-2-CSS
fast/text/international/bidi-LDB-2-HTML
fast/text/international/bidi-LDB-2-formatting-characters
fast/text/international/bidi-explicit-embedding
fast/text/international/bidi-fallback-font-weight
fast/text/international/bidi-ignored-for-first-child-inline
fast/text/international/bidi-innertext
fast/text/international/bidi-layout-across-linebreakfast/text/international/bidi-override
Comment on attachment 5240 [details]
Exclude Hebrew letters and maqaf from ATSUI rendering
Patch looks simple enough. I will take it on faith that it is conceptually
right as well.
|