diff --git a/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt b/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt index d1957709da..2e5886217f 100644 --- a/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt +++ b/app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt @@ -124,6 +124,7 @@ import com.itsaky.androidide.tasks.cancelIfActive import com.itsaky.androidide.tasks.mainThreadHandler import com.itsaky.androidide.ui.CodeEditorView import com.itsaky.androidide.ui.ContentTranslatingDrawerLayout +import com.itsaky.androidide.ui.MemoryChartAxis import com.itsaky.androidide.ui.SwipeRevealLayout import com.itsaky.androidide.uidesigner.UIDesignerActivity import com.itsaky.androidide.utils.ActionMenuUtils.showPopupWindow @@ -1021,14 +1022,7 @@ abstract class BaseEditorActivity : setScaleEnabled(true) axisLeft.isEnabled = false - axisRight.valueFormatter = - object : - IAxisValueFormatter { - override fun getFormattedValue( - value: Float, - axis: AxisBase?, - ): String = "%dMB".format(value.roundToLong()) - } + MemoryChartAxis.configure(axisRight) } } diff --git a/app/src/main/java/com/itsaky/androidide/ui/MemoryChartAxis.kt b/app/src/main/java/com/itsaky/androidide/ui/MemoryChartAxis.kt new file mode 100644 index 0000000000..0b6e07e708 --- /dev/null +++ b/app/src/main/java/com/itsaky/androidide/ui/MemoryChartAxis.kt @@ -0,0 +1,67 @@ +/* + * 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 com.github.mikephil.charting.components.AxisBase +import com.github.mikephil.charting.components.YAxis +import com.github.mikephil.charting.formatter.IAxisValueFormatter +import kotlin.math.roundToLong + +/** + * The memory chart's value axis: whole megabytes, never negative, never repeated (ADFA-5535). + * + * Extracted from the chart setup so the labels it produces can be tested. They could not be checked + * any other way: what MPAndroidChart prints depends on the range it picks for itself, which is only + * decided during a layout and a draw. + */ +object MemoryChartAxis { + /** + * Formats whole megabytes. + * + * The rounding is why [configure] has to bound the interval. Left to itself, MPAndroidChart + * picks a label interval from the data, and an all-zero chart -- every editor open, before the + * first sample lands -- gave it a range of about -1MB to 1MB with six labels. Rounded to whole + * megabytes those collapse onto three strings, each printed twice: "-1MB, -1MB, 0MB, 0MB, 1MB, + * 1MB". + */ + private val FORMATTER = + object : IAxisValueFormatter { + override fun getFormattedValue( + value: Float, + axis: AxisBase?, + ): String = "%dMB".format(value.roundToLong()) + } + + /** + * A megabyte, the smallest interval the label text can tell apart. + * + * Granularity is a floor on the interval, not the interval itself, so this costs nothing on a + * chart with real data -- a memory axis spanning hundreds of megabytes was never going to want + * gridlines a megabyte apart. It only bites in the degenerate case it exists for. + */ + private const val MINIMUM_INTERVAL_MEGABYTES = 1f + + fun configure(axis: YAxis) { + axis.valueFormatter = FORMATTER + // Memory is never negative, so the axis has no business going below zero -- which is also + // half of what made the duplicates readable as "-1MB" twice. + axis.axisMinimum = 0f + axis.granularity = MINIMUM_INTERVAL_MEGABYTES + axis.isGranularityEnabled = true + } +} diff --git a/app/src/test/java/com/itsaky/androidide/ui/MemoryChartAxisTest.kt b/app/src/test/java/com/itsaky/androidide/ui/MemoryChartAxisTest.kt new file mode 100644 index 0000000000..13c2c0161f --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/ui/MemoryChartAxisTest.kt @@ -0,0 +1,106 @@ +/* + * 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 android.graphics.Bitmap +import android.graphics.Canvas +import android.view.View +import androidx.test.core.app.ApplicationProvider +import com.github.mikephil.charting.data.Entry +import com.github.mikephil.charting.data.LineData +import com.github.mikephil.charting.data.LineDataSet +import com.google.common.truth.Truth.assertThat +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * The labels the memory chart's value axis actually prints (ADFA-5535). + * + * Asserted on what MPAndroidChart computes rather than on the formatter alone, because the bug was + * never in the formatter: it was in the interval the library chose to hand it. That interval is + * decided during a layout and a draw, so the chart has to be laid out and drawn here. + */ +@RunWith(RobolectricTestRunner::class) +class MemoryChartAxisTest { + private val context = ApplicationProvider.getApplicationContext() + + private fun chartWith(megabytes: List): SafeLineChart { + val chart = SafeLineChart(context) + MemoryChartAxis.configure(chart.axisRight) + chart.axisLeft.isEnabled = false + chart.data = + LineData(LineDataSet(megabytes.mapIndexed { i, mb -> Entry(i.toFloat(), mb) }, "IDE")) + chart.notifyDataSetChanged() + + chart.measure( + View.MeasureSpec.makeMeasureSpec(WIDTH, View.MeasureSpec.EXACTLY), + View.MeasureSpec.makeMeasureSpec(HEIGHT, View.MeasureSpec.EXACTLY), + ) + chart.layout(0, 0, WIDTH, HEIGHT) + chart.draw(Canvas(Bitmap.createBitmap(WIDTH, HEIGHT, Bitmap.Config.ARGB_8888))) + return chart + } + + private fun labelsOf(chart: SafeLineChart): List { + val axis = chart.axisRight + return (0 until axis.mEntryCount).map { i -> axis.getFormattedLabel(i) } + } + + @Test + fun `an all-zero chart prints no duplicate labels`() { + // Every editor open looks like this until the first sample lands, and so does the moment + // after the sampling rate changes, which clears the buffers. + val labels = labelsOf(chartWith(List(60) { 0f })) + + assertThat(labels).isNotEmpty() + assertThat(labels).containsNoDuplicates() + } + + @Test + fun `an all-zero chart prints no negative megabytes`() { + // The reported symptom was "-1MB, -1MB, 0MB, 0MB, 1MB, 1MB". Memory is never negative, so + // half of that was nonsense before it was even repeated. + val labels = labelsOf(chartWith(List(60) { 0f })) + + labels.forEach { label -> assertThat(label).doesNotContain("-") } + } + + @Test + fun `a chart with real readings still labels distinctly`() { + // Granularity is a floor on the interval, not the interval, so a chart spanning hundreds of + // megabytes is unaffected by it. If this ever fails, the floor has started to bite. + val labels = labelsOf(chartWith(listOf(120f, 480f, 733f, 906f, 512f))) + + assertThat(labels).containsNoDuplicates() + assertThat(labels.size).isAtLeast(3) + } + + @Test + fun `the labels are whole megabytes`() { + val labels = labelsOf(chartWith(listOf(120f, 480f, 733f))) + + labels.forEach { label -> assertThat(label).matches("\\d+MB") } + } + + private companion object { + const val WIDTH = 720 + const val HEIGHT = 400 + } +}