Merge branch 'breaking-0.21' into ff/voxel

.github/workflows/compilation-benchmark.yaml

@@ -29,7 +29,6 @@ jobs:
       - uses: julia-actions/setup-julia@v1
         with:
           version: '1'
-          include-all-prereleases: true
           arch: x64
       - uses: julia-actions/cache@v1
       - name: Benchmark

CHANGELOG.md

@@ -2,7 +2,13 @@
 
 ## [Unreleased]
 
+- Added supported markers hint to unsupported marker warn message.
 - Add `voxels` plot [#3527](https://github.com/MakieOrg/Makie.jl/pull/3527)
+
+## [0.21.0] - 2024-03-0X
+
+- Remove StableHashTraits in favor of calculating hashes directly with CRC32c [#3667](https://github.com/MakieOrg/Makie.jl/pull/3667).
+- **Breaking (sort of)** Added a new `@recipe` variant which allows documenting attributes directly where they are defined and validating that all attributes are known whenever a plot is created. This is not breaking in the sense that the API changes, but user code is likely to break because of misspelled attribute names etc. that have so far gone unnoticed.
 - **Breaking** Reworked line shaders in GLMakie and WGLMakie [#3558](https://github.com/MakieOrg/Makie.jl/pull/3558)
   - GLMakie: Removed support for per point linewidths
   - GLMakie: Adjusted dots (e.g. with `linestyle = :dot`) to bend across a joint
@@ -12,6 +18,7 @@
   - WGLMakie: Added line joints
   - WGLMakie: Added native anti-aliasing which generally improves quality but introduces outline artifacts in some cases (same as GLMakie)
   - Both: Adjusted handling of thin lines which may result in different color intensities
+- Fixed an issue with lines being drawn in the wrong direction in 3D (with perspective projection) [#3651](https://github.com/MakieOrg/Makie.jl/pull/3651).
 
 ## [0.20.8] - 2024-02-22
 

CairoMakie/src/primitives.jl

@@ -27,7 +27,81 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Unio
     end
 
     space = to_value(get(primitive, :space, :data))
-    projected_positions = project_position.(Ref(scene), (Makie.transform_func(primitive),), Ref(space), positions, Ref(model))
+    # Lines need to be handled more carefully with perspective projections to
+    # avoid them inverting.
+    projected_positions, indices = let
+        # Standard transform from input space to clip space
+        points = Makie.apply_transform(Makie.transform_func(primitive), positions, space)
+        res = scene.camera.resolution[]
+        transform = Makie.space_to_clip(scene.camera, space) * model
+        clip_points = map(p -> transform * to_ndim(Vec4f, to_ndim(Vec3f, p, 0f0), 1f0), points)
+
+        # yflip and clip -> screen/pixel coords
+        function clip2screen(res, p)
+            s = Vec2f(0.5f0, -0.5f0) .* p[Vec(1, 2)] / p[4] .+ 0.5f0
+            return res .* s
+        end
+
+        screen_points = sizehint!(Vector{Vec2f}(undef, 0), length(clip_points))
+        indices = sizehint!(Vector{Int}(undef, 0), length(clip_points))
+
+        # Adjust points such that they are always in front of the camera.
+        # TODO: Consider skipping this if there is no perspetive projection.
+        # (i.e. use project_position.(..., positions) and indices = eachindex(positions))
+        for (i, p) in enumerate(clip_points)
+            if p[4] < 0.0               # point behind camera and ...
+                if primitive isa Lines  # ... part of a continuous line
+                    # create an extra point for the incoming line segment at the
+                    # near clipping plane (i.e. on line prev --> this)
+                    if i > 1
+                        prev = clip_points[i-1]
+                        v = p - prev
+                        #
+                        p2 = p + (-p[4] - p[3]) / (v[3] + v[4]) * v
+                        push!(screen_points, clip2screen(res, p2))
+                        push!(indices, i)
+                    end
+
+                    # disconnect the line
+                    push!(screen_points, Vec2f(NaN))
+
+                    # and create another point for the outgoing line segment at
+                    # the near clipping plane (on this ---> next)
+                    if i < length(clip_points)
+                        next = clip_points[i+1]
+                        v = next - p
+                        p2 = p + (-p[4] - p[3]) / (v[3] + v[4]) * v
+                        push!(screen_points, clip2screen(res, p2))
+                        push!(indices, i)
+                    end
+
+                else                    # ... part of a discontinuous set of segments
+                    if iseven(i)
+                        # if this is the last point of the segment we move towards
+                        # the previous (start) point
+                        prev = clip_points[i-1]
+                        v = p - prev
+                        p = p + (-p[4] - p[3]) / (v[3] + v[4]) * v
+                        push!(screen_points, clip2screen(res, p))
+                    else
+                        # otherwise we move to the next (end) point
+                        next = clip_points[i+1]
+                        v = next - p
+                        p = p + (-p[4] - p[3]) / (v[3] + v[4]) * v
+                        push!(screen_points, clip2screen(res, p))
+                    end
+                end
+            else
+                # otherwise we can just draw the point
+                push!(screen_points, clip2screen(res, p))
+            end
+
+            # we always have at least one point
+            push!(indices, i)
+        end
+
+        screen_points, indices
+    end
 
     color = to_color(primitive.calculated_colors[])
 
@@ -64,7 +138,7 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Unio
         draw_multi(
             primitive, ctx,
             projected_positions,
-            color, linewidth,
+            color, linewidth, indices,
             isnothing(linestyle) ? nothing : diff(Float64.(linestyle))
         )
     else
@@ -150,16 +224,16 @@ function draw_single(primitive::LineSegments, ctx, positions)
 end
 
 # if linewidth is not an array
-function draw_multi(primitive, ctx, positions, colors::AbstractArray, linewidth, dash)
-    draw_multi(primitive, ctx, positions, colors, [linewidth for c in colors], dash)
+function draw_multi(primitive, ctx, positions, colors::AbstractArray, linewidth, indices, dash)
+    draw_multi(primitive, ctx, positions, colors, [linewidth for c in colors], indices, dash)
 end
 
 # if color is not an array
-function draw_multi(primitive, ctx, positions, color, linewidths::AbstractArray, dash)
-    draw_multi(primitive, ctx, positions, [color for l in linewidths], linewidths, dash)
+function draw_multi(primitive, ctx, positions, color, linewidths::AbstractArray, indices, dash)
+    draw_multi(primitive, ctx, positions, [color for l in linewidths], linewidths, indices, dash)
 end
 
-function draw_multi(primitive::LineSegments, ctx, positions, colors::AbstractArray, linewidths::AbstractArray, dash)
+function draw_multi(primitive::LineSegments, ctx, positions, colors::AbstractArray, linewidths::AbstractArray, indices, dash)
     @assert iseven(length(positions))
     @assert length(positions) == length(colors)
     @assert length(linewidths) == length(colors)
@@ -194,7 +268,9 @@ function draw_multi(primitive::LineSegments, ctx, positions, colors::AbstractArr
     end
 end
 
-function draw_multi(primitive::Lines, ctx, positions, colors::AbstractArray, linewidths::AbstractArray, dash)
+function draw_multi(primitive::Lines, ctx, positions, colors::AbstractArray, linewidths::AbstractArray, indices, dash)
+    colors = colors[indices]
+    linewidths = linewidths[indices]
     @assert length(positions) == length(colors)
     @assert length(linewidths) == length(colors)
 

GLMakie/assets/shader/line_segment.geom

@@ -57,8 +57,20 @@ void main(void)
     }
 
     // get start and end point of line segment
-    vec3 p1 = screen_space(gl_in[0].gl_Position);
-    vec3 p2 = screen_space(gl_in[1].gl_Position);
+    // restrict to visible area (see lines.geom)
+    vec3 p1, p2;
+    {
+        vec4 _p1 = gl_in[0].gl_Position, _p2 = gl_in[1].gl_Position;
+        vec4 v1 = _p2 - _p1;
+
+        if (_p1.w < 0.0)
+            _p1 = _p1 + (-_p1.w - _p1.z) / (v1.z + v1.w) * v1;
+        if (_p2.w < 0.0)
+            _p2 = _p2 + (-_p2.w - _p2.z) / (v1.z + v1.w) * v1;
+
+        p1 = screen_space(_p1);
+        p2 = screen_space(_p2);
+    }
 
     // get vector in line direction and vector in linewidth direction
     vec3 v1 = (p2 - p1);

GLMakie/assets/shader/lines.geom

@@ -85,19 +85,19 @@ vec2 normal_vector(in vec3 v) { return vec2(-v.y, v.x); }
 ////////////////////////////////////////////////////////////////////////////////
 
 
-vec2 process_pattern(Nothing pattern, bool[4] isvalid, mat2 extrusion, float halfwidth) {
+vec2 process_pattern(Nothing pattern, bool[4] isvalid, mat2 extrusion, float segment_length, float halfwidth) {
     // do not adjust stuff
     f_pattern_overwrite = vec4(-1e12, 1.0, 1e12, 1.0);
     return vec2(0);
 }
-vec2 process_pattern(sampler2D pattern, bool[4] isvalid, mat2 extrusion, float halfwidth) {
+vec2 process_pattern(sampler2D pattern, bool[4] isvalid, mat2 extrusion, float segment_length, float halfwidth) {
     // TODO
     // This is not a case that's used at all yet. Maybe consider it in the future...
     f_pattern_overwrite = vec4(-1e12, 1.0, 1e12, 1.0);
     return vec2(0);
 }
 
-vec2 process_pattern(sampler1D pattern, bool[4] isvalid, mat2 extrusion, float halfwidth) {
+vec2 process_pattern(sampler1D pattern, bool[4] isvalid, mat2 extrusion, float segment_length, float halfwidth) {
     // samples:
     //   -ext1  p1 ext1    -ext2 p2 ext2
     //      1   2   3        4   5   6
@@ -148,13 +148,13 @@ vec2 process_pattern(sampler1D pattern, bool[4] isvalid, mat2 extrusion, float h
     if (isvalid[3]) {
         // float offset = max(abs(extrusion[1][0]), halfwidth + AA_RADIUS);
         float offset = abs(extrusion[1][0]);
-        left   = width * texture(pattern, uv_scale * (g_lastlen[2] - offset)).x;
-        center = width * texture(pattern, uv_scale * (g_lastlen[2]         )).x;
-        right  = width * texture(pattern, uv_scale * (g_lastlen[2] + offset)).x;
+        left   = width * texture(pattern, uv_scale * (g_lastlen[1] + segment_length - offset)).x;
+        center = width * texture(pattern, uv_scale * (g_lastlen[1] + segment_length         )).x;
+        right  = width * texture(pattern, uv_scale * (g_lastlen[1] + segment_length + offset)).x;
 
         if ((left > 0 && center > 0 && right > 0) || (left < 0 && right < 0)) {
             // default/freeze
-            f_pattern_overwrite.z = uv_scale * (g_lastlen[2] - abs(extrusion[1][0]) - AA_RADIUS);
+            f_pattern_overwrite.z = uv_scale * (g_lastlen[1] + segment_length - abs(extrusion[1][0]) - AA_RADIUS);
             f_pattern_overwrite.w = sign(center);
         } else if (left > 0) {
             // shrink backwards
@@ -164,7 +164,7 @@ vec2 process_pattern(sampler1D pattern, bool[4] isvalid, mat2 extrusion, float h
             adjust.y = 1.0;
         } else {
             // default - see above
-            f_pattern_overwrite.z = uv_scale * (g_lastlen[2] - abs(extrusion[1][0]) - AA_RADIUS);
+            f_pattern_overwrite.z = uv_scale * (g_lastlen[1] + segment_length - abs(extrusion[1][0]) - AA_RADIUS);
             f_pattern_overwrite.w = sign(center);
         }
     }
@@ -212,18 +212,47 @@ void main(void)
     // extends the line. First let's get some vectors we need.
 
     // Get the four vertices passed to the shader in pixel space.
-    // Without FAST_PATH the conversions happen on the CPU
-#ifdef FAST_PATH
-    vec3 p0 = screen_space(gl_in[0].gl_Position); // start of previous segment
-    vec3 p1 = screen_space(gl_in[1].gl_Position); // end of previous segment, start of current segment
-    vec3 p2 = screen_space(gl_in[2].gl_Position); // end of current segment, start of next segment
-    vec3 p3 = screen_space(gl_in[3].gl_Position); // end of next segment
-#else
-    vec3 p0 = gl_in[0].gl_Position.xyz; // start of previous segment
-    vec3 p1 = gl_in[1].gl_Position.xyz; // end of previous segment, start of current segment
-    vec3 p2 = gl_in[2].gl_Position.xyz; // end of current segment, start of next segment
-    vec3 p3 = gl_in[3].gl_Position.xyz; // end of next segment
-#endif
+
+    // To apply pixel space linewidths we transform line vertices to pixel space
+    // here. This is dangerous with perspective projection as p.xyz / p.w sends
+    // points from behind the camera to beyond far (clip z > 1), causing lines
+    // to invert. To avoid this we translate points along the line direction,
+    // moving them to the edge of the visible area.
+    vec3 p0, p1, p2, p3;
+    {
+        // All in clip space
+        vec4 clip_p0 = gl_in[0].gl_Position; // start of previous segment
+        vec4 clip_p1 = gl_in[1].gl_Position; // end of previous segment, start of current segment
+        vec4 clip_p2 = gl_in[2].gl_Position; // end of current segment, start of next segment
+        vec4 clip_p3 = gl_in[3].gl_Position; // end of next segment
+
+        vec4 v1 = clip_p2 - clip_p1;
+
+        // With our perspective projection matrix clip.w = -view.z with
+        // clip.w < 0.0 being behind the camera.
+        // Note that if the signs in the projectionmatrix change, this may become wrong.
+        if (clip_p1.w < 0.0) {
+            // the line connects outside the visible area so we may consider it disconnected
+            isvalid[0] = false;
+            // A clip position is visible if -w <= z <= w. To move the line along
+            // the line direction v to the start of the visible area, we solve:
+            //   p.z + t * v.z = +-(p.w + t * v.w)
+            // where (-) gives us the result for the near clipping plane as p.z
+            // and p.w share the same sign and p.z/p.w = -1.0 is the near plane.
+            clip_p1 = clip_p1 + (-clip_p1.w - clip_p1.z) / (v1.z + v1.w) * v1;
+        }
+        if (clip_p2.w < 0.0) {
+            isvalid[3] = false;
+            clip_p2 = clip_p2 + (-clip_p2.w - clip_p2.z) / (v1.z + v1.w) * v1;
+        }
+
+        // transform clip -> screen space, applying xyz / w normalization (which
+        // is now save as all vertices are in front of the camera)
+        p0 = screen_space(clip_p0); // start of previous segment
+        p1 = screen_space(clip_p1); // end of previous segment, start of current segment
+        p2 = screen_space(clip_p2); // end of current segment, start of next segment
+        p3 = screen_space(clip_p3); // end of next segment
+    }
 
     // Since we are measuring from the center of the line we will need half
     // the thickness/linewidth for most things.
@@ -322,7 +351,7 @@ void main(void)
     //   on straight line quad (adjustment becomes +1.0 or -1.0)
     // - or adjust the pattern to start/stop outside of the joint
     //   (f_pattern_overwrite is set, adjustment is 0.0)
-    vec2 adjustment = process_pattern(pattern, isvalid, halfwidth * extrusion, halfwidth);
+    vec2 adjustment = process_pattern(pattern, isvalid, halfwidth * extrusion, segment_length, halfwidth);
 
     // If adjustment != 0.0 we replace a joint by an extruded line, so we no longer
     // need to shrink the line for the joint to fit.

GLMakie/assets/shader/lines.vert

@@ -25,6 +25,7 @@ out float g_lastlen;
 out int g_valid_vertex;
 out float g_thickness;
 
+vec4 to_vec4(vec4 v){return v;}
 vec4 to_vec4(vec3 v){return vec4(v, 1);}
 vec4 to_vec4(vec2 v){return vec4(v, 0, 1);}
 

GLMakie/src/GLAbstraction/GLTypes.jl

@@ -385,6 +385,7 @@ function RenderObject(
                 try
                     data[k] = gl_convert(v)
                 catch e
+                    @error "gl_convert for key `$k` failed"
                     rethrow(e)
                 end
 

GLMakie/src/drawing_primitives.jl

@@ -456,15 +456,11 @@ function draw_atomic(screen::Screen, scene::Scene, @nospecialize(plot::Lines))
             data[:fast] = false
 
             pvm = lift(*, plot, data[:projectionview], data[:model])
-            positions = lift(plot, transform_func, positions, space, pvm,
-                            data[:resolution]) do f, ps, space, pvm, res
+            positions = lift(plot, transform_func, positions, space, pvm) do f, ps, space, pvm
                 transformed = apply_transform(f, ps, space)
-                output = Vector{Point3f}(undef, length(transformed))
-                scale = Vec3f(0.5 * res[1], 0.5 * res[2], 1f0)
-                offset = Vec3f(0.5 * res[1], 0.5 * res[2], 0)
+                output = Vector{Point4f}(undef, length(transformed))
                 for i in eachindex(transformed)
-                    clip = pvm * to_ndim(Point4f, to_ndim(Point3f, transformed[i], 0f0), 1f0)
-                    output[i] = scale .* Point3f(clip) ./ clip[4] .+ offset
+                    output[i] = pvm * to_ndim(Point4f, to_ndim(Point3f, transformed[i], 0f0), 1f0)
                 end
                 output
             end

GLMakie/src/glshaders/lines.jl

@@ -1,14 +1,20 @@
-function sumlengths(points)
+function sumlengths(points, resolution)
+    # normalize w component if availabke
+    f(p::VecTypes{4}) = p[Vec(1, 2)] / p[4]
+    f(p::VecTypes) = p[Vec(1, 2)]
+
+    invalid(p::VecTypes{4}) = p[4] <= 1e-6
+    invalid(p::VecTypes) = false
+
     T = eltype(eltype(typeof(points)))
     result = zeros(T, length(points))
-    i12 = Vec(1, 2)
     for i in eachindex(points)
         i0 = max(i-1, 1)
         p1, p2 = points[i0], points[i]
-        if !(any(map(isnan, p1)) || any(map(isnan, p2)))
-            result[i] = result[i0] + norm(p1[i12] - p2[i12])
-        else
+        if any(map(isnan, p1)) || any(map(isnan, p2)) || invalid(p1) || invalid(p2)
             result[i] = 0f0
+        else
+            result[i] = result[i0] + 0.5 * norm(resolution .* (f(p1) - f(p2)))
         end
     end
     result
@@ -32,6 +38,7 @@ function draw_lines(screen, position::Union{VectorTypes{T}, MatTypes{T}}, data::
     end
 
     color_type = gl_color_type_annotation(data[:color])
+    resolution = data[:resolution]
 
     @gen_defaults! data begin
         total_length::Int32 = const_lift(x-> Int32(length(x)), position)
@@ -66,7 +73,7 @@ function draw_lines(screen, position::Union{VectorTypes{T}, MatTypes{T}}, data::
         valid_vertex        = const_lift(p_vec) do points
             map(p-> Float32(all(isfinite, p)), points)
         end => GLBuffer
-        lastlen             = const_lift(sumlengths, p_vec) => GLBuffer
+        lastlen             = const_lift(sumlengths, p_vec, resolution) => GLBuffer
         pattern_length      = 1f0 # we divide by pattern_length a lot.
         debug               = false
     end