Skip to content

Conversation

@monfera
Copy link
Contributor

@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 #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 #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 changed the title Filled link Filled link path Apr 24, 2017
@monfera
Copy link
Contributor Author

monfera commented Apr 24, 2017

Move 'Nuclear' up/down and close to 'Thermal generation' to see artifacts here and no artifacts here.

@monfera
Copy link
Contributor Author

monfera commented Apr 27, 2017

Over the last few days I started to think about

  • either retracting this PR
  • or making the opposite tradeoff w/ code size, i.e. retaining the stroke based link path while adding the fill based one
  • or adding a configuration option for straight lines (or parametric spline) as an alternative to the current spline

The reason is that the stroke based approach does a better job preserving a constant cross section in the direction normal to the link centerline (the path itself).

I don't currently see how to geometrically meet two goals simultaneously: 1) multiedges with neither gaps, nor overlaps, 2) constant cross section of individual edges; 3) constant cross section of the aggregation of multi-edges, 4) use of the current spline.

It is possible to elaborately create a fill based path that results in a similar, constant cross section ink coverage as the current stroke; it also looks possible to lay them out such that there are no overlaps and gaps between them; but it would require a more elaborate spline calculation mechanism that's more aware of its surroundings than the current one that depends on very few things.

Therefore the full-on option might be:

  1. have both kinds of links present, and the user of d3-sankey will make the call and
  2. have a config option for tweaking or eliminating curvature
  3. OR fully overhaul the pathmaking logic without overlaps, gaps and bulges (nice bonus: arbitrary directions) for example

image

Apparently in some version at least, Tableau makes the tradeoff similar to this PR, i.e. apparent 'thinning' of the straight middle section of the links, though similarly not disturbing here due to the not too steep angles (nice use case for multi-edges except the minor discrepancy that the color scale legend is continuous while multi-edges use discrete colors):
image

Any thoughts on this?

===================

Btw. is it the good place for discussion? TIL some of the OS maintainers prefer that there's no PR before discussion on an Issue and I've also heard of maintainers who insist on, or prefer a PR with code even for the discussion part.

@mbostock
Copy link
Member

Sorry, I haven’t been actively maintaining this library recently, and it looks like @arankek hasn’t been, either. I do want to get back to it eventually, but I don’t think I will be able to do that within the next few weeks.

@monfera
Copy link
Contributor Author

monfera commented Apr 27, 2017

Thanks @mbostock no worries, for now we're folding these couple PRs in the plotly fork, and it supports a version w/ plotly tooltips, callbacks and drag&drop. Off-label use of the forceSimulation!

@NPC
Copy link

NPC commented Apr 27, 2017

@monfera I was dealing with this a few years ago in my project, and implemented a (hacky) check for link distance vs thickness — if it is large enough then use stroke-based links, otherwise use filled links.

This may not be the best approach (and I don't even want to share my condition, it looks odd, I need to review/revise it), but back then I found that for my situation both stroke and fill methods are needed, depending on link size.

@monfera
Copy link
Contributor Author

monfera commented Apr 27, 2017

@NPC thanks, interesting approach! Maybe you have had a somewhat different set of requirements as probably the code didn't aim to support multi-edges more so than today, see the criss-crossing correct?

Undesirable gaps and overlaps appear with multi-edge stroke even with fine (4-6px) to medium (50-80px) thickness. While only Moiré is perceivable with the finest of thickness, OOM 1px, this latter example also shows the middle section of the links thinner than the area around the node, if there's a big Y difference (again, after #19 is cherrypicked, otherwise, spaghetti). Besides having the same weakness, it would add a heuristic, darken the midsection and add Moiré. Maybe you had other ways of working around these or I miss something?

The conditionalization idea in general is good, e.g. switching to fill based if multiedges are present. By doing option 1 in my above post, your heuristic could be done, or it could be driven by multiedge vs not, i.e. the heuristic would stay in userland and the d3-sankey library itself wouldn't need to make opinionated calls (it would provide the link method for both fill and stroke and user code decides).

Btw. the fill based approach is quite nice and seems to work out for us, it's just it may be too big of a departure for a general purpose central repo like this that hasn't been geared toward multi-edge support.

@NPC
Copy link

NPC commented May 1, 2017

@monfera You are correct, I am not dealing with multi-edge links at all, links between same A & B nodes are summed. That simplified my task, I see now that yours is much more advanced.

@mbostock
Copy link
Member

I feel it’s important to maintain the constant width of links.

If you want to generate a path from the outline of a stroked Bézier curve, I recommend reading Mike “Pomax” Kamermans’ Primer on Bézier Curves, and also using his Bezier.js library which implements offset curves. It should be reasonably straightforward plug the output coordinates from d3-sankey into Bezier.js; see also the implementations of d3.sankeyLinkHorizontal and d3.linkHorizontal for the smooth curves used in the examples.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants