From c4b71d603c331ad49dc092b09ef683348c77e22f Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel Date: Thu, 10 Aug 2023 08:11:19 +0200 Subject: [PATCH 1/3] Improve cairomakie lines with array colors --- CairoMakie/src/primitives.jl | 115 +++++++++++++++++++++---- ReferenceTests/src/tests/examples2d.jl | 26 ++++++ 2 files changed, 125 insertions(+), 16 deletions(-) diff --git a/CairoMakie/src/primitives.jl b/CairoMakie/src/primitives.jl index e94d5156759..224e46634f0 100644 --- a/CairoMakie/src/primitives.jl +++ b/CairoMakie/src/primitives.jl @@ -158,30 +158,22 @@ function draw_multi(primitive, ctx, positions, color, linewidths::AbstractArray, draw_multi(primitive, ctx, positions, [color for l in linewidths], linewidths, dash) end -function draw_multi(primitive::Union{Lines, LineSegments}, ctx, positions, colors::AbstractArray, linewidths::AbstractArray, dash) - if primitive isa LineSegments - @assert iseven(length(positions)) - end +function draw_multi(primitive::LineSegments, ctx, positions, colors::AbstractArray, linewidths::AbstractArray, dash) + @assert iseven(length(positions)) @assert length(positions) == length(colors) @assert length(linewidths) == length(colors) - iterator = if primitive isa Lines - 1:length(positions)-1 - elseif primitive isa LineSegments - 1:2:length(positions) - end - - for i in iterator + for i in 1:2:length(positions) if isnan(positions[i+1]) || isnan(positions[i]) continue end - Cairo.move_to(ctx, positions[i]...) - - Cairo.line_to(ctx, positions[i+1]...) if linewidths[i] != linewidths[i+1] error("Cairo doesn't support two different line widths ($(linewidths[i]) and $(linewidths[i+1])) at the endpoints of a line.") end + Cairo.move_to(ctx, positions[i]...) + Cairo.line_to(ctx, positions[i+1]...) Cairo.set_line_width(ctx, linewidths[i]) + !isnothing(dash) && Cairo.set_dash(ctx, dash .* linewidths[i]) c1 = colors[i] c2 = colors[i+1] @@ -199,8 +191,99 @@ function draw_multi(primitive::Union{Lines, LineSegments}, ctx, positions, color Cairo.destroy(pat) end end - # force clearing of path in case of skipped NaN - Cairo.new_path(ctx) +end + +function draw_multi(primitive::Lines, ctx, positions, colors::AbstractArray, linewidths::AbstractArray, dash) + @assert length(positions) == length(colors) + @assert length(linewidths) == length(colors) + + prev_color = colors[begin] + prev_linewidth = linewidths[begin] + prev_position = positions[begin] + prev_nan = isnan(prev_position) + prev_continued = false + + if !prev_nan + # first is not nan, move_to + Cairo.move_to(ctx, positions[begin]...) + else + # first is nan, do nothing + end + + for i in eachindex(positions)[2:end] + this_position = positions[i] + this_color = colors[i] + this_nan = isnan(this_position) + this_linewidth = linewidths[i] + if this_nan + # this is nan + if prev_continued + # and this is prev_continued, so set source and stroke to finish previous line + Cairo.set_line_width(ctx, this_linewidth) + !isnothing(dash) && Cairo.set_dash(ctx, dash .* this_linewidth) + Cairo.set_source_rgba(ctx, red(prev_color), green(prev_color), blue(prev_color), alpha(prev_color)) + Cairo.stroke(ctx) + else + # but this is not prev_continued, so do nothing + end + end + if prev_nan + # previous was nan + if !this_nan + # but this is not nan, so move to this position + Cairo.move_to(ctx, this_position...) + else + # and this is also nan, do nothing + end + else + if this_color == prev_color + # this color is like the previous + if !this_nan + # and this is not nan, so line_to and set prev_continued + this_linewidth != prev_linewidth && error("Encountered two different linewidth values $prev_linewidth and $this_linewidth in `lines` at index $(i-1). Different linewidths in one line are only permitted in CairoMakie when separated by a NaN point.") + Cairo.line_to(ctx, this_position...) + prev_continued = true + + if i == lastindex(positions) + # this is the last element so stroke this + Cairo.set_line_width(ctx, this_linewidth) + !isnothing(dash) && Cairo.set_dash(ctx, dash .* this_linewidth) + Cairo.set_source_rgba(ctx, red(this_color), green(this_color), blue(this_color), alpha(this_color)) + Cairo.stroke(ctx) + end + else + # but this is nan, so do nothing + end + else + prev_continued = false + if !this_nan + this_linewidth != prev_linewidth && error("Encountered two different linewidth values $prev_linewidth and $this_linewidth in `lines` at index $(i-1). Different linewidths in one line are only permitted in CairoMakie when separated by a NaN point.") + # this is not nan + # and this color is different than the previous, so move_to prev and line_to this + # create gradient pattern and stroke + Cairo.move_to(ctx, prev_position...) + Cairo.line_to(ctx, this_position...) + !isnothing(dash) && Cairo.set_dash(ctx, dash .* this_linewidth) + Cairo.set_line_width(ctx, this_linewidth) + + pat = Cairo.pattern_create_linear(prev_position..., this_position...) + Cairo.pattern_add_color_stop_rgba(pat, 0, red(prev_color), green(prev_color), blue(prev_color), alpha(prev_color)) + Cairo.pattern_add_color_stop_rgba(pat, 1, red(this_color), green(this_color), blue(this_color), alpha(this_color)) + Cairo.set_source(ctx, pat) + Cairo.stroke(ctx) + Cairo.destroy(pat) + + Cairo.move_to(ctx, this_position...) + else + # this is nan, do nothing + end + end + end + prev_nan = this_nan + prev_color = this_color + prev_linewidth = linewidths[i] + prev_position = this_position + end end ################################################################################ diff --git a/ReferenceTests/src/tests/examples2d.jl b/ReferenceTests/src/tests/examples2d.jl index 81709161878..e6eefd869ff 100644 --- a/ReferenceTests/src/tests/examples2d.jl +++ b/ReferenceTests/src/tests/examples2d.jl @@ -1119,3 +1119,29 @@ end # Display fig end + +@reference_test "lines (some with NaNs) with array colors" begin + f = Figure() + ax = Axis(f[1, 1]) + hidedecorations!(ax) + hidespines!(ax) + lines!(ax, 1:10, 1:10, color = fill(RGBAf(1, 0, 0, 0.5), 10), linewidth = 5) + lines!(ax, 1:10, 2:11, color = [fill(RGBAf(1, 0, 0, 0.5), 5); fill(RGBAf(0, 0, 1, 0.5), 5)], linewidth = 5) + lines!(ax, 1:10, [3, 4, NaN, 6, 7, NaN, 9, 10, 11, NaN], color = [fill(RGBAf(1, 0, 0, 0.5), 5); fill(RGBAf(0, 0, 1, 0.5), 5)], linewidth = 5) + lines!(ax, 1:10, 4:13, color = repeat([RGBAf(1, 0, 0, 0.5), RGBAf(0, 0, 1, 0.5)], 5), linewidth = 5) + lines!(ax, 1:10, fill(NaN, 10), color = repeat([RGBAf(1, 0, 0, 0.5), RGBAf(0, 0, 1, 0.5)], 5), linewidth = 5) + lines!(ax, 1:10, [6, 7, 8, NaN, 10, 11, 12, 13, 14, 15], color = [:red, :blue, fill(:red, 8)...], linewidth = 5) + lines!(ax, 1:3, [7, 8, 9], color = [:red, :red, :blue], linewidth = 5) + lines!(ax, 1:3, [8, 9, NaN], color = [:red, :red, :blue], linewidth = 5) + lines!(ax, 1:3, [NaN, 10, 11], color = [:red, :red, :blue], linewidth = 5) + lines!(ax, 1:5, [10, 11, NaN, 13, 14], color = [:red, :red, :blue, :blue, :blue], linewidth = [5, 5, 5, 10, 10]) + lines!(ax, 1:10, 11:20, color = [fill(RGBAf(1, 0, 0, 0.5), 5); fill(RGBAf(0, 0, 1, 0.5), 5)], linewidth = 5, linestyle = :dot) + lines!(ax, 1:10, 12:21, color = fill(RGBAf(1, 0, 0, 0.5), 10), linewidth = 5, linestyle = :dot) + f +end + +@reference_test "contour with single alpha color" begin + x = range(-π, π; length=50) + z = @. sin(x) * cos(x') + fig, ax = contour(x, x, z, color=RGBAf(1,0,0,0.4), linewidth=6) +end From 2577f73d88df3c9ba46aa59890462050c2cfb51b Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel Date: Thu, 10 Aug 2023 08:15:30 +0200 Subject: [PATCH 2/3] add NEWS entry --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index e35c93833e7..c85ff033184 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,7 @@ ## master +- Improved CairoMakie rendering of `lines` with repeating colors in an array [#3141](https://github.com/MakieOrg/Makie.jl/pull/3141). - Added `xreversed`, `yreversed` and `zreversed` attributes to `Axis3` [#3138](https://github.com/MakieOrg/Makie.jl/pull/3138). - Fixed incorrect placement of contourlabels with transform functions [#3083](https://github.com/MakieOrg/Makie.jl/pull/3083) - Fixed automatic normal generation for meshes with shading and no normals [#3041](https://github.com/MakieOrg/Makie.jl/pull/3041). From 7a4de2ec61edc857f26f6520702c7c9749ad54b2 Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel Date: Mon, 14 Aug 2023 14:13:49 +0200 Subject: [PATCH 3/3] use butted stroke style to fix dashes --- CairoMakie/src/primitives.jl | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CairoMakie/src/primitives.jl b/CairoMakie/src/primitives.jl index 224e46634f0..212b6ba1f62 100644 --- a/CairoMakie/src/primitives.jl +++ b/CairoMakie/src/primitives.jl @@ -58,8 +58,10 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Unio # stroke each segment separately, this means disjointed segments with probably # wonky dash patterns if segments are short - # we can hide the gaps by setting the line cap to round - Cairo.set_line_cap(ctx, Cairo.CAIRO_LINE_CAP_ROUND) + # Butted segments look the best for varying colors, at least when connection angles are small. + # While round style has nicer sharp joins, it looks bad with alpha colors (double paint) and + # also messes with dash patterns (they are too long because of the caps) + Cairo.set_line_cap(ctx, Cairo.CAIRO_LINE_CAP_BUTT) draw_multi( primitive, ctx, projected_positions,