[GLMakie] Cleanup linestyles #2666
Conversation
- per point linewidth - fix pattern deformation (merging of rectangle and parallelogram) - fix artifacting at pattern edge - cleanup joins - cleanup anti-aliasing
ffreyer
added
the
GLMakie
Compile Times benchmarkNote, that these numbers may fluctuate on the CI servers, so take them with a grain of salt. All benchmark results are based on the mean time and negative percent mean faster than the base branch. Note, that GLMakie + WGLMakie run on an emulated GPU, so the runtime benchmark is much slower. Results are from running: using_time = @ctime using Backend
# Compile time
create_time = @ctime fig = scatter(1:4; color=1:4, colormap=:turbo, markersize=20, visible=true)
display_time = @ctime Makie.colorbuffer(display(fig))
# Runtime
create_time = @benchmark fig = scatter(1:4; color=1:4, colormap=:turbo, markersize=20, visible=true)
display_time = @benchmark Makie.colorbuffer(display(fig))
|
|
I'm quite surprised that the benchmark isn't complaining about this. I thought moving the data -> pixel space calculation to the CPU would surely trigger it. Anyway while working on this pr I also noticed that Cairo(Makie) handles corners with patterned lines differently. Take this for example:
In this pr I'm currently cutting of the dot to avoid creating weird shapes at the corner. (I'm not quite sure what happens anymore. I think you get an additional triangle on the right segment up to the edge of the marker and another one at the tip of the other segment. Might also have AA issues.) What Cairo does instead is that it picks one segment to place the dot on, i.e. the right one as it has more of the dot, and extends that segment to draw the full marker there. That is also something we should be able to do in GLMakie. It would require reworking the pattern internals in GLMakie, but I don't think it would be breaking. It would probably simplify the shader in the end, it should also allow use generic markers in linestyles and it would allow us to match the linecap behavior from Cairo. The problem I see with Cairos approach is that it might become a bit misleading in terms of where a corner is. Specifically with long dashes a segment might extend significantly past a corner where the current approach would bend the dash around the corner. Though dashed and dotted lines kind of have this problem anyway. If the corner is in a gap it is always hard to figure out where exactly the corner is. |
Now that I'm trying to reproduce it I'm not getting this kind of behavior anymore. I could swear Cairo did behave like that though... Now it seems to be doing some of the things I tried to actively avoid in GLMakie, i.e. mixing dots from different line segments to create bends, gaps or step-like shapes: GLMakie | CairoMakie CairoMakie generating a small gap: The branch was still based of the linecaps pr when I looked, so it was probably a result of those size adjustments. |
|
I added a fast path now which moves the data space -> pixel space transformations back into the shader and skips a bunch of steps. Rendering 1k frames of a line with 1k points as fast as possible takes
So as of now this pr makes patterned lines significantly slower but leaves solid lines as fast as before. The extra cost comes from running the data -> pixel space transformations on the CPU. Since that triggers on |
|
I think it's completely fine to make line styles slower to make them look good. Because to me, fast but bad quality is much worse than slower but good quality. People concerned about maximum performance can always use solid lines as a fallback. |
|
Yes I agree. It stings a little that it's that much of a difference, but it's the price you have to pay to get clean linestyles. What's perhaps a little upside is that it's a pretty self contained problem. Pretty much the entire problem is that the 4x4 matrix |
|
This is really cool!! I agree, that the default shouldn't worry too much about performance, while we still have an option we can use for high performance use cases! I've been wanting to separate 2d + 3d shaders for lines + scatter, since the 2d problem is much easier to solve, and has a much higher need for quality... Maybe we can move more into that direction? |
|
I don't think reducing this to 2D would be that much easier. We would still need to do conversion on the CPU since rotations (transformations) and scaling (camera, resolution, transformations) are still relevant. For positions we would still need a 4x4 matrix (or 3x4?) but I guess for just line lengths we could drop down to 2x2. That seems to be ~3.3x as expensive than master (~0.6s) based on a quick benchmark. |
|
I'm going through tests manually to look for any bad behavior that's not showing up in CI and noticed that is practically invisible (in refimages too). This line seems to be from pre-MakieGallery. Should we remove it? For reference, it just peeks out a little bit like this: |
|
I think so, if it's not even visible. Doesn't seem to be a critical functionality check either |
|
I enabled the linestyle test, but there is a difference in how CairoMakie and GLMakie interpret vector linestyles. CairoMakie seems to interpret it as state switches, i.e. or "draw 1 - gap 2 - draw 1 - gap 1 - draw 2 - gap 1", while in it just repeats one cycle: "draw 1 - gap 2 - draw 1". We need to pick which one of these two we want. (The top one in these is the GLMakie
CairoMakie
|
Missing reference imagesFound 1 new images without existing references. |
|
This doesn't look quite right no? |
Missing reference imagesFound 1 new images without existing references. |
|
What about it? The square not bending at the edge is something I did deliberately. Within the section marked in red here The way I set it up is, that a line can be elongated by up to half a linewidth in either direction to fit an on/off transition of the pattern. If more space is requires there pattern will be bend around a corner. If the line ends, the pattern will be cut off right at the edge. I've tried a couple more things, but the only one worth mentioning is what I've shown in the first comments. There I avoided partially filling corners by squishing and stretching pattern section near them. Imo that looks much worse. Having a dot I'm also looking ahead a bit with this - the new version of the shader should be able to handle 2d patterns like scatter makers. Having them bend around a corner could be quite annoying - it would cause something like a circle to distort. Right now it wouldn't (as long as the markers width <= height) This is how it looks in CairoMakie btw |
|
I should probably add the line visualization I made at some point along the way here: It's also a good thing I looked at this again because the offset between line dot and circle here was another little error. I applied some scaling in the wrong place which caused line sections to be too small. 3742e95 fixed that. (I checked with 1560 pixels which is 52 dots at linewidth 10.) |
|
Can we in some way preserve all of the great debug examples you have shown here? They look like they took some time to create and pretty educational as well ;) |
|
I added the half circle example and the animation in the gist now. The light gray backgrounds still have code in the fragment shader similar to the scatter shader. The geom shader also has a ton of sketches in it, probably too many |
Missing reference imagesFound 1 new images without existing references. |
|
Out of curiosity, what's the technical reason for WGLMakie's lack of support for line styles? |
I think WGLMakie doesn't have as many people helping, so it tends to lag behind. I noticed another small problem - the anti-aliasing for (solid) lines at the start and end is a little off. Seems to be linewidth dependent too. |
Missing reference imagesFound 1 new images without existing references. |
WebGL doesn't support geometry shader, and the line caps + connections are easiest implemented with a geometry shader... |
Missing reference imagesFound 1 new images without existing references. |
Missing reference imagesFound 1 new images without existing references. |
|
Thank you, this is really great!! :) |
Description
Dots and dashes look pretty bad in GLMakie. This pr is supposed to clean them up. Here's an example test case:
On master:
On this branch (70ce058)
Corner behavior (0a915ed)
Previous Version
From 70ce058prototype.mp4
prototype.mp4
Change Summary
For the second version of this I allowed line segments to resize themselves to conform to the line pattern. The code now does the following for each line segment of a line:
(p1 + 0.5 * linewidth, Inf)and save start of it (off-to-on) asstart.startis to the left ofp1 - 0.5 * linewidthwe draw a normal line join (truncated or not depending on angle). In this case the the "on" part of the pattern should generally be enough to cover the whole join (except for non-truncated joins with sharper than 90° angles - TODO)startis to the right ofp1 - 0.5 * linewidthwe extend the line to include that part of the pattern.We go through an analogously set of steps for the end of the line segment.
To force anti-aliasing of potentially cut of parts of a pattern, I added
f_uv_minmax. It acts as a limit forf_uv.xwhich introduces another pair of anti-aliasing edges andf_uv_minmax.xandf_uv_minmax.y.Like before this second version also requires conversions to screen space for patterned lines. Without that, continuity between one segment and the next is not given.
Performance should be roughly the same as the last version. Maybe slightly faster without linestyles and a bit slower with them.
Type of change
Delete options that do not apply:
Checklist
Added unit tests for new algorithms, conversion methods, etc.Other TODO's:
linestyle = (sdf, [0, 1, 2])or maybe even(marker, [0, 1, 2])Old Version
On this branch (first commit)
(The first commit combines a lot of commits I made in branch coming off the linecaps pr. That included a lot of debug code which I wanted to clean out)
This addresses a few of the points raised in #2548:
And also:
Changelog
Apply transformations on CPU
The transformations for positions from data space to pixel space have been moved to the CPU (in drawing_primitives.jl). As far as I can tell this is necessary to get the correct cumulative pixel scale lengths of each line vertex, i.e.
lastlen. Before, these lengths were calculated in data space and transformed in the shader. This doesn't work out in the end, as aspect ratio, perspective projection, rotation and scale transformations all influence how data space and pixel space lengths relate. Getting them wrong causes jumps in the pattern progression from one line segment to the next.Fixing dot/dash shapes
On master patterned lines often generate dots/dashes which look like an overlapping rectangle and triangle. This is mainly caused by not considering extrusions to vertices when calculating uv coorinates. On master we have:
Ignoring the scaling factors, this puts
uv.xatg_lastlen[index], i.e. the cumulative length related to the base vertex position of the emitted vertex. For shallow joins these vertices are however moved in (or against) the direction of the line to make it continuous. Sinceuv.xdoes not reflect this, the interpolated index becomes inconsistent at the top and bottom end of the line. This is fixed bywhich considers the offset between the expected start/end of a line
line_posand the given start/endpositionin the direction of the lineline_pos.Pattern Scale
I don't really understand what the code was trying to achieve before. Now the steps go like this:
data[:pattern] = linestyle * mean(linewidth)which scales a pattern in units of linewidth up to pixel units.-2 -1 0 | 0 1 2as you repeat the texture, which generates a messy AA border.)0 .. 0.5pattern_lengthbecomes0 .. 1where the0.5comes from an incomplete transformation from clip space (-1 .. 1) to "pixel space" (-width .. width).Anti-aliasing of truncated miter join
To correctly anti-alias the truncated miter join adding just a triangle is not sufficient. Here's an example to illustrate the problem:
The black parts indicate the line segments and truncated miter without AA and the AA border for line segments is drawn in blue. Simply using the line segment vertices with AA leads to the red miter join, which obviously has less space to anti-alias as the lines. To give them enough space we add the orange rectangle below.
Note that extruding the red triangle outwards further also works in this case, but if the lines become close to (anti-) parallel the extrusion necessary goes to infinity. The rectangle on the other hand can have a fixed height of
AA_THICKNESSwith a width <= the linewidth.Other anti-aliasing & avoiding disjointed patterns
This is basically the "everything else". First of all I got rid of the
dFdxreliantaastepmethods in lines.frag and copied over the methods from the scatter shader. This is mostly because I found the old methods hard to manipulate and reason with, as they pull in information from surrounding pixels.Beyond that I added
vec4 f_uv_minmaxto overwrite what is sampled from patterns. It's interpretation follows fromConsider a line starting at (0, 0) going to (1, 0) (i.e. just extending in +x direction)
uv.xis smaller thanf_uv_minmax.x, i.e. if we are to the left of that value, then we set the signed distancexy.xto bef_uv.x - f_uv_minxmax.y. This means we have a off -> on transition atf_uv_minmax.y.f_uv_minmaxwe create a on -> off transition atf_uv_minmax.w.This is used throughout the geometry shaders:
Type of change
Delete options that do not apply:
Checklist
Added unit tests for new algorithms, conversion methods, etc.Other TODO's:
"We should add a way to draw closed lines" from GLMakie lines TODO #2548 should be easy to set up so why not add it hereSkipping this here because it's not quite as easy as I thought.linestyle = nothingcleanup uv coordinates to be consistently in pixel or (0, 1) spaceI think it's better to leave it as it is for performance, because otherwise we need to add extra conversions in the fragment shader.thicknessis currently the full linewidth but mostly used as half of it. Maybe change this?