Skip to content

[Skia] Font::platformSupportsCodePoint should take the variation selector into account #47228

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 27, 2025
Merged

[Skia] Font::platformSupportsCodePoint should take the variation selector into account #47228

merged 1 commit into from
Jun 27, 2025

Conversation

carlosgcampos
Copy link
Contributor

@carlosgcampos carlosgcampos commented Jun 26, 2025 •
edited by webkit-early-warning-system
Loading

24a3125

[Skia] Font::platformSupportsCodePoint should take the variation selector into account
https://bugs.webkit.org/show_bug.cgi?id=295022

Reviewed by Michael Catanzaro.

Use SkiaHarfBuzzFont::glyph() instead.

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

6a3ad7f

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe   🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2   🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@carlosgcampos carlosgcampos self-assigned this Jun 26, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 26, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 31b3ec5)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ❌ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@carlosgcampos carlosgcampos added the Platform Portability improvements and other general platform improvements not driven directly by site bugs. label Jun 26, 2025
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 26, 2025
mcatanzaro
mcatanzaro approved these changes Jun 26, 2025
View reviewed changes
Copy link
Contributor

@mcatanzaro mcatanzaro left a comment

Choose a reason for hiding this comment

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

Need to update the test expectations for the three newly-failing tests. The new results for the two emoji tests are clearly better, so the expectations just need to be rebaselined.

I'm not sure what's going on with fast/text/unknown-font.html.

@carlosgcampos
Copy link
Contributor Author

Need to update the test expectations for the three newly-failing tests. The new results for the two emoji tests are clearly better, so the expectations just need to be rebaselined.

It's not so easy, they are ref tests. The problem is that now we render correctly the test but still incorrectly the ref, that's why it looks like a progression. So, we need to fix the rendering of the ref.

I'm not sure what's going on with fast/text/unknown-font.html.

This was wrong before too, but it just checked we were rendering anything (the ref mismatch is just empty). Now we fail to render, but for a different existing issue that I'll try to fix in a different bug.

@carlosgcampos carlosgcampos removed the merging-blocked Applied to prevent a change from being merged label Jun 27, 2025
@carlosgcampos carlosgcampos force-pushed the skia-font-supports-codepoint branch from 31b3ec5 to 6a3ad7f Compare June 27, 2025 09:27
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 27, 2025 •
edited
Loading

EWS run on current version of this PR (hash 6a3ad7f)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe   🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2   🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@carlosgcampos carlosgcampos added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jun 27, 2025
@carlosgcampos
[Skia] Font::platformSupportsCodePoint should take the variation sele…
24a3125 
…ctor into account

https://bugs.webkit.org/show_bug.cgi?id=295022

Reviewed by Michael Catanzaro.

Use SkiaHarfBuzzFont::glyph() instead.

Canonical link: https://commits.webkit.org/296713@main
@webkit-commit-queue webkit-commit-queue force-pushed the skia-font-supports-codepoint branch from 6a3ad7f to 24a3125 Compare June 27, 2025 10:53
@webkit-commit-queue
Copy link
Collaborator

Committed 296713@main (24a3125): https://commits.webkit.org/296713@main

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

@webkit-commit-queue webkit-commit-queue merged commit 24a3125 into WebKit:main Jun 27, 2025
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jun 27, 2025
@carlosgcampos carlosgcampos deleted the skia-font-supports-codepoint branch June 27, 2025 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcatanzaro mcatanzaro mcatanzaro approved these changes

@alexgcastro alexgcastro Awaiting requested review from alexgcastro alexgcastro is a code owner

@nikolaszimmermann nikolaszimmermann Awaiting requested review from nikolaszimmermann nikolaszimmermann is a code owner

Assignees

@carlosgcampos carlosgcampos

Labels
Platform Portability improvements and other general platform improvements not driven directly by site bugs.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@carlosgcampos @webkit-early-warning-system @webkit-commit-queue @mcatanzaro @webkit-ews-buildbot