Bug 127162 - [Cairo] HarfbuzzNG should apply draw-range to complex font.
Summary: [Cairo] HarfbuzzNG should apply draw-range to complex font.
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-16 22:39 PST by Seongjun Kim
Modified: 2021-07-20 07:25 PDT (History)
11 users (show)

See Also:


Attachments
Patch (1.80 KB, patch)
2014-01-16 22:56 PST, Seongjun Kim
no flags Details | Formatted Diff | Diff
Patch (2.10 KB, patch)
2014-01-16 23:10 PST, Seongjun Kim
no flags Details | Formatted Diff | Diff
Patch (1.67 KB, patch)
2014-01-17 02:31 PST, Seongjun Kim
no flags Details | Formatted Diff | Diff
Patch (1.60 KB, patch)
2014-01-17 02:56 PST, Seongjun Kim
no flags Details | Formatted Diff | Diff
Patch (1.79 KB, patch)
2014-01-20 17:58 PST, Seongjun Kim
no flags Details | Formatted Diff | Diff
Patch (1.79 KB, patch)
2014-01-20 17:59 PST, Seongjun Kim
no flags Details | Formatted Diff | Diff
The results of a demo page below (35.84 KB, image/png)
2015-01-27 22:47 PST, Seongjun Kim
no flags Details
Patch (5.85 KB, patch)
2015-01-27 23:57 PST, Seongjun Kim
mcatanzaro: review-
mcatanzaro: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Seongjun Kim 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
Comment 1 Seongjun Kim 2014-01-16 22:56:16 PST
Created attachment 221441 [details]
Patch
Comment 2 Seongjun Kim 2014-01-16 23:10:03 PST
Created attachment 221444 [details]
Patch
Comment 3 Seongjun Kim 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.
Comment 4 Seongjun Kim 2014-01-17 02:31:19 PST
Created attachment 221451 [details]
Patch
Comment 5 Seongjun Kim 2014-01-17 02:56:05 PST
Created attachment 221452 [details]
Patch
Comment 6 Seongjun Kim 2014-01-20 17:58:03 PST
Created attachment 221709 [details]
Patch
Comment 7 Seongjun Kim 2014-01-20 17:59:28 PST
Created attachment 221710 [details]
Patch
Comment 8 Dominik Röttsches (drott) 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.
Comment 9 Behdad Esfahbod 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
Comment 10 Seongjun Kim 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+.
Comment 11 Seongjun Kim 2015-01-27 22:48:23 PST
Demo page : http://jsfiddle.net/6sfw764b/2/
Comment 12 Seongjun Kim 2015-01-27 23:57:46 PST
Created attachment 245520 [details]
Patch
Comment 13 Gyuyoung Kim 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 ?
Comment 14 Martin Robinson 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. :)
Comment 15 Michael Catanzaro 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.