perf(xy): avoid computing bar value display width (#2754)

This commit improves a bit the geometries rendering logic by avoiding some computations.
In particular it avoids computing the bar values to display if the option is not enabled.

Author: markov00

packages/charts/src/chart_types/xy_chart/rendering/bars.ts

@@ -19,7 +19,7 @@ import type { BarSeriesStyle, DisplayValueStyle } from '../../../utils/themes/th
 import { IndexedGeometryMap } from '../utils/indexed_geometry_map';
 import type { DataSeries, DataSeriesDatum, XYChartSeriesIdentifier } from '../utils/series';
 import { getSeriesIdentifierFromDataSeries } from '../utils/series';
-import type { BarStyleAccessor, DisplayValueSpec, StackMode } from '../utils/specs';
+import type { BarStyleAccessor, DisplayValueSpec, StackMode, TickFormatter } from '../utils/specs';
 import { LabelOverflowConstraint } from '../utils/specs';
 
 const PADDING = 1; // default padding for now
@@ -30,6 +30,11 @@ type BarTuple = {
   indexedGeometryMap: IndexedGeometryMap;
 };
 
+/** @internal */
+type DisplayValueSpecWithValueFormatter = Omit<DisplayValueSpec, 'valueFormatter'> & {
+  valueFormatter: TickFormatter;
+};
+
 /** @internal */
 export function renderBars(
   measureText: TextMeasure,
@@ -43,17 +48,17 @@ export function renderBars(
   color: Color,
   isBandedSpec: boolean,
   sharedSeriesStyle: BarSeriesStyle,
-  displayValueSettings?: DisplayValueSpec,
+  displayValueSettings?: DisplayValueSpecWithValueFormatter,
   styleAccessor?: BarStyleAccessor,
   stackMode?: StackMode,
 ): BarTuple {
-  const { fontSize, fontFamily } = sharedSeriesStyle.displayValue;
   const initialBarTuple: BarTuple = { barGeometries: [], indexedGeometryMap: new IndexedGeometryMap() } as BarTuple;
   const y1Fn = getY1ScaledValueFn(yScale);
   const y0Fn = getY0ScaledValueFn(yScale);
-
+  const seriesIdentifier = getSeriesIdentifierFromDataSeries(dataSeries);
   return dataSeries.data.reduce((barTuple: BarTuple, datum) => {
     const xScaled = xScale.scale(datum.x);
+
     if (!xScale.isValueInDomain(datum.x) || Number.isNaN(xScaled)) {
       return barTuple; // don't create a bar if not within the xScale domain
     }
@@ -77,7 +82,6 @@ export function renderBars(
     // the actual height of the bar
     const height = yDiff + addedMinBarHeight;
 
-    const seriesIdentifier: XYChartSeriesIdentifier = getSeriesIdentifierFromDataSeries(dataSeries);
     const seriesStyle = getBarStyleOverrides(datum, seriesIdentifier, sharedSeriesStyle, styleAccessor);
 
     const maxPixelWidth = clamp(seriesStyle.rect.widthRatio ?? 1, 0, 1) * xScale.bandwidth;
@@ -87,52 +91,15 @@ export function renderBars(
     const x = xScaled + xScale.bandwidth * orderIndex + xScale.bandwidth / 2 - width / 2;
 
     const y1Value = getDatumYValue(datum, false, isBandedSpec, stackMode);
-    const formattedDisplayValue = displayValueSettings?.valueFormatter?.(y1Value);
-
-    // only show displayValue for even bars if showOverlappingValue
-    const displayValueText =
-      displayValueSettings?.isAlternatingValueLabel && barGeometries.length % 2 ? undefined : formattedDisplayValue;
-
-    const { displayValueWidth, fixedFontScale } = computeBoxWidth(displayValueText ?? '', {
-      padding: PADDING,
-      fontSize,
-      fontFamily,
-      measureText,
-    });
-
-    const isHorizontalRotation = chartRotation % 180 === 0;
-    // Pick the right side of the label's box to use as factor reference
-    const referenceWidth = Math.max(isHorizontalRotation ? displayValueWidth : fixedFontScale, 1);
-
-    const textScalingFactor = getFinalFontScalingFactor(
-      (width * FONT_SIZE_FACTOR) / referenceWidth,
-      fixedFontScale,
-      fontSize,
-    );
-    const overflowConstraints: Set<LabelOverflowConstraint> = new Set(
-      displayValueSettings?.overflowConstraints ?? [
-        LabelOverflowConstraint.ChartEdges,
-        LabelOverflowConstraint.BarGeometry,
-      ],
-    );
-
-    // Based on rotation scale the width of the text box
-    const bboxWidthFactor = isHorizontalRotation ? textScalingFactor : 1;
-
-    const displayValue: BarGeometry['displayValue'] | undefined =
-      displayValueText && displayValueSettings?.showValueLabel
-        ? {
-            fontScale: textScalingFactor,
-            fontSize: fixedFontScale,
-            text: displayValueText,
-            width: bboxWidthFactor * displayValueWidth,
-            height: textScalingFactor * fixedFontScale,
-            overflowConstraints,
-          }
-        : undefined;
 
+    const shouldDisplayValue =
+      displayValueSettings?.showValueLabel &&
+      // only show displayValue for even bars if isAlternatingValueLabel
+      !(displayValueSettings?.isAlternatingValueLabel && barGeometries.length % 2);
     const barGeometry: BarGeometry = {
-      displayValue,
+      displayValue: shouldDisplayValue
+        ? computeDisplayValue(y1Value, sharedSeriesStyle, measureText, chartRotation, width, displayValueSettings)
+        : undefined,
       x,
       y: yScreenSpaceCoord,
       transform: { x: 0, y: 0 },
@@ -169,6 +136,57 @@ export function renderBars(
   }, initialBarTuple);
 }
 
+function computeDisplayValue(
+  y1Value: number | null,
+  sharedSeriesStyle: BarSeriesStyle,
+  measureText: TextMeasure,
+  chartRotation: number,
+  width: number,
+  displayValueSettings: DisplayValueSpecWithValueFormatter,
+): BarGeometry['displayValue'] | undefined {
+  const { fontSize, fontFamily } = sharedSeriesStyle.displayValue;
+  const displayValueText = displayValueSettings.valueFormatter(y1Value);
+
+  if (displayValueText.length === 0) {
+    return;
+  }
+
+  const { displayValueWidth, fixedFontScale } = computeBoxWidth(displayValueText, {
+    padding: PADDING,
+    fontSize,
+    fontFamily,
+    measureText,
+  });
+
+  const isHorizontalRotation = chartRotation % 180 === 0;
+  // Pick the right side of the label's box to use as factor reference
+  const referenceWidth = Math.max(isHorizontalRotation ? displayValueWidth : fixedFontScale, 1);
+
+  const textScalingFactor = getFinalFontScalingFactor(
+    (width * FONT_SIZE_FACTOR) / referenceWidth,
+    fixedFontScale,
+    fontSize,
+  );
+  const overflowConstraints: Set<LabelOverflowConstraint> = new Set(
+    displayValueSettings?.overflowConstraints ?? [
+      LabelOverflowConstraint.ChartEdges,
+      LabelOverflowConstraint.BarGeometry,
+    ],
+  );
+
+  // Based on rotation scale the width of the text box
+  const bboxWidthFactor = isHorizontalRotation ? textScalingFactor : 1;
+
+  return {
+    fontScale: textScalingFactor,
+    fontSize: fixedFontScale,
+    text: displayValueText,
+    width: bboxWidthFactor * displayValueWidth,
+    height: textScalingFactor * fixedFontScale,
+    overflowConstraints,
+  };
+}
+
 /**
  * Workout the text box size and fixedFontSize based on a collection of options
  * @internal
@@ -189,6 +207,10 @@ function computeBoxWidth(
 ): { fixedFontScale: number; displayValueWidth: number } {
   const fixedFontScale = Math.max(typeof fontSize === 'number' ? fontSize : fontSize.min, 1);
 
+  if (text.length === 0) {
+    return { fixedFontScale, displayValueWidth: 0 };
+  }
+
   const computedDisplayValueWidth = measureText(
     text,
     { fontFamily, fontWeight: 'normal', fontStyle: 'normal', fontVariant: 'normal' },

packages/charts/src/chart_types/xy_chart/rendering/points.ts

@@ -62,35 +62,34 @@ export function renderPoints(
   const needSorting = !markSizeOptions.enabled;
 
   let style = buildPointGeometryStyles(color, pointStyle);
-  let styleOverrides: RecursivePartial<PointStyle> | undefined = undefined;
+  const seriesIdentifier = getSeriesIdentifierFromDataSeries(dataSeries);
   const { pointGeometries, minDistanceBetweenPoints } = dataSeries.data.reduce<{
     pointGeometries: SortedArray<PointGeometry>;
     minDistanceBetweenPoints: number;
     prevX: number | undefined;
   }>(
     (acc, datum, dataIndex) => {
       const { x: xValue, mark } = datum;
-      const prev = dataSeries.data[dataIndex - 1];
-      const next = dataSeries.data[dataIndex + 1];
       // don't create the point if not within the xScale domain
       if (!xScale.isValueInDomain(xValue)) return acc;
-
       // don't create the point if it that point was filled
       const x = xScale.scale(xValue);
-
       if (!isFiniteNumber(x)) return acc;
 
+      const prev = dataSeries.data[dataIndex - 1];
+      const next = dataSeries.data[dataIndex + 1];
+
       if (isFiniteNumber(acc.prevX) && !isDatumFilled(datum)) {
         acc.minDistanceBetweenPoints = Math.min(acc.minDistanceBetweenPoints, Math.abs(x - acc.prevX));
       }
       acc.prevX = x;
 
       const yDatumKeyNames: Array<keyof Omit<FilledValues, 'x'>> = isBandedSpec ? ['y0', 'y1'] : ['y1'];
-      const seriesIdentifier: XYChartSeriesIdentifier = getSeriesIdentifierFromDataSeries(dataSeries);
 
       const isPointIsolated = considerIsolatedPoints
         ? isIsolatedPoint(dataIndex, dataSeries.data.length, datum, isBandedSpec, y1Fn, y0Fn, prev, next)
         : false;
+      let styleOverrides: RecursivePartial<PointStyle> | undefined = undefined;
       if (styleAccessor) {
         styleOverrides = getPointStyleOverrides(datum, seriesIdentifier, isPointIsolated, styleAccessor);
         style = buildPointGeometryStyles(color, pointStyle, styleOverrides);

packages/charts/src/chart_types/xy_chart/utils/specs.ts

@@ -386,7 +386,6 @@ export interface DisplayValueSpec {
   isAlternatingValueLabel?: boolean;
   /**
    * Function for formatting values; will use axis tickFormatter if none specified
-   * @defaultValue false
    */
   valueFormatter?: TickFormatter;
   /**

packages/charts/src/utils/bbox/canvas_text_bbox_calculator.ts

@@ -23,14 +23,14 @@ export type TextMeasure = (text: string, font: Omit<Font, 'textColor'>, fontSize
 
 /** @internal */
 export function measureText(ctx: CanvasRenderingContext2D): TextMeasure {
-  return (text, font, fontSize, lineHeight = 1) =>
-    withContext(ctx, (ctx): Size => {
-      if (text.length === 0) {
-        // TODO this is a temporary fix to make the multilayer time axis work
-        return { width: 0, height: fontSize * lineHeight };
-      }
+  return (text, font, fontSize, lineHeight = 1) => {
+    if (text.length === 0) {
+      return { width: 0, height: fontSize * lineHeight };
+    }
+    return withContext(ctx, (ctx): Size => {
       ctx.font = cssFontShorthand(font, fontSize);
       const { width } = ctx.measureText(text);
       return { width, height: fontSize * lineHeight };
     });
+  };
 }