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

add font rasterization settings in paragraph style #1102

Merged
merged 11 commits into from
Feb 21, 2024

Conversation

SergeevPavel
Copy link

Proposed Changes

Allow to configure font rasterization settings

Testing

Check FontRasterization tab in demo application

Issues Fixed

Fixes: [Optional] The bug on https://issuetracker.google.com being fixed

Google CLA

You need to sign the Google Contributor’s License Agreement at https://cla.developers.google.com/.
This is needed since we synchronise most of the code with Google’s AOSP repository. Signing this agreement allows us to synchronise code from your Pull Requests as well.

@SergeevPavel
Copy link
Author

@MatkovIvan please take a look

@MatkovIvan MatkovIvan self-requested a review February 14, 2024 20:53
@SergeevPavel
Copy link
Author

@manu-unter I took this names from Skia, but indeed FontSmoothing sounds better, so let's rename it!

@SergeevPavel SergeevPavel force-pushed the font_rastr_settings branch 2 times, most recently from 15b3055 to 3c84e3f Compare February 15, 2024 14:50
Comment on lines 500 to 504
val edging = when (rasterizationSettings.smoothing) {
FontSmoothing.None-> org.jetbrains.skia.FontEdging.ALIAS
FontSmoothing.AntiAlias -> org.jetbrains.skia.FontEdging.ANTI_ALIAS
FontSmoothing.SubpixelAntiAlias -> org.jetbrains.skia.FontEdging.SUBPIXEL_ANTI_ALIAS
}
Copy link
Member

Choose a reason for hiding this comment

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

  1. Apply auto formatting
  2. it's a good candidate for extraction to internal extension function for this enum (I'd put it in FontRasterizationSettings.skiko.kt) file

@@ -320,8 +356,10 @@ public final class androidx/compose/ui/text/PlaceholderVerticalAlign$Companion {
public final class androidx/compose/ui/text/PlatformParagraphStyle {
public static final field $stable I
public static final field Companion Landroidx/compose/ui/text/PlatformParagraphStyle$Companion;
public fun <init> ()V
Copy link
Member

Choose a reason for hiding this comment

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

You removed constructor without arguments. It breaks binary compatibility

Copy link
Collaborator

@igordmn igordmn left a comment

Choose a reason for hiding this comment

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

Looks good to merge, let me know and I'll merge it

@SergeevPavel
Copy link
Author

@igordmn could you grant me write access to the repo?

@igordmn
Copy link
Collaborator

igordmn commented Feb 21, 2024

Done

@SergeevPavel SergeevPavel merged commit 132e695 into JetBrains:jb-main Feb 21, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants