Refactor SurfaceLike trait

[ffreyer]

Description

This pr breaks up the SurfaceLike trait and its children ContinuousSurface and DiscreteSurface into 3 (4) new traits:

• GridBased which more or less replaces SurfaceLike. The name change is meant to remove the association with Surface plots which is misleading imo.
  • CellBasedGrid <: GridBased which replaces DiscreteSurface with no changes
  • VertexBasedGrid <: GridBased which replaces ContinuousSurface for surface, contour and contourf plots. The only change here is to default to a 1:size(zs, dim) instead of 0:size(zs, dim). This fixes the alignment issue noted in Cannot plot contours in both CairoMakie and GLMakie #3060 and a similar issue noted for surface in Fix mesh/surface with NaN points #2598.
• ImageLike which replaces ContinuousSurface specifically for image plots. This no longer produces arrays in the conversion pipeline, but instead just an interval. In backends an image plot is treated as a single quad so I don't think this removes anything

In #2598 there is a picture which shows how vertices are aligned for surface plots:

After this pr they would be aligned to integer x and y values instead, starting at (1, 1).

Example after pr: (should only change placement and scale)

begin
    f = Figure()
    img = [sin(i + 2 * cos(j)) for i in 1:10, j in 1:10]
    heatmap(f[1, 1], img)
    image(f[1, 2], img)
    a = Axis(f[2, 1])
    surface!(a, img, shading = false)
    xlims!(a, 1, 10); ylims!(a, 1, 10)
    contourf(f[2, 2], img)
    a, p = contour(f[2, 3], img)
    tightlimits!(a)
    f
end

With this and #3102 adding voronoiplot, maybe we can consider #748 done?

TODO:

• Check if CairoMakie surface discoloration comes from this pr

Type of change

• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Changes placements of some plots and may break some convert_arguments methods.

Checklist

• Added an entry in NEWS.md (for new features and breaking changes)
• Added or changed relevant sections in the documentation
• Added unit tests for new algorithms, conversion methods, etc.
• Added reference image tests for new plotting functions, recipes, visual options, etc.

[MakieBot]

Compile Times benchmark

Note, 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))

	using	create	display	create	display
GLMakie	14.49s (14.20, 14.87) 0.24+-	1.40s (1.33, 1.49) 0.06+-	0.91s (0.88, 1.01) 0.05+-	15.67ms (15.50, 15.82) 0.13+-	183.29ms (181.64, 184.98) 1.27+-
master	14.11s (13.75, 14.54) 0.29+-	1.34s (1.29, 1.41) 0.04+-	884.52ms (854.49, 904.78) 16.34+-	15.70ms (15.48, 15.84) 0.15+-	183.43ms (180.49, 187.91) 2.54+-
evaluation	+2.67%, 0.39s slower X (1.47d, 0.02p, 0.26std)	+4.15%, 0.06s slower X (1.18d, 0.05p, 0.05std)	+2.37%, 0.02s invariant (0.62d, 0.28p, 0.03std)	-0.18%, -0.03ms invariant (-0.20d, 0.71p, 0.14std)	-0.08%, -0.14ms invariant (-0.07d, 0.90p, 1.90std)
CairoMakie	10.57s (10.54, 10.61) 0.03+-	1.09s (1.08, 1.11) 0.01+-	213.73ms (210.03, 215.59) 1.79+-	10.81ms (10.72, 10.86) 0.05+-	6.41ms (6.34, 6.48) 0.04+-
master	10.49s (10.43, 10.54) 0.04+-	1.06s (1.03, 1.08) 0.02+-	224.96ms (221.01, 229.56) 2.90+-	10.82ms (10.78, 10.85) 0.02+-	6.37ms (6.31, 6.42) 0.04+-
evaluation	+0.77%, 0.08s slower X (2.36d, 0.00p, 0.03std)	+3.36%, 0.04s slower X (1.95d, 0.00p, 0.02std)	-5.25%, -11.23ms faster✅ (-4.66d, 0.00p, 2.35std)	-0.12%, -0.01ms invariant (-0.33d, 0.55p, 0.04std)	+0.68%, 0.04ms invariant (1.08d, 0.07p, 0.04std)
WGLMakie	16.10s (15.90, 16.30) 0.12+-	1.55s (1.52, 1.60) 0.03+-	14.83s (14.52, 15.03) 0.18+-	19.84ms (17.35, 28.38) 3.88+-	1.46s (1.42, 1.52) 0.04+-
master	16.08s (15.91, 16.39) 0.16+-	1.54s (1.45, 1.64) 0.07+-	14.89s (14.35, 15.37) 0.35+-	17.72ms (17.06, 18.58) 0.68+-	1.48s (1.43, 1.54) 0.04+-
evaluation	+0.13%, 0.02s invariant (0.15d, 0.79p, 0.14std)	+1.15%, 0.02s invariant (0.34d, 0.54p, 0.05std)	-0.39%, -0.06s invariant (-0.20d, 0.71p, 0.27std)	+10.66%, 2.11ms noisy🤷‍♀️ (0.76d, 0.20p, 2.28std)	-1.37%, -0.02s invariant (-0.52d, 0.35p, 0.04std)

[ffreyer]

CairoMakie adds some red tones into the surface plot for some reason:

But that also happens on master and has nothing to do with this pr:

@SimonDanisch @jkrumbiegel What do you think about these changes? Should we add them to the next breaking release?

[jkrumbiegel]

Those changes sound good to me. Can we convert ranges to intervals for ImageLike? Should that error when the number of entries is different from the number of pixels? Or should we just be completely breaking and not allow ranges there at all, as they don't really make sense

[ffreyer]

Oh right, that was a pretty aggressive change and I didn't actually check if it was ok...

The behavior for irregular xs/ys and nonlinear transform functions in image is currently:

• GLMakie uses transformed extrema but keeps linear spacing in the quad
• CairoMakie errors
• WGLMakie uses transformed spacing

Heatmap with interpolate = true works with GLMakie and WGLMakie, but still errors in CairoMakie.

My assumption was that irregular images didn't work across the board so that an interval didn't remove any functionality. Since that's not true the question is whether we want to actually support that. CairoMakie and GLMakie currently treat an image simply as a single quad/rect, which should be a good bit faster than heatmap. If we want to support irregular images we'd probably repeat most of what heatmap does.

I don't really mind reverting the interval change for images, though I might rethink the naming. With vector xs and ys I would think of image as a top-right aligned cell based grid, which would fit into the GridBased branch of conversions but needs to be contrasted with heatmaps center aligned cell grids.

[jkrumbiegel]

whether we want to actually support that. CairoMakie and GLMakie currently treat an image simply as a single quad/rect

Yeah I think that's actually a very good distinction between heatmap and image that we should emphasize? I'm fine with specifying the boundaries of an image quad via intervals, while it makes sense I think to specify cell centers for the heatmap because you can shift those around. Not sure what we currently do if a range passed to heatmap is n or n+1 long (compared to the number of entries in that corresponding matrix dimension). Do we shift behavior automatically or error for one?

[ffreyer]

If n values are passed they are interpreted as centers and expanded to n+1, otherwise we just keep them. CairoMakie, GLMakie and WGLMakie are all ok with getting something other than n+1 values... I guess we should add errors here.

[ffreyer]

Simon is also OK with the restrictions to image.

I think the pr was mostly done, but we should probably emphasize the differences between heatmap and image a bit more in their docstrings.

[ffreyer]

Should we merge this into the beta branch?

[SimonDanisch]

Yes, with squash merge :)