Closed kubo-von closed 3 years ago
Hi @kubo-von , adding support for these in embree-rs would be great. Unfortunately at this time I'm trying to wrap up my thesis and defend, so most of my time is occupied elsewhere.
If you're interested in working on this though I should at least have some time to answer questions or review PRs. If so, I'd recommend starting by checking out the Embree's Curves Tutorial and API, and taking a look at embree-rs's TriangleMesh to see how it wraps and uses the Embree API for setting up the triangle mesh. You can also compare Embree's triangle geometry tutorial with embree-rs's triangle tutorial to see how things fit together. When you start adding the new geometry type you'll also need to modify the Geometry enum in this crate to support it.
Embree has a few additional geometries that I don't have support for here, in the end it would be nice to have them all hooked up to support everything that Embree itself supports (subdivision surfaces, grids, points, curves). It'd be interesting to see how user geometry support could also be added by hooking up some wrapper to go between C and Rust to have the intersection callback be written in Rust.
Hi @Twinklebear, completely understand that!
I'll try to take a stab at it myself, thanks a lot for the advices. I will come back here to report/ bother with questions. :) Additional geometry support and user geometry sounds good as well!
Cheers
Hi @Twinklebear, Got FLAT_LINEAR_CURVE working, Finally it was much easier than I imagined! (all the work was already done)
Commit in my fork (added an example as well)
My question is, would you implement new geometry type for each of the curve bases Embree supports or somehow merge the under one curve geometry type? Thanks!
Awesome!
That's a good question on which design would be best here, so far I've matched 1:1 Embree "geometry type enum" -> class, but there's a lot more variants of the curve types that might make it more annoying to use and implement. What might work well is to have one per-basis as you mention (Linear, Bezier, Hermite, B-Spline, Catmull-Rom) with the different variants of them (flat, normal, cone, round) as different constructors taking the additional optional parameters needed for those variants. Since the basis effects what the "vertex" buffer means, knowing the type when modifying the curve is probably important.
It's a bit of pain to add these all to the Geometry enum, but it's been a bit since I've been working a lot on this project so maybe Rust has improved how this works with traits now?
Sounds good! I will make split it per-basis then, starting with linear one with all its variants.
I think I will try add them to Geometry enum at first. I've worked with traits, but I'm not experienced enough to say if they improved or not. What was the issue you were having with them ?
I was kind of hoping for something where I could still have them not be in a Box, but treat them all as a geometry (so putting them in containers, etc.) without having to add this dispatching per-type. I do remember hearing some stuff about value traits or something(?) but I'm not sure. If this is still the most rust-y way to do it then lets stick with this style though
Also in the mean time I found some free time to migrate the CI for this project over to github actions so we can run some basic checks when you have a PR coming together.
Ah, right! I can stick with this style for now (still finding my feet in all of this) and maybe we can explore converting it all into trait approach after? Nice one! I should have more free time next week, starting Monday, so i will properly jump into this and send a PR :)
Added by #8
Hi @Twinklebear, I was wondering if there's any plans for adding support for other geometry types Embree offers, like curves.. If not, where would be a good place to start if one would one to implement those himself ? Thank you!
Ps: Thanks a lot for creating this crate and especially for the examples, they really helped me to get into rust and raytracing as a beginner.