stable clip paths

[Fil]

new description!

By using (and reusing) the same clip-path when two renders are clipped with the same shape, we get:

• smaller files
• better performance during pointer interactions

Tested manually against #1623. The only reason why I based this PR on #1623 originally is that this forms a good test case: demo); for the same reason, merging this will change the snapshots for the new tests added in #1623, but they are independent concerns.

[Fil]

This seems ready now.

[Fil]

rebased

[mbostock]

These probably shouldn’t be exposed publicly on the context. Maybe use a WeakMap keyed by the context instead of assigning to a new property on the context? (Also note that the Context interface is publicly documented in context.d.ts.)

[mbostock]

Are there only at most two keys in the clip cache (frame and projection)?

[Fil]

Currently yes, these are the only keys; related: #1109

[Fil]

This was clearly over engineered. I don't think we even need a WeakMap, as we could just do a quick check through the DOM with querySelector; we just need to pass the type of clip (frame, sphere) in a class or in the id, then select against it e.g. with [id^=plot-clip-sphere-].

[mbostock]

I don't think we even need a WeakMap, as we could just do a quick check through the DOM with querySelector

Why do you think that (going through the DOM) is simpler or faster than a WeakMap?

[Fil]

Using the DOM feels simpler [information-wise], in the sense that we wouldn't need to track the association of type of clip and clipPath in parallel, as we could just look it up. (It's hard to remember what I had in mind a year ago, though — I think I was probably trying to change the output as little as possible.) But I'm happy with this version too.

[mbostock]

Using the DOM feels simpler [information-wise], in the sense that we wouldn't need to track the association of type of clip and clipPath in parallel, as we could just look it up.

I don’t follow that the DOM is simpler. In the DOM approach, you have to use some (serializable) identifier in the DOM such as an id prefix ([id^=plot-clip-sphere-]), and then use querySelector to find it. In the WeakMap approach, you use a symbolic identifier (the reference to the WeakMap) and you use map.get to find it. The main difference I see is that the WeakMap approach is completely symbolic and hidden, whereas the DOM approach exposes its implementation, and requires pattern matching or other messiness via querySelector since it’s not a symbolic reference.

[mbostock]

The way I think about the WeakMap is that there’s a context.frameClip and context.projectionClip property that are lazily set in getFrameClip and getProjectionClip. The only thing the WeakMap provides is that these properties aren’t publicly exposed.

[Fil]

Makes sense; you're right that there is as much tracking in both cases, just differently. To be clear my comment about over engineering was about the original introduction of the parallel tracking with Map; I could have used a className instead, but indeed it would have been visible in the output.