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

Add :solid_pattern linestyle to allow switching between patterned and solid lines #4570

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ffreyer
Copy link
Collaborator

@ffreyer ffreyer commented Nov 5, 2024

Description

Fixes #3693
Fixes #803

This linestyle generates a solid line pattern so it renders with shaders used for linestyles but never actually stops drawing. This enables switching between a solid pattern and other patterns (e.g. :dot, :dash, etc).

We could also adjust :solid to do that, though that would make linestyle = :solid slower than nothing (and potentially different if there are any bugs/peculiarities in the linestyle shaders).

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • Added an entry in CHANGELOG.md (for new features and breaking changes)
  • Added or changed relevant sections in the documentation
  • Added unit tests for new algorithms, conversion methods, etc.
  • Added reference image tests for new plotting functions, recipes, visual options, etc.

@MakieBot
Copy link
Collaborator

MakieBot commented Nov 5, 2024

Benchmark Results

SHA: cbb5096828111f6fa63ed396a3277c7e866e9866

Warning

These results are subject to substantial noise because GitHub's CI runs on shared machines that are not ideally suited for benchmarking.

GLMakie
CairoMakie
WGLMakie

@jkrumbiegel
Copy link
Member

Would it make sense to special case in the shader when the numbers appear that correspond to :solid or :solid_pattern? To me it seems bad to have both just because it's a peculiarity of our shader setup that you can't switch. Rendering doesn't have to be slow if it picks the fast branch right away?

@ffreyer
Copy link
Collaborator Author

ffreyer commented Nov 5, 2024

I'm not sure what you mean. Currently :solid converts to nothing and results in solid line shaders. :dot, :dash, etc converts to an array which results in patterned line shaders. :solid_pattern follows the latter conversion path. There is nothing else that plays into that decision.

We could set up some logic to convert :solid to an array when patterned line shaders are chosen, but that only works if you start with :dot etc then. If you start with :solid or nothing, we'd have to predict the future

@jkrumbiegel
Copy link
Member

I mean turn :solid into an array but check that array in the shader as a special fast path. Then the users don't have to pick "fast vs slow but exchangeable"

@ffreyer
Copy link
Collaborator Author

ffreyer commented Nov 5, 2024

Hmm, that could work too, but it would require padding position of solid lines to Point4f. (Because with linestyles the projections happen on the CPU and clipping won't work if you do p4d[Vec(1, 2, 3)] / p4d[4] beforehand.) I don't know how much of a performance impact that has...

@ffreyer ffreyer marked this pull request as draft November 6, 2024 16:27
@ffreyer
Copy link
Collaborator Author

ffreyer commented Nov 6, 2024

This seems rather convoluted to set up right now, compared to the plot update branch. I'll probably try it there instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Work in progress
Development

Successfully merging this pull request may close these issues.

Cannot set linestyle to :solid with observable MethodError updating Observable linestyle
4 participants