Skip to content

Conversation

SimonDanisch
Copy link
Member

The mesh type from GeometryBasics has been pretty insane (in terms of type complexity & usability) and making it the core representation in the mesh plot type has been a mistake for compilation latency and usability reasons.

This refactors the mesh plot type to get rid of GeometryBasics.Mesh, and instead just write out the mesh attributes we need.
We gain the following advantages:

  • make it easy to switch to the slimmed version of GeometryBasics Slim down GeometryBasics and remove all type complexity JuliaGeometry/GeometryBasics.jl#173
  • enable users to do mplot = mesh!(ax, ..., normals=my_normals) and mplot.normals = new_normals etc...Before that, all these would have been wrapped up in mplot.mesh and it wasn't possible to update them separately
  • prepare for a world were the basic plot types are well documented and typed.
    The fully typed mesh type will look something like:
struct MeshPlot{T, N}
    vertex_colors::Union{Nothing, RGBColors}
    image::Union{Nothing, Sampler{T}}
    colormap::Union{Nothing, Vector{RGBAf}}
    colorrange::Union{Nothing, Vec2f}

    normals::Union{Nothing, Vector{Vec3f}}
    texturecoordinates::Union{Nothing, Vector{Vec2f}}
    vertices::Vector{Point{N, Float32}}
    faces::Vector{GLTriangleFace}

    backlight::Float32
    shading::Bool
    fetch_pixel::Bool
    uv_scale::Vec2f
    transparency::Bool
end

And will then very cleanly lead to type stable code in the backends without the current mess, where nobody knows what the mesh plot type is even supposed to contain.

Switching to this representation will need to wait though and will be done in the attributes refactor.

@MakieBot
Copy link
Collaborator

Compile Times benchmark

@SimonDanisch SimonDanisch reopened this Mar 23, 2022
@SimonDanisch SimonDanisch added the breaking a PR with breaking changes label May 12, 2022
SimonDanisch added a commit that referenced this pull request Jun 22, 2022
* concretely type pattern fields

* transparency + fxaa should be fixed

* add propertynames for proper auto-complete for plot attributes

* fix pattern + add test
@SimonDanisch
Copy link
Member Author

Closed in favor of #2056

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking a PR with breaking changes
Projects
Development

Successfully merging this pull request may close these issues.

2 participants