Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: markArea display filter correction #16861

Merged
merged 11 commits into from
Apr 15, 2022
15 changes: 15 additions & 0 deletions src/component/marker/MarkAreaView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,21 @@ function markAreaFilter(coordSys: CoordinateSystem, item: MarkAreaMergedItemOpti
) {
return true;
}

/*Another zone filter is applied, which might not be necessary.
Directly returning true means displaying markArea component permanently,
Copy link
Contributor

Choose a reason for hiding this comment

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

We prefer using inline comment // here.

Also this comment seems to be outdated after new commits?

Copy link
Member Author

Choose a reason for hiding this comment

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

For this one, I mean we are using a function zonefilter here, but actually returning true can achieve almost the same effect as using this function. The reason is that, when returning true, markArea is always shown and will not be clipped however the grid changes, which solves the problem. When the coordinates does not include the markArea, it's automatically not shown. So in my opinion a function to check if the coordinates intersect with markArea is unnecessary. Of course, keeping the markArea rendered may be problematic when there are huge amount of them, but in most cases one or two markAreas would hardly affect performance.

Copy link
Contributor

@pissang pissang Apr 14, 2022

Choose a reason for hiding this comment

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

Got it. Thanks for the detailed explanation. Perhaps we can tweak the description a bit to make it more clear. For example:

Directly returning true may also do the work. But filtering ahead will avoid creating too much unnecessary elements when there are huge amount of elements.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. Comment updated.

but it won't hurt performance much and can sometimes save the work of filtering*/
return markerHelper.zoneFilter(coordSys, {
jiawulin001 marked this conversation as resolved.
Show resolved Hide resolved
coord: fromCoord,
x: item.x0,
y: item.y0
},
{
coord: toCoord,
x: item.x1,
y: item.y1
}
)
}
return markerHelper.dataFilter(coordSys, {
coord: fromCoord,
Expand Down
13 changes: 13 additions & 0 deletions src/component/marker/markerHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,19 @@ export function dataFilter(
? coordSys.containData(item.coord) : true;
}

export function zoneFilter(
// Currently only polar and cartesian has containData.
coordSys: CoordinateSystem & {
containZone?(data1: ScaleDataValue[],data2: ScaleDataValue[]): boolean
},
item1: MarkerPositionOption,
item2: MarkerPositionOption
) {
// Alwalys return true if there is no coordSys
return (coordSys && coordSys.containZone && item1.coord && item2.coord && !hasXOrY(item1) && !hasXOrY(item2))
? coordSys.containZone(item1.coord, item2.coord) : true;
}

export function createMarkerDimValueGetter(
inCoordSys: boolean,
dims: SeriesDimensionDefine[]
Expand Down
13 changes: 13 additions & 0 deletions src/coord/cartesian/Cartesian2D.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import Grid from './Grid';
import Scale from '../../scale/Scale';
import { invert } from 'zrender/src/core/matrix';
import { applyTransform } from 'zrender/src/core/vector';
import {rectRectIntersect} from '../../util/graphic'

export const cartesian2DDimensions = ['x', 'y'];

Expand Down Expand Up @@ -105,6 +106,18 @@ class Cartesian2D extends Cartesian<Axis2D> implements CoordinateSystem {
&& this.getAxis('y').containData(data[1]);
}

containZone(data1: ScaleDataValue[], data2: ScaleDataValue[]): boolean {
const zoneCorner1 = this.dataToPoint(data1);
const zoneCorner2 = this.dataToPoint(data2);
const area = this.getArea();
return rectRectIntersect(
jiawulin001 marked this conversation as resolved.
Show resolved Hide resolved
[zoneCorner1,zoneCorner2],
[
[area.x,area.y], [area.x + area.width, area.y + area.height]
]
)
}

dataToPoint(data: ScaleDataValue[], clamp?: boolean, out?: number[]): number[] {
out = out || [];
const xVal = data[0];
Expand Down
21 changes: 21 additions & 0 deletions src/util/graphic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,27 @@ export function createIcon(
}
}

/**
* Return `true` if the given two polygons are intersect.
* Note that we do not count colinear as intersect here because no
* requirement for that. We could do that if required in future.
* Input takes in the coordinates of the two diagonal points
*/
export function rectRectIntersect(
rect1: vector.VectorArray[],
rect2: vector.VectorArray[]
): boolean {
const a1x = Math.min(rect1[0][0], rect1[1][0]);
const a2x = Math.max(rect1[0][0], rect1[1][0]);
const a1y = Math.min(rect1[0][1], rect1[1][1]);
const a2y = Math.max(rect1[0][1], rect1[1][1]);
const b1x = Math.min(rect2[0][0], rect2[1][0]);
const b2x = Math.max(rect2[0][0], rect2[1][0]);
const b1y = Math.min(rect2[0][1], rect2[1][1]);
const b2y = Math.max(rect2[0][1], rect2[1][1]);
return !((a2x < b1x) || (a1y > b2y) || (a1x > b2x) || (a2y < b1y));
}

/**
* Return `true` if the given line (line `a`) and the given polygon
* are intersect.
Expand Down
93 changes: 91 additions & 2 deletions test/markArea.html

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.