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

LineDash and 0-width lines #68

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

chiguireitor
Copy link

Added setLineDash support to series drawing
Added support for 0 line width on series (they get skipped)

Signed-off-by: johnvillar [email protected]

Added support for 0 line width on series (they get skipped)

Signed-off-by: johnvillar <[email protected]>
smoothie.js Outdated

if (seriesOptions.lineWidth === 0) {
context.restore();
continue;
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you continue here, then the call to dropOldData call below is skipped too, and the data will build up indefinitely in the time series object.

Copy link
Author

Choose a reason for hiding this comment

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

Good catch... in fact i've already changed it a bit, as the "lineDash" property doesn't gets saved in the context.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this also prohibit using a line width of zero and a fill style in order to have a filled curve with no line?

Copy link
Author

Choose a reason for hiding this comment

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

Indeed, that would be the effect.

Just changed it to the line 783 where it makes more sense (i was setting lineWidth to 0 and stroke was still there). That should solve the problem of having 0 width lines with fillStyles.

Added a way to append data to a series via object instead of positional parameters
Added a provision to add metadata to series points
Metadata can control interpolation from point to point
Added "jump" interpolation that just moves to the series' point without drawing
Added a way to add text to a series point

Signed-off-by: johnvillar <[email protected]>
@alexcastillo
Copy link

@chiguireitor

Can you please give an example on how to use this?

@chiguireitor
Copy link
Author

chiguireitor commented Apr 30, 2016

Yeah sure, it works wonders to create graphs like this:
graph

To hide the dotted series i do:

style = ghsChart.getTimeSeriesOptions(seriesGhsAv[x])
style.lineWidth = 0

And to create them, as i create them dotted, i use this code:

ghsChart.addTimeSeries(serieAv, { strokeStyle: rgbCol, lineWidth:0, lineDash: [2, 3]})

@alexcastillo
Copy link

Very nice, thank you!
I hope this PR gets merged soon.

@chiguireitor
Copy link
Author

Good thing is, i'm actively using it in one product i'm developing, so at least it will get maintained if it doesn't gets pulled.

Ooh, also, another nifty thing, you can include text on the time series now, although i think i haven't pushed those changes yet, as it is a little rough around the edges atm. I use it to include fontawesome icons on my graphs to mark events.

@alexcastillo
Copy link

I would love that! Please push your changes when you get a chance.
Also, how would you go about making the charts responsive?

@chiguireitor
Copy link
Author

chiguireitor commented Apr 30, 2016

I'll push changes as soon as i fix some quirks.

Responsiveness is pretty hard atm, as the charts are just a canvas with some line calls, but that could be fixed, although it isn't my priority right now (still some time before i implement the cloud server for my product which needs to be responsive).

@alexcastillo
Copy link

Fair enough. Please keep me posted on any improvements. I will probably be submitting some PRs soon as I have chosen this library to visualize EEG data for my project.

@chiguireitor
Copy link
Author

Aaah EEG is a pretty nice fit for this project. Be sure to post links when it's up to peruse the goodies.

@alexcastillo
Copy link

I already have something up. You can check it out here: https://github.com/NeuroJS/openbci-dashboard

@mg1075
Copy link

mg1075 commented Nov 19, 2016

@chiguireitor - were you ever able to make smoothiecharts responsive?

@chiguireitor
Copy link
Author

Nope, didn't continue modifying it because the project didn't need anything
else besides what i achieved. However, making it responsive is kinda
complicated because canvas is not responsive friendly at times. Gotta work
it out a lot.

El vie., 18 de nov. de 2016 a la(s) 22:43, mg1075 [email protected]
escribió:

@chiguireitor https://github.com/chiguireitor - were you ever able to
make smoothiecharts responsive?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#68 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAbqXjb3UqbgDv_qJ1keY8CwzPZ8fwpvks5q_mJMgaJpZM4Hh2ks
.

John Villar

@mg1075
Copy link

mg1075 commented Nov 19, 2016

@chiguireitor - ok, thanks for the update.

drewnoakes added a commit that referenced this pull request Aug 25, 2017
This follows on from #68. Unfortunately the dashes jump back and forth
during rendering, which is quite visually annoying. Fixing this would be
quite difficult I suspect.
@drewnoakes
Copy link
Collaborator

@mg1075, smoothiecharts has been responsive for a few versions now.

@chiguireitor I took a look at the dashed line code in isolation but unfortunately it creates jerkiness in the animation, which isn't very smoooooth. In your example you possibly don't notice that because it seems that your scroll rate is so low, but for high FPS animation the dashes appear to jump forwards and backwards along the line, which I think is probably unavoidable. I pushed code for that to the linedash branch, and you can play around with it in the builder.

You have some other bits in this PR too which are worth separate review.

@chiguireitor
Copy link
Author

Nice, maybe in the future i'll make a better PR... this whole year and a half i have got more git wisdom to understand my original PR was bs.

About the dashes lines jerkiness, it's a common artifact from dashed lines in fast movement to do that, prolly not a good idea to use dashes/dotted stuff on fast moving charts.

The rest of the PR whould be in it own PR, i concur.... maybe i'll get to this eventually this year as we have another project needing smooth graphs.

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.

4 participants