Bug 92445 - [Chromium] HarfBuzzShaper can't handle segmented text run
Summary: [Chromium] HarfBuzzShaper can't handle segmented text run
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kenichi Ishibashi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-26 18:45 PDT by Kenichi Ishibashi
Modified: 2012-07-29 18:47 PDT (History)
2 users (show)

See Also:

Attachments
Patch (9.05 KB, patch)
2012-07-26 19:09 PDT, Kenichi Ishibashi 		no flags Details | Formatted Diff | Diff
Patch for landing (9.02 KB, patch)
2012-07-29 16:55 PDT, Kenichi Ishibashi 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Kenichi Ishibashi 2012-07-26 18:45:47 PDT 
The same problem of https://bugs.webkit.org/show_bug.cgi?id=72095 happens when we build chromium with use_harfbuzz_ng=1 GYP flag.
Comment 1 Kenichi Ishibashi 2012-07-26 19:09:50 PDT 
Created attachment 154810 [details]
Patch
Comment 2 Kenichi Ishibashi 2012-07-26 19:11:31 PDT 
Hi Tony,

Could you take a look?
Comment 3 Tony Chang 2012-07-27 10:32:01 PDT 
Comment on attachment 154810 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=154810&action=review

> Source/WebCore/platform/graphics/harfbuzz/ng/HarfBuzzShaper.cpp:194
> +    return index >= m_fromIndex && index < m_toIndex;

Nit: I think "m_fromIndex <= index && index < m_toIndex" is a little easier to read.

> Source/WebCore/platform/graphics/harfbuzz/ng/HarfBuzzShaper.h:100
> +    bool shouldDrawCharacter(int index);

Nit: Maybe name this shouldDrawCharacterAt?
Comment 4 Kenichi Ishibashi 2012-07-29 16:55:45 PDT 
Created attachment 155187 [details]
Patch for landing
Comment 5 Kenichi Ishibashi 2012-07-29 16:56:28 PDT 
Comment on attachment 154810 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=154810&action=review

Thank you for review!

>> Source/WebCore/platform/graphics/harfbuzz/ng/HarfBuzzShaper.cpp:194
>> +    return index >= m_fromIndex && index < m_toIndex;
> 
> Nit: I think "m_fromIndex <= index && index < m_toIndex" is a little easier to read.

Done.

>> Source/WebCore/platform/graphics/harfbuzz/ng/HarfBuzzShaper.h:100
>> +    bool shouldDrawCharacter(int index);
> 
> Nit: Maybe name this shouldDrawCharacterAt?

Done.
Comment 6 WebKit Review Bot 2012-07-29 18:47:14 PDT 
Comment on attachment 155187 [details]
Patch for landing

Clearing flags on attachment: 155187

Committed r123991: <http://trac.webkit.org/changeset/123991>
Comment 7 WebKit Review Bot 2012-07-29 18:47:18 PDT 
All reviewed patches have been landed.  Closing bug.