Initial curve geometry support

[kubo-von]

This adds initial support for curve geometry with bspline basis (flat/normal oriented/round) and linear basis (just flat) After we're sure this is done right, I will add the rest of basis the same way. There's curve_geometry in examples which shows the both curve basis working.

Things I'm not sure about:

• In Embree API, there's CONE_LINEAR_CURVE and ROUND_LINEAR_CURVE but they don't seem to be in available in the sys.rs ?
• I've added the curve type as int function argument ( 0 - flat, 1 - normal oriented, 2 - round) - not sure if that's the best way, maybe using string keyword is more intuitive?
• I've added the curve flag buffer but not sure which values should be used there for left/right/both? I just copied 0x3 from the curve tutorial in my example.
• I'm not sure if I've set the byteStride for the new buffers I added correctly.

Thanks a for for your help!

[Twinklebear]

Awesome!

For your questions:

In Embree API, there's CONE_LINEAR_CURVE and ROUND_LINEAR_CURVE but they don't seem to be in available in the sys.rs ?

It looks like the last time I updated sys.rs was back on Embree 3.5, I just regenerated them now so those are in the RTCGeometryType enum now. There might be a minor merge conflict since I did a cargo fmt on the rest of the code here, but it looks like it shouldn't be bad to fix.

I've added the curve type as int function argument ( 0 - flat, 1 - normal oriented, 2 - round) - not sure if that's the best way, maybe using string keyword is more intuitive?

I think an enum will be clearest here, since that's basically how we'll end up using it to pick the corresponding geometry type flag (and is how you're using the int now):

pub enum CurveType {
    Flat,
    NormalOriented,
    Round
}

This could be in src/curve.rs so it's available to all the curves

I've added the curve flag buffer but not sure which values should be used there for left/right/both? I just copied 0x3 from the curve tutorial in my example.

I'm not so familiar with this item either but it looks like it's an optional thing based on the documentation https://www.embree.org/api.html#linear-basis ? Could we make that optional here too for a low memory option? Then we could take an extra bool argument if the caller wants this flag buffer or not.

I'm not sure if I've set the byteStride for the new buffers I added correctly.

It is actually correct to be a stride of 1 for the flags buffers, these look to get a special exception for the curve geometry.

[kubo-von]

Hey @Twinklebear , Thanks for the quick and thorough response! I've fixed the byte stride, removed flags buffer in bspline (I think I got confused there) and added enum for curve type.

I've tried to make the Normal buffer for bspline optional, but atm its make Embree segfault when bspline type is Normal Oriented and no Normal Buffer is set. What do you think would be the best way to solve this ?

Thanks!

[Twinklebear]

Oh interesting, so then maybe the best path is to not take an enum for the curve type and to instead express it through the function names. This would enforce at the API level that a normal buffer is required for a normal oriented curve, and optional for the other types.

So we'd have functions like

impl<'a> BsplineCurve<'a> {
    pub fn normal_oriented(device: &'a Device, num_segments: usize, num_verts: usize) -> BsplineCurve<'a> { ... }

    pub fn round(device: &'a Device, num_segments: usize, num_verts: usize, use_normals: bool) -> BsplineCurve<'a> { ... }

    pub fn flat(device: &'a Device, num_segments: usize, num_verts: usize, use_normals: bool) -> BsplineCurve<'a> { ... }

where use normals can be an option on the round and flat curves. And we'd have the same for the linear curve. Then these can all call in to an internal method in the class (basically just calling unanimated as before) but we can enforce that the curve configurations being made are valid.

[kubo-von]

Thanks @Twinklebear, sorry for late reply, got swamped by work.

I've addressed those two things, segfault is no more!

The option introduced a bit of clumpsiness: Normal buffer cannot be mapped as the other ones, but by: this curve.normal_buffer.as_mut().unwrap().map(); And I needed to use temp_normal_buffer when creating the normal buffer But I guess there is no way around. Do you thing all is good to implement the rest of basis this way ?

[Twinklebear]

No worries!

The option buffer does make the API a bit awkward, but I'm also not sure of a better option to do it either. Let's keep it how you have it for now and see if we think of something a bit more convenient later on.

Yeah, I think implementing the rest of the bases in the same way would work well!

[kubo-von]

Alright, I think I am getting close to finishing this. :) Hermite curve gave me a hard time, still have no idea who normal derivative buffer serves for, but it seems to work. The last 2 issues I am having is that linear round curve is not rendering and linear cone is causing segfault. Will try to figure it out this week, but if you have any suggestions why that might be? Thanks

[Twinklebear]

I'm not sure, I haven't used the curve geometry before so am not that familiar. One thing that might help is using a debug device https://github.com/Twinklebear/embree-rs/blob/master/src/device.rs#L16 , then Embree might print some info about wrong or missing parameters?

[kubo-von]

Thanks the debug device at least showed me that the segfault was happening during the intersection, that made me look again at my code again and found issues. Had some curve type mismatch, curve flags set in a bad way + using old version of Embree (silly me!) All curve basis/types seem to be working fine now!

[kubo-von]

If you think that everything's ok can I resolve the geometry.rs go ahead with the commit ? (not sure if it goes to master directly or you have to approve)

[Twinklebear]

Sorry @kubo-von I didn't get a chance to come back to this, you should be able to resolve the conflicts on the command line or I can do that when merging it manually. Awesome to see it's all working! I'll check it out shortly and should be able to merge

[Twinklebear]

I added some small notes but these are things I can also easily fix after merging if you'd prefer. Everything else looks great!

[kubo-von]

Thanks! Yeah if it's ok for you to fix after merge that will be maybe faster :) Forgot about the debug device, but it was Device::new() I think

[Twinklebear]

I'll make those final tweaks and then put up a release on cargo to add the curves API

[kubo-von]

Awesome, thanks! I think I will try the point geometry next, should be similar :)

[Twinklebear]

That'd be great! I think points should be even easier than curves since there's just a few variants if I remember correctly