Skip to content

Conversation

Meziu
Copy link
Member

@Meziu Meziu commented Aug 31, 2025

Fixes #37 (and possibly #36, but textures are not yet implemented)

This PR changes the rendering paradigm to use a RenderPass struct. This struct is effectively a marker of the frame's lifetime, and handles all draw and binding calls (instead of Instance). It does not hold references nor copies the render data.

The struct's calls take references of 'frame lifetime, so that it is statically checked that no items used in the rendering can be dropped before C3D_FrameEnd.

Example with shader programs

If the program is instantiated outside the render_frame_with function (and thus, due to borrowing, lives for longer), the rendering is done as intended.

Instead, putting the shader program initialization within the render call, would cause this error:

error[E0597]: `program` does not live long enough
   --> citro3d/examples/triangle.rs:103:31
    |
100 |         instance.render_frame_with(|mut pass| {
    |                                     -------- has type `RenderPass<'1>`
101 |             let program = shader::Program::new(vertex_shader).unwrap();
    |                 ------- binding `program` declared here
102 |             let projection_uniform_idx = program.get_uniform("projection").unwrap();
103 |             pass.bind_program(&program);
    |             ------------------^^^^^^^^-
    |             |                 |
    |             |                 borrowed value does not live long enough
    |             argument requires that `program` is borrowed for `'1`
...
144 |         });
    |         - `program` dropped here while still borrowed

Or, adding a drop() call after calling bind_program() would cause this error:

error[E0505]: cannot move out of `program` because it is borrowed
   --> citro3d/examples/triangle.rs:105:18
    |
 87 |     let program = shader::Program::new(vertex_shader).unwrap();
    |         ------- binding `program` declared here
...
103 |         instance.render_frame_with(|mut pass| {
    |                                     -------- has type `RenderPass<'1>`
104 |             pass.bind_program(&program);
    |             ---------------------------
    |             |                 |
    |             |                 borrow of `program` occurs here
    |             argument requires that `program` is borrowed for `'1`
105 |             drop(program);
    |                  ^^^^^^^ move out of `program` occurs here

Pros and cons

  • Statically checked lifetimes at compilation time

  • No copying of data within redundant internal representation (only uses the raw C3D context).

  • Data that gets copied within the C3D context (such as uniform buffers) can be handled without the special lifetime requirements.

  • Loss of global state, so each frame everything needs to be bound again (not a problem in big applications where things are bound and unbound constantly, but smaller games suffer added complexity).

  • I weren't able to encode the lifetime requirement into the render_to closure we previously used in our examples, so that became a standalone function (with some quite verbose arguments...).
    Edit: managed using a wrapping function around the closure that forces lifetimes as wanted.

Draft

Left to be implemented before un-drafting the PR:

@Meziu Meziu changed the title Require frame-long timelines for all items used during a render pass Require frame-long timelines for borrowed items used during a frame Aug 31, 2025
Copy link
Contributor

@0x00002a 0x00002a left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall, couple nitpicks, the Frame thing is important for correctness imo

@Meziu Meziu changed the title Require frame-long timelines for borrowed items used during a frame Require frame-long lifetimes for borrowed items used during a frame Aug 31, 2025
@Meziu
Copy link
Member Author

Meziu commented Sep 2, 2025

Added a partial Drop implementation. As more of the API is covered, FFI calls should be replaced by safe function calls, and due to #78 only attrib::Infos are currently not technically covered by the safety measures. Textures will need a close look when implemented.

@Meziu Meziu marked this pull request as ready for review September 2, 2025 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

C3D_BindProgram stores a pointer to a Program (use-after-free in bind_program)
2 participants