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+

Dave Hyatt
Reported 2017-03-07 13:21:45 PST
Japanese fonts in vertical text should support synthesized italics
Attachments
Patch (35.22 KB, patch)
2017-03-07 13:25 PST, Dave Hyatt 		no flags
DetailsFormatted DiffDiff
Patch (35.60 KB, patch)
2017-03-07 16:08 PST, Dave Hyatt 		no flags
DetailsFormatted DiffDiff
Patch (12.80 KB, patch)
2017-04-03 14:04 PDT, Dave Hyatt 		simon.fraser: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Dave Hyatt
Comment 1 2017-03-07 13:25:42 PST
Created attachment 303714 [details] Patch
Dave Hyatt
Comment 2 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.
Dave Hyatt
Comment 3 2017-03-07 16:08:23 PST
Created attachment 303741 [details] Patch
Dave Hyatt
Comment 4 2017-03-08 11:17:51 PST
rdar://problem/22122041
Jon Lee
Comment 5 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?
Dave Hyatt
Comment 6 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.
Dave Hyatt
Comment 7 2017-04-03 14:04:27 PDT
Created attachment 306106 [details] Patch
Simon Fraser (smfr)
Comment 8 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.
Dave Hyatt
Comment 9 2017-04-03 15:29:05 PDT
Fixed in r214848.
Ryan Haddad
Comment 10 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?
Ryan Haddad
Comment 11 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
Note You need to log in before you can comment on or make changes to this bug.