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

Plot: Line styles #482

Merged
merged 10 commits into from
Jul 6, 2021
Merged

Plot: Line styles #482

merged 10 commits into from
Jul 6, 2021

Conversation

EmbersArc
Copy link
Contributor

@EmbersArc EmbersArc commented Jun 12, 2021

Closes #474 and closes #524.

@emilk
Copy link
Owner

emilk commented Jul 1, 2021

Tested, and it looks so pretty! I'm sorry I'm so slow to review, and I don't have the time right now either.

The dotting and dashing is such a nice feature that it would be nice to expose it outside of the plotter, i.e. add it to epaint. Maybe part of a constructor for Shape (Shape::dashed(points, dash_length, stroke))? It could internally call the dashes_from_line function and return a Shape::Vec.

In a more distant future the LineStyle could even become a member of Stroke. The dotting/dashing could then be done by the tessalator directly, which could save on some allocations. Not for this PR though!

@EmbersArc
Copy link
Contributor Author

@emilk Thanks for the suggestions, I've moved the functions to epaint.

@EmbersArc EmbersArc requested a review from emilk July 3, 2021 14:10
points: &[Pos2],
stroke: impl Into<Stroke>,
dash_length: f32,
dash_gap_ratio: f32,
Copy link
Owner

Choose a reason for hiding this comment

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

dash_gap_ratio deserves an explanation. Length of dash divided by length of the gap between them? it is a bit of a weird API imho. dash_length and gap_length seems simpler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied in 5418cd7.

if highlight {
stroke.width *= 2.0;
}
dashes_from_line(&line, *px, stroke, shapes);
let golden_ratio = (5.0_f32.sqrt() - 1.0) / 2.0; // 0.61803398875
Copy link
Owner

Choose a reason for hiding this comment

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

hint: let golden_ratio = (5.0_f32.sqrt() + 1.0) / 2.0; // 1.61803398875 to avoid the division later :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, but made redundant by the other change :)

@EmbersArc EmbersArc requested a review from emilk July 6, 2021 18:09
@emilk emilk merged commit 7c5a2d6 into emilk:master Jul 6, 2021
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.

HLines and VLines cannot have custom stroke colors Plot: Dashed and dotted lines
2 participants