jalberse / shimmer

Physically based rendering in Rust
Apache License 2.0
33 stars 0 forks source link

Actually make use of a scratch buffer in rendering #49

Open jalberse opened 5 months ago

jalberse commented 5 months ago

I have a scratch_buffer declared (using bumpalo, for now), but we don't actually allocate with it.

Using bumpalo might be tricky with handling lifetimes - its alloc returns an exclusive mutable reference.

It might be easier to avoid lifetimes by using an ID-based arena allocator instead. We might want an abstraction around the IDs (storing in BSDFs?). This would avoid lifetime issues, I think - we just pass the scratch buffer/arena and the IDs.

jalberse commented 5 months ago

We would only use this for BxDFs right now; BSDFs don't need it. BSSRDFs would need it in the future, as well as for ray majorant sections in Media eventually.

jalberse commented 5 months ago

The lifetimes are proving tricky enough (or my 2AM brain sleepy enough) that I'm going to table this for now.

This was probably premature optimization right now anyways; I should look at WPA first to see where we're spending time anyways, I'm not sure what kind of performance gain this can give us, especially with only Diffuse BxDFs right now.

Progress is in the allocating branch.

jalberse commented 5 months ago

The issue with the lifetimes I was running into is that in e.g. DiffuseMaterial::get_bsdf(), we're trying to return a BSDF which holds a &'a BxDF returned from the Bump::alloc() for the BxDF. ~really~, we just need to ensure that our BSDF doesn't outlive our Bump, which it doesn't. But the compiler gets hung up on the lifetime of &self (the material and the interaction).

I tried kicking up the instantiation of the BxDF and/or BSDF, and passing in the reference, but the fact that the BSDF holds a reference to the BxDF (and thus already needs it available) made that unwieldy. I'm tired though, maybe there's merit here.

I vaguely think we need to work with lifetime subtyping: https://web.mit.edu/rust-lang_v1.25/arch/amd64_ubuntu1404/share/doc/rust/html/book/second-edition/ch19-02-advanced-lifetimes.html

Specifically, that the lifetime of the Bump will outlive all the other lifetimes involved, which should be true of at least the surface interaction. That's where I would start looking, anyways.

Slightly related - we should only need one implementation of MaterialI::get_bsdf(), but varying on the type returned. Can we do a "concrete variant"? I'd just do this after I figure out the lifetime stuff.