From 227a43e64375d6964aa61bbc2915d7ce13b5b885 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Thu, 14 Sep 2023 11:25:58 +0200 Subject: [PATCH 1/4] Use (and re-use) the same clip-path when two renders are clipped with the same shape --- src/style.js | 75 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 47 insertions(+), 28 deletions(-) diff --git a/src/style.js b/src/style.js index deeb66346f..96a2d7abf0 100644 --- a/src/style.js +++ b/src/style.js @@ -1,9 +1,9 @@ -import {geoPath, group, namespaces} from "d3"; +import {geoPath, group, namespaces, select} from "d3"; import {create} from "./context.js"; import {defined, nonempty} from "./defined.js"; import {formatDefault} from "./format.js"; import {isNone, isNoneish, isRound, maybeColorChannel, maybeNumberChannel} from "./options.js"; -import {keyof, number, string} from "./options.js"; +import {keyof, keyword, number, string} from "./options.js"; import {warn} from "./warnings.js"; export const offset = (typeof window !== "undefined" ? window.devicePixelRatio > 1 : typeof it === "undefined") ? 0 : 0.5; // prettier-ignore @@ -297,43 +297,62 @@ export function* groupIndex(I, position, mark, channels) { } } -// TODO avoid creating a new clip-path each time? +// TODO Accept other types of clips (paths, urls, x, y, other marks…)? +// https://github.com/observablehq/plot/issues/181 +export function maybeClip(clip) { + if (clip === true) clip = "frame"; + else if (clip === false) clip = null; + else if (clip != null) clip = keyword(clip, "clip", ["frame", "sphere"]); + return clip; +} + +function clipDefs({ownerSVGElement}) { + const svg = select(ownerSVGElement); + const defs = svg.select("defs.clip"); + return defs.size() ? defs : svg.insert("defs", ":first-child").attr("class", "clip"); +} + // Note: may mutate selection.node! function applyClip(selection, mark, dimensions, context) { let clipUrl; const {clip = context.clip} = mark; switch (clip) { case "frame": { - const {width, height, marginLeft, marginRight, marginTop, marginBottom} = dimensions; - const id = getClipId(); - clipUrl = `url(#${id})`; - selection = create("svg:g", context) - .call((g) => - g - .append("svg:clipPath") - .attr("id", id) - .append("rect") - .attr("x", marginLeft) - .attr("y", marginTop) - .attr("width", width - marginRight - marginLeft) - .attr("height", height - marginTop - marginBottom) - ) - .each(function () { - this.appendChild(selection.node()); - selection.node = () => this; // Note: mutation! - }); + const clips = context.clips ?? (context.clips = new Map()); + if (!clips.has("frame")) { + const {width, height, marginLeft, marginRight, marginTop, marginBottom} = dimensions; + const id = getClipId(); + clips.set("frame", id); + clipDefs(context) + .append("clipPath") + .attr("id", id) + .append("rect") + .attr("x", marginLeft) + .attr("y", marginTop) + .attr("width", width - marginRight - marginLeft) + .attr("height", height - marginTop - marginBottom); + } + selection = create("svg:g", context).each(function () { + this.appendChild(selection.node()); + selection.node = () => this; // Note: mutation! + }); + clipUrl = `url(#${clips.get("frame")})`; break; } case "sphere": { + const clips = context.clips ?? (context.clips = new Map()); const {projection} = context; if (!projection) throw new Error(`the "sphere" clip option requires a projection`); - const id = getClipId(); - clipUrl = `url(#${id})`; - selection - .append("clipPath") - .attr("id", id) - .append("path") - .attr("d", geoPath(projection)({type: "Sphere"})); + if (!clips.has("projection")) { + const id = getClipId(); + clips.set("projection", id); + clipDefs(context) + .append("clipPath") + .attr("id", id) + .append("path") + .attr("d", geoPath(projection)({type: "Sphere"})); + } + clipUrl = `url(#${clips.get("projection")})`; break; } } From 0b25ebfa976152c55c88cf0d47a77c283c221a86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Thu, 14 Sep 2023 11:27:54 +0200 Subject: [PATCH 2/4] update test snapshots --- test/output/aaplBollingerCandlestick.svg | 18 +- test/output/aaplCloseClip.svg | 13 +- test/output/armadillo.svg | 13 +- test/output/bandClip.svg | 8 +- test/output/bandClip2.svg | 8 +- test/output/contourVapor.svg | 8 +- test/output/differenceFilterX.svg | 18 +- test/output/differenceFilterY1.svg | 18 +- test/output/differenceFilterY2.svg | 18 +- test/output/differenceX.svg | 18 +- test/output/differenceY.svg | 18 +- test/output/differenceY1.svg | 18 +- test/output/differenceYClip.svg | 22 +- test/output/differenceYClipVariable.svg | 22 +- test/output/differenceYConstant.svg | 18 +- test/output/differenceYCurve.svg | 18 +- test/output/differenceYNegative.svg | 14 +- test/output/differenceYOrdinal.svg | 18 +- test/output/differenceYOrdinalFlip.svg | 18 +- test/output/differenceYRandom.svg | 18 +- test/output/differenceYReverse.svg | 18 +- test/output/differenceYVariable.svg | 38 +- test/output/differenceYZero.svg | 18 +- test/output/hexbin.svg | 8 +- test/output/hexbinFillX.svg | 8 +- test/output/hexbinIdentityReduce.svg | 8 +- test/output/hexbinR.html | 18 +- test/output/hexbinText.svg | 18 +- test/output/hexbinZ.html | 8 +- test/output/hexbinZNull.svg | 8 +- test/output/penguinDensityFill.html | 18 +- test/output/penguinDensityZ.html | 18 +- test/output/projectionClipBerghaus.svg | 13 +- test/output/rasterVaporEqualEarth.svg | 8 +- .../rasterVaporEqualEarthBarycentric.svg | 8 +- test/output/roundedRectNegativeX.html | 8 +- test/output/roundedRectNegativeY.html | 8 +- test/output/roundedRectNegativeY1.html | 8 +- test/output/trafficHorizon.html | 953 ++++-------------- test/output/usStateCapitalsVoronoi.svg | 14 +- test/output/usStateCentroidVoronoi.svg | 14 +- test/output/usStateGeoCentroidVoronoi.svg | 14 +- test/output/youngAdults.html | 150 ++- 43 files changed, 576 insertions(+), 1132 deletions(-) diff --git a/test/output/aaplBollingerCandlestick.svg b/test/output/aaplBollingerCandlestick.svg index 1115051552..56c9d9f6fd 100644 --- a/test/output/aaplBollingerCandlestick.svg +++ b/test/output/aaplBollingerCandlestick.svg @@ -1,4 +1,9 @@ + + + + + + + + + + Gentoo + + + + + ≥8,000 + + + + + - - - - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + - - - - + diff --git a/test/output/usStateCapitalsVoronoi.svg b/test/output/usStateCapitalsVoronoi.svg index c34eef764b..9c71962ee8 100644 --- a/test/output/usStateCapitalsVoronoi.svg +++ b/test/output/usStateCapitalsVoronoi.svg @@ -1,4 +1,9 @@ + + + + + - - - - - - - - + diff --git a/test/output/usStateCentroidVoronoi.svg b/test/output/usStateCentroidVoronoi.svg index cc0ca4df88..1b53a84143 100644 --- a/test/output/usStateCentroidVoronoi.svg +++ b/test/output/usStateCentroidVoronoi.svg @@ -1,4 +1,9 @@ + + + + + - - - - - - - - + diff --git a/test/output/usStateGeoCentroidVoronoi.svg b/test/output/usStateGeoCentroidVoronoi.svg index 2ad229543d..27e53f8040 100644 --- a/test/output/usStateGeoCentroidVoronoi.svg +++ b/test/output/usStateGeoCentroidVoronoi.svg @@ -1,4 +1,9 @@ + + + + + - - - - - - - - + diff --git a/test/output/youngAdults.html b/test/output/youngAdults.html index f15b78b8a6..c63748f5ec 100644 --- a/test/output/youngAdults.html +++ b/test/output/youngAdults.html @@ -34,6 +34,11 @@

…by age and sex. Data: Eurostat

Y25-29 + + + + +

…by age and sex. Data: Eurostat

- + - - + + - - + + - + - + - - + + - - + + - + - + - - + + - - + + - + - + - - + + - - + + - + - + - - + + - - + + - + - + - - + + - - + + - + - + - - + + - - + + - + - + - - + + - - + + - + - + - - + + - - + + - + - + - - + + - - + + - + - - - - + - - - - + - - - - + - - - - + - - - - + From c28da54e5f95d862ff0938c6fa1e14d6731fd220 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Sun, 4 Aug 2024 21:18:18 -0400 Subject: [PATCH 3/4] no defs; use WeakMap --- src/style.js | 69 ++++----- test/output/aaplBollingerCandlestick.svg | 8 +- test/output/aaplCloseClip.svg | 8 +- test/output/armadillo.svg | 8 +- test/output/bandClip.svg | 8 +- test/output/bandClip2.svg | 8 +- test/output/contourVapor.svg | 8 +- test/output/differenceFilterX.svg | 18 +-- test/output/differenceFilterY1.svg | 18 +-- test/output/differenceFilterY2.svg | 18 +-- test/output/differenceX.svg | 18 +-- test/output/differenceY.svg | 18 +-- test/output/differenceY1.svg | 18 +-- test/output/differenceYClip.svg | 8 +- test/output/differenceYClipVariable.svg | 8 +- test/output/differenceYConstant.svg | 18 +-- test/output/differenceYCurve.svg | 18 +-- test/output/differenceYNegative.svg | 14 +- test/output/differenceYOrdinal.svg | 18 +-- test/output/differenceYOrdinalFlip.svg | 18 +-- test/output/differenceYRandom.svg | 18 +-- test/output/differenceYReverse.svg | 18 +-- test/output/differenceYVariable.svg | 38 +++-- test/output/differenceYZero.svg | 18 +-- test/output/hexbin.svg | 8 +- test/output/hexbinFillX.svg | 8 +- test/output/hexbinIdentityReduce.svg | 8 +- test/output/hexbinR.html | 8 +- test/output/hexbinText.svg | 8 +- test/output/hexbinZ.html | 8 +- test/output/hexbinZNull.svg | 8 +- test/output/penguinDensityFill.html | 8 +- test/output/penguinDensityZ.html | 8 +- test/output/projectionClipBerghaus.svg | 8 +- test/output/rasterVaporEqualEarth.svg | 8 +- .../rasterVaporEqualEarthBarycentric.svg | 8 +- test/output/roundedRectNegativeX.html | 8 +- test/output/roundedRectNegativeY.html | 8 +- test/output/roundedRectNegativeY1.html | 8 +- test/output/trafficHorizon.html | 8 +- test/output/usStateCapitalsVoronoi.svg | 13 +- test/output/usStateCentroidVoronoi.svg | 13 +- test/output/usStateGeoCentroidVoronoi.svg | 13 +- test/output/youngAdults.html | 138 +++++++++--------- 44 files changed, 324 insertions(+), 400 deletions(-) diff --git a/src/style.js b/src/style.js index 0478170da2..0c2b931ec7 100644 --- a/src/style.js +++ b/src/style.js @@ -311,53 +311,25 @@ export function maybeClip(clip) { return clip; } -function clipDefs({ownerSVGElement}) { - const svg = select(ownerSVGElement); - const defs = svg.select("defs.clip"); - return defs.size() ? defs : svg.insert("defs", ":first-child").attr("class", "clip"); -} - // Note: may mutate selection.node! function applyClip(selection, mark, dimensions, context) { let clipUrl; const {clip = context.clip} = mark; switch (clip) { case "frame": { - const clips = context.clips ?? (context.clips = new Map()); - if (!clips.has("frame")) { - const {width, height, marginLeft, marginRight, marginTop, marginBottom} = dimensions; - const id = getClipId(); - clips.set("frame", id); - clipDefs(context) - .append("clipPath") - .attr("id", id) - .append("rect") - .attr("x", marginLeft) - .attr("y", marginTop) - .attr("width", width - marginRight - marginLeft) - .attr("height", height - marginTop - marginBottom); - } + // Wrap the G element with another (untransformed) G element, applying the + // clip to the parent G element so that the clip path is not affected by + // the mark’s transform. To simplify the adoption of this fix, mutate the + // passed-in selection.node to return the parent G element. selection = create("svg:g", context).each(function () { this.appendChild(selection.node()); selection.node = () => this; // Note: mutation! }); - clipUrl = `url(#${clips.get("frame")})`; + clipUrl = getFrameClip(context, dimensions); break; } case "sphere": { - const clips = context.clips ?? (context.clips = new Map()); - const {projection} = context; - if (!projection) throw new Error(`the "sphere" clip option requires a projection`); - if (!clips.has("projection")) { - const id = getClipId(); - clips.set("projection", id); - clipDefs(context) - .append("clipPath") - .attr("id", id) - .append("path") - .attr("d", geoPath(projection)({type: "Sphere"})); - } - clipUrl = `url(#${clips.get("projection")})`; + clipUrl = getProjectionClip(context); break; } } @@ -370,6 +342,35 @@ function applyClip(selection, mark, dimensions, context) { applyAttr(selection, "clip-path", clipUrl); } +function memoizeClip(clip) { + const cache = new WeakMap(); + return (context, dimensions) => { + let url = cache.get(context); + if (!url) { + const id = getClipId(); + select(context.ownerSVGElement).append("clipPath").attr("id", id).call(clip, context, dimensions); + cache.set(context, (url = `url(#${id})`)); + } + return url; + }; +} + +const getFrameClip = memoizeClip((clipPath, context, dimensions) => { + const {width, height, marginLeft, marginRight, marginTop, marginBottom} = dimensions; + clipPath + .append("rect") + .attr("x", marginLeft) + .attr("y", marginTop) + .attr("width", width - marginRight - marginLeft) + .attr("height", height - marginTop - marginBottom); +}); + +const getProjectionClip = memoizeClip((clipPath, context) => { + const {projection} = context; + if (!projection) throw new Error(`the "sphere" clip option requires a projection`); + clipPath.append("path").attr("d", geoPath(projection)({type: "Sphere"})); +}); + // Note: may mutate selection.node! export function applyIndirectStyles(selection, mark, dimensions, context) { applyClip(selection, mark, dimensions, context); diff --git a/test/output/aaplBollingerCandlestick.svg b/test/output/aaplBollingerCandlestick.svg index 56c9d9f6fd..b8a7f75915 100644 --- a/test/output/aaplBollingerCandlestick.svg +++ b/test/output/aaplBollingerCandlestick.svg @@ -1,9 +1,4 @@ - - - - - + + + diff --git a/test/output/aaplCloseClip.svg b/test/output/aaplCloseClip.svg index 9fcad0e9e3..61820798db 100644 --- a/test/output/aaplCloseClip.svg +++ b/test/output/aaplCloseClip.svg @@ -1,9 +1,4 @@ - - - - - + + + diff --git a/test/output/armadillo.svg b/test/output/armadillo.svg index 4497b1d395..4c9e4ee088 100644 --- a/test/output/armadillo.svg +++ b/test/output/armadillo.svg @@ -1,9 +1,4 @@ - - - - - + + + diff --git a/test/output/bandClip.svg b/test/output/bandClip.svg index fb9612a4d4..8d481ce295 100644 --- a/test/output/bandClip.svg +++ b/test/output/bandClip.svg @@ -1,9 +1,4 @@ - - - - - + + + A diff --git a/test/output/bandClip2.svg b/test/output/bandClip2.svg index 4ba9ec4d1c..06d376c3bc 100644 --- a/test/output/bandClip2.svg +++ b/test/output/bandClip2.svg @@ -1,9 +1,4 @@ - - - - -

…by age and sex. Data: Eurostat

Y25-29 - - - - -

…by age and sex. Data: Eurostat

 - + - - + + - - + + - + - + - - + + - - + + - + - + - - + + - - + + - + - + - - + + - - + + - + - + - - + + - - + + - + - + - - + + - - + + - + - + - - + + - - + + - + - + - - + + - - + + - + - + - - + + - - + + - + - + - - + + - - + + - + + + + - + - + - + - + - + From 6509206400a0f3391ff88d2839cc0bad89b006c1 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Sun, 4 Aug 2024 21:19:51 -0400 Subject: [PATCH 4/4] remove unused maybeClip --- src/style.js | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/style.js b/src/style.js index 0c2b931ec7..94e4742ee0 100644 --- a/src/style.js +++ b/src/style.js @@ -3,7 +3,7 @@ import {create} from "./context.js"; import {defined, nonempty} from "./defined.js"; import {formatDefault} from "./format.js"; import {isNone, isNoneish, isRound, maybeColorChannel, maybeNumberChannel} from "./options.js"; -import {keyof, keyword, number, string} from "./options.js"; +import {keyof, number, string} from "./options.js"; import {warn} from "./warnings.js"; export const offset = (typeof window !== "undefined" ? window.devicePixelRatio > 1 : typeof it === "undefined") ? 0 : 0.5; // prettier-ignore @@ -302,15 +302,6 @@ export function* groupIndex(I, position, mark, channels) { } } -// TODO Accept other types of clips (paths, urls, x, y, other marks…)? -// https://github.com/observablehq/plot/issues/181 -export function maybeClip(clip) { - if (clip === true) clip = "frame"; - else if (clip === false) clip = null; - else if (clip != null) clip = keyword(clip, "clip", ["frame", "sphere"]); - return clip; -} - // Note: may mutate selection.node! function applyClip(selection, mark, dimensions, context) { let clipUrl;