From 5fd34544723144e3769e411a69e1d46150dbf25a Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Tue, 4 Aug 2026 20:14:34 +0200 Subject: [PATCH] [JEWEL-1389] Optimize CircularProgressIndicator animation Advance a primitive frame index at the configured frame cadence instead of invalidating the draw scope for every display frame. Preserve the existing rendering while skipping the fully transparent spinner segment, and cover the redraw behavior with Compose UI tests. closes https://github.com/JetBrains/intellij-community/pull/3599 (cherry picked from commit 0a8f4ea77addb7419712fdacdd0786061d275816) (cherry picked from commit 30ac4a39a75e3b613d1240051ed6e73526c2afdf) IJ-MR-220560 GitOrigin-RevId: b611c193585dc32efba4e8c6a16d16e8e1e99fc6 --- .../CircularProgressIndicatorUiTest.kt | 84 +++++++++++++++++++ .../ui/component/CircularProgressIndicator.kt | 79 +++++++++++------ 2 files changed, 136 insertions(+), 27 deletions(-) create mode 100644 platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/CircularProgressIndicatorUiTest.kt diff --git a/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/CircularProgressIndicatorUiTest.kt b/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/CircularProgressIndicatorUiTest.kt new file mode 100644 index 000000000000..6ac49d10bb29 --- /dev/null +++ b/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/CircularProgressIndicatorUiTest.kt @@ -0,0 +1,84 @@ +// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package org.jetbrains.jewel.ui.component + +import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.drawWithContent +import androidx.compose.ui.graphics.Color +import androidx.compose.ui.test.junit4.v2.createComposeRule +import kotlin.test.assertEquals +import kotlin.test.assertTrue +import kotlin.time.Duration +import kotlin.time.Duration.Companion.milliseconds +import org.jetbrains.jewel.intui.standalone.theme.IntUiTheme +import org.jetbrains.jewel.ui.component.styling.CircularProgressStyle +import org.junit.Rule +import org.junit.Test + +class CircularProgressIndicatorUiTest { + @get:Rule val rule = createComposeRule() + + private val frameTime = 125.milliseconds + + @Test + fun `should only redraw once per animation frame`() { + val durationMillis = 2_000L + val drawPasses = countDrawPasses(frameTime, durationMillis) + + // One draw per animation frame, plus at most one for rounding at each end. + val expected = (durationMillis / frameTime.inWholeMilliseconds).toInt() + assertTrue( + drawPasses <= expected + 1, + "Expected at most ${expected + 1} draw passes in ${durationMillis}ms, but got $drawPasses. " + + "The spinner is likely redrawing on every display frame instead of once per animation frame.", + ) + assertTrue(drawPasses > 0, "Expected the spinner to animate, but it never redrew") + } + + @Test + fun `should redraw more often with a shorter frame time`() { + val slowPasses = countDrawPasses(200.milliseconds, 2_000) + rule.mainClock.autoAdvance = true + + assertTrue(slowPasses in 1..12, "Expected around 10 draw passes for a 200ms frame time, but got $slowPasses") + } + + @Test + fun `should not animate when frame time is not positive`() { + val drawPasses = countDrawPasses(Duration.ZERO, 2_000) + + assertEquals(0, drawPasses, "A non-positive frame time should render a static spinner, not spin or busy-loop") + } + + private fun style(frameTime: Duration = this.frameTime) = + CircularProgressStyle(frameTime = frameTime, color = Color(0xFF6F737A)) + + /** + * Counts draw passes while the animation runs on the virtual clock. The spinner must only redraw when its snapped + * frame index changes, not on every display frame; see JEWEL-1389. + */ + private fun countDrawPasses(frameTime: Duration, durationMillis: Long): Int { + var drawPasses = 0 + rule.mainClock.autoAdvance = false + rule.setContent { + IntUiTheme { + CircularProgressIndicator( + modifier = + Modifier.drawWithContent { + drawPasses++ + drawContent() + }, + style = style(frameTime), + ) + } + } + rule.waitForIdle() + drawPasses = 0 // Discard the initial layout and draw pass + + repeat((durationMillis / MILLIS_PER_DISPLAY_FRAME).toInt()) { rule.mainClock.advanceTimeByFrame() } + return drawPasses + } + + private companion object { + private const val MILLIS_PER_DISPLAY_FRAME = 16 + } +} diff --git a/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/CircularProgressIndicator.kt b/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/CircularProgressIndicator.kt index f29e07428403..0fd3b248f6be 100644 --- a/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/CircularProgressIndicator.kt +++ b/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/CircularProgressIndicator.kt @@ -1,15 +1,12 @@ package org.jetbrains.jewel.ui.component -import androidx.compose.animation.core.LinearEasing -import androidx.compose.animation.core.RepeatMode -import androidx.compose.animation.core.animateFloat -import androidx.compose.animation.core.infiniteRepeatable -import androidx.compose.animation.core.rememberInfiniteTransition -import androidx.compose.animation.core.tween import androidx.compose.foundation.Canvas import androidx.compose.foundation.layout.size import androidx.compose.runtime.Composable -import androidx.compose.runtime.getValue +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.MutableIntState +import androidx.compose.runtime.mutableIntStateOf +import androidx.compose.runtime.remember import androidx.compose.ui.Modifier import androidx.compose.ui.geometry.CornerRadius import androidx.compose.ui.geometry.Offset @@ -19,9 +16,13 @@ import androidx.compose.ui.graphics.drawscope.rotate import androidx.compose.ui.graphics.takeOrElse import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp +import kotlin.time.Duration import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.delay +import kotlinx.coroutines.isActive import org.jetbrains.jewel.foundation.theme.JewelTheme +import org.jetbrains.jewel.foundation.util.JewelLogger import org.jetbrains.jewel.ui.component.styling.CircularProgressStyle import org.jetbrains.jewel.ui.theme.circularProgressStyle @@ -63,24 +64,13 @@ private fun CircularProgressIndicatorImpl( ) { val defaultColor = if (JewelTheme.isDark) Color(0xFF6F737A) else Color(0xFFA8ADBD) val color = style.color.takeOrElse { defaultColor } - val framesCount = spinnerSegmentOpacities.size - val frameTimeMillis = style.frameTime.inWholeMilliseconds.toInt() - - val transition = rememberInfiniteTransition("CircularProgressIndicator") - val rotationRatio by - transition.animateFloat( - initialValue = 0f, - targetValue = 1f, - animationSpec = - infiniteRepeatable( - animation = tween(easing = LinearEasing, durationMillis = frameTimeMillis * framesCount), - repeatMode = RepeatMode.Restart, - ), - ) + val frameIndex = rememberSpinnerFrameIndex(style.frameTime) Canvas(modifier = modifier.size(iconSize)) { - val frameIndex = (rotationRatio * framesCount).toInt() % framesCount - val snappedRotation = frameIndex * FULL_ROTATION_DEGREES / framesCount + // Reading the frame index inside the draw lambda, rather than in the composable scope, keeps invalidations + // scoped to the draw phase. Since the index only changes once per animation frame, we also avoid redrawing + // on every display frame, which is what an animated float value would have caused. + val snappedRotation = frameIndex.intValue * degreesPerSegment val diameter = size.minDimension val rectWidth = diameter * 2f / ICON_VIEW_BOX_SIZE @@ -90,14 +80,19 @@ private fun CircularProgressIndicatorImpl( val segmentSize = Size(rectWidth, rectHeight) rotate(degrees = snappedRotation, pivot = center) { - for (i in 0 until framesCount) { - rotate(degrees = -i * FULL_ROTATION_DEGREES / framesCount, pivot = center) { + for (i in spinnerSegmentOpacities.indices) { + val alpha = spinnerSegmentOpacities[i] + + // Fully transparent segments still cost time to draw, but are invisible: skip them + if (alpha == 0f) continue + + rotate(degrees = -i * degreesPerSegment, pivot = center) { drawRoundRect( color = color, topLeft = segmentTopLeft, size = segmentSize, cornerRadius = cornerRadius, - alpha = spinnerSegmentOpacities[i], + alpha = alpha, ) } } @@ -105,7 +100,37 @@ private fun CircularProgressIndicatorImpl( } } +/** + * Drives the spinner animation by ticking an integer frame index once every [frameTime]. + * + * Compared to running a float-valued animation and quantising it at draw time, this only invalidates when the value + * actually changes, instead of on every display frame. + */ +@Composable +private fun rememberSpinnerFrameIndex(frameTime: Duration): MutableIntState { + val frameIndex = remember { mutableIntStateOf(0) } + + LaunchedEffect(frameTime) { + // A non-positive frame time would turn the loop below into a busy loop; show a static spinner instead. + if (frameTime <= Duration.ZERO) { + JewelLogger.getInstance("CircularProgressIndicator") + .warn( + "Non-positive frameTime received. Indicator will be static until a positive duration is provided." + ) + return@LaunchedEffect + } + + while (isActive) { + delay(frameTime) + frameIndex.intValue = (frameIndex.intValue + 1) % spinnerSegmentOpacities.size + } + } + + return frameIndex +} + private const val FULL_ROTATION_DEGREES = 360f private const val ICON_VIEW_BOX_SIZE = 16f -private val spinnerSegmentOpacities = listOf(1f, 0.93f, 0.78f, 0.69f, 0.62f, 0.48f, 0.38f, 0f) +private val spinnerSegmentOpacities = floatArrayOf(1f, 0.93f, 0.78f, 0.69f, 0.62f, 0.48f, 0.38f, 0f) +private val degreesPerSegment = FULL_ROTATION_DEGREES / spinnerSegmentOpacities.size