Precompilation of some recipes broken in MakieCore ^0.8.12

[termi-official]

Looks like #4655 broke precompilation in cases when there are multiple recipes with the same name. In the case of FerriteViz we have a custom wireframe recipe with a custom mesh format which works with v0.8.11.

Reproducer for the exact lines which break precompilation is given by instantiating Ferrite-FEM/FerriteViz.jl#103 with MakieCore at least v0.8.12 .

[SimonDanisch]

I'm not a big fan of redefining an existing recipe.
I think you should get around this by overloading:

const PlotterWireframe = Wireframe{<:Tuple{<:MakiePlotter{dim}}}

function Makie.default_theme(::Scene, ::PlotterWireframe)
    Attributes(
        plotnodes=true,
        color=theme(scene, :linecolor),
        strokewidth=theme(scene, :linewidth),
        markersize=theme(scene, :markersize),
        deformation_field=:default,
        visible=true,
        deformation_scale=1,
        textsize=15,
        offset=(0.0, 0.0),
        nodelabels=false,
        nodelabelcolor=:darkblue,
        celllabels=false,
        celllabelcolor=:darkred,
        cellsets=false,
        depth_shift=-0.0001f0
    )
end

function Makie.plot!(p::PlotterWireframe)
    ...
end

You may need to overload more functions, like here:
https://github.com/MakieOrg/Makie.jl/blob/master/src/basic_recipes/datashader.jl#L561-L592

I'm not sure if I'd recommend this though.
I really don't think a wireframe should do more than plotting a wireframe, and the above hack may not be stable for the next breaking version.

Considering how much this wireframe plot recipe actually does, it looks like it may be more sensible to use plot(plotter::MakiePlotter), and have that handle all things like labels/wireframe/etc.
I think it should work with something like:

function Makie.plot!(p::Makie.Plot(MakiePlotter))
...
end

[termi-official]

I'm not a big fan of redefining an existing recipe.

I agree on this, but this was the easiest solution back then to add additional dispatches. To recap the plan on my end, I need to find some time to resolve JuliaGeometry/GeometryBasics.jl#161 first. Once this has been resolved FerriteViz will essentially be reduced to just give routines to generate and manipulate a Makie-compatible meshes together with the data mapping efficiently. This should allow us to make some package for PDE specific visualization routines, which can then be reused across the different PDE packages, so we can finally avoid duplicating the almost same visualization code across the packages.

I really don't think a wireframe should do more than plotting a wireframe [...]

I am taking from your answer also that there is no easy forward-compatible way to add more dispatches to existing recipes (/extend recipes with custom input types) right now, even if we would have the same theme etc?

I also understand the recommendation to factor up the functionality further, adding stuff like plot_nodelabels/... separately. I will tackle this.

Furthermore, looking at

Makie.jl/src/basic_recipes/wireframe.jl

Lines 27 to 28 in 7a95dbc

	indices = decompose(LineFace{GLIndex}, g)
	points = decompose(Point, g)

would it be forward compatible to add dispatches for decompose on our custom plotter then?

Considering how much this wireframe plot recipe actually does, it looks like it may be more sensible to use plot(plotter::MakiePlotter), and have that handle all things like labels/wireframe/etc.
I think it should work with something like:

function Makie.plot!(p::Makie.Plot(MakiePlotter))
...
end

Can you ELI5 (or point me to the docs) on how the calls propagate from wireframe(plotter) to plot!(p::Makie.Plot(MakiePlotter))?

[SimonDanisch]

Can you ELI5 (or point me to the docs) on how the calls propagate from wireframe(plotter) to plot!(p::Makie.Plot(MakiePlotter))?

It doesn't... I meant that if you draw much more than a wireframe, you shouldn't call wireframe(plotter), but rather something like plot(plotter; wireframe=true) ...
And then call wireframe!(plotter) inside the plot recipe, and implement that via convert_argument without any additional scatter/label plots.