JuliaGeometry / Meshes.jl

Computational geometry in Julia
https://juliageometry.github.io/MeshesDocs/stable
Other
390 stars 84 forks source link

Moving surface types from OpticSim.jl into Meshes.jl #93

Closed BrianGun closed 1 year ago

BrianGun commented 3 years ago

@juliohm suggested perhaps it would be a good idea to put some of our geometry code from OpticSim.jl in Meshes.jl.

We currently support Bezier and NURBS patches, and Zernike, QType, and Chebyshev polynomials. I've never seen the last three surface types used anywhere except in optics so they may not be great candidates for inclusion in Meshes.jl. Bezier and NURBS patches are used in many fields. We implemented just enough functionality to make these two surface types useable in OpticSim. Not sure if that is enough for your purposes.

For this code to be useful to us in OpticSim we need partials with respect to the parameterizing variables. Our ray intersection returns intervals instead of intersection points. The intervals are used in our CSG computations. Would it make sense to move the partial and ray intersection code to Meshes.jl? It doesn't seem to fit with the philosophy of the rest of the package.

Have to also say that I'm 100% time committed between my job and maintaining OpticSim.jl so I wouldn't be able to maintain this code here on Meshes.jl. Is there a contributor here who wants to take on the job?

juliohm commented 3 years ago

Thank you @BrianGun for moving the discussion here. We have a very broad scope in Meshes.jl and the more primitives we have the better (unless they come with heavy dependencies, is it the case?). In the future, we can assume that maintainers will be distributed across different downstream applications, including OpticSim.jl for example.

How do you feel we should proceed here? We can certainly assign ourselves to help with the migration of types. That will potentially help us improve our API and type hierarchy as well.

Best,

BrianGun commented 3 years ago

@juliohm if by dependencies you mean package dependencies then no there are not heavy dependencies. The code is largely self contained. Most of the algorithms are taken from the [ NURBS book](: https://www.amazon.com/NURBS-Book-Monographs-Visual-Communication/dp/3540615458/ref=asc_df_3540615458/?tag=bingshoppinga-20&linkCode=df0&hvadid=&hvpos=&hvnetw=o&hvrand=&hvpone=&hvptwo=&hvqmt=e&hvdev=c&hvdvcmdl=&hvlocint=&hvlocphy=&hvtargid=pla-4584207577648506&psc=1)

The place to start would be for you all to take a look at the files Bezier.jl, BSpline.jl, Knots.jl, and Spline.jl in the OpticSim.jl/src/Geometry/Primitives/Curves directory. These files have all the functionality for these surface types. We have functions to compute surfacepoints, parametric partials, surface normals, to convert from NURBS to Bezier patches, to refine knot vectors, which allows you to subdivide patches, and probably some other stuff I'm forgetting right now.

Once you get a feel for them then you can tell us what part of the functionality fits into the Meshes.jl design philosophy. If you want all of the functionality then it would definitely make sense for us to take a dependency on you. If you only want a small chunk then it may make less sense.

If we all decide it makes sense to move the code then we can help you figure out how to do it.

These functions have not been as thoroughly tested as some of the others in OpticSim. We made them work well enough for our needs at the time so there could be bugs lurking there. None that we are aware of though.

juliohm commented 3 years ago

That is a very good plan @BrianGun , thank you for sharing the pointers. We will take a look and get back to you afterwards.

juliohm commented 3 years ago

Hi @BrianGun , I had a chance to take a look at the curves defined in OpticSim.jl, thanks for the pointers. In Meshes.jl, @serenity4 has recently added BezierCurve with two methods of evaluation. Do you think we could extend our implementation to include the partials and normals necessary to your use case in OpticSim.jl? I think that could be a good starting point.

serenity4 commented 3 years ago

Hi @BrianGun, This migration would be very much appreciated. We were actually just thinking of defining B-splines/NURBS in Meshes, among others. I'm not an expert in these domains, but I'll take a look when I have some time and then we can figure out how to make the transition.

BrianGun commented 3 years ago

Sounds good. Will wait to hear from you then.

One other thing that is extremely important for us is that the number of allocations when evaluating the surfaces be essentially zero otherwise multithreaded performance in our ray tracer is terrible. I think we had hand unrolled loops in some of these functions for that reason.

I also have code for computing analytic intersections between lines and curves that is not part of the ray tracing project, and started on lines and surface patches. Would you be interested in adding that as well?

hyrodium commented 3 years ago

I'm also developing a package for B-spline, so I can help this issue. :muscle: https://github.com/hyrodium/BasicBSpline.jl

serenity4 commented 3 years ago

Great! I am currently working to figure out a good way to integrate transforms into the library and my bandwidth won't really let me focus on this issue for now. Your help would be definitely appreciated, especially since you seem to have more experience than me in these topics.

I also have code for computing analytic intersections between lines and curves that is not part of the ray tracing project, and started on lines and surface patches. Would you be interested in adding that as well?

Sure! We intend to provide as many tools as possible for geometry processing to avoid fragmentation of functionality and duplication of efforts. Once the types mentioned first are moved to Meshes we can take a look at that.

BrianGun commented 3 years ago

It might be a while before I can get around to this. OpticSim is keeping me busy, among other things. Ping me in a month and I'll see if there is time in my schedule.

BrianGun commented 3 years ago

@hydrodium it looks like your package supports non-uniform knots. Does it also support rational B-Splines? If it doesn't I've got code for this case NURBS which you are welcome to swipe if it is useful.

hyrodium commented 3 years ago

@BrianGun Thanks for the comment! I'll check the code. No, the current BasicBSpline.jl doesn't provide methods for NURBS, but I was planning to add them (and also T-spline) in the future.

I think B-spline basis function is not just for geometry, so it's better to depend on other B-spline package, I guess.

juliohm commented 2 years ago

Hi everyone, any update on this integration? It would be really nice to have all these spline models united :)

BrianGun commented 2 years ago

i have no objection to the integration but don't have time right now to look at this. Is there somebody working on meshes.jl who wants to take this on?

juliohm commented 1 year ago

I am closing this issue since no one is actively working on it. Thanks for considering the migration @BrianGun :)