Add support for frame rate voting #2205
Merged
Conversation
This file contains hidden or 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
ASalavei
reviewed
Jun 30, 2025
compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/RootNodeOwner.skiko.kt
Outdated
Show resolved
Hide resolved
MatkovIvan
reviewed
Jun 30, 2025
compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/OwnedLayerManager.kt
Outdated
Show resolved
Hide resolved
compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/RootNodeOwner.skiko.kt
Outdated
Show resolved
Hide resolved
...ose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/GraphicsLayerOwnerLayer.skiko.kt
Outdated
Show resolved
Hide resolved
ASalavei
requested changes
Jun 30, 2025
compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/PlatformContext.skiko.kt
Outdated
Show resolved
Hide resolved
MatkovIvan
reviewed
Jul 2, 2025
compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/RootNodeOwner.skiko.kt
Outdated
Show resolved
Hide resolved
compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/RootNodeOwner.skiko.kt
Outdated
Show resolved
Hide resolved
compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/PlatformContext.skiko.kt
Show resolved
Hide resolved
compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/OwnedLayerManager.kt
Outdated
Show resolved
Hide resolved
compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/node/RootNodeOwner.skiko.kt
Show resolved
Hide resolved
MatkovIvan
reviewed
Jul 2, 2025
MatkovIvan
reviewed
Jul 2, 2025
| if (frameRate.isNaN() || frameRate < currentFrameRateCategory) { | ||
| currentFrameRateCategory = frameRate | ||
| } | ||
| } else if (frameRate < currentFrameRateCategory || (frameRate.isNaN() && isCurrentFrameRateCategoryUnset)) { |
There was a problem hiding this comment.
isNaN check should be done before comparison
There was a problem hiding this comment.
I thought that the comparison would return false is frameRate.isNaN() == true or is that a convention?
There was a problem hiding this comment.
NaNis considered greater than any other element includingPOSITIVE_INFINITY
Found that statement, but it needs to be checked. Anyway, checking NaN before is safer anyway
|
LGTM, waiting for tests |
svastven
force-pushed
the
svastven/frame-rate-voting
branch
2 times, most recently
from
cffa3e7 to
b9e59d9
Compare
MatkovIvan
reviewed
Jul 7, 2025
| ) | ||
| } | ||
|
|
||
| protected open val platformContext: PlatformContext get() = TestContext() |
There was a problem hiding this comment.
Note: It's public APIs for users. I'm not sure that we really want to expose it (even as experimental)
cc @m-sasha as owner of this API
MatkovIvan
reviewed
Jul 8, 2025
compose/ui/ui/src/uikitInstrumentedTest/kotlin/androidx/compose/ui/modifiers/FrameRateTest.kt
Outdated
Show resolved
Hide resolved
MatkovIvan
approved these changes
Jul 9, 2025
| @Test | ||
| fun testNoVotedFrameRate() = runTest { | ||
| val surface = Surface.makeRasterN32Premul(100, 100) | ||
| val owner = RootNodeOwner( |
There was a problem hiding this comment.
As alternative: Use scene to test integration with public modifiers API. Since finding nodes/"clicking" is problematic, changes can be done via direct setting state values and re-drawing scene.
Not blocking because of it.
ASalavei
approved these changes
Jul 9, 2025
...e/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeHostingViewController.uikit.kt
Outdated
Show resolved
Hide resolved
...e/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeHostingViewController.uikit.kt
Outdated
Show resolved
Hide resolved
svastven
force-pushed
the
svastven/frame-rate-voting
branch
from
0c1a786 to
e1bdda7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
In this PR:
Fixes - https://youtrack.jetbrains.com/issue/CMP-7802
Testing
This should be tested by QA
Release Notes
Features - iOS