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

occlusionY initializer #1957

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

occlusionY initializer #1957

wants to merge 7 commits into from

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Dec 21, 2023

occlusionY.mp4

quoting @visnup: I always need some occlusionX or Y 😅

For #27

TODO:

  • fix the display of y in the tip (if using tip: true), it should be the unscaled value.
  • decide whether we keep the deduplication of (y, text) pairs—this could be achieved by combining occlusionY and a group transform, resulting in cleaner code and a better separation of concerns (but more user code to write for that particular case) [code removed and replaced by group]
  • rename ("occlusion" says the opposite of what it does, and the name should be a verb); my preference is currently to call this spreadX/spreadY)

examples:

@Fil Fil requested a review from mbostock December 21, 2023 16:15
Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

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

This sounds a lot like the dodge transform and this statement isn’t quite enough to help me understand the difference:

The occlusion transform differs from the dodge transform in that it only adjusts the nodes’ existing positions.

We should draw the distinction in a tip at the top of the page, and expand it. And when you say “rearranges the x (horizontal) position” I interpret “rearranges” as “reorders” the x-coordinates, but that’s not what you mean at all.

I guess the difference is that dodgeY doesn’t take a y channel as input, it just computes y from the anchor (baseline), whereas occlusionY does take a y channel as input and adjusts those values to enforce a minimum distance, while minimizing movement and preserving order? I wonder if we could instead incorporate this into the dodge transform rather than making a new transform.

We should continue to name transforms with a verb. This isn’t “occlusion-ing” and it’s not “occluding” either because it’s trying to prevent occlusion. I think “dodge” is the best name for this type of thing. “Repel”, “offset”, “collide”, and “avoid” are other verbs that come to mind…

Your earlier occlusion helper might better be named an “occlude” transform as that one hides things! 😅

@Fil
Copy link
Contributor Author

Fil commented Dec 21, 2023

I wonder if we could instead incorporate this into the dodge transform rather than making a new transform.

Yes I had the same feeling, but didn't want to complicate things at first. It might mean renaming minDistance (to r, with a default of 5.5 instead of 11, but then it's not the same default as dodge…), and applying it on the opposite direction too? I can investigate whether that's possible. Now that we have this baseline, maybe it's worth doing this extra step.

I like "repel" a lot.

@Fil
Copy link
Contributor Author

Fil commented Dec 22, 2023

Other verbs: Fan out, Spread, Stretch, Radiate, Unfold...

@visnup
Copy link
Member

visnup commented Dec 22, 2023

I had to check but disocclude is an actual word. https://en.wiktionary.org/wiki/disocclude

That's a lot of syllables though.

@Fil
Copy link
Contributor Author

Fil commented Jan 3, 2024

I would love to unify this with the dodge transform, unfortunately I'm not seeing a lot of common ground. Maybe this needs more work.

What's different:

dodgeY occlusionY
y channel not supported required
x channel required optional
spread x + y y only
default r 3, channel minDist 11, constant
anchor supported not supported

A way to unify them would be to let dodgeY switch to the occlusionY algo when a y channel is specified, but this would mean that almost everything changes meaning:

  • what to do when x is a channel? Can we support disoccluding more (by pushing the point higher?)?
  • minDist could become r (at the price of not having the best default for text labels), but then what to do when r is a channel?
  • anchor could be ignored in that case, since we could argue that the point is anchored to its y value

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.

3 participants