Updates for GeometryBasics refactor (#4319)

* get Makie to compile * get GLMakie to compile * fix nromal rename, meshscatter * fix pointtype in mesh conversion * avoid matrix in vector rotation * fix more tests * update GeometryBasics functions/interface * fix voronoiplot clipping * update CairoMakie * update RPRMakie * prepare WGLMakie * Update CI to use the GeometryBasics refactor branch (#4326) * Update ci.yml * Update Docs.yml * Update reference_tests.yml * add MeshIO branch --------- Co-authored-by: ffreyer <frederic481994@hotmail.de> * update _faces * normals -> normal * add ShaderAbstractions to ci * update docs, normals deprecation * fix branch names * fix catmesh getting normals regenerated * update for ShaderAbstractions * handle voxel clipping in CairoMakie like in GLMakie * fix another two "normals" * add dev branches for RPRMakie CI * fix missing kwarg name in conversion * update for FaceViews * add example for per-face colors and normals * improve typing * fix Sampler, allow mipmap via Sampler * try to fix benchmarks * add to correct env * remove from master project * fix docs * fix Rect decomposition * fix poly converts * fix typo * improve precompilation by avoiding constprop and more direct icon loading via PNGFiles * fix CairoMakie precompile * fix old syntax * fix ambient light * add temp dependencies to relocatbility test * bump RPR to required version * update changelog * fix test * fix other branch in ray cast test * fix relocatability ci * fix rpr and relocatability --------- Co-authored-by: Anshul Singhvi <asinghvi17@simons-rock.edu> Co-authored-by: SimonDanisch <sdanisch@protonmail.com>
MakieOrg · Oct 16, 2024 · d6f9fdb · d6f9fdb
1 parent 53b7e7e
commit d6f9fdb
Show file tree

Hide file tree

Showing 61 changed files with 300 additions and 231 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -47,6 +47,9 @@ jobs:
           pkg"registry up"
           Pkg.update()
           pkg"dev . ./MakieCore"
+          pkg"add GeometryBasics#ff/refactor"
+          pkg"add MeshIO#ff/GeometryBasics_refactor"
+          pkg"add ShaderAbstractions#ff/GeometryBasics_refactor"
           Pkg.test("Makie"; coverage=true)
 
       - uses: julia-actions/julia-processcoverage@v1

diff --git a/.github/workflows/reference_tests.yml b/.github/workflows/reference_tests.yml
@@ -46,6 +46,9 @@ jobs:
           # dev mono repo versions
           pkg"registry up"
           Pkg.update()
+          pkg"add MeshIO#ff/GeometryBasics_refactor"
+          pkg"add GeometryBasics#ff/refactor"
+          pkg"add ShaderAbstractions#ff/GeometryBasics_refactor"
           pkg"dev . ./MakieCore ./CairoMakie ./ReferenceTests"
       - name: Run the tests
         continue-on-error: true
@@ -96,6 +99,9 @@ jobs:
           # dev mono repo versions
           pkg"registry up"
           Pkg.update()
+          pkg"add MeshIO#ff/GeometryBasics_refactor"
+          pkg"add GeometryBasics#ff/refactor"
+          pkg"add ShaderAbstractions#ff/GeometryBasics_refactor"
           pkg"dev . ./MakieCore ./GLMakie ./ReferenceTests"
       - name: Run the tests
         id: referencetests
@@ -146,6 +152,9 @@ jobs:
           # dev mono repo versions
           pkg"registry up"
           Pkg.update()
+          pkg"add MeshIO#ff/GeometryBasics_refactor"
+          pkg"add GeometryBasics#ff/refactor"
+          pkg"add ShaderAbstractions#ff/GeometryBasics_refactor"
           pkg"dev . ./MakieCore ./WGLMakie ./ReferenceTests"
       - name: Run the tests
         continue-on-error: true

diff --git a/.github/workflows/rprmakie.yaml b/.github/workflows/rprmakie.yaml
@@ -44,6 +44,9 @@ jobs:
           using Pkg;
           pkg"registry up"
           Pkg.update()
+          pkg"add GeometryBasics#ff/refactor"
+          pkg"add MeshIO#ff/GeometryBasics_refactor"
+          pkg"add ShaderAbstractions#ff/GeometryBasics_refactor"
           # dev mono repo versions
           pkg"dev . ./MakieCore ./RPRMakie ./ReferenceTests"
       - name: Run the tests

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ## [Unreleased]
 
 - Improved performance of `record` by avoiding unnecessary copying in common cases [#4475](https://github.com/MakieOrg/Makie.jl/pull/4475).
+- Updated to GeoemtryBasic 0.5 [#4319](https://github.com/MakieOrg/Makie.jl/pull/4319)
 
 ## [0.21.14] - 2024-10-11
 

diff --git a/CairoMakie/src/display.jl b/CairoMakie/src/display.jl
@@ -80,7 +80,7 @@ function Makie.backend_show(screen::Screen{SVG}, io::IO, ::MIME"image/svg+xml",
     # xlink:href="someid" (but not xlink:href="data:someothercontent" which is how image data is attached)
     # url(#someid)
     svg = replace(svg, r"((?:(?:id|xlink:href)=\"(?!data:)[^\"]+)|url\(#[^)]+)" => SubstitutionString("\\1-$salt"))
-    
+
     print(io, svg)
     return screen
 end

diff --git a/CairoMakie/src/overrides.jl b/CairoMakie/src/overrides.jl
@@ -10,7 +10,7 @@ function draw_plot(scene::Scene, screen::Screen, poly::Poly)
     # dispatch on input arguments to poly to use smarter drawing methods than
     # meshes if possible.
     # however, since recipes exist, we can't explicitly handle all cases here
-    # so, we should also take a look at converted 
+    # so, we should also take a look at converted
     # First, we check whether a `draw_poly` method exists for the input arguments
     # before conversion:
     return if Base.hasmethod(draw_poly, Tuple{Scene, Screen, typeof(poly), typeof.(to_value.(poly.args))...})
@@ -85,10 +85,14 @@ draw_poly(scene::Scene, screen::Screen, poly, rect::Rect2) = draw_poly(scene, sc
 draw_poly(scene::Scene, screen::Screen, poly, bezierpath::BezierPath) = draw_poly(scene, screen, poly, [bezierpath])
 
 function draw_poly(scene::Scene, screen::Screen, poly, shapes::Vector{<:Union{Rect2, BezierPath}})
-    model = poly.model[]
-    space = to_value(get(poly, :space, :data))
-    clipped_shapes = clip_shape.(Ref(poly.clip_planes[]), shapes, space, Ref(model))
-    projected_shapes = project_shape.(Ref(poly), space, clipped_shapes, Ref(model))
+    model = poly.model[]::Mat4d
+    space = to_value(get(poly, :space, :data))::Symbol
+    planes = poly.clip_planes[]::Vector{Plane3f}
+
+    projected_shapes = map(shapes) do shape
+        clipped = clip_shape(planes, shape, space, model)
+        return project_shape(poly, space, clipped, model)
+    end
 
     color = to_cairo_color(poly.color[], poly)
 
@@ -188,7 +192,7 @@ end
 function draw_poly(scene::Scene, screen::Screen, poly, polygons::AbstractArray{<: MultiPolygon})
     model = poly.model[]
     space = to_value(get(poly, :space, :data))
-    projected_polys = map(polygons) do polygon 
+    projected_polys = map(polygons) do polygon
         project_multipolygon(poly, space, polygon, poly.clip_planes[], model)
     end
 

diff --git a/CairoMakie/src/precompiles.jl b/CairoMakie/src/precompiles.jl
@@ -15,6 +15,7 @@ let
         include(shared_precompile)
     end
 end
+precompile(openurl, (String,))
 precompile(draw_atomic_scatter, (Scene, Cairo.CairoContext, Tuple{typeof(identity),typeof(identity)},
                     Vector{ColorTypes.RGBA{Float32}}, Vec{2,Float32}, ColorTypes.RGBA{Float32},
                     Float32, BezierPath, Vec{2,Float32}, Quaternionf,

diff --git a/CairoMakie/src/primitives.jl b/CairoMakie/src/primitives.jl
@@ -9,22 +9,6 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Unio
 
     isempty(positions) && return
 
-    # workaround for a LineSegments object created from a GLNormalMesh
-    # the input argument is a view of points using faces, which results in
-    # a vector of tuples of two points. we convert those to a list of points
-    # so they don't trip up the rest of the pipeline
-    # TODO this shouldn't be necessary anymore!
-    if positions isa SubArray{<:Point3, 1, P, <:Tuple{Array{<:AbstractFace}}} where P
-        positions = let
-            pos = Point3f[]
-            for tup in positions
-                push!(pos, tup[1])
-                push!(pos, tup[2])
-            end
-            pos
-        end
-    end
-
     # color is now a color or an array of colors
     # if it's an array of colors, each segment must be stroked separately
     color = to_color(primitive.calculated_colors[])
@@ -1200,7 +1184,7 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Maki
 
     pos = primitive[1][]
     # For correct z-ordering we need to be in view/camera or screen space
-    model = copy(model)
+    model = copy(model)::Mat4d
     view = scene.camera.view[]
 
     zorder = sortperm(pos, by = p -> begin
@@ -1253,17 +1237,17 @@ function draw_atomic(scene::Scene, screen::Screen, @nospecialize(primitive::Maki
     pos = Makie.voxel_positions(primitive)
     scale = Makie.voxel_size(primitive)
     colors = Makie.voxel_colors(primitive)
-    marker = GeometryBasics.normal_mesh(Rect3f(Point3f(-0.5), Vec3f(1)))
-    
-    # Face culling
+    marker = GeometryBasics.clear_faceviews(normal_mesh(Rect3f(Point3f(-0.5), Vec3f(1))))
+
+
     if !isempty(primitive.clip_planes[]) && Makie.is_data_space(primitive.space[])
         valid = [is_visible(primitive.clip_planes[], p) for p in pos]
         pos = pos[valid]
         colors = colors[valid]
     end
 
     # For correct z-ordering we need to be in view/camera or screen space
-    model = copy(primitive.model[])
+    model = copy(primitive.model[])::Mat4d
     view = scene.camera.view[]
 
     zorder = sortperm(pos, by = p -> begin

diff --git a/CairoMakie/src/utils.jl b/CairoMakie/src/utils.jl
@@ -14,7 +14,7 @@ end
 
 # much faster than dot-ing `project_position` because it skips all the repeated mat * mat
 function project_position(
-        scene::Scene, space::Symbol, ps::Vector{<: VecTypes{N, T1}}, 
+        scene::Scene, space::Symbol, ps::Vector{<: VecTypes{N, T1}},
         indices::Vector{<:Integer}, model::Mat4, yflip::Bool = true
     ) where {N, T1}
 
@@ -44,7 +44,7 @@ function _project_position(scene::Scene, space, ps::AbstractArray{<: VecTypes{N,
 end
 
 function project_position(
-        scene::Scene, space::Symbol, ps::AbstractArray{<: VecTypes{N, T1}}, 
+        scene::Scene, space::Symbol, ps::AbstractArray{<: VecTypes{N, T1}},
         indices::Base.OneTo, model::Mat4, yflip::Bool = true
     ) where {N, T1}
 
@@ -149,14 +149,15 @@ function clip_shape(clip_planes::Vector{Plane3f}, shape::Rect2, space::Symbol, m
     if !Makie.is_data_space(space) || isempty(clip_planes)
         return shape
     end
-    
+
     xy = origin(shape)
     w, h = widths(shape)
-    ps = [xy, xy + Vec2(w, 0), xy + Vec2f(w, h), xy + Vec2(0, h)]
+    ps = Vec2f[xy, xy + Vec2f(w, 0), xy + Vec2f(w, h), xy + Vec2f(0, h)]
     if any(p -> Makie.is_clipped(clip_planes, p), ps)
         push!(ps, xy)
         ps = clip_poly(clip_planes, ps, space, model)
-        return BezierPath([MoveTo(ps[1]), LineTo.(ps[2:end])..., ClosePath()])
+        commands = Makie.PathCommand[MoveTo(ps[1]), LineTo.(ps[2:end])..., ClosePath()]
+        return BezierPath(commands::Vector{Makie.PathCommand})
     else
         return shape
     end
@@ -173,7 +174,7 @@ function project_polygon(@nospecialize(scenelike), space, poly::Polygon{N, T}, c
 
     ext_proj = PT[project(p) for p in clip_poly(clip_planes, ext, space, model)]
     interiors_proj = Vector{PT}[
-        PT[project(p) for p in clip_poly(clip_planes, decompose(PT, points), space, model)] 
+        PT[project(p) for p in clip_poly(clip_planes, decompose(PT, points), space, model)]
         for points in poly.interiors]
 
     return Polygon(ext_proj, interiors_proj)
@@ -195,7 +196,7 @@ end
     # at clip planes
     per_point_colors = colors <: AbstractArray
     per_point_linewidths = (T <: Lines) && (linewidths <: AbstractArray)
-    
+
     quote
         @get_attribute(plot, (space, model))
 
@@ -207,7 +208,7 @@ end
         @inbounds for (i, point) in enumerate(points)
             clip_points[i] = transform * to_ndim(Vec4d, to_ndim(Vec3d, point, 0), 1)
         end
-        
+
         # yflip and clip -> screen/pixel coords
         res = scene.camera.resolution[]
 
@@ -219,14 +220,14 @@ end
         end
 
         # Fix lines with points far outside the clipped region not drawing at all
-        # TODO this can probably be done more efficiently by checking -1 ≤ x, y ≤ 1 
+        # TODO this can probably be done more efficiently by checking -1 ≤ x, y ≤ 1
         #      directly and calculating intersections directly (1D)
         push!(clip_planes,
             Plane3f(Vec3f(-1, 0, 0), -1f0), Plane3f(Vec3f(+1, 0, 0), -1f0),
             Plane3f(Vec3f(0, -1, 0), -1f0), Plane3f(Vec3f(0, +1, 0), -1f0)
         )
 
-        
+
         # outputs
         screen_points = sizehint!(Vec2f[], length(clip_points))
         $(if per_point_colors
@@ -314,8 +315,8 @@ end
                     end)
                 elseif !hidden
                     # if not hidden, always push the first element to 1:end-1 line points
-                    
-                    # if the start of the segment is disconnected (moved), make sure the 
+
+                    # if the start of the segment is disconnected (moved), make sure the
                     # line separates before it
                     if disconnect1 && !last_is_nan
                         push!(screen_points, Vec2f(NaN))
@@ -326,7 +327,7 @@ end
                             :(push!(color_output, c1))
                         end)
                     end
-                    
+
                     last_is_nan = false
                     push!(screen_points, clip2screen(p1, res))
                     $(if per_point_linewidths
@@ -336,7 +337,7 @@ end
                         :(push!(color_output, c1))
                     end)
 
-                    # if the end of the segment is disconnected (moved), add the adjusted 
+                    # if the end of the segment is disconnected (moved), add the adjusted
                     # point and separate it from from the next segment
                     if disconnect2
                         last_is_nan = true
@@ -351,8 +352,8 @@ end
                 end
             end
 
-            # If last_is_nan == true, the last segment is either hidden or the moved 
-            # end point has been added. If it is false we're missing the last regular 
+            # If last_is_nan == true, the last segment is either hidden or the moved
+            # end point has been added. If it is false we're missing the last regular
             # clip_points
             if !last_is_nan
                 push!(screen_points, clip2screen(clip_points[end], res))
@@ -365,7 +366,7 @@ end
             end
 
         else  # LineSegments
-            
+
             for i in 1:2:length(clip_points)-1
                 $(if per_point_colors
                     quote
@@ -525,8 +526,8 @@ end
 """
     cairo_scatter_marker(marker)
 
-Convert a Makie marker to a Cairo-compatible marker.  This defaults to calling 
-`Makie.to_spritemarker`, but can be overridden for specific markers that can 
+Convert a Makie marker to a Cairo-compatible marker.  This defaults to calling
+`Makie.to_spritemarker`, but can be overridden for specific markers that can
 be directly rendered to vector formats using Cairo.
 """
 cairo_scatter_marker(marker) = Makie.to_spritemarker(marker)

diff --git a/GLMakie/src/GLAbstraction/GLExtendedFunctions.jl b/GLMakie/src/GLAbstraction/GLExtendedFunctions.jl
@@ -177,7 +177,9 @@ function glGenRenderbuffers(format::GLenum, attachment::GLenum, dimensions)
     return renderbuffer[1]
 end
 
-function glTexImage(ttype::GLenum, level::Integer, internalFormat::GLenum, w::Integer, h::Integer, d::Integer, border::Integer, format::GLenum, datatype::GLenum, data)
+Makie.@noconstprop function glTexImage(ttype::GLenum, level::Integer, internalFormat::GLenum, w::Integer,
+                                         h::Integer, d::Integer, border::Integer, format::GLenum,
+                                         datatype::GLenum, data)
     glTexImage3D(GL_PROXY_TEXTURE_3D, level, internalFormat, w, h, d, border, format, datatype, C_NULL)
     for l in  0:level
         result = glGetTexLevelParameteriv(GL_PROXY_TEXTURE_3D, l, GL_TEXTURE_WIDTH)
@@ -200,7 +202,8 @@ function glTexImage(ttype::GLenum, level::Integer, internalFormat::GLenum, w::In
     glTexImage3D(ttype, level, internalFormat, w, h, d, border, format, datatype, data)
 end
 
-function glTexImage(ttype::GLenum, level::Integer, internalFormat::GLenum, w::Integer, h::Integer, border::Integer, format::GLenum, datatype::GLenum, data)
+Makie.@noconstprop function glTexImage(ttype::GLenum, level::Integer, internalFormat::GLenum, w::Integer,
+                                         h::Integer, border::Integer, format::GLenum, datatype::GLenum, data)
     maxsize = glGetIntegerv(GL_MAX_TEXTURE_SIZE)
     glTexImage2D(GL_PROXY_TEXTURE_2D, level, internalFormat, w, h, border, format, datatype, C_NULL)
     for l in 0:level
@@ -220,7 +223,8 @@ function glTexImage(ttype::GLenum, level::Integer, internalFormat::GLenum, w::In
     glTexImage2D(ttype, level, internalFormat, w, h, border, format, datatype, data)
 end
 
-function glTexImage(ttype::GLenum, level::Integer, internalFormat::GLenum, w::Integer, border::Integer, format::GLenum, datatype::GLenum, data)
+Makie.@noconstprop function glTexImage(ttype::GLenum, level::Integer, internalFormat::GLenum, w::Integer,
+                                         border::Integer, format::GLenum, datatype::GLenum, data)
     glTexImage1D(GL_PROXY_TEXTURE_1D, level, internalFormat, w, border, format, datatype, C_NULL)
     for l in 0:level
         result = glGetTexLevelParameteriv(GL_PROXY_TEXTURE_1D, l, GL_TEXTURE_WIDTH)

diff --git a/GLMakie/src/GLAbstraction/GLTexture.jl b/GLMakie/src/GLAbstraction/GLTexture.jl
@@ -82,7 +82,7 @@ function set_packing_alignment(a) # at some point we should specialize to array/
     glPixelStorei(GL_UNPACK_SKIP_ROWS, 0)
 end
 
-function Texture(
+Makie.@noconstprop function Texture(
         data::Ptr{T}, dims::NTuple{NDim, Int};
         internalformat::GLenum = default_internalcolorformat(T),
         texturetype   ::GLenum = default_texturetype(NDim),
@@ -139,7 +139,7 @@ function Texture(s::ShaderAbstractions.Sampler{T, N}; kwargs...) where {T, N}
         pointer(s.data), size(s.data),
         minfilter = s.minfilter, magfilter = s.magfilter,
         x_repeat = s.repeat[1], y_repeat = s.repeat[min(2, N)], z_repeat = s.repeat[min(3, N)],
-        anisotropic = s.anisotropic; kwargs...
+        mipmap = s.mipmap, anisotropic = s.anisotropic; kwargs...
     )
     obsfunc = ShaderAbstractions.connect!(s, tex)
     push!(tex.observers, obsfunc)

diff --git a/GLMakie/src/GLAbstraction/GLTypes.jl b/GLMakie/src/GLAbstraction/GLTypes.jl
@@ -342,7 +342,6 @@ function RenderObject(
         pre::Pre, post,
         context=current_context()
     ) where Pre
-
     switch_context!(context)
 
     # This is a lazy workaround for disabling updates of `requires_update` when

diff --git a/GLMakie/src/GLAbstraction/GLUniforms.jl b/GLMakie/src/GLAbstraction/GLUniforms.jl
@@ -108,13 +108,15 @@ function glsl_typename(t::Type{T}) where T <: Mat
     string(opengl_prefix(eltype(t)), "mat", M==N ? M : string(N, "x", M))
 end
 toglsltype_string(t::Observable) = toglsltype_string(to_value(t))
-toglsltype_string(x::T) where {T<:Union{Real, Mat, StaticVector, Texture, Colorant, TextureBuffer, Nothing}} = "uniform $(glsl_typename(x))"
+function toglsltype_string(x::T) where {T<:Union{Real, Mat, StaticVector, Texture, Colorant, TextureBuffer, Nothing}}
+    return "uniform $(glsl_typename(x))"
+end
 #Handle GLSL structs, which need to be addressed via single fields
 function toglsltype_string(x::T) where T
     if isa_gl_struct(x)
         string("uniform ", T.name.name)
     else
-        error("can't splice $T into an OpenGL shader. Make sure all fields are of a concrete type and isbits(FieldType)-->true")
+        error("can't splice $T into an OpenGL shader. Make sure all fields are of a concrete type and isbits(FieldType)-->true\n\n$x")
     end
 end
 toglsltype_string(t::Union{GLBuffer{T}, GPUVector{T}}) where {T} = string("in ", glsl_typename(T))
@@ -237,7 +239,7 @@ gl_convert(a::T) where {T <: NATIVE_TYPES} = a
 gl_convert(s::Observable{T}) where {T <: NATIVE_TYPES} = s
 gl_convert(s::Observable{T}) where T = const_lift(gl_convert, s)
 gl_convert(x::StaticVector{N, T}) where {N, T} = map(gl_promote(T), x)
-gl_convert(x::Mat{N, M, T}) where {N, M, T} = map(gl_promote(T), x)
+gl_convert(x::Mat{N, M, T}) where {N, M, T} = Mat{N, M, gl_promote(T)}(x)
 gl_convert(a::AbstractVector{<: AbstractFace}) = indexbuffer(s)
 gl_convert(t::Type{T}, a::T; kw_args...) where T <: NATIVE_TYPES = a
 gl_convert(::Type{<: GPUArray}, a::StaticVector) = gl_convert(a)