Skip to content
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

[Cocoa] font-variation-settings:opsz is not applied when text-rendering:optimizeLegibility is set #12301

Conversation

litherum
Copy link
Contributor

@litherum litherum commented Apr 3, 2023

8bbfbb1

[Cocoa] font-variation-settings:opsz is not applied when text-rendering:optimizeLegibility is set
https://bugs.webkit.org/show_bug.cgi?id=254897
rdar://106814619

Reviewed by Simon Fraser.

text-rendering:optimizeLegibility turns on kCTFontOpticalSizeAttribute:”auto” in Core Text,
and font-variation-settings:opsz turns on kCTFontVariationAttribute:opsz in Core Text. When
both are applied, font-variation-settings is supposed to win. However, in Core Text,
kCTFontOpticalSizeAttribute wins.

The solution for this is to set kCTFontOpticalSizeAttribute to a specific size, given by
font-variation-settings:opsz. This should only happen when OpticalSizingTypes::Everything
is enabled, so this patch adds an optional<float> to that struct. All this is guarded by
a USE() so we can remove it if/when Core text changes to match the CSS behavior.

Test: fast/text/variations/optical-sizing-optimizeLegibility.html

* Source/WTF/wtf/PlatformUse.h:
* Source/WebCore/platform/graphics/FontTaggedSettings.h:
* Source/WebCore/platform/graphics/cocoa/UnrealizedCoreTextFont.cpp:
(WebCore::UnrealizedCoreTextFont::addAttributesForOpticalSizing):
(WebCore::UnrealizedCoreTextFont::modifyFromContext):
* Source/WebCore/platform/graphics/cocoa/UnrealizedCoreTextFont.h:

Canonical link: https://commits.webkit.org/262831@main

bcd51a2

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 🧪 gtk-wk2
🛠 🧪 jsc ✅ 🛠 tv ✅ 🧪 api-ios ✅ 🧪 api-gtk
✅ 🛠 🧪 jsc-arm64 🧪 mac-wk2 🧪 mac-AS-debug-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 jsc-armv7-tests
✅ 🛠 🧪 merge ✅ 🛠 watch ✅ 🛠 jsc-mips
✅ 🛠 watch-sim ✅ 🧪 jsc-mips-tests

@litherum litherum self-assigned this Apr 3, 2023
@litherum litherum added the Text For bugs in text layout and rendering, including international text support. label Apr 3, 2023
@litherum litherum force-pushed the eng/Cocoa-font-variation-settingsopsz-is-not-applied-when-text-renderingoptimizeLegibility-is-set branch from 9da23d5 to 4805c60 Compare April 3, 2023 02:15
@litherum
Copy link
Contributor Author

litherum commented Apr 3, 2023

This depends on #12300

@litherum litherum marked this pull request as ready for review April 3, 2023 02:16
@litherum
Copy link
Contributor Author

litherum commented Apr 3, 2023

Oh, this needs a test.

@litherum litherum marked this pull request as draft April 3, 2023 02:17
@litherum litherum force-pushed the eng/Cocoa-font-variation-settingsopsz-is-not-applied-when-text-renderingoptimizeLegibility-is-set branch from 4805c60 to 08e4eb3 Compare April 3, 2023 02:57
@litherum litherum marked this pull request as ready for review April 3, 2023 02:58
@litherum litherum force-pushed the eng/Cocoa-font-variation-settingsopsz-is-not-applied-when-text-renderingoptimizeLegibility-is-set branch from 08e4eb3 to 267419f Compare April 8, 2023 03:58
@litherum litherum force-pushed the eng/Cocoa-font-variation-settingsopsz-is-not-applied-when-text-renderingoptimizeLegibility-is-set branch from 267419f to 2f726a9 Compare April 8, 2023 04:05
@litherum litherum requested a review from mdubet April 8, 2023 04:09
@litherum litherum requested a review from rreno April 11, 2023 04:21
|| (PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MAX_ALLOWED < 180000) \
|| (PLATFORM(APPLETV) && __TV_OS_VERSION_MAX_ALLOWED < 180000) \
|| (PLATFORM(WATCHOS) && __WATCH_OS_VERSION_MAX_ALLOWED < 110000)
#define USE_CORE_TEXT_OPTICAL_SIZING_WORKAROUND 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Workaround for what? A radar link in a comment would be good.

@litherum litherum force-pushed the eng/Cocoa-font-variation-settingsopsz-is-not-applied-when-text-renderingoptimizeLegibility-is-set branch from 2f726a9 to bcd51a2 Compare April 11, 2023 17:01
@litherum litherum added the merge-queue Applied to send a pull request to merge-queue label Apr 11, 2023
…ng:optimizeLegibility is set

https://bugs.webkit.org/show_bug.cgi?id=254897
rdar://106814619

Reviewed by Simon Fraser.

text-rendering:optimizeLegibility turns on kCTFontOpticalSizeAttribute:”auto” in Core Text,
and font-variation-settings:opsz turns on kCTFontVariationAttribute:opsz in Core Text. When
both are applied, font-variation-settings is supposed to win. However, in Core Text,
kCTFontOpticalSizeAttribute wins.

The solution for this is to set kCTFontOpticalSizeAttribute to a specific size, given by
font-variation-settings:opsz. This should only happen when OpticalSizingTypes::Everything
is enabled, so this patch adds an optional<float> to that struct. All this is guarded by
a USE() so we can remove it if/when Core text changes to match the CSS behavior.

Test: fast/text/variations/optical-sizing-optimizeLegibility.html

* Source/WTF/wtf/PlatformUse.h:
* Source/WebCore/platform/graphics/FontTaggedSettings.h:
* Source/WebCore/platform/graphics/cocoa/UnrealizedCoreTextFont.cpp:
(WebCore::UnrealizedCoreTextFont::addAttributesForOpticalSizing):
(WebCore::UnrealizedCoreTextFont::modifyFromContext):
* Source/WebCore/platform/graphics/cocoa/UnrealizedCoreTextFont.h:

Canonical link: https://commits.webkit.org/262831@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Cocoa-font-variation-settingsopsz-is-not-applied-when-text-renderingoptimizeLegibility-is-set branch from bcd51a2 to 8bbfbb1 Compare April 11, 2023 18:20
@webkit-commit-queue
Copy link
Collaborator

Committed 262831@main (8bbfbb1): https://commits.webkit.org/262831@main

Reviewed commits have been landed. Closing PR #12301 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Apr 11, 2023
@webkit-commit-queue webkit-commit-queue merged commit 8bbfbb1 into WebKit:main Apr 11, 2023
@litherum litherum deleted the eng/Cocoa-font-variation-settingsopsz-is-not-applied-when-text-renderingoptimizeLegibility-is-set branch April 11, 2023 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Text For bugs in text layout and rendering, including international text support.
Projects
None yet
6 participants