-
-
Notifications
You must be signed in to change notification settings - Fork 313
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
add linecaps (GLMakie, CairoMakie) #2536
Closed
Closed
Changes from 12 commits
Commits
Show all changes
54 commits
Select commit
Hold shift + click to select a range
fe0043f
prototype linecaps in shader
ffreyer 5c14b08
fix triangle
ffreyer 1035f9a
fix antialiasing & add uniforms
ffreyer 2868301
add line shortening and mirrored triangle
ffreyer ac54023
only shorter discontinous lines
ffreyer 58238c4
fix printing
ffreyer 21c260d
make linewidth and linecap_length per vertex
ffreyer 41a200f
add linecaps to linesegments
ffreyer 2103da7
fix cut off triangles
ffreyer 8af7d28
minor cleanup
ffreyer fe26ed7
improve triangle fix
ffreyer df4cc82
get linestyles working
ffreyer 0ed7fd3
add linecap attribute
ffreyer 0bcc438
add refimg test
ffreyer 4b3f232
update NEWS
ffreyer 344d5ba
add linecap to CairoMakie
ffreyer a0e8004
update NEWS
ffreyer d571ae9
exclude linecap test from WGLMakie
ffreyer afc879a
minor cleanup
ffreyer 1503d8f
Merge branch 'master' into ff/linecaps
ffreyer 6ac6bf7
fix test failure
ffreyer 5e206a9
simplify fragment shader (drop triangle caps)
ffreyer ff118ea
Merge branch 'master' into ff/linecaps
ffreyer 61802c4
Merge branch 'master' into ff/linecaps
SimonDanisch a4ee760
remove unnecessary lastlen and maxlen
ffreyer 7fd4efb
update recipes (linecap passthrough & tweaks)
ffreyer 3916802
use lines without point dublication
ffreyer 3fe7351
fix test failures
ffreyer fa7cbd9
update MakieLayout
ffreyer 812ff3f
fix rebase
ffreyer 8ff1bed
fix typo
ffreyer d05b002
update legendelements
ffreyer 401e1d8
add linecap examples
ffreyer 035ba35
mention linecap in poly docstring
ffreyer 3da3d96
Merge branch 'master' into ff/linecaps
ffreyer 3e92c0e
no linecap for latexstrings
ffreyer ef2bd88
fix typo
ffreyer 918d268
remove linecap attribute
ffreyer c368811
fix convert error
ffreyer 33f36b4
fix another typo
ffreyer 5054952
and another one
ffreyer e8e0943
Merge branch 'master' into ff/linecaps
ffreyer 11a12ba
make test name more unique
ffreyer a0b2a1b
Merge branch 'master' into ff/linecaps
ffreyer ef78de7
rework linecap_length into more general length_offset
ffreyer bbf3445
add reference test for offsets
ffreyer 06afc32
add length_offset to CairoMakie
ffreyer 29e9c1d
add offsets to WGLMakie and exclude test
ffreyer 22e42a4
add entry for length_offset
ffreyer 6ada0f6
remove experimental stuff
ffreyer e9a853b
fix screen to clip transformation
ffreyer 835b582
Merge branch 'master' into ff/linecaps
ffreyer f99ccaf
Merge branch 'master' into ff/linecaps
ffreyer add06f8
adjust linestyle spacing with linecaps
ffreyer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the comment in the scatter shader
Makie.jl/GLMakie/assets/shader/distance_shape.frag
Line 26 in 4dab995
I assume we should find a way around this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did some benchmarking with
I get the same ~5% CI got for this branch vs master.
I tried hacking together a two pass version which does the old lines in the first pass and caps in the second pass. This way we can swap out the
flat in int f_type
foruniform int linecap
anduniform int render_pass
. This ends up being about 10% slower than master though, doing both passes. Doing just the line pass is 3-4% slower.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's ok for linecaps! Can we optionally disable it and get the 5% back for high perf use cases? We could also think about having a different recipe for that though 🤷 I think the default should make good lines, and performance should be the special case!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reducing the number of options in the fragment shader (5e206a9) helped a bit ... I think. I'm never sure with gpu benchmarking. When I tested it I got a bunch of benchmarks that were ~2% slower than master and then a bunch up to 5% slower 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran them again without background processes and I'm getting ~2% now (812ff3f)