Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add triangle_mesh shortcut #197

Closed
wants to merge 1 commit into from

Conversation

JonasIsensee
Copy link
Contributor

@JonasIsensee JonasIsensee commented Jul 10, 2023

When plotting a vector of many meshes in 2D and 3D a large portion of time is taken up by triangle_mesh or more specifically decompose_triangulate_fallback to first decompose and then reassemble the meshes.

This PR makes triangle_mesh(::TriangleMesh) an identity operation.

EDIT: this change does not yet yield the correct dispatch.
However, here's an example to show that it is worth it:

using GLMakie, GeometryBasics

julia> m = GeometryBasics.mesh([Point2f(0.0,0.0), Point2f(0.0,1.0), Point2f(1.0,0.0)], facetype = GLTriangleFace)
Mesh{2, Float32, Triangle}:
 Triangle(Float32[0.0, 0.0], Float32[1.0, 0.0], Float32[0.0, 1.0])

julia> mv = [deepcopy(m) for i=1:200000];

julia> @time mesh(mv) # second time
  1.179934 seconds (7.24 M allocations: 567.325 MiB, 6.40% gc time)

# here's an inelegant pass-through definition
julia> GeometryBasics.triangle_mesh(primitive::typeof(m)) = primitive

julia> @time mesh(mv) # second time
  0.064663 seconds (438.06 k allocations: 85.145 MiB, 34.41% gc time)

@JonasIsensee
Copy link
Contributor Author

JonasIsensee commented Jul 10, 2023

Correction: this doesn't work yet.
I'm struggling with the correct dispatch pattern.

TriangleMesh{N} is seen as less specific compared to Meshable{N}.
Given that there is such a detailed hierarchy of abstract mesh types it seems odd that the
Meshable union contains Mesh{Dim, T} directly instead of e.g. AbstractMesh.

const Meshable{Dim,T} = Union{Tesselation{Dim,T},Mesh{Dim,T},AbstractPolygon{Dim,T},

@JonasIsensee
Copy link
Contributor Author

superceded by #198

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant