Plot: avoid timeseries downsample if we are already below display lim…

…it (#7333)

This change updates the TimestampDatasetsBuilderImpl to avoid doing a
downsample if we are already under the viewport size for the number of
items we want to display. While this does technically create a dataset
that could have multiple datums at the same pixel coordinate, this is a
tradeoff to avoid downsample rounding errors for extremely close datums
at different zoom levels.

As the user zooms in, we might resolve these datums to the same pixel or
different pixels based on rounding - even though in absolute terms they
are further apart (i.e. going from 0.42 to 0.477 distance apart). This
tends to be most obvious once you've already zoomed to a point where
data was displayed without downsampling but then zooming in again
changes the rounding and the logic would think the data is downsampled.
In practice, the purpose of downsampling is to avoid rendering _too
many_ datums on a plot since rendering takes time. But when the dataset
is already so small that it is under our viewport size and max point
threshold then there's no reason to run downsampling and we can display
all the datums regardless of whether they would appear at the same pixel
or not.

packages/studio-base/src/panels/Plot/builders/TimestampDatasetsBuilderImpl.ts

@@ -196,24 +196,38 @@ export class TimestampDatasetsBuilderImpl {
       };
 
       const maxPoints = MAX_POINTS / numSeries;
-      const downsampledIndicies =
-        dataset.showLine === true
+
+      // We already have fewer items than the viewport width so there's no need to downsample
+      //
+      // Points could still "overlap" but we don't care because we are under our threshold for
+      // the number of points we want to return for a dataset.
+      //
+      // This avoids situations where downsampling might resolve datums to the same pixel at
+      // different zoom levels (due to pixel rounding for datums) as we zoom in and cause us to
+      // remove data points and hide dots. This creates a counter-intuitive UX where dots can
+      // dissapear when zooming in.
+      const min = Math.min(downsampleViewport.width, maxPoints);
+
+      const downsampledIndices =
+        items.length < min
+          ? items.map((item) => item.index)
+          : dataset.showLine === true
           ? downsampleTimeseries(items, downsampleViewport, maxPoints)
           : downsampleScatter(items, downsampleViewport);
 
       // When a series is downsampled the points are disabled as a visual indicator that
       // data is downsampled.
       //
       // If show line is false then we must show points otherwise nothing will be displayed
-      if (downsampledIndicies.length < items.length && dataset.showLine === true) {
+      if (downsampledIndices.length < items.length && dataset.showLine === true) {
         dataset.pointRadius = 0;
       }
 
       // We only need to add the NaN entry if using lines and there is both full and current data
       // to create a discontinuity after the full data.
       let shouldAddNan = dataset.showLine === true && series.full.length > 0;
 
-      for (const index of downsampledIndicies) {
+      for (const index of downsampledIndices) {
         const item = allData[index];
         if (!item) {
           continue;