Skip to content

Commit

Permalink
fix: render orphan data points on lines and areas (opensearch-project…
Browse files Browse the repository at this point in the history
…#900)

This commits adds the ability to render orphan data points displayed on a line or area chart. An orphan data point is a datapoint that doesn't have a next or a previous data point to connect to.

fix opensearch-project#783
  • Loading branch information
markov00 authored Nov 12, 2020
1 parent 818e83b commit 3e2c739
Show file tree
Hide file tree
Showing 20 changed files with 418 additions and 264 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
7 changes: 7 additions & 0 deletions packages/osd-charts/integration/tests/area_stories.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,11 @@ describe('Area series stories', () => {
);
});
});
describe('Area with orphan data points', () => {
it('render correctly fit function', async () => {
await common.expectChartAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/line-chart--test-orphan-data-points&knob-enable fit function=&knob-switch to area=true',
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ export function renderAreas(ctx: CanvasRenderingContext2D, props: AreaGeometries
});

areas.forEach(({ panel, value: area }) => {
const { seriesPointStyle, seriesIdentifier } = area;
if (!seriesPointStyle.visible) {
const { seriesPointStyle, seriesIdentifier, points } = area;
const visiblePoints = seriesPointStyle.visible ? points : points.filter(({ orphan }) => orphan);
if (visiblePoints.length === 0) {
return;
}
const geometryStateStyle = getGeometryStateStyle(seriesIdentifier, sharedStyle, highlightedLegendItem);
Expand All @@ -85,9 +86,9 @@ export function renderAreas(ctx: CanvasRenderingContext2D, props: AreaGeometries
rotation,
renderingArea,
(ctx) => {
renderPoints(ctx, area.points, seriesPointStyle, geometryStateStyle);
renderPoints(ctx, visiblePoints, seriesPointStyle, geometryStateStyle);
},
{ area: clippings, shouldClip: area.points[0]?.value.mark !== null },
{ area: clippings, shouldClip: points[0]?.value.mark !== null },
);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ function renderPerPanelBars(
rotation: Rotation = 0,
) {
return ({ panel, value: bars }: PerPanel<BarGeometry[]>) => {
if (bars.length === 0) {
return;
}
withPanelTransform(
ctx,
panel,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ export function renderBubbles(ctx: CanvasRenderingContext2D, props: BubbleGeomet
},
[],
);
if (allPoints.length === 0) {
return;
}

renderPointGroup(
ctx,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,28 +46,30 @@ export function renderLines(ctx: CanvasRenderingContext2D, props: LineGeometries
const { lines, sharedStyle, highlightedLegendItem, clippings, renderingArea, rotation } = props;

lines.forEach(({ panel, value: line }) => {
const { seriesLineStyle, seriesPointStyle } = line;
const { seriesLineStyle, seriesPointStyle, points } = line;

if (seriesLineStyle.visible) {
withPanelTransform(ctx, panel, rotation, renderingArea, (ctx) => {
renderLine(ctx, line, sharedStyle, clippings, highlightedLegendItem);
});
}

if (seriesPointStyle.visible) {
withPanelTransform(
ctx,
panel,
rotation,
renderingArea,
(ctx) => {
const geometryStyle = getGeometryStateStyle(line.seriesIdentifier, sharedStyle, highlightedLegendItem);
renderPoints(ctx, line.points, line.seriesPointStyle, geometryStyle);
},
// TODO: add padding over clipping
{ area: clippings, shouldClip: line.points[0]?.value.mark !== null },
);
const visiblePoints = seriesPointStyle.visible ? points : points.filter(({ orphan }) => orphan);
if (visiblePoints.length === 0) {
return;
}
const geometryStyle = getGeometryStateStyle(line.seriesIdentifier, sharedStyle, highlightedLegendItem);
withPanelTransform(
ctx,
panel,
rotation,
renderingArea,
(ctx) => {
renderPoints(ctx, visiblePoints, line.seriesPointStyle, geometryStyle);
},
// TODO: add padding over clipping
{ area: clippings, shouldClip: line.points[0]?.value.mark !== null },
);
});
});
}
Expand Down
17 changes: 10 additions & 7 deletions packages/osd-charts/src/chart_types/xy_chart/rendering/area.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ import { PointStyleAccessor } from '../utils/specs';
import { renderPoints } from './points';
import {
getClippedRanges,
getY0ScaledValueOrThrow,
getY1ScaledValueOrThrow,
isYValueDefined,
getY0ScaledValueOrThrowFn,
getY1ScaledValueOrThrowFn,
getYDatumValueFn,
isYValueDefinedFn,
MarkSizeOptions,
} from './utils';

Expand All @@ -56,15 +57,17 @@ export function renderArea(
areaGeometry: AreaGeometry;
indexedGeometryMap: IndexedGeometryMap;
} {
const y1Fn = getY1ScaledValueOrThrow(yScale);
const y0Fn = getY0ScaledValueOrThrow(yScale);
const definedFn = isYValueDefined(yScale, xScale);
const y1Fn = getY1ScaledValueOrThrowFn(yScale);
const y0Fn = getY0ScaledValueOrThrowFn(yScale);
const definedFn = isYValueDefinedFn(yScale, xScale);
const y1DatumAccessor = getYDatumValueFn();
const y0DatumAccessor = getYDatumValueFn('y0');
const pathGenerator = area<DataSeriesDatum>()
.x(({ x }) => xScale.scaleOrThrow(x) - xScaleOffset)
.y1(y1Fn)
.y0(y0Fn)
.defined((datum) => {
return definedFn(datum) && (hasY0Accessors ? definedFn(datum, 'y0') : true);
return definedFn(datum, y1DatumAccessor) && (hasY0Accessors ? definedFn(datum, y0DatumAccessor) : true);
})
.curve(getCurveFactory(curve));

Expand Down
15 changes: 11 additions & 4 deletions packages/osd-charts/src/chart_types/xy_chart/rendering/line.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,13 @@ import { IndexedGeometryMap } from '../utils/indexed_geometry_map';
import { DataSeries, DataSeriesDatum } from '../utils/series';
import { PointStyleAccessor } from '../utils/specs';
import { renderPoints } from './points';
import { getClippedRanges, getY1ScaledValueOrThrow, isYValueDefined, MarkSizeOptions } from './utils';
import {
getClippedRanges,
getY1ScaledValueOrThrowFn,
getYDatumValueFn,
isYValueDefinedFn,
MarkSizeOptions,
} from './utils';

/** @internal */
export function renderLine(
Expand All @@ -49,14 +55,15 @@ export function renderLine(
lineGeometry: LineGeometry;
indexedGeometryMap: IndexedGeometryMap;
} {
const y1Fn = getY1ScaledValueOrThrow(yScale);
const definedFn = isYValueDefined(yScale, xScale);
const y1Fn = getY1ScaledValueOrThrowFn(yScale);
const definedFn = isYValueDefinedFn(yScale, xScale);
const y1Accessor = getYDatumValueFn();

const pathGenerator = line<DataSeriesDatum>()
.x(({ x }) => xScale.scaleOrThrow(x) - xScaleOffset)
.y(y1Fn)
.defined((datum) => {
return definedFn(datum);
return definedFn(datum, y1Accessor);
})
.curve(getCurveFactory(curve));

Expand Down
54 changes: 42 additions & 12 deletions packages/osd-charts/src/chart_types/xy_chart/rendering/points.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,21 @@
* under the License.
*/
import { Scale } from '../../../scales';
import { Color } from '../../../utils/commons';
import { Color, isNil } from '../../../utils/commons';
import { Dimensions } from '../../../utils/dimensions';
import { BandedAccessorType, PointGeometry } from '../../../utils/geometry';
import { LineStyle, PointStyle } from '../../../utils/themes/theme';
import { GeometryType, IndexedGeometryMap } from '../utils/indexed_geometry_map';
import { DataSeries, DataSeriesDatum, FilledValues, XYChartSeriesIdentifier } from '../utils/series';
import { PointStyleAccessor, StackMode } from '../utils/specs';
import {
getY0ScaledValueOrThrow,
getY1ScaledValueOrThrow,
getY0ScaledValueOrThrowFn,
getY1ScaledValueOrThrowFn,
getYDatumValueFn,
isDatumFilled,
isYValueDefined,
isYValueDefinedFn,
MarkSizeOptions,
YDefinedFn,
} from './utils';

/** @internal */
Expand All @@ -55,12 +57,14 @@ export function renderPoints(
: () => 0;
const geometryType = spatial ? GeometryType.spatial : GeometryType.linear;

const y1Fn = getY1ScaledValueOrThrow(yScale);
const y0Fn = getY0ScaledValueOrThrow(yScale);
const yDefined = isYValueDefined(yScale, xScale);
const y1Fn = getY1ScaledValueOrThrowFn(yScale);
const y0Fn = getY0ScaledValueOrThrowFn(yScale);
const yDefined = isYValueDefinedFn(yScale, xScale);

const pointGeometries = dataSeries.data.reduce((acc, datum) => {
const pointGeometries = dataSeries.data.reduce((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;
Expand All @@ -78,7 +82,8 @@ export function renderPoints(
const points: PointGeometry[] = [];
const yDatumKeyNames: Array<keyof Omit<FilledValues, 'x'>> = hasY0Accessors ? ['y0', 'y1'] : ['y1'];

yDatumKeyNames.forEach((yDatumKeyName, index) => {
yDatumKeyNames.forEach((yDatumKeyName, keyIndex) => {
const valueAccessor = getYDatumValueFn(yDatumKeyName);
// skip rendering point if y1 is null
const radius = getRadius(mark);
let y: number | null;
Expand All @@ -91,7 +96,7 @@ export function renderPoints(
return;
}

const originalY = getDatumYValue(datum, index === 0, hasY0Accessors, dataSeries.stackMode);
const originalY = getDatumYValue(datum, keyIndex === 0, hasY0Accessors, dataSeries.stackMode);
const seriesIdentifier: XYChartSeriesIdentifier = {
key: dataSeries.key,
specId: dataSeries.specId,
Expand All @@ -102,6 +107,7 @@ export function renderPoints(
smHorizontalAccessorValue: dataSeries.smHorizontalAccessorValue,
};
const styleOverrides = getPointStyleOverrides(datum, seriesIdentifier, styleAccessor);
const orphan = isOrphanDataPoint(dataIndex, dataSeries.data.length, yDefined, prev, next);
const pointGeometry: PointGeometry = {
radius,
x,
Expand All @@ -111,7 +117,7 @@ export function renderPoints(
x: xValue,
y: originalY,
mark,
accessor: hasY0Accessors && index === 0 ? BandedAccessorType.Y0 : BandedAccessorType.Y1,
accessor: hasY0Accessors && keyIndex === 0 ? BandedAccessorType.Y0 : BandedAccessorType.Y1,
datum: datum.datum,
},
transform: {
Expand All @@ -121,10 +127,11 @@ export function renderPoints(
seriesIdentifier,
styleOverrides,
panel,
orphan,
};
indexedGeometryMap.set(pointGeometry, geometryType);
// use the geometry only if the yDatum in contained in the current yScale domain
if (yDefined(datum, yDatumKeyName)) {
if (yDefined(datum, valueAccessor)) {
points.push(pointGeometry);
}
});
Expand Down Expand Up @@ -228,3 +235,26 @@ export function getRadiusFn(
return circleRadius ? Math.sqrt(circleRadius + baseMagicNumber) + lineWidth : lineWidth;
};
}

function yAccessorForOrphanCheck(datum: DataSeriesDatum): number | null {
return datum.filled?.y1 ? null : datum.y1;
}

function isOrphanDataPoint(
index: number,
length: number,
yDefined: YDefinedFn,
prev?: DataSeriesDatum,
next?: DataSeriesDatum,
): boolean {
if (index === 0 && (isNil(next) || !yDefined(next, yAccessorForOrphanCheck))) {
return true;
}
if (index === length - 1 && (isNil(prev) || !yDefined(prev, yAccessorForOrphanCheck))) {
return true;
}
return (
(isNil(prev) || !yDefined(prev, yAccessorForOrphanCheck)) &&
(isNil(next) || !yDefined(next, yAccessorForOrphanCheck))
);
}
Loading

0 comments on commit 3e2c739

Please sign in to comment.