Skip to content

Commit 541d45d

Browse files
maximecbmaximecb
committed
Fix mutable reference warnings
1 parent b7d9355 commit 541d45d

2 files changed

Lines changed: 91 additions & 87 deletions

File tree

‎vm/src/host.rs‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
extern crate sdl2;
22
use std::collections::HashMap;
3+
use std::cell::RefCell;
34
use std::io::Write;
45
use std::io::Read;
56
use std::io::{stdout, stdin};
@@ -68,22 +69,24 @@ impl HostFn
6869
}
6970
}
7071

71-
/// SDL context (used for UI and audio)
72-
/// This is a global variable because it doesn't implement
73-
/// the Send trait, and so can't be referenced from another thread
74-
static mut SDL: Option<sdl2::Sdl> = None;
72+
thread_local! {
73+
/// SDL context (used for UI and audio)
74+
/// This is thread-local because it doesn't implement the Send trait,
75+
/// and so can't be referenced from another thread
76+
static SDL: RefCell<Option<sdl2::Sdl>> = RefCell::new(None);
77+
}
7578

76-
pub fn get_sdl_context() -> &'static mut sdl2::Sdl
79+
/// Get a handle to the SDL context, lazily initializing it
80+
/// Sdl is a cheap reference-counted handle, so we return it by value
81+
pub fn get_sdl_context() -> sdl2::Sdl
7782
{
78-
unsafe
79-
{
80-
// Lazily initialize the SDL context
81-
if SDL.is_none() {
82-
SDL = Some(sdl2::init().unwrap());
83+
SDL.with_borrow_mut(|sdl| {
84+
if sdl.is_none() {
85+
*sdl = Some(sdl2::init().unwrap());
8386
}
8487

85-
SDL.as_mut().unwrap()
86-
}
88+
sdl.as_ref().unwrap().clone()
89+
})
8790
}
8891

8992
/// Get the syscall with a given index

‎vm/src/window.rs‎

Lines changed: 76 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -8,58 +8,54 @@ use sdl2::mouse::MouseButton;
88
use sdl2::surface::Surface;
99
use sdl2::render::Texture;
1010
use sdl2::render::TextureAccess;
11+
use sdl2::render::TextureCreator;
12+
use sdl2::video::WindowContext;
1113
use sdl2::pixels::PixelFormatEnum;
14+
use std::cell::RefCell;
1215
use std::mem::size_of;
1316
use std::time::Duration;
1417
use crate::host::{get_sdl_context};
1518
use crate::vm::{VM, Thread, Value};
1619

17-
/// SDL video subsystem
18-
/// This is a global variable because it doesn't implement
19-
/// the Send trait, and so can't be referenced from another thread
20-
static mut SDL_VIDEO: Option<sdl2::VideoSubsystem> = None;
20+
thread_local! {
21+
/// SDL video subsystem
22+
/// This is thread-local because it doesn't implement the Send trait,
23+
/// and so can't be referenced from another thread
24+
static SDL_VIDEO: RefCell<Option<sdl2::VideoSubsystem>> = RefCell::new(None);
2125

22-
/// Lazily initialize the SDL video subsystem
23-
fn get_video_subsystem() -> &'static mut sdl2::VideoSubsystem
24-
{
25-
unsafe
26-
{
27-
let sdl = get_sdl_context();
26+
// Note: we're keeping this as a thread-local global to avoid the
27+
// Window type bubbling up everywhere.
28+
// TODO: eventually we will likely want to allow multiple windows
29+
static WINDOW: RefCell<Option<Window>> = RefCell::new(None);
30+
}
2831

29-
if SDL_VIDEO.is_none() {
30-
SDL_VIDEO = Some(sdl.video().unwrap());
32+
/// Get a handle to the SDL video subsystem, lazily initializing it
33+
/// VideoSubsystem is a cheap reference-counted handle, returned by value
34+
fn get_video_subsystem() -> sdl2::VideoSubsystem
35+
{
36+
SDL_VIDEO.with_borrow_mut(|video| {
37+
if video.is_none() {
38+
*video = Some(get_sdl_context().video().unwrap());
3139
}
3240

33-
SDL_VIDEO.as_mut().unwrap()
34-
}
41+
video.as_ref().unwrap().clone()
42+
})
3543
}
3644

37-
struct Window<'a>
45+
struct Window
3846
{
3947
width: u32,
4048
height: u32,
4149
window_id: u32,
4250

4351
// SDL canvas to draw into
4452
canvas: sdl2::render::Canvas<sdl2::video::Window>,
45-
texture_creator: sdl2::render::TextureCreator<sdl2::video::WindowContext>,
46-
texture: Option<Texture<'a>>,
47-
}
48-
49-
// Note: we're leaving this global to avoid the Window lifetime
50-
// bubbling up everywhere.
51-
// TODO: eventually we will likely want to allow multiple windows
52-
static mut WINDOW: Option<Window> = None;
53-
54-
fn get_window(window_id: u32) -> &'static mut Window<'static>
55-
{
56-
if window_id != 0 {
57-
panic!("for now, only one window supported");
58-
}
5953

60-
unsafe {
61-
WINDOW.as_mut().unwrap()
62-
}
54+
// Texture creator for the canvas. This is leaked to give it a 'static
55+
// lifetime: it lives as long as the window, which exists for the entire
56+
// duration of the program. This lets the texture below be 'static too.
57+
texture_creator: &'static TextureCreator<WindowContext>,
58+
texture: Option<Texture<'static>>,
6359
}
6460

6561
pub fn window_create(thread: &mut Thread, width: Value, height: Value, title: Value, flags: Value) -> Value
@@ -68,10 +64,8 @@ pub fn window_create(thread: &mut Thread, width: Value, height: Value, title: Va
6864
panic!("window functions should only be called from the main thread");
6965
}
7066

71-
unsafe {
72-
if WINDOW.is_some() {
73-
panic!("for now, only one window supported");
74-
}
67+
if WINDOW.with_borrow(|w| w.is_some()) {
68+
panic!("for now, only one window supported");
7569
}
7670

7771
let width: u32 = width.as_usize().try_into().unwrap();
@@ -92,7 +86,10 @@ pub fn window_create(thread: &mut Thread, width: Value, height: Value, title: Va
9286
canvas.clear();
9387
canvas.present();
9488

95-
let texture_creator = canvas.texture_creator();
89+
// Leak the texture creator so it has a 'static lifetime. It lives as long
90+
// as the window, which exists for the entire duration of the program.
91+
let texture_creator: &'static TextureCreator<WindowContext> =
92+
Box::leak(Box::new(canvas.texture_creator()));
9693

9794
let window = Window {
9895
width,
@@ -103,9 +100,7 @@ pub fn window_create(thread: &mut Thread, width: Value, height: Value, title: Va
103100
texture: None,
104101
};
105102

106-
unsafe {
107-
WINDOW = Some(window)
108-
}
103+
WINDOW.with_borrow_mut(|w| *w = Some(window));
109104

110105
// TODO: return unique window id
111106
Value::from(0)
@@ -117,46 +112,52 @@ pub fn window_draw_frame(thread: &mut Thread, window_id: Value, src_addr: Value)
117112
panic!("window functions should only be called from the main thread");
118113
}
119114

120-
let window = get_window(window_id.as_u32());
121-
122-
// Get the address to copy pixel data from
123-
let data_len = (4 * window.width * window.height) as usize;
124-
let data_ptr = thread.get_heap_ptr_mut(src_addr.as_usize(), data_len);
125-
126-
// If no frame has been drawn yet
127-
if window.texture.is_none() {
128-
// Creat the texture to render into
129-
// Pixels use the BGRA byte order (0xAA_RR_GG_BB on a little-endian machine)
130-
window.texture = Some(window.texture_creator.create_texture(
131-
PixelFormatEnum::BGRA32,
132-
TextureAccess::Streaming,
133-
window.width,
134-
window.height
135-
).unwrap());
136-
137-
// We show and raise the window at the moment the first frame is drawn
138-
// This avoids showing a blank window too early
139-
window.canvas.window_mut().show();
140-
window.canvas.window_mut().raise();
115+
if window_id.as_u32() != 0 {
116+
panic!("for now, only one window supported");
141117
}
142118

143-
// Clear the canvas
144-
window.canvas.clear();
119+
WINDOW.with_borrow_mut(|window| {
120+
let window = window.as_mut().unwrap();
121+
122+
// Get the address to copy pixel data from
123+
let data_len = (4 * window.width * window.height) as usize;
124+
let data_ptr = thread.get_heap_ptr_mut(src_addr.as_usize(), data_len);
125+
126+
// If no frame has been drawn yet
127+
if window.texture.is_none() {
128+
// Creat the texture to render into
129+
// Pixels use the BGRA byte order (0xAA_RR_GG_BB on a little-endian machine)
130+
window.texture = Some(window.texture_creator.create_texture(
131+
PixelFormatEnum::BGRA32,
132+
TextureAccess::Streaming,
133+
window.width,
134+
window.height
135+
).unwrap());
136+
137+
// We show and raise the window at the moment the first frame is drawn
138+
// This avoids showing a blank window too early
139+
window.canvas.window_mut().show();
140+
window.canvas.window_mut().raise();
141+
}
142+
143+
// Clear the canvas
144+
window.canvas.clear();
145145

146-
// Update the texture
147-
let pitch = 4 * window.width as usize;
148-
let pixel_slice = unsafe { std::slice::from_raw_parts(data_ptr, data_len) };
149-
window.texture.as_mut().unwrap().update(None, pixel_slice, pitch).unwrap();
146+
// Update the texture
147+
let pitch = 4 * window.width as usize;
148+
let pixel_slice = unsafe { std::slice::from_raw_parts(data_ptr, data_len) };
149+
window.texture.as_mut().unwrap().update(None, pixel_slice, pitch).unwrap();
150150

151-
// Copy the texture into the canvas
152-
window.canvas.copy(
153-
&window.texture.as_ref().unwrap(),
154-
None,
155-
None
156-
).unwrap();
151+
// Copy the texture into the canvas
152+
window.canvas.copy(
153+
&window.texture.as_ref().unwrap(),
154+
None,
155+
None
156+
).unwrap();
157157

158-
// Update the screen with any rendering performed since the previous call
159-
window.canvas.present();
158+
// Update the screen with any rendering performed since the previous call
159+
window.canvas.present();
160+
});
160161
}
161162

162163
const EVENT_TEXT_MAX_BYTES: usize = 64;

0 commit comments

Comments
 (0)