Bug 169301

Summary: Japanese fonts in vertical text should support synthesized italics
Product: WebKit Reporter: Dave Hyatt <hyatt>
Component: New BugsAssignee: Dave Hyatt <hyatt>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, jonlee, mmaxfield, ryanhaddad, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch simon.fraser: review+

Description Dave Hyatt 2017-03-07 13:21:45 PST
Japanese fonts in vertical text should support synthesized italics
Comment 1 Dave Hyatt 2017-03-07 13:25:42 PST
Created attachment 303714 [details]
Patch
Comment 2 Dave Hyatt 2017-03-07 13:50:20 PST
Not really sure if the slant direction is what we want here or not. It matches Blink, but I don't know if that's what we want here.
Comment 3 Dave Hyatt 2017-03-07 16:08:23 PST
Created attachment 303741 [details]
Patch
Comment 4 Dave Hyatt 2017-03-08 11:17:51 PST
rdar://problem/22122041
Comment 5 Jon Lee 2017-03-09 17:19:11 PST
Comment on attachment 303741 [details]
Patch

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

> Source/WebCore/platform/graphics/cocoa/FontCascadeCocoa.mm:246
> +        static float obliqueCJKSkew = tanf(-syntheticObliqueAngle() * piFloat / 180);

isn't this just -obliqueSkew?

> Source/WebCore/platform/graphics/cocoa/FontCascadeCocoa.mm:253
>              matrix = CGAffineTransformConcat(matrix, CGAffineTransformMake(1, 0, -obliqueSkew, 1, 0, 0));

I then think this simplifies to
if (platformData.orientation() == Vertical && font.isTextOrientationFallback()
 matrix = ... obliqueSkew ...
else
 matrix = ... -obliqueSkew ...

> LayoutTests/ChangeLog:10
> +        * platform/mac/fast/text/international/synthesized-italic-vertical-expected.txt: Added.

Could a ref test have one Ahem character italicized in vertical writing mode, and the reference have the same character in horizontal writing mode and have a skew transform applied?
Comment 6 Dave Hyatt 2017-03-09 17:57:27 PST
Every ref test I've tried to make here has had small differences. The characters don't start off in the same place for vertical vs. horizontal.
Comment 7 Dave Hyatt 2017-04-03 14:04:27 PDT
Created attachment 306106 [details]
Patch
Comment 8 Simon Fraser (smfr) 2017-04-03 15:12:00 PDT
Comment on attachment 306106 [details]
Patch

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

> Source/WebCore/platform/graphics/FontCascadeFonts.cpp:305
> +        // If there is a baked-in rotated glyph, we will use it unless syntheticOblique is set. If
> +        // synthetic oblique is set, we fall back to the horizontal glyph. This allows us to guarantee that vertical fonts without isTextOrientationFallback() set
> +        // contain CJK characters only and thus we can get the oblique slant correct.

Awkward comment wrapping.
Comment 9 Dave Hyatt 2017-04-03 15:29:05 PDT
Fixed in r214848.
Comment 10 Ryan Haddad 2017-04-04 11:32:48 PDT
(In reply to Dave Hyatt from comment #9)
> Fixed in r214848.

The expected result files for fast/text/international/synthesized-italic-vertical-latin.html were removed with this change. Did you intend to delete the test, too?
Comment 11 Ryan Haddad 2017-04-04 13:22:29 PDT
(In reply to Ryan Haddad from comment #10)
> (In reply to Dave Hyatt from comment #9)
> > Fixed in r214848.
> 
> The expected result files for
> fast/text/international/synthesized-italic-vertical-latin.html were removed
> with this change. Did you intend to delete the test, too?

This appears to have been accidental. Added the results back in http://trac.webkit.org/projects/webkit/changeset/214892