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

turn off flutter hack by default, implement methods to control it #997

Merged
merged 9 commits into from
Oct 31, 2024

Conversation

SergeevPavel
Copy link
Collaborator

No description provided.

@eymar
Copy link
Member

eymar commented Oct 29, 2024

Looks like layoutParagraph test failed on web and linux.
What's the reason to disable that hack by default?

@SergeevPavel
Copy link
Collaborator Author

@eymar the hack is basically truncation of some float values in paragraph e.g. maxIntrinsicWidth, as far as I understood it was required to make some Flutter tests pass. The reason why I decided to turn it off is demonstrated in this test

fun `layout paragraph with its maxIntrinsicWidth shouldn't lead to wraps`() = runTest {

The issue is observable with Compose TextMeasurer, here we layout paragraph with maxIntrinicWidth which may lead to unexpected line wraps with some combination of font size and line length

@SergeevPavel
Copy link
Collaborator Author

Also could you take a look what's wrong with WebTarget tests , is the fault result of my changes?


@Test
fun paragraphStyleRoundingHackTests() {
ParagraphStyle().use { paragraphStyle ->
Copy link
Member

@eymar eymar Oct 30, 2024

Choose a reason for hiding this comment

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

Changing this one to this would make k/js happy :D

    @Test
    fun paragraphStyleRoundingHackTests() {
        ParagraphStyle().use { paragraphStyle ->
            assertFalse(paragraphStyle.isApplyRoundingHackEnabled)
            paragraphStyle.isApplyRoundingHackEnabled = true
            assertTrue(paragraphStyle.isApplyRoundingHackEnabled)
        }
    }

_nGetApplyRoundingHack returns 0 (there is no false/true in wasm). And JS equality check fails:
0 === false --> false


Not using "add a suggestion" because new imports are needed anyway.

Copy link
Member

Choose a reason for hiding this comment

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

jfyi: to run the k/js tests locally:

./gradlew :cleanJsBrowserTest :jsBrowserTest -Pskiko.wasm.enabled=true -Pskiko.js.enabled=true

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks!

Co-authored-by: Oleksandr Karpovich <[email protected]>
@SergeevPavel SergeevPavel merged commit f722906 into master Oct 31, 2024
6 checks passed
@SergeevPavel SergeevPavel deleted the disable-flutter-hack branch October 31, 2024 09:51
mazunin-v-jb added a commit to JetBrains/compose-multiplatform-core that referenced this pull request Nov 6, 2024
mazunin-v-jb added a commit to JetBrains/compose-multiplatform-core that referenced this pull request Nov 7, 2024
mazunin-v-jb added a commit to JetBrains/compose-multiplatform-core that referenced this pull request Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants