From 1f9c4984fdfaa8e674a8618d471459f0e3313d51 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Sun, 6 Sep 2026 19:15:41 -0700 Subject: [PATCH 1/2] ADFA-5527: follow the font scale in the charts, up to a ceiling MPAndroidChart sizes its text in dp, so nothing the charts drew responded to the system font scale: a user who asked for larger text got it everywhere in the IDE except inside these plots, where the text is already the smallest on the screen. Measured before this change, an axis label went 19px to 20px between font scales 1.0 and 2.0 while an ordinary TextView beside it went 36px to 54px. Followed only to 1.5, because a plot is dense by nature and the strip is a fixed 248dp. Option 1 of the four written up on the ticket. Growing the text is not enough on its own, and the first version of this change proved it: the label count stayed put, so the memory page's nine value labels went from 29px apart to 6px. Bigger text, crowded axis -- worse, not better. The count now falls as the text grows, from six at scale 1 to four at the ceiling, as a hint rather than a command so that granularity still has the last word. That matters on the temperature axis, which is pinned to whole degrees. The annotation rows scale too. Eight rows sized for scale-1 text would have overlapped exactly when the labels grew, which is what the staggering exists to prevent. Verified on device, not by eye. At font scale 2.0 the memory page shows five value labels instead of nine, with the smallest gap between them at 28px against 31px at scale 1.0 -- the same readability with visibly larger text. On the temperature and power page, five labels per axis, whole degrees and whole watts, no repeats, smallest gaps 97px and 28px. Not in the metrics carousel stack, deliberately. It touches the shared renderer and the annotation geometry, and five reviewed PRs are waiting on approval; adding it there would invalidate all of them to fix something that predates them. Branched off ADFA-5509 because MetricsChartRenderer does not exist on stage yet. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QeW3M24yD6HNhEnbuNW7Hz --- .../androidide/ui/MetricsChartRenderer.kt | 74 ++++++++- .../ui/MetricsChartTextScaleTest.kt | 151 ++++++++++++++++++ 2 files changed, 222 insertions(+), 3 deletions(-) create mode 100644 app/src/test/java/com/itsaky/androidide/ui/MetricsChartTextScaleTest.kt diff --git a/app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt b/app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt index ca152daaad..d06591e9b5 100644 --- a/app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt +++ b/app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt @@ -40,6 +40,7 @@ import com.itsaky.androidide.utils.resolveAttr import com.itsaky.androidide.utils.showIdeCategoryTooltipIfPresent import kotlin.math.ceil import kotlin.math.floor +import kotlin.math.roundToInt import kotlin.math.roundToLong /** @@ -419,6 +420,7 @@ abstract class MetricsChartRenderer( xAxis.textColor = textColor data.setValueTextColor(textColor) + applyTextScale(this) styleValueAxes(this, textColor) setBackgroundColor(bgColor) setGridBackgroundColor(bgColor) @@ -435,6 +437,37 @@ abstract class MetricsChartRenderer( chart.invalidate() } + /** + * Sizes every piece of text the chart draws, following the system font scale up to a ceiling. + * + * MPAndroidChart sizes its text in dp, so nothing it draws responded to the font scale at all: + * a user who asked for larger text got it everywhere in the IDE except inside these plots, + * where the text is already the smallest on the screen (ADFA-5527). + * + * Followed only to [MAX_TEXT_SCALE], because a plot is dense by nature and the strip is a + * fixed [R.dimen.editor_mem_usage_view_height]. At the full 2.0 the axis labels collide with + * each other and the eight staggered annotation rows overlap, so honouring the scale + * literally would make the chart less readable rather than more. A ceiling gives most of the + * benefit and keeps the plot legible at the extreme. + */ + @UiThread + private fun applyTextScale(chart: SafeLineChart) { + val scale = textScaleFor(chart.context) + chart.legend.textSize = BASE_TEXT_SIZE_DP * scale + chart.xAxis.textSize = BASE_TEXT_SIZE_DP * scale + chart.axisLeft.textSize = BASE_TEXT_SIZE_DP * scale + chart.axisRight.textSize = BASE_TEXT_SIZE_DP * scale + chart.data?.setValueTextSize(BASE_VALUE_TEXT_SIZE_DP * scale) + + // Bigger text needs fewer labels. Growing the text alone left the count untouched, so the + // memory page's nine value labels went from 29px apart to 6px -- crowded enough that the + // change made the axis worse rather than better. The count is a hint: granularity still + // has the last word, which is what keeps the temperature axis on whole degrees. + val labels = (BASE_LABEL_COUNT / scale).roundToInt().coerceAtLeast(MIN_LABEL_COUNT) + chart.axisLeft.setLabelCount(labels, false) + chart.axisRight.setLabelCount(labels, false) + } + /** * Colours the value axes' labels. Called from [setData], not [configure], because the styling * here is re-applied on every redraw and would otherwise overwrite whatever a subclass had set @@ -501,7 +534,7 @@ abstract class MetricsChartRenderer( labelPosition = LimitLine.LimitLabelPosition.RIGHT_BOTTOM // Rows are counted up from the bottom of the plot, and the offset is in dp: // LimitLine converts it on the way in. - yOffset = ANNOTATION_LABEL_ROW_HEIGHT_DP * slotFor(annotation.sequence) + yOffset = annotationRowHeightFor(chart.context) * slotFor(annotation.sequence) }, ) } @@ -590,7 +623,8 @@ abstract class MetricsChartRenderer( chart.invalidate() } - private companion object { + @VisibleForTesting + internal companion object { /** * Samples shown at once. Thousands are retained; a minute is what fits legibly in the strip. */ @@ -609,7 +643,41 @@ abstract class MetricsChartRenderer( */ const val ANNOTATION_LABEL_SLOTS = 8 - /** One row, in dp. The label text is 10dp, so this leaves a little air between rows. */ + /** + * One row, in dp, at a font scale of 1. The label text is [BASE_TEXT_SIZE_DP], so this + * leaves a little air between rows; it is scaled with the text by [annotationRowHeightFor], + * or the rows would overlap exactly when the labels grew (ADFA-5527). + */ const val ANNOTATION_LABEL_ROW_HEIGHT_DP = 12f + + /** MPAndroidChart's own default for axis and legend text, which this matches at scale 1. */ + const val BASE_TEXT_SIZE_DP = 10f + + /** MPAndroidChart's own default for value labels. */ + const val BASE_VALUE_TEXT_SIZE_DP = 9f + + /** + * The most the chart will grow its text by, whatever the system font scale. + * + * 1.5 rather than the platform's maximum of 2.0: see [applyTextScale]. Eight annotation + * rows at 1.5 still fit the plot, where at 2.0 they do not. + */ + const val MAX_TEXT_SCALE = 1.5f + + /** Value-axis labels at a font scale of 1, which is MPAndroidChart's own default. */ + const val BASE_LABEL_COUNT = 6 + + /** Never fewer than this, or the axis stops conveying a scale at all. */ + const val MIN_LABEL_COUNT = 3 + + /** The font scale the charts follow: the system's, held to [MAX_TEXT_SCALE]. */ + @JvmStatic + fun textScaleFor(context: Context): Float = + context.resources.configuration.fontScale + .coerceIn(1f, MAX_TEXT_SCALE) + + /** One annotation row, scaled with the label text it has to leave room for. */ + @JvmStatic + fun annotationRowHeightFor(context: Context): Float = ANNOTATION_LABEL_ROW_HEIGHT_DP * textScaleFor(context) } } diff --git a/app/src/test/java/com/itsaky/androidide/ui/MetricsChartTextScaleTest.kt b/app/src/test/java/com/itsaky/androidide/ui/MetricsChartTextScaleTest.kt new file mode 100644 index 0000000000..6d3680cbdc --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/ui/MetricsChartTextScaleTest.kt @@ -0,0 +1,151 @@ +/* + * This file is part of AndroidIDE. + * + * AndroidIDE is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * AndroidIDE is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with AndroidIDE. If not, see . + */ + +package com.itsaky.androidide.ui + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.utils.MetricsAnnotationStore +import com.itsaky.androidide.utils.NetworkUsageWatcher +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * The chart text-scale policy of ADFA-5527: follow the system font scale, up to a ceiling. + * + * MPAndroidChart sizes its text in dp, so before this the charts ignored the font scale entirely + * -- a user who asked for larger text got it everywhere in the IDE except inside these plots. The + * scale is followed only to [MetricsChartRenderer.MAX_TEXT_SCALE], because the strip is a fixed + * height and at the platform's full 2.0 the axis labels collide and the eight staggered annotation + * rows overlap. + */ +@RunWith(RobolectricTestRunner::class) +class MetricsChartTextScaleTest { + private val context: Context get() = ApplicationProvider.getApplicationContext() + + private fun chart(): SafeLineChart { + val chart = SafeLineChart(context) + NetworkUsageChartRenderer( + usageProvider = { + NetworkUsageWatcher.NetworkUsage(LongArray(SAMPLES) { 1_000L }, LongArray(SAMPLES) { 500L }) + }, + ).attach(chart) + return chart + } + + private val base get() = MetricsChartRenderer.BASE_TEXT_SIZE_DP + + @Test + fun `at the default scale the text is the size it always was`() { + val chart = chart() + + // Matching MPAndroidChart's own default, so nothing moves for a user who has not changed + // the setting. + assertThat(chart.xAxis.textSize).isWithin(TOLERANCE).of(base) + assertThat(chart.legend.textSize).isWithin(TOLERANCE).of(base) + assertThat(chart.axisRight.textSize).isWithin(TOLERANCE).of(base) + } + + @Test + @Config(fontScale = 1.3f) + fun `a modest font scale is followed exactly`() { + val chart = chart() + + assertThat(chart.xAxis.textSize).isWithin(TOLERANCE).of(base * 1.3f) + assertThat(chart.legend.textSize).isWithin(TOLERANCE).of(base * 1.3f) + } + + @Test + @Config(fontScale = 2.0f) + fun `the largest font scale is held to the ceiling`() { + val chart = chart() + + // Not base * 2: eight annotation rows at that size do not fit the plot, and the axis + // labels collide with each other. + assertThat(chart.xAxis.textSize) + .isWithin(TOLERANCE) + .of(base * MetricsChartRenderer.MAX_TEXT_SCALE) + } + + @Test + @Config(fontScale = 0.85f) + fun `a font scale below one does not shrink the chart further`() { + val chart = chart() + + // The chart's text is already the smallest on the screen; following a reduction would + // make the labels unreadable rather than merely small. + assertThat(chart.xAxis.textSize).isWithin(TOLERANCE).of(base) + } + + @Test + @Config(fontScale = 2.0f) + fun `the annotation rows a chart actually draws grow with the labels`() { + // Asserted on the drawn marker, not on the helper: an earlier version of this test called + // annotationRowHeightFor directly, so it passed even with the renderer still using the + // unscaled constant at the call site. + var now = 1_000_000L + val store = MetricsAnnotationStore(nowMillis = { now }) + store.record("first") + now += MetricsAnnotationStore.THROTTLE_INTERVAL_MS + store.record("second") + + val chart = SafeLineChart(context) + NetworkUsageChartRenderer( + usageProvider = { + NetworkUsageWatcher.NetworkUsage(LongArray(SAMPLES) { 1_000L }, LongArray(SAMPLES) { 500L }) + }, + annotations = store, + sampleInterval = { 1_000L }, + ).attach(chart) + + // Rows sized for scale-1 text would overlap exactly when the text grew, which is what the + // staggering exists to prevent. Two consecutive markers sit one row apart. + val offsets = + chart.xAxis.limitLines + .map { it.yOffset } + .sorted() + assertThat(offsets).hasSize(2) + val expected = + MetricsChartRenderer.ANNOTATION_LABEL_ROW_HEIGHT_DP * MetricsChartRenderer.MAX_TEXT_SCALE + assertThat(offsets[1] - offsets[0]).isWithin(TOLERANCE).of(expected) + } + + @Test + @Config(fontScale = 2.0f) + fun `eight annotation rows still fit the plot at the ceiling`() { + // The reason the ceiling is 1.5. The strip is a fixed height, and this is the constraint + // that sets the limit -- if it ever fails, the ceiling is too high or the strip too short. + val rows = MetricsChartRenderer.ANNOTATION_LABEL_SLOTS + val used = rows * MetricsChartRenderer.annotationRowHeightFor(context) + + assertThat(used).isLessThan(PLOT_HEIGHT_DP) + } + + private companion object { + const val SAMPLES = 60 + const val TOLERANCE = 0.01f + + /** + * The plot's share of editor_mem_usage_view_height (248dp), less the title row, the + * legend and the x axis. Deliberately conservative. + */ + const val PLOT_HEIGHT_DP = 150f + } +} From 38e68786d3922909493cbc1fb95c31628aa0a8a2 Mon Sep 17 00:00:00 2001 From: David Schachter Date: Mon, 7 Sep 2026 05:40:00 -0700 Subject: [PATCH 2/2] ADFA-5527: measure the plot the eight rows have to fit in The test that justifies the 1.5 ceiling compared eight rows against a hand-picked 150dp, with a comment admitting the number was a guess. A guess pins nothing: no layout change could move it, so the test could not tell a shorter strip or a taller title row from a safe one. It now measures the whole way down -- the strip at the dimen the layout uses, the pager after a real measure and layout at 360dp wide, and the plot as the content rect a real chart page reports once a renderer has put its legend and axis on it. Nothing is allowed for by hand. Raising MAX_TEXT_SCALE to 2.0 now fails it, 192dp of rows against a 169dp plot, which is the claim the test makes about why the ceiling is where it is. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017CCQUU7tBzZL61EmQhJP8j --- .../ui/MetricsChartTextScaleTest.kt | 55 +++++++++++++++++-- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/app/src/test/java/com/itsaky/androidide/ui/MetricsChartTextScaleTest.kt b/app/src/test/java/com/itsaky/androidide/ui/MetricsChartTextScaleTest.kt index 6d3680cbdc..29ace075c5 100644 --- a/app/src/test/java/com/itsaky/androidide/ui/MetricsChartTextScaleTest.kt +++ b/app/src/test/java/com/itsaky/androidide/ui/MetricsChartTextScaleTest.kt @@ -18,8 +18,13 @@ package com.itsaky.androidide.ui import android.content.Context +import android.view.LayoutInflater +import android.view.View +import androidx.appcompat.view.ContextThemeWrapper import androidx.test.core.app.ApplicationProvider import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.R +import com.itsaky.androidide.databinding.LayoutMemUsageBinding import com.itsaky.androidide.utils.MetricsAnnotationStore import com.itsaky.androidide.utils.NetworkUsageWatcher import org.junit.Test @@ -132,20 +137,58 @@ class MetricsChartTextScaleTest { fun `eight annotation rows still fit the plot at the ceiling`() { // The reason the ceiling is 1.5. The strip is a fixed height, and this is the constraint // that sets the limit -- if it ever fails, the ceiling is too high or the strip too short. + // + // Measured, not guessed. This used to compare against a hand-picked 150dp with a comment + // admitting it was conservative, which pinned the ceiling against a number no layout change + // could ever move. The strip is laid out at the ceiling font scale and the pager reports + // what the title row -- itself grown by that scale -- left it. val rows = MetricsChartRenderer.ANNOTATION_LABEL_SLOTS val used = rows * MetricsChartRenderer.annotationRowHeightFor(context) - assertThat(used).isLessThan(PLOT_HEIGHT_DP) + assertThat(used).isLessThan(plotHeightDp()) + } + + /** + * The plot area a chart page actually gets, in dp, with the system font scale at its largest. + * + * Measured the whole way down, with nothing allowed for by hand: the strip's height is the + * dimen the layout uses, the pager's share of it comes from a real measure and layout of the + * real strip, and the plot's share of *that* is the content rect a real chart page reports + * after a real renderer has put its legend and axis on it. So shortening the strip fails this, + * and so does anything above or inside the plot growing with the font scale. + */ + private fun plotHeightDp(): Float { + val themed = ContextThemeWrapper(context, R.style.Theme_AndroidIDE) + val strip = LayoutMemUsageBinding.inflate(LayoutInflater.from(themed)) + val metrics = context.resources.displayMetrics + val stripHeightPx = context.resources.getDimensionPixelSize(R.dimen.editor_mem_usage_view_height) + val widthPx = (STRIP_WIDTH_DP * metrics.density).toInt() + + strip.root.measure( + View.MeasureSpec.makeMeasureSpec(widthPx, View.MeasureSpec.EXACTLY), + View.MeasureSpec.makeMeasureSpec(stripHeightPx, View.MeasureSpec.EXACTLY), + ) + strip.root.layout(0, 0, widthPx, stripHeightPx) + + val page = + LayoutInflater + .from(themed) + .inflate(R.layout.item_metrics_chart, strip.metricsPager, false) as SafeLineChart + NetworkUsageChartRenderer( + usageProvider = { + NetworkUsageWatcher.NetworkUsage(LongArray(SAMPLES) { 1_000L }, LongArray(SAMPLES) { 500L }) + }, + ).attach(page) + page.layOutAndDraw(width = strip.metricsPager.width, height = strip.metricsPager.height) + + return page.viewPortHandler.contentHeight() / metrics.density } private companion object { const val SAMPLES = 60 const val TOLERANCE = 0.01f - /** - * The plot's share of editor_mem_usage_view_height (248dp), less the title row, the - * legend and the x axis. Deliberately conservative. - */ - const val PLOT_HEIGHT_DP = 150f + /** A narrow phone, so the title row wraps here if it is ever going to. */ + const val STRIP_WIDTH_DP = 360f } }