[SOLVED] Lifetimes vs. Arc in an example render loop

wahn · February 16, 2018, 2:59pm

I'm at a point where I would like to use lifetimes, but so far I have avoided them and used std::sync::Arc a lot. The code base is pretty large right now, so maybe it's best to discuss things on basis of the current docs.

The C++ counterpart has two base classes where several algorithms can derive from to render the same scene various ways:

class Integrator {
  public:
    // Integrator Interface
    virtual ~Integrator();
    virtual void Render(const Scene &scene) = 0;
};
...
class SamplerIntegrator : public Integrator {
  public:
...
    void Render(const Scene &scene);
...
};

I had problems abstracting the Rust structs and traits the same way so currently I have two render functions (that's where the code is multi-threaded):

In C++ several classes derive from SamplerIntegrator:

$ rg -tcpp "public SamplerIntegrator" ~/git/github/pbrt-v3/src/
/home/jan/git/github/pbrt-v3/src/integrators/path.h
49:class PathIntegrator : public SamplerIntegrator {

/home/jan/git/github/pbrt-v3/src/integrators/whitted.h
49:class WhittedIntegrator : public SamplerIntegrator {

/home/jan/git/github/pbrt-v3/src/integrators/ao.h
48:class AOIntegrator : public SamplerIntegrator {

/home/jan/git/github/pbrt-v3/src/integrators/directlighting.h
52:class DirectLightingIntegrator : public SamplerIntegrator {

/home/jan/git/github/pbrt-v3/src/integrators/volpath.h
49:class VolPathIntegrator : public SamplerIntegrator {

On the Rust side I have implemented three of them so far, AOIntegrator, PathIntegrator, and DirectLightingIntegrator:

$ rg -trust "impl SamplerIntegrator"
src/integrators/ao.rs
40:impl SamplerIntegrator for AOIntegrator {

src/integrators/path.rs
51:impl SamplerIntegrator for PathIntegrator {

src/integrators/directlighting.rs
161:impl SamplerIntegrator for DirectLightingIntegrator {

They all use the first function render(...) and the trait methods abstract the algorithms, so the main render loop stays the same for all of them ...

The second function render_bdpt(...) is the one I'm currently working on (for bidirectional path tracing).

So the first problem is that I didn't manage to use traits for all of the integrators (yet) and that the signature for both functions is more complicated (which means that I do not store pointers to the camera and the sampler in the structs implementing the SamplerIntegrator trait):

pub fn render(
    scene: &Scene,
    camera: &Box<Camera + Send + Sync>,
    sampler: &mut Box<Sampler + Send + Sync>,
    integrator: &mut Box<SamplerIntegrator + Send + Sync>,
    num_threads: u8,
) {...}
...
pub fn render_bdpt(
    scene: &Scene,
    camera: &Box<Camera + Send + Sync>,
    sampler: &mut Box<Sampler + Send + Sync>,
    integrator: &mut Box<BDPTIntegrator>,
    num_threads: u8,
) {...}

But let's ignore that for now. I will post another message with my questions ...

wahn · February 16, 2018, 3:23pm

The last commit compiles with Rust stable (again), so feel free to modify the current code in a possible reply to my questions ...

So, let's talk about the second function, render_bdpt(...). The C++ counterpart creates subpaths from the camera and the lights, and tries to connect them:

void BDPTIntegrator::Render(const Scene &scene) {
...
        ParallelFor2D([&](const Point2i tile) {
 ...
                do {
...
                    Vertex *cameraVertices = arena.Alloc<Vertex>(maxDepth + 2);
                    Vertex *lightVertices = arena.Alloc<Vertex>(maxDepth + 1);
                    int nCamera = GenerateCameraSubpath(
                        scene, *tileSampler, arena, maxDepth + 2, *camera,
                        pFilm, cameraVertices);
...
                    int nLight = GenerateLightSubpath(
                        scene, *tileSampler, arena, maxDepth + 1,
                        cameraVertices[0].time(), *lightDistr, lightToIndex,
                        lightVertices);
...

                    for (int t = 1; t <= nCamera; ++t) {
                        for (int s = 0; s <= nLight; ++s) {
...
                            Spectrum Lpath = ConnectBDPT(
                                scene, lightVertices, cameraVertices, s, t,
                                *lightDistr, lightToIndex, *camera, *tileSampler,
                                &pFilmNew, &misWeight);
...
                        }
                    }
...
                } while (tileSampler->StartNextSample());
...
}

I managed to implement generate_camera_subpath(...) so far, the vertices get created correctly, but I have problems to return the resulting Vec<...> to the render loop:

pub fn render_bdpt(
    scene: &Scene,
    camera: &Box<Camera + Send + Sync>,
    sampler: &mut Box<Sampler + Send + Sync>,
    integrator: &mut Box<BDPTIntegrator>,
    num_threads: u8,
) {
...
                                while !done {
...
                                    // need to return generated vertices here
                                    let n_camera: usize = generate_camera_subpath(
                                        scene,
                                        &mut tile_sampler,
                                        integrator.max_depth + 2,
                                        camera,
                                        &p_film,
                                    );
...
                                }
...
}

wahn · February 16, 2018, 3:38pm

The src/integrators/bdpt.rs contains two functions which need attention regarding returning the resulting vertices, generate_camera_subpath(...) and random_walk(...):

pub fn generate_camera_subpath(
    scene: &Scene,
    sampler: &mut Box<Sampler + Send + Sync>,
    max_depth: u32,
    camera: &Box<Camera + Send + Sync>,
    p_film: &Point2f,
) -> usize {
...
    random_walk(
        scene,
        &mut ray,
        sampler,
        &mut beta,
        pdf_dir,
        max_depth - 1_u32,
        TransportMode::Radiance,
        vertex,
    ) + 1_usize
}
pub fn random_walk<'c, 'l, 'p, 's>(
    scene: &'p Scene,
    ray: &mut Ray,
    sampler: &mut Box<Sampler + Send + Sync>,
    beta: &mut Spectrum,
    pdf: Float,
    max_depth: u32,
    mode: TransportMode,
    vertex: Vertex,
) -> usize {
    // those two lines where previously in generate_camera_subpath()
    let mut path: Vec<Vertex> = Vec::with_capacity(max_depth as usize);
    path.push(vertex);
...
                // update previous vertex
                path[(bounces - 1) as usize].pdf_rev = new_pdf_rev;
                // store new vertex
                path.push(vertex);
...
    bounces
}

Maybe I'm overdoing it here with different lifetimes? Should I use one? Basically all those lifetimes refer to a single camera, several lights, several primitves, and shapes. All of them are traits:

Camera trait
Light trait
Primitive trait
Shape trait

Could I possibly use the lifetime of the Scene?

Whatever I tried so far failed. That's why I'm asking now regarding the current commit, until I proceed. I could post some variations and the resulting compiler errors ...

vitalyd · February 16, 2018, 10:00pm

Can you distill your question(s) to some reduced example that captures the essence of it? I suspect you’ll get more feedback that way.

abusch · February 19, 2018, 1:09am

Good questions... Unsurprisingly, I'm struggling with the same sort of problems Which is part of the reason why I've avoided BDPT for now... (working on VolPathIntegrator instead).

One idea that has been floating in my mind for a bit, which is kind of inspired by how the rust compiler is structured (I think), is to use 2 levels of memory arena: one for "static" data like scene, lights, primitives, materials, etc..., and one for data that needs to be created during rendering (i.e per tile or per ray). That should reduce the number of lifetimes to 2. However, one obstacle I foresee is that the memory arena implementations I've seen so far place some fairly strong restrictions on what types can be created from it. In particular they often require your types to be Copy. For good reasons, mind you, but a lot types aren't Copy

wahn · February 19, 2018, 10:15am

Hi @vitalyd,
thanks for your suggestion, I think I will try to stick to the current source code for now and describe step by step what I try to do and post the error messages I get, when it fails to compile. Maybe that will make it easier to extract the essence of the problem ...

wahn · February 19, 2018, 10:38am

Creating an empty vector with the needed capacity to the multi-threaded loop without passing it to the relevant function compiles:

diff --git a/pbrt/src/lib.rs b/pbrt/src/lib.rs
index e8401bf..b673761 100644
--- a/pbrt/src/lib.rs
+++ b/pbrt/src/lib.rs
@@ -51,7 +51,7 @@ use core::lightdistrib::create_light_sample_distribution;
 use core::pbrt::{Float, Spectrum};
 use core::sampler::Sampler;
 use core::scene::Scene;
-use integrators::bdpt::BDPTIntegrator;
+use integrators::bdpt::{BDPTIntegrator, Vertex};
 use integrators::bdpt::generate_camera_subpath;
 
 // see github/tray_rust/src/sampler/block_queue.rs
@@ -415,6 +415,8 @@ pub fn render_bdpt(
                                     }
                                         + tile_sampler.get_2d();
                                     // trace the camera subpath
+                                    let mut camera_vertices: Vec<Vertex> =
+                                        Vec::with_capacity((integrator.max_depth + 2) as usize);
                                     let n_camera: usize = generate_camera_subpath(
                                         scene,
                                         &mut tile_sampler,

Passing the vector to the generate_camera_subpath(...) without modifying it, compiles as well:

diff --git a/pbrt/src/integrators/bdpt.rs b/pbrt/src/integrators/bdpt.rs
index 51e29b9..a9ec852 100644
--- a/pbrt/src/integrators/bdpt.rs
+++ b/pbrt/src/integrators/bdpt.rs
@@ -287,6 +287,7 @@ pub fn generate_camera_subpath(
     max_depth: u32,
     camera: &Box<Camera + Send + Sync>,
     p_film: &Point2f,
+    path: &mut Vec<Vertex>,
 ) -> usize {
     if max_depth == 0 {
         return 0_usize;
diff --git a/pbrt/src/lib.rs b/pbrt/src/lib.rs
index b673761..e5452d5 100644
--- a/pbrt/src/lib.rs
+++ b/pbrt/src/lib.rs
@@ -423,6 +423,7 @@ pub fn render_bdpt(
                                         integrator.max_depth + 2,
                                         camera,
                                         &p_film,
+                                        &mut camera_vertices,
                                     );
                                     // Get a distribution for sampling
                                     // the light at the start of the

Just passing the path along to the next function without really using it works as well (there is a local path, which over-shadows the function parameter path):

diff --git a/pbrt/src/integrators/bdpt.rs b/pbrt/src/integrators/bdpt.rs
index 51e29b9..a747d42 100644
--- a/pbrt/src/integrators/bdpt.rs
+++ b/pbrt/src/integrators/bdpt.rs
@@ -313,6 +314,7 @@ pub fn generate_camera_subpath(
         max_depth - 1_u32,
         TransportMode::Radiance,
         vertex,
+        path,
     ) + 1_usize
 }
 
@@ -325,6 +327,7 @@ pub fn random_walk<'c, 'l, 'p, 's>(
     max_depth: u32,
     mode: TransportMode,
     vertex: Vertex,
+    path: &mut Vec<Vertex>,
 ) -> usize {
     // those two lines where previously in generate_camera_subpath()
     let mut path: Vec<Vertex> = Vec::with_capacity(max_depth as usize);

But as soon as I use the real path function parameter Rust declines to compile:

diff --git a/pbrt/src/integrators/bdpt.rs b/pbrt/src/integrators/bdpt.rs
index a747d42..b42f1d0 100644
--- a/pbrt/src/integrators/bdpt.rs
+++ b/pbrt/src/integrators/bdpt.rs
@@ -330,7 +330,6 @@ pub fn random_walk<'c, 'l, 'p, 's>(
     path: &mut Vec<Vertex>,
 ) -> usize {
     // those two lines where previously in generate_camera_subpath()
-    let mut path: Vec<Vertex> = Vec::with_capacity(max_depth as usize);
     path.push(vertex);
     let mut bounces: usize = 0_usize;
     if max_depth == 0_u32 {

$ cargo test --release
...
   Compiling ply-rs v0.1.1
   Compiling pbrt v0.2.7 (file:///home/jan/git/self_hosted/Rust/pbrt)
error[E0623]: lifetime mismatch
   --> src/integrators/bdpt.rs:333:15
    |
329 |     vertex: Vertex,
    |             ------ these two types are declared with different lifetimes...
330 |     path: &mut Vec<Vertex>,
    |                    ------
...
333 |     path.push(vertex);
    |               ^^^^^^ ...but data from `vertex` flows into `path` here

error[E0621]: explicit lifetime required in the type of `path`
   --> src/integrators/bdpt.rs:411:27
    |
330 |     path: &mut Vec<Vertex>,
    |     ---- consider changing the type of `path` to `&mut std::vec::Vec<integrators::bdpt::Vertex<'_, '_, 'p, '_>>`
...
411 |                 path.push(vertex);
    |                           ^^^^^^ lifetime `'p` required

error[E0623]: lifetime mismatch
   --> src/integrators/bdpt.rs:333:15
    |
329 |     vertex: Vertex,
    |             ------ these two types are declared with different lifetimes...
330 |     path: &mut Vec<Vertex>,
    |                    ------
...
333 |     path.push(vertex);
    |               ^^^^^^ ...but data from `vertex` flows into `path` here

error[E0621]: explicit lifetime required in the type of `path`
   --> src/integrators/bdpt.rs:411:27
    |
330 |     path: &mut Vec<Vertex>,
    |     ---- consider changing the type of `path` to `&mut std::vec::Vec<integrators::bdpt::Vertex<'_, '_, 'p, '_>>`
...
411 |                 path.push(vertex);
    |                           ^^^^^^ lifetime `'p` required

error: aborting due to 5 previous errors

error: Could not compile `pbrt`.
warning: build failed, waiting for other jobs to finish...
error: aborting due to 5 previous errors

error: Could not compile `pbrt`.

wahn · February 19, 2018, 10:52am

Adding the (camera) vertex earlier, before passing the path on, doesn't work either:

diff --git a/pbrt/src/integrators/bdpt.rs b/pbrt/src/integrators/bdpt.rs
index a747d42..9dd35a7 100644
--- a/pbrt/src/integrators/bdpt.rs
+++ b/pbrt/src/integrators/bdpt.rs
@@ -304,6 +304,7 @@ pub fn generate_camera_subpath(
     ray.scale_differentials(1.0 as Float / (sampler.get_samples_per_pixel() as Float).sqrt());
     // generate first vertex on camera subpath and start random walk
     let vertex: Vertex = Vertex::create_camera(camera, &ray, &beta);
+    path.push(vertex);
     let (pdf_pos, pdf_dir) = camera.pdf_we(&ray);
     random_walk(
         scene,
@@ -313,7 +314,6 @@ pub fn generate_camera_subpath(
         pdf_dir,
         max_depth - 1_u32,
         TransportMode::Radiance,
-        vertex,
         path,
     ) + 1_usize
 }
@@ -326,12 +326,8 @@ pub fn random_walk<'c, 'l, 'p, 's>(
     pdf: Float,
     max_depth: u32,
     mode: TransportMode,
-    vertex: Vertex,
     path: &mut Vec<Vertex>,
 ) -> usize {
-    // those two lines where previously in generate_camera_subpath()
-    let mut path: Vec<Vertex> = Vec::with_capacity(max_depth as usize);
-    path.push(vertex);
     let mut bounces: usize = 0_usize;
     if max_depth == 0_u32 {
         return bounces;

It fails with:

   Compiling ply-rs v0.1.1
   Compiling pbrt v0.2.7 (file:///home/jan/git/self_hosted/Rust/pbrt)
error[E0623]: lifetime mismatch
   --> src/integrators/bdpt.rs:307:15
    |
288 |     camera: &Box<Camera + Send + Sync>,
    |             -------------------------- these two types are declared with different lifetimes...
289 |     p_film: &Point2f,
290 |     path: &mut Vec<Vertex>,
    |                    ------
...
307 |     path.push(vertex);
    |               ^^^^^^ ...but data from `camera` flows into `path` here

error[E0621]: explicit lifetime required in the type of `path`
   --> src/integrators/bdpt.rs:408:27
    |
329 |     path: &mut Vec<Vertex>,
    |     ---- consider changing the type of `path` to `&mut std::vec::Vec<integrators::bdpt::Vertex<'_, '_, 'p, '_>>`
...
408 |                 path.push(vertex);
    |                           ^^^^^^ lifetime `'p` required

error[E0623]: lifetime mismatch
   --> src/integrators/bdpt.rs:307:15
    |
288 |     camera: &Box<Camera + Send + Sync>,
    |             -------------------------- these two types are declared with different lifetimes...
289 |     p_film: &Point2f,
290 |     path: &mut Vec<Vertex>,
    |                    ------
...
307 |     path.push(vertex);
    |               ^^^^^^ ...but data from `camera` flows into `path` here

error[E0621]: explicit lifetime required in the type of `path`
   --> src/integrators/bdpt.rs:408:27
    |
329 |     path: &mut Vec<Vertex>,
    |     ---- consider changing the type of `path` to `&mut std::vec::Vec<integrators::bdpt::Vertex<'_, '_, 'p, '_>>`
...
408 |                 path.push(vertex);
    |                           ^^^^^^ lifetime `'p` required

error: aborting due to 2 previous errors

error: Could not compile `pbrt`.
warning: build failed, waiting for other jobs to finish...
error: aborting due to 2 previous errors

error: Could not compile `pbrt`.

wahn · February 19, 2018, 12:52pm

I'm not sure why this works now, but I managed to compile again (and storing the vertices in the path). The signatures of the two functions in question simply use the same lifetime 'a like this:

pub fn generate_camera_subpath<'a>(
    scene: &'a Scene,
    sampler: &mut Box<Sampler + Send + Sync>,
    max_depth: u32,
    camera: &'a Box<Camera + Send + Sync>,
    p_film: &Point2f,
    path: &'a mut Vec<Vertex<'a, 'a, 'a>>,
) -> usize {
...
    let vertex: Vertex = Vertex::create_camera(camera, &ray, &beta);
    path.push(vertex);
...
}

pub fn random_walk<'a>(
    scene: &'a Scene,
    ray: &mut Ray,
    sampler: &mut Box<Sampler + Send + Sync>,
    beta: &mut Spectrum,
    pdf: Float,
    max_depth: u32,
    mode: TransportMode,
    path: &'a mut Vec<Vertex<'a, 'a, 'a>>,
) -> usize {
...
            let mut vertex: Vertex =
                Vertex::create_surface(isect.clone(), &beta, pdf_fwd, &path[bounces as usize]);
...
                // store new vertex
                path.push(vertex);
...
    bounces
}

I guess that's what I meant by:

Maybe I’m overdoing it here with different lifetimes?

wahn · February 19, 2018, 1:19pm

https://github.com/wahn/rs_pbrt/commit/c79e0dd4337d310de2a60b90964b1ffe7a1f49fd

vitalyd · February 20, 2018, 2:38am

Can’t say if you’re overdoing it with lifetimes, but I can say that you will likely need to specify lifetime parameters explicitly and not rely on lifetime elision. Unless your code lines up properly with default elision rules, it’s very likely that the elided lifetimes will not have the right relationships. You will need to pay particular attention to any types that are or become invariant (such as when referenced behind a mutable reference) since the compiler will be a lot more strict about the lifetimes. Conversely, for types that hold immutable references to variant types, you usually can get away with just a single lifetime parameter that covers all of them.

Topic		Replies	Views
Rust's std::sync::Arc vs. C++ std::shared_ptr, lifetimes, etc. questions on some example code help	28	5200	January 12, 2023
Beginner confusion: lifetimes of App, Window, and callback help	7	63	March 13, 2025
Addressing lifetimes in a basic example of a lib help	7	396	May 1, 2021
Help with struct lifetimes help	8	575	July 19, 2020
Confusion understanding lifetimes in a complex iterator help	7	229	July 22, 2024

[SOLVED] Lifetimes vs. Arc in an example render loop

Related topics