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

RFC: make horizontal lines render in alignment with monitor pixels #169

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

briancappello
Copy link
Contributor

Hi. I noticed that horizontal lines were often rendering in a "blurry" way, and this PR is the result of my trip down the rabbit hole of SVG rendering. (See this answer on StackOverflow, and the comment on it.)

The short of it is, horizontal/vertical lines should be drawn on half pixel values, instead of integer pixel values. You can see upstream d3 does this when drawing axes (link to relevant source code).

If you look at my changes, you'll notice that for drawing the crosshairs I used -0.5, while for indicator plots I used +0.5. I don't have a logical explanation for you there, other than that visually, those modifications seem to reliably give the desired result. (Yea, kinda hand wavy... I wish I had a better understanding of why, but, no such luck :( To see what I mean, compare the overbought/oversold lines to the axis ticks d3 draws, and the crosshair line to the crosshair cursor the OS draws. It helps if you turn off dasharray/opacity styles.)

Other comments:

  • Vertical lines seem to already be correct. Perhaps a side effect of using d3.scale for positioning? Did not investigate.
  • I didn't clean up the tests, because, well, these few changes break a very disproportionate number of them and I wanted to hear your thoughts before spending any more time on this particular issue.

Thanks!

@andredumas
Copy link
Owner

Very nice @briancappello. You've been very busy! Yes the SVG rendering is a very deep rabbit hole indeed. I've never really noticed enough to look into it. I will give this change a try.

Side note: I'm surprised with the link to d3 v4, I observed after doing the v4 update, axis didn't look as 'good' as they did for d3 v3. I guessed it was due to the css changes but never looked closely into it...

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