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

Should the output function only be in the example file? #20

Closed
elbert5770 opened this issue Aug 28, 2023 · 3 comments
Closed

Should the output function only be in the example file? #20

elbert5770 opened this issue Aug 28, 2023 · 3 comments

Comments

@elbert5770
Copy link

It seems kind of fragile to not have the output function in the main file, MarchingCubes.jl.

@t-bltg
Copy link
Member

t-bltg commented Aug 28, 2023

I don't really understand what you mean, could you clarify ?

@t-bltg t-bltg closed this as not planned Won't fix, can't repro, duplicate, stale Sep 4, 2023
@elbert5770
Copy link
Author

elbert5770 commented Sep 8, 2023

"example.jl" implies a file that stores a simple example, and in fact this is the case (i.e. scenario()). However, the file also contains the following crucial functions, which aren't even used in the example. It seems like they should be in MarchingCubes.jl

output(PlyIO::Module, m::MC, fn::AbstractString = "test.ply") = begin
    ply = PlyIO.Ply()
    push!(
        ply,
        PlyIO.PlyElement(
            "vertex",
            PlyIO.ArrayProperty("x", map(v -> Float32(v[1]), m.vertices)),
            PlyIO.ArrayProperty("y", map(v -> Float32(v[2]), m.vertices)),
            PlyIO.ArrayProperty("z", map(v -> Float32(v[3]), m.vertices)),
            PlyIO.ArrayProperty("nx", map(n -> Float32(n[1]), m.normals)),
            PlyIO.ArrayProperty("ny", map(n -> Float32(n[2]), m.normals)),
            PlyIO.ArrayProperty("nz", map(n -> Float32(n[3]), m.normals)),
        ),
    )
    vertex_indices = PlyIO.ListProperty("vertex_indices", UInt8, Int32)
    for i  eachindex(m.triangles)
        push!(vertex_indices, m.triangles[i] .- 1)
    end
    push!(ply, PlyIO.PlyElement("face", vertex_indices))

    PlyIO.save_ply(ply, fn, ascii = true)
    nothing
end

makemesh_Meshes(Meshes::Module, m::MC) = begin
    points = map(Meshes.Point3  Tuple, m.vertices)
    tris = Meshes.connect.(map(Tuple, m.triangles), Meshes.Triangle)
    Meshes.SimpleMesh(points, [tris;])
end

makemesh_GeometryBasics(GeometryBasics::Module, m::MC) = begin
    vertices = map(GeometryBasics.Point3f, m.vertices)
    normals = map(GeometryBasics.Vec3f, m.normals)
    triangles = map(t -> GeometryBasics.TriangleFace(t...), m.triangles)
    GeometryBasics.Mesh(GeometryBasics.meta(vertices; normals), triangles)
end

makemesh(mod::Module, m::MC) =
    if (mod_str = string(mod)) == "Meshes"
        makemesh_Meshes(mod, m)
    elseif mod_str == "GeometryBasics"
        makemesh_GeometryBasics(mod, m)
    else
        throw(ArgumentError("un-supported module `$mod`"))
    end

@t-bltg
Copy link
Member

t-bltg commented Sep 8, 2023

Those functions were solely added to show an example of using the MarchingCubes algorithm as a library.

Furthermore, they use modules arguments (e.g. PlyIO), without specifying any version constraints, and are hence not guaranteed to work in the future.

They are also related to how you visualize your computations, and since everyone has its own preferred graphics / plotting library, they are not universal.

This is why I don't think they should be part of the library.

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

No branches or pull requests

2 participants