NEW 127162
[Cairo] HarfbuzzNG should apply draw-range to complex font.
https://bugs.webkit.org/show_bug.cgi?id=127162
Summary [Cairo] HarfbuzzNG should apply draw-range to complex font.
Seongjun Kim
Reported 2014-01-16 22:39:37 PST
HarfbuzzNG didn't clip complex font because HarfbuzzNG didn't apply draw-range. Test page: http://black.company100.com/isair/arabic-ellipsis.html
Attachments
Patch (1.80 KB, patch)
2014-01-16 22:56 PST, Seongjun Kim
no flags
Patch (2.10 KB, patch)
2014-01-16 23:10 PST, Seongjun Kim
no flags
Patch (1.67 KB, patch)
2014-01-17 02:31 PST, Seongjun Kim
no flags
Patch (1.60 KB, patch)
2014-01-17 02:56 PST, Seongjun Kim
no flags
Patch (1.79 KB, patch)
2014-01-20 17:58 PST, Seongjun Kim
no flags
Patch (1.79 KB, patch)
2014-01-20 17:59 PST, Seongjun Kim
no flags
The results of a demo page below (35.84 KB, image/png)
2015-01-27 22:47 PST, Seongjun Kim
no flags
Patch (5.85 KB, patch)
2015-01-27 23:57 PST, Seongjun Kim
mcatanzaro: review-
mcatanzaro: commit-queue-
Seongjun Kim
Comment 1 2014-01-16 22:56:16 PST
Seongjun Kim
Comment 2 2014-01-16 23:10:03 PST
Seongjun Kim
Comment 3 2014-01-16 23:44:28 PST
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.
Seongjun Kim
Comment 4 2014-01-17 02:31:19 PST
Seongjun Kim
Comment 5 2014-01-17 02:56:05 PST
Seongjun Kim
Comment 6 2014-01-20 17:58:03 PST
Seongjun Kim
Comment 7 2014-01-20 17:59:28 PST
Dominik Röttsches (drott)
Comment 8 2014-01-21 00:15:48 PST
(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.
Behdad Esfahbod
Comment 9 2014-01-28 07:55:30 PST
(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
Seongjun Kim
Comment 10 2015-01-27 22:47:34 PST
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+.
Seongjun Kim
Comment 11 2015-01-27 22:48:23 PST
Seongjun Kim
Comment 12 2015-01-27 23:57:46 PST
Gyuyoung Kim
Comment 13 2015-02-11 02:02:19 PST
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 ?
Martin Robinson
Comment 14 2015-05-31 20:23:01 PDT
(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. :)
Michael Catanzaro
Comment 15 2016-01-14 19:18:52 PST
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.
Note You need to log in before you can comment on or make changes to this bug.