ADFA-5515: keep the chart on the newest samples across a carousel rebind

app/src/main/java/com/itsaky/androidide/ui/MetricsChartRenderer.kt

@@ -22,6 +22,7 @@ import android.graphics.Bitmap
 import android.os.SystemClock
 import android.util.TypedValue
 import android.view.MotionEvent
+import android.view.View
 import androidx.annotation.CallSuper
 import androidx.annotation.UiThread
 import androidx.annotation.VisibleForTesting
@@ -186,8 +187,21 @@ abstract class MetricsChartRenderer(
 	 */
 	@UiThread
 	fun attach(chart: SafeLineChart) {
+		// A rebind can attach the replacement before the view it replaced is recycled, so the
+		// outgoing chart is let go of here rather than waiting for a [detachIfAttached] that, by
+		// then, no longer names it.
+		//
+		// The whole teardown, not just the listener. [detach] also clears [userHasZoomed], and
+		// releasing only the listener leaked it onto the replacement: a user who had panned once
+		// got a chart whose follow-window was disabled for good, because showNewestWindow returns
+		// early on the flag and every later redraw takes the same early return. That is the
+		// oldest-samples symptom this ticket was filed for -- reachable only after a pan, which is
+		// why the resume paths reproduce it and a fresh chart never does. Subclasses clear their own
+		// per-chart state through the same override.
+		this.chart?.let { outgoing -> if (outgoing !== chart) detach() }
 		this.chart = chart
 		configure(chart)
+		chart.addOnLayoutChangeListener(newestWindowOnLayout)
 		rebuild()
 	}
 
@@ -198,6 +212,7 @@ abstract class MetricsChartRenderer(
 	@CallSuper
 	open fun detach() {
 		userHasZoomed = false
+		chart?.removeOnLayoutChangeListener(newestWindowOnLayout)
 		chart = null
 	}
 
@@ -308,10 +323,36 @@ abstract class MetricsChartRenderer(
 			return
 		}
 
+		// Before the first layout there is no plot area to place a window in, and applying one
+		// anyway is worse than waiting: the scale is clamped against an empty content rect, and the
+		// layout that follows resets the chart's transform. [newestWindowOnLayout] re-applies it as
+		// soon as there is something to apply it to (ADFA-5515).
+		if (!chart.viewPortHandler.hasChartDimens()) {
+			return
+		}
+
 		chart.setVisibleXRangeMaximum(VISIBLE_SAMPLES.toFloat())
-		chart.moveViewToX(newestIndex - VISIBLE_SAMPLES.toFloat() + 1f)
+		// Not moveViewToX: its scroll is deferred to a later frame and would be converted through
+		// a different transform from the scale just set here (ADFA-5515).
+		chart.moveViewToXNow(newestIndex - VISIBLE_SAMPLES.toFloat() + 1f)
 	}
 
+	/**
+	 * Re-applies the newest window whenever the chart is laid out.
+	 *
+	 * A layout that changes the chart's size resets its transform, which drops the window and shows
+	 * the whole buffer from its oldest end. Nothing put it back until the next sample landed a
+	 * redraw, so every rebind -- and the carousel is rebound on every resume -- opened on an empty
+	 * plot for a second or more (ADFA-5515).
+	 */
+	private val newestWindowOnLayout =
+		View.OnLayoutChangeListener { view, _, _, _, _, _, _, _, _ ->
+			val chart = view as? SafeLineChart ?: return@OnLayoutChangeListener
+			if (chart === this.chart) {
+				showNewestWindow(chart)
+			}
+		}
+
 	/**
 	 * The sample indices currently on screen, for a series of [sampleCount] samples.
 	 *

app/src/main/java/com/itsaky/androidide/ui/SafeLineChart.kt

@@ -130,6 +130,27 @@ class SafeLineChart : LineChart {
 		}
 	}
 
+	/**
+	 * Scrolls the plot so that [xValue] is its leftmost value, now rather than on a later frame.
+	 *
+	 * What [moveViewToX] does, minus the deferral. That one queues the scroll as a viewport job
+	 * which MPAndroidChart hands to `View.post`, and by the time it runs the transform it converts
+	 * its x value through is no longer the one the caller set it up against -- a layout in between
+	 * resets the transform to identity, and the clamp afterwards restores the scale around a
+	 * translation computed for a different one. The viewport then lands neither where it was nor
+	 * where it was asked to go (ADFA-5515).
+	 *
+	 * On a chart that is not attached to a window the job is worse than late: `View.post` drops it
+	 * in the view's run queue, which is only flushed on attach, so it never runs at all.
+	 */
+	fun moveViewToXNow(xValue: Float) {
+		// The left axis, as moveViewToX itself uses; only the x component is read back.
+		val transformer = getTransformer(YAxis.AxisDependency.LEFT) ?: return
+		val target = floatArrayOf(xValue, 0f)
+		transformer.pointValuesToPixel(target)
+		viewPortHandler.centerViewPort(target, this)
+	}
+
 	override fun onDraw(canvas: Canvas) {
 		try {
 			super.onDraw(canvas)

app/src/test/java/com/itsaky/androidide/ui/ChartLayout.kt

@@ -29,12 +29,9 @@ const val CHART_HEIGHT = 400
 /**
  * Lays this chart out and draws it once, which is what every assertion about its viewport needs.
  *
- * Two separate reasons, both easy to leave out and neither of which fails loudly. Without the
- * layout the plot area has no extent, so every coordinate lands on its edge and a hit test cannot
- * tell inside from outside. Without the draw the scroll to the newest samples has not run --
- * MPAndroidChart queues `moveViewToX` as a job that only executes during a draw pass -- so the
- * chart still reports the *oldest* samples as visible and a window assertion reads the wrong end
- * of the buffer.
+ * Without the layout the plot area has no extent, so every coordinate lands on its edge, a hit test
+ * cannot tell inside from outside, and the renderer has nothing to place its viewport in. The draw
+ * is what renders the axes and annotations that assertions about them read back.
  *
  * Four test classes had grown their own copy of this, with the comment explaining it in three of
  * them and the draw missing from one.

app/src/test/java/com/itsaky/androidide/ui/MetricsAnnotationSpanTest.kt

@@ -84,7 +84,7 @@ class MetricsAnnotationSpanTest {
 
 		// Pan back to where that marker lives, and record that the user drove the viewport.
 		chart.setVisibleXRangeMaximum(VISIBLE_WINDOW.toFloat())
-		chart.moveViewToX(0f)
+		chart.moveViewToXNow(0f)
 		draw(chart)
 		val event = MotionEvent.obtain(0L, 0L, MotionEvent.ACTION_MOVE, 10f, 10f, 0)
 		chart.onChartGestureListener.onChartTranslate(event, -50f, 0f)

app/src/test/java/com/itsaky/androidide/ui/MetricsChartAxisTapTest.kt

@@ -84,7 +84,7 @@ class MetricsChartAxisTapTest {
 
 		// Zoom first: an unzoomed chart shows everything, so there is nothing a pan could move.
 		chart.setVisibleXRangeMaximum(VISIBLE_WINDOW.toFloat())
-		chart.moveViewToX(0f)
+		chart.moveViewToXNow(0f)
 		drawOnce(chart)
 		assertThat(chart.lowestVisibleX).isLessThan(10f)
 		assertThat(chart.highestVisibleX).isLessThan(SAMPLES / 2f)

app/src/test/java/com/itsaky/androidide/ui/MetricsChartNewestWindowTest.kt

@@ -0,0 +1,143 @@
+/*
+ *  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 <https://www.gnu.org/licenses/>.
+ */
+
+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.NetworkUsageWatcher
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+
+/**
+ * Where the chart's viewport ends up when a page is bound before it has been laid out.
+ *
+ * That is the order the carousel always binds in -- `onBindViewHolder` attaches the renderer while
+ * RecyclerView is still laying the page out -- and the editor rebinds the whole carousel on every
+ * resume. Returning from the APK install prompt therefore left the chart parked in the zeroed head
+ * of the buffer, reading -9999s and drawing nothing, until the next sample landed a redraw a second
+ * or more later: an empty plot at the moment the user has just run a build and is looking straight
+ * at it (ADFA-5515).
+ */
+@RunWith(RobolectricTestRunner::class)
+class MetricsChartNewestWindowTest {
+	private val context = ApplicationProvider.getApplicationContext<Context>()
+
+	private fun renderer() =
+		NetworkUsageChartRenderer(
+			usageProvider = {
+				NetworkUsageWatcher.NetworkUsage(LongArray(SAMPLES) { 1_000L }, LongArray(SAMPLES) { 500L })
+			},
+		)
+
+	/**
+	 * The window the chart is meant to settle on: the newest [MetricsChartRenderer.VISIBLE_SAMPLES]
+	 * of them, ending on the newest sample.
+	 *
+	 * The window is asked to start one sample further right than this and is clamped back, since it
+	 * cannot extend past the end of the data -- so the newest sample sits exactly on the right edge.
+	 */
+	private fun assertShowsNewestSamples(chart: SafeLineChart) {
+		val newest = (SAMPLES - 1).toFloat()
+		assertThat(chart.highestVisibleX).isWithin(TOLERANCE).of(newest)
+		assertThat(chart.lowestVisibleX)
+			.isWithin(TOLERANCE)
+			.of(newest - MetricsChartRenderer.VISIBLE_SAMPLES)
+	}
+
+	@Test
+	fun `a page bound before it is laid out still opens on the newest samples`() {
+		val chart = SafeLineChart(context)
+
+		// The carousel's order: attach first, lay out second.
+		renderer().attach(chart)
+		chart.layOutAndDraw()
+
+		assertShowsNewestSamples(chart)
+	}
+
+	@Test
+	fun `a page bound after it is laid out opens on the newest samples`() {
+		val chart = SafeLineChart(context)
+		chart.layOutAndDraw()
+
+		renderer().attach(chart)
+
+		assertShowsNewestSamples(chart)
+	}
+
+	@Test
+	fun `a resize puts the window back without waiting for a sample`() {
+		val chart = SafeLineChart(context)
+		renderer().attach(chart)
+		chart.layOutAndDraw()
+
+		// A size change resets the chart's transform, dropping the window. Only a redraw used to
+		// restore it, which is why a rotation showed samples from half an hour ago until the next
+		// tick.
+		chart.layOutAndDraw(width = CHART_WIDTH, height = CHART_HEIGHT + 40)
+
+		assertShowsNewestSamples(chart)
+	}
+
+	@Test
+	fun `a detached renderer stops following the chart it left`() {
+		val chart = SafeLineChart(context)
+		val renderer = renderer()
+		renderer.attach(chart)
+		chart.layOutAndDraw()
+		renderer.detach()
+
+		// A resize drops the window, and nothing should put it back: the page is on its way to
+		// another renderer, and a stale listener would fight whichever one binds next.
+		chart.layOutAndDraw(width = CHART_WIDTH, height = CHART_HEIGHT + 40)
+
+		assertThat(chart.lowestVisibleX).isWithin(TOLERANCE).of(0f)
+	}
+
+	@Test
+	fun `a rebind onto a new page forgets a pan on the old one`() {
+		val renderer = renderer()
+		val first = SafeLineChart(context)
+		renderer.attach(first)
+		first.layOutAndDraw()
+
+		// The user pans. From here the viewport on *this* chart is theirs, not the renderer's.
+		checkNotNull(first.onChartGestureListener).onChartTranslate(null, -20f, 0f)
+
+		// A resume rebinds the carousel, which attaches the replacement page before the outgoing
+		// one is recycled -- so the detach naming the old chart arrives afterwards and finds a
+		// different one bound. The pan belonged to the page the user left; the fresh page must
+		// still open on the newest samples.
+		val second = SafeLineChart(context)
+		renderer.attach(second)
+		renderer.detachIfAttached(first)
+		second.layOutAndDraw()
+
+		assertShowsNewestSamples(second)
+	}
+
+	private companion object {
+		/** Longer than the visible window, so there is a wrong end of the buffer to park in. */
+		const val SAMPLES = 200
+
+		/** The viewport is computed in pixels and read back as a value, so it lands near-exactly. */
+		const val TOLERANCE = 0.01f
+	}
+}