Attempt fixing charts shrinking on certain zoom values in Chrome

[bojidar-bg]

This is an attempt at fixing #11224 and possibly #12076.
I managed to reproduce the issue with the steps from #11224 on Chromium 138 on Linux, but it is very sensitive to screen width so I'm not sure if I caught all cases. At any rate, with the submitted fix, I'm no longer able to reproduce the issue myself.

Would appreciate if someone with Chrome on Windows could try reproducing the issue with the steps from #12076 (comment) and this PR.

[magnusvk]

Thanks for the fix! @LeeLenaleee any chance you could release a new version with this fix? We have users that are complaining about this and would love to pull this in.

[joshkel]

These changes appear to have some unintended side effects; after upgrading from Chart.js 4.5.0 to 4.5.1, we started encountering errors in our code. Within retinaScale:

  const deviceHeight = round1(chart.height * pixelRatio);
  const deviceWidth = round1(chart.width * pixelRatio);

  (chart as PrivateChart).height = round1(chart.height);
  (chart as PrivateChart).width = round1(chart.width);

  const canvas = chart.canvas;
  // ...
  if (chart.currentDevicePixelRatio !== pixelRatio
      || canvas.height !== deviceHeight
      || canvas.width !== deviceWidth) {
    (chart as PrivateChart).currentDevicePixelRatio = pixelRatio;
    canvas.height = deviceHeight;
    canvas.width = deviceWidth;
    chart.ctx.setTransform(pixelRatio, 0, 0, pixelRatio, 0, 0);
    return true;
  }

HTMLCanvasElement's width and height are integers. Therefore, if deviceWidth and deviceHeight are non-integers, the if block is always evaluated, and retinaScale always returns true.

In our case, this resulted in extra resize and update events during chart initialization that some custom in-house plugins weren't prepared to handle. This is a potential bug within our custom plugins, so I can address that, but having guaranteed extra resize and update events (because retinaScale is never satisfied that the chart is properly sized) seems like a mistake.

If preserving decimals within the chart's internal width/height is necessary to prevent bugs, then I assume that the fix is to still truncate to int when comparing to the canvas width/height.

[bojidar-bg]

Hm.. I'll have to test whether using e.g. Math.abs(canvas.height - deviceHeight) > 0.5 in place of canvas.height !== deviceHeight results in any regressions with the original issue.

Is the only prerequisite for the multiple resize events a fractional display scaling ratio? If not, I would appreciate a minimal reproduction example (:

[joshkel]

Thanks, @bojidar-bg.

I should have mentioned - we're seeing this with fractional width / height (due to some percentage-based flex layouts). We haven't touched the scaling ratio. See https://codepen.io/joshkel/pen/azdjLGZ. With Chart.js 4.5.0, you should see one onResize event in your browser's console. With Chart.js 4.5.1, I see 3.

I really think you want canvas.height !== Math.floor(deviceHeight) (or equivalent) - not Math.abs(canvas.height - deviceHeight) > 0.5. Assigning canvas.height = deviceHeight does the equivalent of a Math.floor (it truncates to int), so I believe the if clause should match that behavior.

[kk-adi]

Hi @bojidar-bg
I also faced some issues after updating from 4.5.0 to 4.5.1.

My app crashes when the zoom level is different than 100%.
I believe the problem is caused by the extra resize, see the stacktrace:

hook.js:608 React Router caught the following error during render TypeError: Cannot read properties of undefined (reading 'length')
    at Object.draw (chartjs-plugin-datalabels.esm.js:1003:31)
    at Object.afterDatasetsDraw (chartjs-plugin-datalabels.esm.js:1302:12)
    at callback (helpers.core.ts:109:15)
    at PluginService._notify (core.plugins.js:65:11)
    at PluginService.notify (core.plugins.js:46:25)
    at Chart.notifyPlugins (core.controller.js:1130:26)
    at Chart._drawDatasets (core.controller.js:775:10)
    at Chart.draw (core.controller.js:724:10)
    at Chart.render (core.controller.js:693:12)
    at Chart._resize (core.controller.js:299:14)

Different from @joshkel who uses some custom in-house plugins, we use the official plugin chartjs-plugin-datalabels.

After debugging, I pinpointed the issue to _labels being undefined (only with zoom != 100%)

  afterDatasetsDraw: function(chart) {
    layout.draw(chart, chart[EXPANDO_KEY]._labels);
  },

Moreover, the extra render causes also another issue:

hook.js:608 React Router caught the following error during render Error: Canvas is already in use. Chart with ID '0' must be destroyed before the canvas with ID '' can be reused.

None of the problems exist if the zoom level is 100% so very likely to be related to this discussion and zoom levels.

[bojidar-bg]

@kk-adi Mm... mind opening an issue on https://github.com/chartjs/chartjs-plugin-datalabels itself? I can try to look at it in the coming week, but it'd be nice if this PR's discussion does not become an issue list (:

[kk-adi]

@bojidar-bg sure, I'll open an issue there. I just wanted to have it also here because it's closely related to the zoom values that were changed in this PR

chartjs/chartjs-plugin-datalabels#438