Copilot commented on code in PR #21601:
URL: https://github.com/apache/echarts/pull/21601#discussion_r3177879124
##########
src/processor/dataSample.ts:
##########
@@ -86,14 +115,27 @@ export default function dataSample(seriesType: string):
StageHandler {
const coordSys = seriesModel.coordinateSystem;
const count = data.count();
// Only cartesian2d support down sampling. Disable it when there
is few data.
- if (count > 10 && coordSys.type === 'cartesian2d' && sampling) {
+ if (count > 10 && coordSys.type === 'cartesian2d' && sampling &&
sampling !== 'none') {
const baseAxis = coordSys.getBaseAxis();
const valueAxis = coordSys.getOtherAxis(baseAxis);
const extent = baseAxis.getExtent();
const dpr = api.getDevicePixelRatio();
// Coordinste system has been resized
const size = Math.abs(extent[1] - extent[0]) * (dpr || 1);
- const rate = Math.round(count / size);
+ if (!isFinite(size) || size <= 0) {
+ return;
+ }
+ if (count <= size) {
+ return;
+ }
+ const baseDim = data.mapDimension(baseAxis.dim);
+ let dataCount = count;
+ if (baseDim != null) {
+ if (!dataExtentInAxisExtent(data, baseAxis, baseDim)) {
+ dataCount = countDataInAxisExtent(data, baseAxis,
baseDim);
+ }
+ }
Review Comment:
`dataExtentInAxisExtent` uses `data.getApproximateExtent(baseDim)` to decide
whether to skip counting visible points. But in `dataZoom.filterMode: 'empty'`,
`AxisProxy.filterData` sets `seriesData.setApproximateExtent(valueWindow, dim)`
for the zoom axis dims, so this check will typically return true even when most
data points are outside the current axis window (they were mapped to `NaN`). As
a result `dataCount` stays as the full `count`, and the sampling rate remains
locked to the full dataset size—so `empty` still gets downsampled when zoomed
in. Consider removing this short-circuit (always compute `dataCount` from the
current axis window) or switching the guard to something that isn’t affected by
`setApproximateExtent` in `empty` mode.
##########
src/processor/dataSample.ts:
##########
@@ -72,6 +74,33 @@ const indexSampler = function (frame: ArrayLike<number>) {
return Math.round(frame.length / 2);
};
+function countDataInAxisExtent(data: SeriesData, baseAxis: Axis, baseDim:
string) {
+ let count = 0;
+ const scale = baseAxis.scale;
+ const store = data.getStore();
+ const dimIdx = data.getDimensionIndex(baseDim);
+ if (dimIdx < 0) {
+ return data.count();
+ }
+ for (let i = 0, len = data.count(); i < len; i++) {
+ const value = store.get(dimIdx, i) as number;
+ if (scale.contain(value)) {
+ count++;
+ }
+ }
+ return count;
Review Comment:
`countDataInAxisExtent` does a full scan over `data.count()` to compute the
in-window count. When sampling is still applied afterwards (`rate > 1`), this
adds an extra O(N) pass on top of the downsampling pass, which can noticeably
increase processing time for very large series during zoom/pan. If possible,
consider a cheaper way to derive the in-window count (e.g., using an
index/range selection facility if available) or restructure so the count is
computed in the same traversal as downsampling to avoid a double pass.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
