float32 conversion

CHANGELOG.md

@@ -12,6 +12,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/CairoMakie.jl

@@ -12,6 +12,7 @@ using Makie: to_value, to_colormap, extrema_nan
 using Makie.Observables
 using Makie: spaces, is_data_space, is_pixel_space, is_relative_space, is_clip_space
 using Makie: numbers_to_colors
+using Makie: Mat3f, Mat4f, Mat3d, Mat4d
 
 # re-export Makie, including deprecated names
 for name in names(Makie, all=true)

CairoMakie/src/primitives.jl

@@ -27,7 +27,82 @@ 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[]
+        f32convert = Makie.f32_convert_matrix(scene.float32convert, space)
+        transform = Makie.space_to_clip(scene.camera, space) * model * f32convert
+        clip_points = map(p -> transform * to_ndim(Vec4d, to_ndim(Vec3d, p, 0), 1), 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 +139,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 +225,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 +269,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)
 
@@ -298,7 +375,7 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Scat
     model = primitive.model[]
     positions = primitive[1][]
     isempty(positions) && return
-    size_model = transform_marker ? model : Mat4f(I)
+    size_model = transform_marker ? model : Mat4d(I)
 
     font = to_font(to_value(get(primitive, :font, Makie.defaultfont())))
 
@@ -554,7 +631,7 @@ function draw_glyph_collection(
     strokecolors = glyph_collection.strokecolors
 
     model = _deref(_model)
-    model33 = transform_marker ? model[Vec(1, 2, 3), Vec(1, 2, 3)] : Mat3f(I)
+    model33 = transform_marker ? model[Vec(1, 2, 3), Vec(1, 2, 3)] : Mat3d(I)
     id = Mat4f(I)
 
     glyph_pos = let
@@ -563,7 +640,7 @@ function draw_glyph_collection(
 
         Makie.clip_to_space(scene.camera, markerspace) *
         Makie.space_to_clip(scene.camera, space) *
-        model * to_ndim(Point4f, to_ndim(Point3f, p, 0), 1)
+        model * to_ndim(Point4d, to_ndim(Point3d, p, 0), 1)
     end
 
     Cairo.save(ctx)
@@ -598,8 +675,8 @@ function draw_glyph_collection(
         # origin. The resulting vectors give the directions in which the character
         # needs to be stretched in order to match the 3D projection
 
-        xvec = rotation * (scale3[1] * Point3f(1, 0, 0))
-        yvec = rotation * (scale3[2] * Point3f(0, -1, 0))
+        xvec = rotation * (scale3[1] * Point3d(1, 0, 0))
+        yvec = rotation * (scale3[2] * Point3d(0, -1, 0))
 
         glyphpos = _project_position(scene, markerspace, gp3, id, true)
         xproj = _project_position(scene, markerspace, gp3 + model33 * xvec, id, true)
@@ -691,7 +768,7 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Unio
     else
         ys = regularly_spaced_array_to_range(ys)
     end
-    model = primitive.model[]::Mat4f
+    model = primitive.model[]::Mat4d
     interpolate = to_value(primitive.interpolate)
 
     # Debug attribute we can set to disable fastpath
@@ -825,10 +902,10 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Maki
 end
 
 function draw_mesh2D(scene, screen, @nospecialize(plot), @nospecialize(mesh))
-    vs =  decompose(Point2f, mesh)::Vector{Point2f}
+    vs = decompose(Point2f, mesh)::Vector{Point2f}
     fs = decompose(GLTriangleFace, mesh)::Vector{GLTriangleFace}
     uv = decompose_uv(mesh)::Union{Nothing, Vector{Vec2f}}
-    model = plot.model[]::Mat4f
+    model = plot.model[]::Mat4d
     color = hasproperty(mesh, :color) ? to_color(mesh.color) : plot.calculated_colors[]
     cols = per_face_colors(color, nothing, fs, nothing, uv)
     space = to_value(get(plot, :space, :data))::Symbol
@@ -895,7 +972,7 @@ function draw_mesh3D(scene, screen, attributes, mesh; pos = Vec4f(0), scale = 1f
 
     per_face_col = per_face_colors(color, matcap, meshfaces, meshnormals, meshuvs)
 
-    model = attributes.model[]::Mat4f
+    model = attributes.model[]::Mat4d
     space = to_value(get(attributes, :space, :data))::Symbol
     func = Makie.transform_func(attributes)
 
@@ -927,7 +1004,7 @@ function draw_mesh3D(
     i = Vec(1, 2, 3)
     normalmatrix = transpose(inv(model[i, i]))
 
-    local_model = rotation * Makie.scalematrix(Vec3f(scale))
+    local_model = rotation * Makie.scalematrix(Vec3d(scale))
     # pass transform_func as argument to function, so that we get a function barrier
     # and have `transform_func` be fully typed inside closure
     vs = broadcast(meshpoints, (transform_func,)) do v, f

CairoMakie/src/utils.jl

@@ -8,10 +8,35 @@ function project_position(scene::Scene, transform_func::T, space, point, model::
     _project_position(scene, space, point, model, yflip)
 end
 
-function _project_position(scene::Scene, space, point, model, yflip::Bool)
+# much faster than dot-ing `project_position` because it skips all the repeated mat * mat
+function _project_position(scene::Scene, space, ps::Vector{<: VecTypes{N, T1}}, model, yflip::Bool) where {N, T1}
+    transform = let
+        f32convert = Makie.f32_convert_matrix(scene.float32convert, space)
+        M = Makie.space_to_clip(scene.camera, space) * model * f32convert
+        res = scene.camera.resolution[]
+        px_scale  = Vec3d(0.5 * res[1], 0.5 * (yflip ? -res[2] : res[2]), 1)
+        px_offset = Vec3d(0.5 * res[1], 0.5 * res[2], 0)
+        M = Makie.transformationmatrix(px_offset, px_scale) * M
+        M[Vec(1,2,4), Vec(1,2,3,4)] # skip z, i.e. calculate (x, y, w)
+    end
+
+    output = similar(ps, Point2f)
+
+    @inbounds for i in eachindex(ps)
+        p4d = to_ndim(Point4d, to_ndim(Point3d, ps[i], 0), 1)
+        px_pos = transform * p4d
+        output[i] = px_pos[Vec(1, 2)] / px_pos[3]
+    end
+
+    return output
+end
+
+function _project_position(scene::Scene, space, point::VecTypes{N, T1}, model, yflip::Bool) where {N, T1}
+    T = promote_type(Float32, T1) # always Float, at least Float32
     res = scene.camera.resolution[]
-    p4d = to_ndim(Vec4f, to_ndim(Vec3f, point, 0f0), 1f0)
-    clip = Makie.space_to_clip(scene.camera, space) * model * p4d
+    p4d = to_ndim(Vec4{T}, to_ndim(Vec3{T}, point, 0), 1)
+    f32convert = Makie.f32_convert_matrix(scene.float32convert, space)
+    clip = Makie.space_to_clip(scene.camera, space) * model * f32convert * p4d
     @inbounds begin
         # between -1 and 1
         p = (clip ./ clip[4])[Vec(1, 2)]
@@ -29,12 +54,12 @@ function project_position(@nospecialize(scenelike), space, point, model, yflip::
     project_position(scene, Makie.transform_func(scenelike), space, point, model, yflip)
 end
 
-function project_scale(scene::Scene, space, s::Number, model = Mat4f(I))
-    project_scale(scene, space, Vec2f(s), model)
+function project_scale(scene::Scene, space, s::Number, model = Mat4d(I))
+    project_scale(scene, space, Vec2d(s), model)
 end
 
-function project_scale(scene::Scene, space, s, model = Mat4f(I))
-    p4d = model * to_ndim(Vec4f, s, 0f0)
+function project_scale(scene::Scene, space, s, model = Mat4d(I))
+    p4d = model * to_ndim(Vec4d, s, 0)
     if is_data_space(space)
         @inbounds p = (scene.camera.projectionview[] * p4d)[Vec(1, 2)]
         return p .* scene.camera.resolution[] .* 0.5

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);