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

Sankey link/node pixel level alignment #2021

Merged
merged 6 commits into from
Sep 20, 2017
Merged

Conversation

monfera
Copy link
Contributor

@monfera monfera commented Sep 19, 2017

Tweaking the alignment (OOM: one pixel) between nodes and links. For past considerations, see #2017 (comment).

The result of this is generally better alignment, while preserving even box borders and multiedges with little to no overlap artifacts. For examples:

image

Magnified:
image

Vertical:
image

Mike Bostock's example:
image

@monfera
Copy link
Contributor Author

monfera commented Sep 20, 2017

The changed baselines show disappearing (and in some cases, appearing) node boundaries. On a retina screen, things show up fine:

image

I'll check it on devicePixelRatio === 1 devices tomorrow.

@monfera
Copy link
Contributor Author

monfera commented Sep 20, 2017

@alexcjohnson I tried a number of approaches - including stuff like dropping the path pixel fractions with things like var result = d.sankey.link()(d.link).replace(/\.[^( |Z|L|C)]*/g, ''); - but the most robust one is the avoidance of pixel truncation and use of geometricPrecision. I'll have a couple of codepen examples in a bit to eyeball if it's fine, and if so, I'll update the offending couple of jasmine tests (minor shape shifts).

@monfera
Copy link
Contributor Author

monfera commented Sep 20, 2017

@alexcjohnson here are two pens that - for now - point to a custom build with the shape-rendering: geometricPrecision option:

Does it look fine?

@monfera
Copy link
Contributor Author

monfera commented Sep 20, 2017

Also, some screenshot examples:

vertical:
image

minimal-value version (where you saw the most visible misalignment): https://codepen.io/monfera/full/EwPrKP/
image

@alexcjohnson
Copy link
Collaborator

Nice, I like it, thanks! 💃 once tests pass.

@monfera monfera merged commit 2cc7804 into master Sep 20, 2017
@monfera monfera deleted the sankey-micro-alignment branch September 20, 2017 20:50
@monfera
Copy link
Contributor Author

monfera commented Sep 20, 2017

Note: this item surfaced during #2017 of which the original driver was #2014.

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.

2 participants