WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
149774
font-variant-caps does not work if the font does not support font features
https://bugs.webkit.org/show_bug.cgi?id=149774
Summary
font-variant-caps does not work if the font does not support font features
Myles C. Maxfield
Reported
2015-10-02 16:55:42 PDT
We need to implement synthesis in this case.
Attachments
WIP
(17.11 KB, patch)
2015-11-20 22:13 PST
,
Myles C. Maxfield
no flags
Details
Formatted Diff
Diff
WIP
(17.58 KB, patch)
2015-11-21 17:37 PST
,
Myles C. Maxfield
no flags
Details
Formatted Diff
Diff
First draft
(34.78 KB, patch)
2015-12-03 22:57 PST
,
Myles C. Maxfield
no flags
Details
Formatted Diff
Diff
Patch
(71.42 KB, patch)
2015-12-05 00:52 PST
,
Myles C. Maxfield
no flags
Details
Formatted Diff
Diff
Patch
(70.93 KB, patch)
2015-12-05 01:00 PST
,
Myles C. Maxfield
no flags
Details
Formatted Diff
Diff
Patch
(71.27 KB, patch)
2015-12-05 13:51 PST
,
Myles C. Maxfield
no flags
Details
Formatted Diff
Diff
Patch
(88.51 KB, patch)
2015-12-07 23:59 PST
,
Myles C. Maxfield
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews103 for mac-yosemite
(365.51 KB, application/zip)
2015-12-08 00:47 PST
,
Build Bot
no flags
Details
Archive of layout-test-results from ews107 for mac-yosemite-wk2
(481.42 KB, application/zip)
2015-12-08 00:51 PST
,
Build Bot
no flags
Details
Archive of layout-test-results from ews115 for mac-yosemite
(450.31 KB, application/zip)
2015-12-08 00:53 PST
,
Build Bot
no flags
Details
Patch
(88.70 KB, patch)
2015-12-08 01:11 PST
,
Myles C. Maxfield
koivisto
: review+
buildbot
: commit-queue-
Details
Formatted Diff
Diff
Archive of layout-test-results from ews102 for mac-yosemite
(975.33 KB, application/zip)
2015-12-08 01:59 PST
,
Build Bot
no flags
Details
Archive of layout-test-results from ews106 for mac-yosemite-wk2
(835.66 KB, application/zip)
2015-12-08 02:14 PST
,
Build Bot
no flags
Details
Archive of layout-test-results from ews115 for mac-yosemite
(1.02 MB, application/zip)
2015-12-08 02:16 PST
,
Build Bot
no flags
Details
Patch for committing
(88.99 KB, patch)
2015-12-08 19:18 PST
,
Myles C. Maxfield
no flags
Details
Formatted Diff
Diff
Patch for committing
(89.07 KB, patch)
2015-12-08 19:36 PST
,
Myles C. Maxfield
no flags
Details
Formatted Diff
Diff
Patch for committing
(96.31 KB, patch)
2015-12-09 23:51 PST
,
Myles C. Maxfield
no flags
Details
Formatted Diff
Diff
Show Obsolete
(15)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2015-10-02 17:06:43 PDT
<
rdar://problem/22959746
>
Myles C. Maxfield
Comment 2
2015-11-19 18:12:30 PST
I'm going to break this into two bugs - one for font-variant-position and one for font-variant-caps.
Myles C. Maxfield
Comment 3
2015-11-19 18:20:13 PST
True small caps requires font features, which will force us down our complex text code path. I need to make sure this won't be a performance regression.
Myles C. Maxfield
Comment 4
2015-11-19 18:38:46 PST
Let's assume that it is not a performance regression, for now. In that case, one way to do it would be: FontCascade::variantFont() and FontCascade::fontForCombiningCharacterSequence() should return information regarding if a real font or a synthesized font was returned. Then, when ComplexTextController::collectComplexTextRuns() notices that a synthesized font was returned, it should scan backward for the longest contiguous collection of fonts which match the real font, and turn them all into the the synthesized font, in addition to setting some state which will trigger any subsequent uses of the true font to be replaced as well. When ComplexTextController::collectComplexTextRuns() reaches a font which doesn't match this contiguous run, it should reset that state. In this case where FontCascade::variantFont() returns the synthesized font, this means that it should actually return two fonts - one is the font to scan back for, and the other is the replacement font. Getting this first is "free," since we always need to try the real font before trying the synthesized font.
Myles C. Maxfield
Comment 5
2015-11-19 18:50:48 PST
(In reply to
comment #4
)
> Let's assume that it is not a performance regression, for now. In that case, > one way to do it would be: > > FontCascade::variantFont() and > FontCascade::fontForCombiningCharacterSequence() should return information > regarding if a real font or a synthesized font was returned. Then, when > ComplexTextController::collectComplexTextRuns() notices that a synthesized > font was returned, it should scan backward for the longest contiguous > collection of fonts which match the real font, and turn them all into the > the synthesized font, in addition to setting some state which will trigger > any subsequent uses of the true font to be replaced as well. When > ComplexTextController::collectComplexTextRuns() reaches a font which doesn't > match this contiguous run, it should reset that state. > > In this case where FontCascade::variantFont() returns the synthesized font, > this means that it should actually return two fonts - one is the font to > scan back for, and the other is the replacement font. Getting this first is > "free," since we always need to try the real font before trying the > synthesized font.
Yeah, I think this approach is the best solution. If the complex code path does turn out to be a regression, this approach should also work with WidthIterator (though we would have bigger problems to get CTFontTransformGlyphs() to work .... probably)
Myles C. Maxfield
Comment 6
2015-11-20 15:28:46 PST
(In reply to
comment #3
)
> True small caps requires font features, which will force us down our complex > text code path. I need to make sure this won't be a performance regression.
Verified this is not a regression.
Myles C. Maxfield
Comment 7
2015-11-20 22:13:07 PST
Created
attachment 266028
[details]
WIP
Myles C. Maxfield
Comment 8
2015-11-21 17:37:55 PST
Created
attachment 266034
[details]
WIP
Myles C. Maxfield
Comment 9
2015-11-22 11:15:23 PST
(In reply to
comment #6
)
> (In reply to
comment #3
) > > True small caps requires font features, which will force us down our complex > > text code path. I need to make sure this won't be a performance regression. > > Verified this is not a regression.
This means I should remove small-caps support entire from the simple codepath
Myles C. Maxfield
Comment 10
2015-12-03 22:57:26 PST
Created
attachment 266604
[details]
First draft
Myles C. Maxfield
Comment 11
2015-12-05 00:52:42 PST
Created
attachment 266711
[details]
Patch
Myles C. Maxfield
Comment 12
2015-12-05 01:00:53 PST
Created
attachment 266712
[details]
Patch
Myles C. Maxfield
Comment 13
2015-12-05 13:51:54 PST
Created
attachment 266721
[details]
Patch
Myles C. Maxfield
Comment 14
2015-12-07 13:47:35 PST
Comment on
attachment 266721
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=266721&action=review
> Source/WebCore/platform/graphics/cocoa/FontCocoa.mm:393 > + // <
rdar://problem/23770219
>: Unable to remove font feature from web font. Until that radar is fixed, re-use the small-caps font for synthesis.
Instead of removing the feature, I should just explicitly set it to "off"
Myles C. Maxfield
Comment 15
2015-12-07 13:49:19 PST
Comment on
attachment 266721
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=266721&action=review
>> Source/WebCore/platform/graphics/cocoa/FontCocoa.mm:393 >> + // <
rdar://problem/23770219
>: Unable to remove font feature from web font. Until that radar is fixed, re-use the small-caps font for synthesis. > > Instead of removing the feature, I should just explicitly set it to "off"
kDefaultLowerCaseSelector
Myles C. Maxfield
Comment 16
2015-12-07 23:59:39 PST
Created
attachment 266849
[details]
Patch
Build Bot
Comment 17
2015-12-08 00:47:42 PST
Comment on
attachment 266849
[details]
Patch
Attachment 266849
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/531737
Number of test failures exceeded the failure limit.
Build Bot
Comment 18
2015-12-08 00:47:46 PST
Created
attachment 266853
[details]
Archive of layout-test-results from ews103 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 19
2015-12-08 00:51:06 PST
Comment on
attachment 266849
[details]
Patch
Attachment 266849
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/531740
Number of test failures exceeded the failure limit.
Build Bot
Comment 20
2015-12-08 00:51:09 PST
Created
attachment 266854
[details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Build Bot
Comment 21
2015-12-08 00:53:22 PST
Comment on
attachment 266849
[details]
Patch
Attachment 266849
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/531727
Number of test failures exceeded the failure limit.
Build Bot
Comment 22
2015-12-08 00:53:25 PST
Created
attachment 266855
[details]
Archive of layout-test-results from ews115 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-yosemite Platform: Mac OS X 10.10.5
Myles C. Maxfield
Comment 23
2015-12-08 01:11:43 PST
Created
attachment 266859
[details]
Patch
Build Bot
Comment 24
2015-12-08 01:59:49 PST
Comment on
attachment 266859
[details]
Patch
Attachment 266859
[details]
did not pass mac-ews (mac): Output:
http://webkit-queues.webkit.org/results/531892
New failing tests: css3/font-variant-petite-caps-synthesis.html
Build Bot
Comment 25
2015-12-08 01:59:54 PST
Created
attachment 266863
[details]
Archive of layout-test-results from ews102 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 26
2015-12-08 02:14:11 PST
Comment on
attachment 266859
[details]
Patch
Attachment 266859
[details]
did not pass mac-wk2-ews (mac-wk2): Output:
http://webkit-queues.webkit.org/results/531928
New failing tests: css3/font-variant-petite-caps-synthesis.html
Build Bot
Comment 27
2015-12-08 02:14:14 PST
Created
attachment 266865
[details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Build Bot
Comment 28
2015-12-08 02:16:47 PST
Comment on
attachment 266859
[details]
Patch
Attachment 266859
[details]
did not pass mac-debug-ews (mac): Output:
http://webkit-queues.webkit.org/results/531929
New failing tests: css3/font-variant-petite-caps-synthesis.html
Build Bot
Comment 29
2015-12-08 02:16:51 PST
Created
attachment 266867
[details]
Archive of layout-test-results from ews115 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-yosemite Platform: Mac OS X 10.10.5
Antti Koivisto
Comment 30
2015-12-08 02:44:41 PST
r=me
Myles C. Maxfield
Comment 31
2015-12-08 19:18:35 PST
Created
attachment 266961
[details]
Patch for committing
Myles C. Maxfield
Comment 32
2015-12-08 19:36:44 PST
Created
attachment 266962
[details]
Patch for committing
Myles C. Maxfield
Comment 33
2015-12-09 23:51:13 PST
Created
attachment 267076
[details]
Patch for committing
WebKit Commit Bot
Comment 34
2015-12-10 00:37:25 PST
Comment on
attachment 267076
[details]
Patch for committing Clearing flags on attachment: 267076 Committed
r193894
: <
http://trac.webkit.org/changeset/193894
>
Myles C. Maxfield
Comment 35
2015-12-10 00:47:16 PST
***
Bug 4355
has been marked as a duplicate of this bug. ***
Ryan Haddad
Comment 36
2015-12-10 07:36:16 PST
The tests added with this change are failing on Windows: <
https://build.webkit.org/builders/Apple%20Win%207%20Release%20%28Tests%29/builds/55487
>
Myles C. Maxfield
Comment 37
2015-12-10 10:51:07 PST
(In reply to
comment #36
)
> The tests added with this change are failing on Windows: > > <
https://build.webkit.org/builders/Apple%20Win%207%20Release%20%28Tests%29/
> builds/55487>
These are expected to fail. Do you think you could mark them as such? (There should be other similar css3/font-variant-* tests that are already marked as failing on Windows.)
Ryan Haddad
Comment 38
2015-12-10 10:55:23 PST
Done in
r193914
.
Michael Catanzaro
Comment 39
2015-12-31 10:41:25 PST
Should this bug be closed?
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug