Bug 127162 – [Cairo] HarfbuzzNG should apply draw-range to complex font.

• Component: Text
• Status: NEW

[Seongjun Kim]

HarfbuzzNG didn't clip complex font because HarfbuzzNG didn't apply draw-range.

Test page: http://black.company100.com/isair/arabic-ellipsis.html

[Seongjun Kim]

Created attachment 221441 [details]
Patch

[Seongjun Kim]

Created attachment 221444 [details]
Patch

[Seongjun Kim]

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]

Created attachment 221451 [details]
Patch

[Seongjun Kim]

Created attachment 221452 [details]
Patch

[Seongjun Kim]

Created attachment 221709 [details]
Patch

[Seongjun Kim]

Created attachment 221710 [details]
Patch

[Dominik Röttsches (drott)]

(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]

(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]

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]

Demo page : http://jsfiddle.net/6sfw764b/2/

[Seongjun Kim]

Created attachment 245520 [details]
Patch

[Gyuyoung Kim]

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]

(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 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.