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

waffle tips #2132

Merged
merged 17 commits into from
Nov 21, 2024
Merged

waffle tips #2132

merged 17 commits into from
Nov 21, 2024

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Aug 12, 2024

Positions the tips on waffle marks by “hacking” the x1, x2, y1 and y2 channels, replacing them with the centroid of each waffle, in pixel space, in an initializer after the stack transform runs.

This kind of works, but it's not satisfying at the moment, because the mark has too little control on where the tips should appear. Currently it can only set px, py to say where the tip should be triggered, but the tip's position can only be controlled by "data" channels (x, y, x1, x2, y1, y2), read by the derive function.

TODO:

  • make it less hacky (possibly by allowing px, py to also drive to position of the tip, not just of the pointer?, or by making derive aware of the scaled values (which we could then hack similarly, instead of hacking the source channels)
  • make the shape (and centroid) computation happen in the initializer instead of render? would feel cleaner, and would allow to make a tip mark with the same geometry but without actually calling the waffle mark.
  • more tests (waffleX, facets…)
  • restore the y channel in the tip contents.
  • [ ] allow tip: {maxRadius: Infinity} (would be welcome in the waffleTip test) (tracked as Erroneous type definition for the tip {maxRadius} option #2134 since it's not linked to this mark)

closes #2129

@mbostock
Copy link
Member

A quick note because I’m not sure I have the bandwidth to work on this now, but I think this should work more like the implicit centroid transform for the geo mark. For one thing, we don’t want the waffle mark to do anything if the tip option isn’t being used. And for two, it might be possible to move this transform/initializer onto the derived tip mark by transforming the tip option (that gets passed through as options to the tip mark), rather than doing the transform on the waffle mark itself.

@Fil Fil force-pushed the fil/waffle-tips branch from 819e3f2 to 7dcccea Compare August 14, 2024 16:35
@Fil Fil marked this pull request as ready for review August 14, 2024 16:47
@Fil Fil requested a review from mbostock August 14, 2024 16:47
@Fil
Copy link
Contributor Author

Fil commented Aug 15, 2024

Would it be interesting to have a way for an initializer to suppress an existing channel (e.g. by returning channels: {y1: null}) ? In this case we could censor y1 and y2, and return y alone, removing the need for the new hint.

(tracked as #2136)

@Fil
Copy link
Contributor Author

Fil commented Aug 15, 2024

Another way of having the right tooltips without the hint would be to change the pair formatting of (a, b) to display a when format(a) === format(b). I think it's a good idea in its own right, too? If the format does not discriminate between the two values of an interval, it seems better to display, say "3" than "3—3".

(tracked as #2135)

@Fil
Copy link
Contributor Author

Fil commented Aug 15, 2024

we don’t want the waffle mark to do anything if the tip option isn’t being used

I now disagree with this, because I want to highlight the particular waffle that is being moused over, and that requires the same secondary initializer to derive centroids, otherwise the highlighted waffle is decided by the corresponding bar position.

export function wafflePointer() {
  const random = d3.randomLcg(42);
  const data = Array.from({length: 100}, (_, i) => ({x: i % 3, fill: random()}));
  return Plot.plot({
    y: {inset: 12},
    marks: [
      Plot.waffleY(data, {x: "x", y: 1, fill: "#888"}),
      Plot.waffleY(data, Plot.pointer({x: "x", y: 1, fill: "fill"}))
    ]
  });
}

EDIT: I tried unsuccessfully to see if we could use the same actual centroid transform.

@Fil Fil marked this pull request as ready for review August 15, 2024 16:43
@Fil
Copy link
Contributor Author

Fil commented Aug 29, 2024

I've fixed the waffle centroids. The number of cases when you include the logic for fractional values is… insane.
Capture d’écran 2024-08-29 à 11 01 09

@mbostock mbostock mentioned this pull request Oct 19, 2024
Copy link
Member

Choose a reason for hiding this comment

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

I love how this image visually captures the surprising number of possible configurations!

@Fil
Copy link
Contributor Author

Fil commented Oct 30, 2024

For a completely different approach, we could use the spatial metaphor, and pass the path to the poi transform (#2098).

@Fil Fil mentioned this pull request Oct 30, 2024
3 tasks
@Fil Fil marked this pull request as draft October 30, 2024 16:23
@Fil
Copy link
Contributor Author

Fil commented Oct 30, 2024

integrated in #2215

@mbostock
Copy link
Member

A challenge I have reviewing this PR is that I don’t feel we’ve developed principles to guide these changes. This “fixes” the tip option and/or the pointer transform for the waffle mark, but in what sense was it “broken”? What is contract that all marks must adhere to, and what should users’ expectations be around how the tip option and the pointer transform work?

In 0.6.16, the waffle mark in effect masquerades as a bar mark with respect to pointing and tips. For example with waffleY, there’s typically a band scale on x and a linear scale on y and the waffle mark is defined by three values [x, y1, y2]. While in many cases approximating a waffle as a bar is “good enough” there are obviously lots of cases where it isn’t a good approximation which leads to confusing behavior.

Consider this simple example:

untitled - 2024-11-20T172228 394

Plot.waffleY([0.5, 2, 4], {x: null, fill: Plot.indexOf}).plot({color: {type: "categorical"}})

(Immediately putting aside another apparent issue, which is that the y-scale only extends from [0, 6.5], but the waffle visually extends to y = 7.5… that’s because here each cell of the waffle is 3 y-units tall, as there are three columns. We’d need the waffle mark to provide a hint to the y scale that the domain needs to be taller than the apparent value from the y1 and y2 channels.)

For the purposes of pointing, this is treated as a barY mark:

untitled - 2024-11-20T172610 824

Plot.barY([0.5, 2, 4], {x: null, fill: Plot.indexOf}).plot({color: {type: "categorical"}})

(Immediately putting aside another apparent issue, which is that when using the tip option here, the pointer must be within 40 pixels horizontally of the bar centroid, so there are large parts of the chart where you can hover over a bar and not see any tooltip.)

I can’t draw the exact Voronoi shape easily, but I can show the “bar centroids” of the waffle marks to get a sense of where the pointer is activated, so we can see the (already established) problem:

untitled - 2024-11-20T173318 330

Plot.plot({
  color: {
    range: ["#4269d0", "#efb118", "#ff725c"]
  },
  marks: [
    () => svg`<defs>
  <pattern id="pat1-0" x="0" y="0" width="10" height="10" patternUnits="userSpaceOnUse" stroke="#4269d0" stroke-width="1">
    <line x1="2" y1="-2" x2="16" y2="12" />
    <line x1="-8" y1="-2" x2="6" y2="12" />
  </pattern>
  <pattern id="pat1-1" x="0" y="0" width="10" height="10" patternUnits="userSpaceOnUse" stroke="#efb118" stroke-width="1">
    <line x1="2" y1="-2" x2="16" y2="12" />
    <line x1="-8" y1="-2" x2="6" y2="12" />
  </pattern>
  <pattern id="pat1-2" x="0" y="0" width="10" height="10" patternUnits="userSpaceOnUse" stroke="#ff725c" stroke-width="1">
    <line x1="2" y1="-2" x2="16" y2="12" />
    <line x1="-8" y1="-2" x2="6" y2="12" />
  </pattern>
</defs>`,
    Plot.waffleY([0.5, 2, 4], { x: null, fill: (_, i) => `url(#pat1-${i})`, stroke: Plot.indexOf, gap: 4 }),
    Plot.dot([0.5, 2, 4], Plot.stackY({ x: null, y: Plot.identity, stroke: Plot.indexOf, r: 6 }))
  ]
})

The blue dot is the confusing one here, since it’s triggered outside of the visible extent of the mark.

I have to run, but some parting quick thoughts:

  • The geo mark has similar problems with respect to the tip option and the pointer transform, and we’ve already done some work there. I’d like the guiding principles to apply to both marks.
  • The geo mark works by apply an implicit centroid transform when the tip option is used. I imagine a similar approach could be taken for the waffle mark, where a centroid (in screen space) is somehow provided to the derived tip.
  • The geo mark doesn’t work out of the box with the pointer transform. You have to explicitly specify a centroid transform if you want to use the pointer transform with the geo mark.
  • The fact that the waffle mark works with the tip and pointer transform at all is somewhat coincidental; it falls out of how the waffle mark was implemented on top of the bar mark.
  • I think we need to develop some guiding principles on what channels should “mean”, at the very least for the standard x and y channels used by most marks; is the waffle mark abusing these channels since it looks differently?
  • Alternatively, we can say that it’s fine as-is, and even if the waffle mark supports the tip option, maybe you still have to specify a centroid or poi transform if you want to use the pointer transform.
  • Alternatively, maybe the waffle mark should produce a geometry channel and a downstream pointer transform should be able to consume that, rather than needing x and y channels?

Needs more thought!

@Fil
Copy link
Contributor Author

Fil commented Nov 21, 2024

These comments point to a common difficulty in that our marks don't have an abstract representation of the "visual space" they occupy.

A rect is simple (x, y, width and height), but even for the humble dot, it's not as obvious. For example the pointer does not care for the dot's shape or radius — if two dots are overlap with different radii, there's no way to build the “natural” interaction where you would select the smallest if you're closer to the center. (In practice the "closest center" heuristic works relatively well, though.)

For text, we have close to nothing… we don't even know how much space exactly it occupies on screen.

Another difficult situation is tips for clipped marks, that might happen at places where the mark is clipped or hidden #2213

The geo mark works by apply an implicit centroid transform when the tip option is used.

Should waffles be treated as a kind of geo mark? It was the route I tried initially, but first it depended on #2098 because waffles often have "two polygons" and their centroid is wrong — and second it meant doing more/slower computation than this closed form solution. What I did here in other words was to think about them as a geo shape, and derive their poi analytically.

But I'd also want marks to expose a function that would return "a multipolygon (approximately) occupied by the shape in screen space", and that could be used to compute occlusion, interaction, etc. (That would be the “contract”, in a sense: a mark would say “I'm showing something here, if you point here it makes sense to point to me, I'm taking this space so please maybe don't put other stuff here if you want me to be visible…”)

I think this whole issue also relates, at a higher-level, to @joshpoll’s work about gestalt and relations between marks.

@mbostock
Copy link
Member

For now I think the simpler contract for marks would be to provide a “representative point” (or point of interest), since that this point could be used by the pointer transform and by extension the tip option (the derived tip mark). For marks that use the x, y, x1, y1, x2, and y2 channels in the “normal” way, they wouldn’t need to do anything and the pointer transform and tip mark would use the existing anchorX and anchorY logic:

export function anchorX({x1: X1, x2: X2, x: X = X1}, cx) {
return X1 && X2 ? (i) => (X1[i] + X2[i]) / 2 : X ? (i) => X[i] : () => cx;
}
export function anchorY({y1: Y1, y2: Y2, y: Y = Y1}, cy) {
return Y1 && Y2 ? (i) => (Y1[i] + Y2[i]) / 2 : Y ? (i) => Y[i] : () => cy;
}

But under this contract the geo mark would need to provide x and y channels — ideally lazily computed, only if needed by a pointer transform or derived tip. Or alternatively the pointer transform and derived tip would need to know about the geometry channel, and could then compute a centroid or poi itself. The question is whether we want the rendering mark to be responsible for identifying its representative point, or if we want a more expressive representation (maybe even a signed distance field!) for pointing and to put the smarts in the pointer transform and tip mark. But I think we can start by having the marks supply a representative point. Easier to expand the representation in the future than to contract it.

Which means the waffle mark would then be responsible for providing a representative point (since it uses the standard channels in a way that’s not well-aligned with its visual representation). That’s further complicated by the waffle mark building on top of the stack transform, which needs to use x, y, x1, y1, x2, and y2. So, probably we need to introduce new channels so marks can expose their representative point more explicitly. The obvious choice there would be to use the px and py channels since these are already used by the pointer transform. But I’m not sure if that will work; that might have some funny interaction… we’ll see.

I also want to fix the cropping issue caused by the y-scale not extending sufficiently to cover the extent of the waffle. I think for that we’ll need either another channel (y3?) bound to the y scale or a hint attached to an existing channel to force the domain to extend slightly.

@mbostock
Copy link
Member

mbostock commented Nov 21, 2024

Maybe having the pointer transform and tip mark understand the geometry channel could be simpler? I want to try that too.

@mbostock
Copy link
Member

Ah, okay, interesting. So in this PR you use a new polygon channel rather than the existing geometry channel, but you also redefine the x and y channels to contain the waffle centroid so that the downstream pointer transform and derived tip mark can use them. Which is clever because it means we don’t have to make the pointer transform and tip mark capable of consuming the geometry channel, and we can still use the standard x and y channels because the waffle initializer runs after the stack transform.

@Fil
Copy link
Contributor Author

Fil commented Nov 21, 2024

I wish I'd been clever, but the truth is that it's the unit tests that guided me (🙏 ): every other thing I tried failed because of the many assumptions that make it impossible to use px, x1, x2, x without breaking something.

src/marks/waffle.js Outdated Show resolved Hide resolved
@mbostock mbostock marked this pull request as ready for review November 21, 2024 17:34
src/marks/waffle.js Outdated Show resolved Hide resolved
Comment on lines 203 to 281
export function waffleX(data, options = {}) {
export function waffleX(data, {tip, ...options} = {}) {
if (!hasXY(options)) options = {...options, y: indexOf, x2: identity};
return new WaffleX(data, maybeStackX(maybeIntervalX(maybeIdentityX(options))));
return new WaffleX(data, {tip, ...maybeStackX(maybeIntervalX(maybeIdentityX(options)))});
Copy link
Member

Choose a reason for hiding this comment

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

What is this change doing? It’s not covered by tests, and I don’t notice any difference in manual testing.

Copy link
Member

@mbostock mbostock Nov 21, 2024

Choose a reason for hiding this comment

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

Ah, so it’s removing the default behavior of the stack transform when the tip option is true, which is to use pointer: "y" on on waffleX, and pointer: "x" on waffleY. I get why you’re doing it, but I’m not sure it’s an improvement — in a way it makes things worse by having lots of dead space where the tip isn’t activated even though you’re hovering over a waffle; you need to be within 40px of the waffle centroid for it to activate, but it isn’t always obvious where that centroid is, and why the tip isn’t activating.

I wonder if this means we should set the maxRadius to Infinity for waffle marks (just so it’s clear that something is happening), or if it means the overall approach of choosing a single representative point for a mark is insufficient, and we need to supply the pointer transform with a full geometry so it can do e.g. a signed distance check.

Copy link
Member

@mbostock mbostock Nov 21, 2024

Choose a reason for hiding this comment

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

Perhaps another possibility is to change the pointer behavior when x (or y) is a band scale. But that’s a bit weird because the provided x here is already in screen space… (and the x scale may not even exist).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think setting maxRadius to infinity is the correct interim choice. But it would be neat to have a full geometry (both for geo and waffles) in the future.

Note that, for the geo mark, one thing that works well currently with a centroid-based pointer — and will be harder to solve with full geometry —, is to make "small countries" discoverable even when their shape is very small (islands are easy, the hard part is small countries surrounded by larger countries).

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I’m not sure how we’d set maxRadius to Infinity for waffle marks. We can do it on the tip option for the derived tip, but that wouldn’t affect how the pointer transform behaves when applied to the waffle mark. 🤔 It kind of makes me think the waffle mark should supply a geometry channel, and then the pointer transform should compute the signed distance to the geometries… (That would also let us fix pointing at bars wider than 40px, too.)

@@ -288,7 +289,7 @@ export interface MarkOptions {
title?: ChannelValue;

/** Whether to generate a tooltip for this mark, and any tip options. */
tip?: boolean | TipPointer | (TipOptions & {pointer?: TipPointer});
tip?: boolean | TipPointer | (TipOptions & PointerOptions & {pointer?: TipPointer});
Copy link
Member

Choose a reason for hiding this comment

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

This allows e.g. tip: {maxRadius: Infinity}.

@mbostock mbostock merged commit eb85419 into main Nov 21, 2024
1 check passed
@mbostock mbostock deleted the fil/waffle-tips branch November 21, 2024 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Waffle mark tooltips are always on axis line
2 participants