Fix bezier marker stroke (#2961)

* add missing ClosePath * update NEWS [skip ci] * test bezier stroke [skip ci] * fix stroke/glow/AA scaling * remove extra padding in Bezier path renders * mention GLMakie changes * fix dot --------- Co-authored-by: Simon <[email protected]>
MakieOrg · Jul 3, 2023 · a82e8a5 · a82e8a5
1 parent 80735a2
commit a82e8a5
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 18 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -2,12 +2,13 @@
 
 ## master
 
+- Fix incomplete stroke with some Bezier markers in CairoMakie and blurry strokes in GLMakie [#2961](https://github.com/MakieOrg/Makie.jl/pull/2961)
 - Added the ability to use custom triangulations from DelaunayTriangulation.jl [#2896](https://github.com/MakieOrg/Makie.jl/pull/2896).
 - Adjusted scaling of scatter/text stroke, glow and anti-aliasing width under non-uniform 2D scaling (Vec2f markersize/fontsize) in GLMakie [#2950](https://github.com/MakieOrg/Makie.jl/pull/2950).
-- Fixed broken AA for lines with strongly varying linewidth [#2953](https://github.com/MakieOrg/Makie.jl/pull/2953).
 - Scaled `errorbar` whiskers and `bracket` correctly with transformations [#3012](https://github.com/MakieOrg/Makie.jl/pull/3012).
 - Updated `bracket` when the screen is resized or transformations change [#3012](https://github.com/MakieOrg/Makie.jl/pull/3012).
 
+
 ## v0.19.6
 
 - Fixed broken AA for lines with strongly varying linewidth [#2953](https://github.com/MakieOrg/Makie.jl/pull/2953).

diff --git a/ReferenceTests/src/tests/primitives.jl b/ReferenceTests/src/tests/primitives.jl
@@ -253,6 +253,27 @@ end
     f
 end
 
+@reference_test "BezierPath marker stroke" begin
+    f = Figure(resolution = (800, 800))
+    ax = Axis(f[1, 1])
+
+    # Same as above
+    markers = [
+        :rect, :circle, :cross, :x, :utriangle, :rtriangle, :dtriangle, :ltriangle, :pentagon, 
+        :hexagon, :octagon, :star4, :star5, :star6, :star8, :vline, :hline, 'x', 'X'
+    ]
+
+    for (i, marker) in enumerate(markers)
+        scatter!(
+            Point2f.(1:5, i), marker = marker, 
+            markersize = range(10, 30, length = 5), color = :orange,
+            strokewidth = 2, strokecolor = :black
+        )
+    end
+
+    f
+end
+
 
 @reference_test "complex_bezier_markers" begin
     f = Figure(resolution = (800, 800))

diff --git a/WGLMakie/test/runtests.jl b/WGLMakie/test/runtests.jl
@@ -54,7 +54,8 @@ excludes = Set([
     "scatter with stroke",
     "scatter with glow",
     "lines and linestyles",
-    "Textured meshscatter" # not yet implemented
+    "Textured meshscatter", # not yet implemented
+    "BezierPath marker stroke", # not yet implemented
 ])
 Makie.inline!(Makie.automatic)
 

diff --git a/src/bezier.jl b/src/bezier.jl
@@ -201,7 +201,8 @@ function bezier_ngon(n, radius, angle)
         for a in range(0, 2pi, length = n+1)[1:end-1]]
     BezierPath([
         MoveTo(points[1]);
-        LineTo.(points[2:end])
+        LineTo.(points[2:end]);
+        ClosePath()
     ])
 end
 
@@ -212,7 +213,8 @@ function bezier_star(n, inner_radius, outer_radius, angle)
         for (i, a) in enumerate(range(0, 2pi, length = 2n+1)[1:end-1])]
     BezierPath([
         MoveTo(points[1]);
-        LineTo.(points[2:end])
+        LineTo.(points[2:end]);
+        ClosePath()
     ])
 end
 
@@ -493,24 +495,23 @@ function render_path(path, bitmap_size_px = 256)
     # in the outline, 1 unit = 1/64px
     scale_factor = bitmap_size_px * 64
 
-    # we transform the path into the unit square and we can
-    # scale and translate this to a 4096x4096 grid, which is 64px x 64px
-    # when rendered to bitmap
+    # We transform the path into a rectangle of size (aspect, 1) or (1, aspect)
+    # such that aspect ≤ 1. We then scale that rectangle up to a size of 4096 by 
+    # 4096 * aspect, which results in at most a 64px by 64px bitmap
 
     # freetype has no ClosePath and EllipticalArc, so those need to be replaced
     path_replaced = replace_nonfreetype_commands(path)
 
-    path_unit_square = fit_to_unit_square(path_replaced, false)
+    aspect = widths(bbox(path)) / maximum(widths(bbox(path)))
+    path_unit_rect = fit_to_bbox(path_replaced, Rect2f(Point2f(0), aspect))
 
-    path_transformed = Makie.scale(
-        path_unit_square,
-        scale_factor,
-    )
+    path_transformed = Makie.scale(path_unit_rect, scale_factor)
 
     outline_ref = make_outline(path_transformed)
 
-    w = bitmap_size_px
-    h = bitmap_size_px
+    # Adjust bitmap size to match path aspect
+    w = ceil(Int, bitmap_size_px * aspect[1])
+    h = ceil(Int, bitmap_size_px * aspect[2])
     pitch = w * 1 # 8 bit gray
     pixelbuffer = zeros(UInt8, h * pitch)
     bitmap_ref = Ref{FT_Bitmap}()

diff --git a/src/utilities/texture_atlas.jl b/src/utilities/texture_atlas.jl
@@ -70,7 +70,7 @@ function Base.show(io::IO, atlas::TextureAtlas)
     println(io, "  font_render_callback: ", length(atlas.font_render_callback))
 end
 
-const SERIALIZATION_FORMAT_VERSION = "v1"
+const SERIALIZATION_FORMAT_VERSION = "v2"
 
 # basically a singleton for the textureatlas
 function get_cache_path(resolution::Int, pix_per_glyph::Int)
@@ -486,8 +486,7 @@ end
 
 function marker_scale_factor(atlas::TextureAtlas, path::BezierPath)
     # padded_width = (unpadded_target_width + unpadded_target_width * pad_per_unit)
-    path_width = widths(Makie.bbox(path))
-    return (1f0 .+ bezierpath_pad_scale_factor(atlas, path)) .* path_width
+    return (1f0 .+ bezierpath_pad_scale_factor(atlas, path)) .* widths(Makie.bbox(path))
 end
 
 function rescale_marker(atlas::TextureAtlas, pathmarker::BezierPath, font, markersize)
@@ -512,7 +511,7 @@ end
 
 function offset_bezierpath(atlas::TextureAtlas, bp::BezierPath, markersize::Vec2, markeroffset::Vec2)
     bb = bbox(bp)
-    pad_offset = (origin(bb) .- 0.5f0 .* bezierpath_pad_scale_factor(atlas, bp) .* widths(bb))
+    pad_offset = origin(bb) .- 0.5f0 .* bezierpath_pad_scale_factor(atlas, bp) .* widths(bb)
     return markersize .* pad_offset
 end