Regression: "font-optical-sizing: auto" has no effect in Safari 16 on macOS Ventura & iOS 16 #10126
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
EWS run on previous version of this PR (hash 6fe47c5)
|
litherum
force-pushed
the
eng/Regression-font-optical-sizing-auto-has-no-effect-in-Safari-16-on-macOS-Ventura--iOS-16
branch
from
6fe47c5
to
e15848c
Compare
|
EWS run on previous version of this PR (hash e15848c)
|
|
The ios-wk2 failure looks real |
litherum
force-pushed
the
eng/Regression-font-optical-sizing-auto-has-no-effect-in-Safari-16-on-macOS-Ventura--iOS-16
branch
from
e15848c
to
9229d7f
Compare
|
EWS run on previous version of this PR (hash 9229d7f)
|
|
Seems like the team prefers to leave this untested rather than introduce megabytes of fonts into LayoutTests. I'll delete the test (actually I'll triple-check there's no smaller test I can make quickly) before landing, and I'll take a follow-up action item to make a smaller test. |
alanbaradlay
approved these changes
Feb 16, 2023
|
More test failures! |
litherum
force-pushed
the
eng/Regression-font-optical-sizing-auto-has-no-effect-in-Safari-16-on-macOS-Ventura--iOS-16
branch
from
9229d7f
to
e190365
Compare
|
EWS run on previous version of this PR (hash e190365)
|
litherum
force-pushed
the
eng/Regression-font-optical-sizing-auto-has-no-effect-in-Safari-16-on-macOS-Ventura--iOS-16
branch
from
e190365
to
a4b5aeb
Compare
|
EWS run on previous version of this PR (hash a4b5aeb) |
litherum
force-pushed
the
eng/Regression-font-optical-sizing-auto-has-no-effect-in-Safari-16-on-macOS-Ventura--iOS-16
branch
from
a4b5aeb
to
8002949
Compare
|
EWS run on current version of this PR (hash 8002949)
|
… macOS Ventura & iOS 16 https://bugs.webkit.org/show_bug.cgi?id=247987 rdar://102432138 Reviewed by Alan Baradlay. This patch hooks up the newly-created UnrealizedCoreTextFont to the logic that was formerly in preparePlatformFont(). The purpose of UnrealizedCoreTextFont is so that we can make all the font modifications we want without actually hitting Core Text; once we're done making modifications there's a final "commit" step that finally ends up making a font. Most of the reason for this is for preparePlatformFont(): its job used to be to take a font as input, modify it, and produce a new font, but now with this patch its functionality is represented as a set of modifications just on a dictionary of attributes inside the UnrealizedCoreTextFont. This means we create no temporary fonts, and also means we are going fast enough that we can enable optical sizing on every font - because doing so is just an attribute in a dictionary, rather than creating a whole new CTFont. That is the fix for this bug: enabling optical sizing on all fonts. All the previous patches in this series of patches I've been writing were just to set up UnrealizedCoreTextFont so that we could make enabling optical sizing fast enough to do unconditionally, and to not create temporary font objects when doing so. There is one caveat to this: Without inspecting a CTFont, we can't know whether a font is an OpenType or a TrueType font, and we need to know that in order to set the appropriate values for the 'wght', 'wdth', 'slnt', and 'ital' variation axes. This patch works around that by exploiting the fact that the vast majority of fonts out there are A) not variable fonts, and B) are OpenType fonts, so its actually beneficial to just assume the font is an OpenType font and set it up accordingly, and then check after we've created it whether our guess was right. If our guess wasn't right, we can just fix up the font object we've just created, rather than creating a whole new one from scratch. I originally wrote 2 tests, and the he test font Newsreader was downloaded from https://fonts.google.com/specimen/Newsreader/about?preview.size=35&preview.layout=row&category=Serif&vfonly=true and is licensed with the Open Font License, which means we can use it in layout tests. We have to keep it and all its files unmodified, which is why there are so many extra font files - those just come with the one font file we actually want to test this. However, because we have to check in the entire release unmodified, that would add megabytes to the WebKit repo forever, we've decided to check in this patch without a test and then I'll asynchronously write a test that can use a smaller font. * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp: (WebCore::preparePlatformFont): (WebCore::variationCapabilitiesForFontDescriptor): (WebCore::appendOpenTypeFeature): Deleted. (WebCore::normalizeGXWeight): Deleted. (WebCore::normalizeCTWeight): Deleted. (WebCore::normalizeSlope): Deleted. (WebCore::denormalizeGXWeight): Deleted. (WebCore::denormalizeCTWeight): Deleted. (WebCore::denormalizeSlope): Deleted. (WebCore::denormalizeVariationWidth): Deleted. (WebCore::normalizeVariationWidth): Deleted. (WebCore::FontType::FontType): Deleted. (WebCore::addLightPalette): Deleted. (WebCore::addDarkPalette): Deleted. (WebCore::addAttributesForCustomFontPalettes): Deleted. (WebCore::addAttributesForFontPalettes): Deleted. * Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.h: * Source/WebCore/platform/graphics/cocoa/FontInterrogation.h: Added. (WebCore::FontInterrogation::FontInterrogation): * Source/WebCore/platform/graphics/cocoa/FontMetricsNormalization.h: Added. (WebCore::normalizeGXWeight): (WebCore::normalizeCTWeight): (WebCore::normalizeSlope): (WebCore::denormalizeGXWeight): (WebCore::denormalizeCTWeight): (WebCore::denormalizeSlope): (WebCore::denormalizeVariationWidth): (WebCore::normalizeVariationWidth): * Source/WebCore/platform/graphics/cocoa/SystemFontDatabaseCoreText.cpp: * Source/WebCore/platform/graphics/cocoa/UnrealizedCoreTextFont.cpp: (WebCore::appendOpenTypeFeature): (WebCore::addLightPalette): (WebCore::addDarkPalette): (WebCore::addAttributesForCustomFontPalettes): (WebCore::addAttributesForFontPalettes): (WebCore::applyFeatures): (WebCore::applyVariations): (WebCore::modifyFromContext): (WebCore::UnrealizedCoreTextFont::modifyFromContext): (WebCore::UnrealizedCoreTextFont::realize const): * Source/WebCore/platform/graphics/cocoa/UnrealizedCoreTextFont.h: Canonical link: https://commits.webkit.org/260447@main
webkit-early-warning-system
force-pushed
the
eng/Regression-font-optical-sizing-auto-has-no-effect-in-Safari-16-on-macOS-Ventura--iOS-16
branch
from
8002949
to
78f657c
Compare
|
Committed 260447@main (78f657c): https://commits.webkit.org/260447@main Reviewed commits have been landed. Closing PR #10126 and removing active labels. |
webkit-commit-queue
removed
the
merge-queue
This was referenced Apr 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
78f657c
8002949
🛠 wincairo🛠 ios-sim🧪 ios-wk2🧪 gtk-wk2🧪 api-ios🧪 mac-wk1🧪 api-gtk🧪 mac-AS-debug-wk2