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

Filled link path #2

Merged
merged 1 commit into from
Apr 28, 2017
Merged

Filled link path #2

merged 1 commit into from
Apr 28, 2017

Conversation

monfera
Copy link

@monfera monfera commented Apr 24, 2017

Purports to solve these items:

  • the circular artifacts around nodes when space for links is constrained, prominent with thick links
  • partial overlap between adjacent links when using multiedges (multiple lines between the same pair of nodes, see Stable sorting links for better parallel edge handling d3/d3-sankey#19)
  • allows styling of the link fill as well as the link stroke separately

A not particularly useful example, showing alignment of multi-edge link boundaries without gap or overlap (to reproduce the example, merge d3#19 also):
image

The same, without styling the link path stroke, i.e. just the fill:
image

The same, using a translucent black, to better show there are no disturbing gaps or overlaps - the same pairs of nodes are still multi-edges as above, and the nodes are moved vertically to stress test the link curves:
image

There's also some loss:

  • when the link is thick, and slope is steep, the link appears to be thinner in the middle than at the nodes - while layouting tries to avoid steep drops of strong connections, it's still an artifact that the stroke-width based logic doesn't have
  • compatibility is broken - we could introduce a separate method for the fill based link but in D3, usually code size consideration is important (also, it's in the D3 org and made by Mike Bostock but isn't part of D3 proper)
  • it's no longer possible to tweak the former path width to subtly play with link width, or use a constant stroke-width instead of the usual d.link.dy

@monfera monfera requested a review from etpinard April 24, 2017 13:23
@monfera monfera self-assigned this Apr 24, 2017
@etpinard
Copy link

etpinard commented Apr 24, 2017

Probably best if @alexcjohnson reviewed this, as he requested this change (I think).

@monfera monfera requested review from alexcjohnson and removed request for etpinard April 24, 2017 14:30
@monfera
Copy link
Author

monfera commented Apr 24, 2017

@alexcjohnson this is a before/after comparison: Move 'Nuclear' up/down and close to 'Thermal generation' to see artifacts here and no artifacts here.

@alexcjohnson
Copy link
Collaborator

Looks great. I guess in an ideal world we would smoothly transition between constant perpendicular thickness (which is what you get from the previous behavior, and I think gives the most "correct" visual significance to the link value) and constant vertical thickness (this behavior, which results in thinner perpendicular thickness at steep angles) but that seems like a fairly complicated optimization problem, while this simple solution has so much going for it: fixes fat-link artifacts, avoids overlaps in multi-part links, and allows link outline strokes. So I'd call this a big win!
💃

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

💃

@monfera monfera merged commit ba90e7a into master Apr 28, 2017
@cpsievert
Copy link

when the link is thick, and slope is steep, the link appears to be thinner in the middle than at the nodes

Perhaps due to the sine illusion?

@alexcjohnson
Copy link
Collaborator

when the link is thick, and slope is steep, the link appears to be thinner in the middle than at the nodes

Perhaps due to the sine illusion?

Exactly - though I'd argue it's not an illusion, it really is thinner because what's important in terms of visually conveying "this is how much material is flowing along this link" is not the vertical width (which is what's conserved in the sine illusion) but the perpendicular width (which is conserved with the previous stroke-width approach). The vertical width doesn't, in itself, mean anything. If we ever extend this to support cycles or nodes in different orientations, this will become very important.

@monfera
Copy link
Author

monfera commented May 1, 2017

@etpinard neat, I wasn't aware of the sine illusion aspect, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants