Refactor: Bundle Window and Surface into single struct with proper lifetimes #604

New issue

Open

opened 2026-01-05 22:40:23 +00:00 by SakulFlee · 0 comments

SakulFlee commented

2026-01-05 22:40:23 +00:00

Owner

Problem

Currently, Runtime stores Window and Surface as separate Option fields, using transmute to cast Surface<'w> to Surface<'static> to avoid lifetime parameters. This approach has several issues:

Unsafe transmute bypasses lifetime checking - We're lying to the compiler about the actual lifetime relationship between Window and Surface
Drop order bugs - Even with manual Drop implementation, we're experiencing issues with surface cleanup
No compile-time safety - The compiler can't prevent accidentally dropping the window while the surface still exists
Future refactoring risk - Changes to field order or drop logic could introduce undefined behavior

Current code structure

struct Runtime {
    window: Option<Window>,
    instance: Option<Instance>,
    surface: Option<Surface<'static>>,  // transmuted from Surface<'w>
}

// Creation uses unsafe transmute
self.surface = Some(unsafe {
    transmute(
        self.instance
            .as_ref()
            .unwrap()
            .create_surface(self.window.as_ref().unwrap())
            .expect("Failed creating Surface!"),
    )
});

Proposed Solution

Bundle Window, Surface, and potentially Instance into a single struct that properly encapsulates their lifetime relationship:

struct WindowSurface<'w> {
    instance: Instance,
    window: Window,
    surface: Surface<'w>,
}

impl<'w> WindowSurface<'w> {
    fn new(event_loop: &EventLoop, settings: &RuntimeSettings) -> Self {
        let window = event_loop
            .create_window(
                Window::default_attributes()
                    .with_active(true)
                    .with_inner_size(settings.size)
                    .with_title(settings.name.clone()),
            )
            .unwrap();
        
        let instance = make_instance();
        let surface = instance
            .create_surface(&window)
            .expect("Failed creating Surface!");
        
        Self { instance, window, surface }
    }
}

impl Drop for WindowSurface<'_> {
    fn drop(&mut self) {
        // Explicit guaranteed drop order
        // Surface drops first, then window
    }
}

struct Runtime<'w> {
    window_surface: Option<WindowSurface<'w>>,
}

Benefits

Type safety - Remove unsafe transmute, let the compiler enforce lifetime correctness
Clear ownership - Window and Surface are explicitly bound together as a single unit
Reliable cleanup - Drop order is enforced in one place, fixing existing drop bugs
Better encapsulation - All window/surface logic lives in one struct
Easier to test - Can test WindowSurface creation/destruction independently
Prevents misuse - Impossible to accidentally drop window before surface at compile time

Acceptance Criteria

Create WindowSurface struct that bundles window, surface, and instance
Remove unsafe transmute from surface creation
Add lifetime parameter 'w to Runtime struct
Implement proper Drop for WindowSurface to ensure correct cleanup order
Verify that existing drop bugs are resolved
Update all window/surface access code to use the new structure
Confirm that lifetime parameter doesn't spread beyond necessary boundaries

Notes

The lifetime parameter on Runtime<'w> should not spread further if Runtime is owned by the parent struct (e.g., App). This is a one-time addition that buys us significant safety guarantees.

## Problem Currently, `Runtime` stores `Window` and `Surface` as separate `Option` fields, using `transmute` to cast `Surface<'w>` to `Surface<'static>` to avoid lifetime parameters. This approach has several issues: 1. **Unsafe transmute bypasses lifetime checking** - We're lying to the compiler about the actual lifetime relationship between Window and Surface 2. **Drop order bugs** - Even with manual `Drop` implementation, we're experiencing issues with surface cleanup 3. **No compile-time safety** - The compiler can't prevent accidentally dropping the window while the surface still exists 4. **Future refactoring risk** - Changes to field order or drop logic could introduce undefined behavior ### Current code structure ```rust struct Runtime { window: Option<Window>, instance: Option<Instance>, surface: Option<Surface<'static>>, // transmuted from Surface<'w> } // Creation uses unsafe transmute self.surface = Some(unsafe { transmute( self.instance .as_ref() .unwrap() .create_surface(self.window.as_ref().unwrap()) .expect("Failed creating Surface!"), ) }); ``` ## Proposed Solution Bundle `Window`, `Surface`, and potentially `Instance` into a single struct that properly encapsulates their lifetime relationship: ```rust struct WindowSurface<'w> { instance: Instance, window: Window, surface: Surface<'w>, } impl<'w> WindowSurface<'w> { fn new(event_loop: &EventLoop, settings: &RuntimeSettings) -> Self { let window = event_loop .create_window( Window::default_attributes() .with_active(true) .with_inner_size(settings.size) .with_title(settings.name.clone()), ) .unwrap(); let instance = make_instance(); let surface = instance .create_surface(&window) .expect("Failed creating Surface!"); Self { instance, window, surface } } } impl Drop for WindowSurface<'_> { fn drop(&mut self) { // Explicit guaranteed drop order // Surface drops first, then window } } struct Runtime<'w> { window_surface: Option<WindowSurface<'w>>, } ``` ## Benefits 1. **Type safety** - Remove unsafe `transmute`, let the compiler enforce lifetime correctness 2. **Clear ownership** - Window and Surface are explicitly bound together as a single unit 3. **Reliable cleanup** - Drop order is enforced in one place, fixing existing drop bugs 4. **Better encapsulation** - All window/surface logic lives in one struct 5. **Easier to test** - Can test WindowSurface creation/destruction independently 6. **Prevents misuse** - Impossible to accidentally drop window before surface at compile time ## Acceptance Criteria - [ ] Create `WindowSurface` struct that bundles window, surface, and instance - [ ] Remove unsafe `transmute` from surface creation - [ ] Add lifetime parameter `'w` to `Runtime` struct - [ ] Implement proper `Drop` for `WindowSurface` to ensure correct cleanup order - [ ] Verify that existing drop bugs are resolved - [ ] Update all window/surface access code to use the new structure - [ ] Confirm that lifetime parameter doesn't spread beyond necessary boundaries ## Notes The lifetime parameter on `Runtime<'w>` should not spread further if `Runtime` is owned by the parent struct (e.g., `App`). This is a one-time addition that buys us significant safety guarantees.

SakulFlee added the

Difficulty: Hard

label

2026-01-07 01:32:24 +00:00

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

SakulFlee/Orbital#604

No description provided.

Rows
Columns