-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for font-variant #2197
Conversation
Hello! One or more of the commit messages in this PR do not match the Ladybird code submission policy, please check the |
b8f9c1d
to
0d502f3
Compare
wpt css/css-fonts before: Ran 496 tests finished in 169.8 seconds. wpt css/css-fonts after: Ran 496 tests finished in 184.2 seconds. This fixes at least 130 tests. A few of the WPT tests have been imported into the testsuite that had to be tweaked to allow custom fonts to be loaded. |
915114a
to
ff0fe3a
Compare
ff0fe3a
to
e249906
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for making a start on font features! I've mostly stuck to looking at the CSS code as that's my area.
Your final commit looks like it's mostly moving code that was added earlier in the PR. Please apply those changes to the earlier commits instead, it makes the git history much easier to follow.
Thanks for the review! I had to do significant refactoring to support writing to the shorthand property, that why the last commit looks weird. I'll recreate the different commits to avoid shuffling things around unnecessarily and maintain the git history clean. |
1097659
to
3c0347b
Compare
@AtkinsSJ Thanks for the thorough review, it's been really helpful and is improving the PR significantly. I'm still learning quite a bit of C++ while doing this. |
9f56fd4
to
67c4de1
Compare
b6b3020
to
4c1d4aa
Compare
4c1d4aa
to
ae4805e
Compare
ae4805e
to
f406ad8
Compare
9538296
to
fc381a3
Compare
0957ec5
to
078ea86
Compare
078ea86
to
c67ea04
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot to mention these before! Otherwise I agree with Jelle, overall this is in decent shape and can be merged.
This is required by WPT tests so we can validate the OpenType/features are properly implemented.
347d02c
to
0747d95
Compare
This PR adds support for:
The CSS properties are parsed, styled and passed on to layout which ultimately passes them on to harfbuzz which shapes them based OpenType Tags, using
dlig
,clig
,calt
etcThis PR includes font-feature-settings honoring feature precedence
font-variant-alternates
lacks support for most css functions such asstylistic(feature-name)
font-variant-east-asian
requires better script shaping support to be actually useful.font-variant-emoji
is only parsed/styled, layout/rendering is a todo.