Bug 75289 - Complex path should be used when UVS exists in text run
Summary: Complex path should be used when UVS exists in text run
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kenichi Ishibashi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-27 19:31 PST by Kenichi Ishibashi
Modified: 2012-01-11 02:23 PST (History)
5 users (show)

See Also:

Attachments
Patch (15.63 KB, patch)
2011-12-27 20:27 PST, Kenichi Ishibashi 		no flags Details | Formatted Diff | Diff
Patch (update expectations) (15.59 KB, patch)
2011-12-27 21:30 PST, Kenichi Ishibashi 		no flags Details | Formatted Diff | Diff
Patch(revised to ToT) (15.59 KB, patch)
2012-01-05 23:03 PST, Kenichi Ishibashi 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) 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 2011-12-27 19:31:39 PST 
I added Unicode variation selector support as http://trac.webkit.org/changeset/102915, but the design wasn't good. When a text run contains UVS, complex path should be used because UVS requires glyph substitution depending on the context. UVS would be supported naturally by platform-specific text rendering engines if it support UVS.
Comment 1 Kenichi Ishibashi 2011-12-27 20:27:07 PST 
Created attachment 120632 [details]
Patch
Comment 2 Kenichi Ishibashi 2011-12-27 21:30:28 PST 
Created attachment 120634 [details]
Patch (update expectations)
Comment 3 Kenichi Ishibashi 2012-01-05 23:03:00 PST 
Created attachment 121403 [details]
Patch(revised to ToT)
Comment 4 Kenichi Ishibashi 2012-01-05 23:07:31 PST 
(In reply to comment #3)
> Created an attachment (id=121403) [details]
> Patch(revised to ToT)

Hi mitz, 

Could you please review this patch? This patch adds unicode variation selector ranges to Font::codePath() so that the function returns "Complex" for that ranges. The patch also removes the code that I added as r102915.
Comment 5 Kenichi Ishibashi 2012-01-09 22:12:20 PST 
Comment on attachment 121403 [details]
Patch(revised to ToT)

Thank you for the review!
Comment 6 WebKit Review Bot 2012-01-10 02:46:50 PST 
Comment on attachment 121403 [details]
Patch(revised to ToT)

Clearing flags on attachment: 121403

Committed r104545: <http://trac.webkit.org/changeset/104545>
Comment 7 WebKit Review Bot 2012-01-10 02:46:54 PST 
All reviewed patches have been landed.  Closing bug.
Comment 8 John Knottenbelt 2012-01-10 08:24:59 PST 
Hi, I just committed a revised baseline in http://trac.webkit.org/changeset/104579 .

Please could you check that it's correct.
Comment 9 Kenichi Ishibashi 2012-01-10 15:46:46 PST 
It's correct. Thank you for rebaselining!

(In reply to comment #8)
> Hi, I just committed a revised baseline in http://trac.webkit.org/changeset/104579 .
> 
> Please could you check that it's correct.
Comment 10 mitz 2012-01-10 22:48:40 PST 
This change broke <fast/text/unicode-variation-selector.html> on OS X Lion.
Comment 11 Kenichi Ishibashi 2012-01-11 02:23:16 PST 
(In reply to comment #10)
> This change broke <fast/text/unicode-variation-selector.html> on OS X Lion.

Sorry for about the regression. It seems I should also add UVS support to ComplexTextController class. I'll file a bug for it.