| Summary: | [Cairo] HarfbuzzNG should apply draw-range to complex font. | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Seongjun Kim <isair> | ||||||||||||||||||
| Component: | Text | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||||||
| Status: | NEW --- | ||||||||||||||||||||
| Severity: | Normal | CC: | behdad, bfulgham, bugs-noreply, commit-queue, crzwdjk, d-r, hausmann, isair, mmaxfield, mrobinson, rniwa | ||||||||||||||||||
| Priority: | P2 | ||||||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||
|
Description
Seongjun Kim
2014-01-16 22:39:37 PST
Created attachment 221441 [details]
Patch
Created attachment 221444 [details]
Patch
Test page looks different in Safari, Chrome and Firefox. So I cannot add layout test. I don't know what is correct behavior. My patch make MiniBrowser in gtk and efl to draw complex text like Chrome. Created attachment 221451 [details]
Patch
Created attachment 221452 [details]
Patch
Created attachment 221709 [details]
Patch
Created attachment 221710 [details]
Patch
(In reply to comment #3) > Test page looks different in Safari, Chrome and Firefox. > So I cannot add layout test. I don't know what is correct behavior. > > > My patch make MiniBrowser in gtk and efl to draw complex text like Chrome. Interesting issue - I would tend to think Firefox' behaviour is correct. I don't think it's correct that the arabic 7glyph overlays the o from hello. Maybe CC Behdad on this bug. (In reply to comment #8) > (In reply to comment #3) > > Test page looks different in Safari, Chrome and Firefox. > > So I cannot add layout test. I don't know what is correct behavior. > > > > > > My patch make MiniBrowser in gtk and efl to draw complex text like Chrome. > > Interesting issue - I would tend to think Firefox' behaviour is correct. I don't think it's correct that the arabic 7glyph overlays the o from hello. Maybe CC Behdad on this bug. Both are wrong, but Chrome is closer to correct. The problem with Firefox is that it is cutting the visual text. Chrome / Webkit correctly cut text logically. The clipping can be fixed. Other than that, the only thing I would fix is to put the ellipsis where text was cut, ie. if it was in the Arabic run, put the ellipsis on the left side of the Arabic run since it's RTL. Same issue is discussed here: https://code.google.com/p/chromium/issues/detail?id=155836 Created attachment 245516 [details]
The results of a demo page below
The results of a demo page below
I want to minimalize this issue. The position of elipsis mark would be another issue.
This issue is about a bug about complex text rendering in WebKitGtk+.
Demo page : http://jsfiddle.net/6sfw764b/2/ Created attachment 245520 [details]
Patch
Comment on attachment 245520 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=245520&action=review Isn't there any tests ? > Source/WebCore/ChangeLog:3 > + [Cairo] HarfbuzzNG should apply draw-range to complex texxt. typo: %s/texxt/text/g > Source/WebCore/platform/graphics/cairo/FontCairoHarfbuzzNG.cpp:42 > +float FontCascade::getGlyphsAndAdvancesForComplexText(const TextRun& run, int form, int to, GlyphBuffer& glyphBuffer, ForTextEmphasisOrNot /* forTextEmphasis */) const Looks like a typo : %s/form/from/g ? (In reply to comment #13) > Comment on attachment 245520 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=245520&action=review > > Isn't there any tests ? I'm definitely curious about the answer to this question. :) Comment on attachment 245520 [details]
Patch
Thanks for this patch. Asides from the typos that Gyuyoung found, the code changes look sane to me. I'm glad to see someone is spending time on complex text.
But please add a layout test, else this is just going to break again. r=me with a layout test.
|