Skip to content

Conversation

alexfauquette
Copy link
Member

@alexfauquette alexfauquette commented Jul 29, 2025

Fix #18908

The issue came from a trick that works for static charts but not zoom ones.

To avoid having to manage coordinates all points outside of the drawing area were moved to a "trash coordinate" at (-drawing_width, -drawing_height) such that any moint inside the drawing area will be closer to the pointer than the trash coordinate.

Here is an illustration of the transformation where from the real data (the left chart) we move all points outside the drawing are (black rectangle) to the trash point (the red one) and give this representation to the delauney computation

image

If you have no data in the drawing area, the closer point is the trash point which contains all the scatter point. So Delauney retruns the first data point.

This PR effectively remove the data point that are outside of the drawing area and store an index mappping from delauney to series

@alexfauquette alexfauquette added type: bug It doesn't behave as expected. scope: charts Changes related to the charts. labels Jul 29, 2025
@mui-bot
Copy link

mui-bot commented Jul 29, 2025

Deploy preview: https://deploy-preview-18950--material-ui-x.netlify.app/

Bundle size report

Bundle Parsed Size Gzip Size
@mui/x-data-grid 0B(0.00%) 0B(0.00%)
@mui/x-data-grid/DataGrid 0B(0.00%) 0B(0.00%)
@mui/x-data-grid-pro 0B(0.00%) 0B(0.00%)
@mui/x-data-grid-pro/DataGridPro 0B(0.00%) 0B(0.00%)
@mui/x-data-grid-premium 0B(0.00%) 0B(0.00%)
@mui/x-data-grid-premium/DataGridPremium 0B(0.00%) 0B(0.00%)
@mui/x-charts 🔺+81B(+0.03%) 🔺+24B(+0.03%)
@mui/x-charts-pro 🔺+81B(+0.02%) 🔺+24B(+0.02%)
@mui/x-date-pickers 0B(0.00%) 0B(0.00%)
@mui/x-date-pickers-pro 0B(0.00%) 0B(0.00%)
@mui/x-tree-view 0B(0.00%) 0B(0.00%)
@mui/x-tree-view-pro 0B(0.00%) 0B(0.00%)

Details of bundle changes

Generated by 🚫 dangerJS against f14d09c

Copy link

codspeed-hq bot commented Jul 29, 2025

CodSpeed Performance Report

Merging #18950 will improve performances by 16.89%

Comparing alexfauquette:fix-voronoi (f14d09c) with master (6d189c4)1

Summary

⚡ 1 improvements
✅ 9 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
ScatterChartPro with big data amount and zoomed in 64.3 ms 55 ms +16.89%

Footnotes

  1. No successful run was found on master (4aeb330) during the generation of this report, so 6d189c4 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

*/
startIndex: number;
/**
* The first index in the voronoi data that is outside this series.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: since this is called "endIndex", it should probably represent the last index, not the first one of the next series.

No need to fix it now as it is already like this and I might need to change it for the performance improvement.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's to have the same behavior as the slice(start, end) method. start is included, and end is excluded

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but at the moment it seems the end is included?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be clearer if I use the same expression as in the slice() docs from MDN?

Suggested change
* The first index in the voronoi data that is outside this series.
* The index at which the series end in voronoid data.
* Ends up to but not including `endIndex`.

@alexfauquette alexfauquette merged commit b6a7156 into mui:master Jul 29, 2025
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: charts Changes related to the charts. type: bug It doesn't behave as expected.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[charts] Weird behavior when finding closest point in zoomed in scatter chart
3 participants