From beaa1744fe7fd141dfcbe2a0f4684300eabf296c Mon Sep 17 00:00:00 2001 From: Dan Spencer Date: Sat, 11 Apr 2015 01:22:08 -0600 Subject: [PATCH 1/4] Remove Texture lifetime, add "is renderer alive" Rc to Texture * Remove `owned` field from Texture. * Remove RefMut from Renderer, make Renderer::drawer() take &mut self. --- examples/demo.rs | 2 +- examples/renderer-texture.rs | 2 +- examples/renderer-yuv.rs | 2 +- src/sdl2/render.rs | 184 +++++++++++++++++++---------------- 4 files changed, 103 insertions(+), 87 deletions(-) diff --git a/examples/demo.rs b/examples/demo.rs index cf7dfff4..bb18bce3 100644 --- a/examples/demo.rs +++ b/examples/demo.rs @@ -13,7 +13,7 @@ pub fn main() { Err(err) => panic!("failed to create window: {}", err) }; - let renderer = match Renderer::from_window(window, RenderDriverIndex::Auto, ACCELERATED) { + let mut renderer = match Renderer::from_window(window, RenderDriverIndex::Auto, ACCELERATED) { Ok(renderer) => renderer, Err(err) => panic!("failed to create renderer: {}", err) }; diff --git a/examples/renderer-texture.rs b/examples/renderer-texture.rs index 6951a9e6..1ef766f0 100644 --- a/examples/renderer-texture.rs +++ b/examples/renderer-texture.rs @@ -14,7 +14,7 @@ pub fn main() { Err(err) => panic!("failed to create window: {}", err) }; - let renderer = match Renderer::from_window(window, RenderDriverIndex::Auto, ACCELERATED) { + let mut renderer = match Renderer::from_window(window, RenderDriverIndex::Auto, ACCELERATED) { Ok(renderer) => renderer, Err(err) => panic!("failed to create renderer: {}", err) }; diff --git a/examples/renderer-yuv.rs b/examples/renderer-yuv.rs index 9ed23084..03e835cf 100644 --- a/examples/renderer-yuv.rs +++ b/examples/renderer-yuv.rs @@ -14,7 +14,7 @@ pub fn main() { Err(err) => panic!("failed to create window: {}", err) }; - let renderer = match Renderer::from_window(window, RenderDriverIndex::Auto, ACCELERATED) { + let mut renderer = match Renderer::from_window(window, RenderDriverIndex::Auto, ACCELERATED) { Ok(renderer) => renderer, Err(err) => panic!("failed to create renderer: {}", err) }; diff --git a/src/sdl2/render.rs b/src/sdl2/render.rs index 61f04f5a..5ca3eef6 100644 --- a/src/sdl2/render.rs +++ b/src/sdl2/render.rs @@ -23,17 +23,7 @@ //! This API is not designed to be used from multiple threads, see //! [this bug](http://bugzilla.libsdl.org/show_bug.cgi?id=1995) for details. //! -//! # Rust differences -//! -//! The Rust version of the render API deviates slightly from the original, -//! in order to be more idiomatic with Rust and to adhere to its notion of -//! memory safety. -//! -//! All `Texture` types are restricted to live for only as long as -//! the parent `Renderer`. -//! Consequentially, this means that `Renderer` never mutates and that all -//! drawing functionality is put behind interior mutability using -//! `RenderDrawer<'renderer>`. +//! --- //! //! None of the draw methods in `RenderDrawer` are expected to fail. //! If they do, a panic is raised and the program is aborted. @@ -51,11 +41,11 @@ use std::ptr; use libc::{c_int, uint32_t, c_double, c_void}; use rect::Point; use rect::Rect; -use std::cell::{RefCell, RefMut}; +use std::cell::UnsafeCell; use std::ffi::CStr; use num::FromPrimitive; use std::vec::Vec; -use std::marker::PhantomData; +use std::rc::Rc; use sys::render as ll; @@ -158,12 +148,15 @@ pub enum RendererParent { pub struct Renderer { raw: *const ll::SDL_Renderer, parent: Option, - drawer_borrow: RefCell<()> + is_alive: Rc> } impl Drop for Renderer { fn drop(&mut self) { - unsafe { ll::SDL_DestroyRenderer(self.raw) }; + unsafe { + *self.is_alive.get() = false; + ll::SDL_DestroyRenderer(self.raw); + }; } } @@ -274,30 +267,20 @@ impl Renderer { /// Provides drawing methods for the renderer. /// - /// # Remarks - /// This method is not `&mut self`. - /// It uses interior mutability via `RenderDrawer<'renderer>` to preserve the - /// Renderer's lifetime, and therefore the lifetimes of any Textures that - /// belong to the Renderer. - /// - /// Only one `RenderDrawer` per `Renderer` can be active at a time. - /// If this method is called and an existing `RenderDrawer` from this - /// instance is active, the program will panic. - /// /// # Examples /// ```no_run /// use sdl2::render::Renderer; /// use sdl2::rect::Rect; /// - /// fn test_draw(renderer: &Renderer) { + /// fn test_draw(renderer: &mut Renderer) { /// let mut drawer = renderer.drawer(); /// drawer.clear(); /// drawer.draw_rect(Rect::new(50, 50, 150, 175)); /// drawer.present(); /// } /// ``` - pub fn drawer(&self) -> RenderDrawer { - RenderDrawer::new(self.raw, self.drawer_borrow.borrow_mut()) + pub fn drawer(&mut self) -> RenderDrawer { + RenderDrawer::new(self.raw, &self.is_alive) } /// Unwraps the window or surface the rendering context was created from. @@ -309,7 +292,7 @@ impl Renderer { Renderer { raw: raw, parent: Some(parent), - drawer_borrow: RefCell::new(()) + is_alive: Rc::new(UnsafeCell::new(true)) } } } @@ -337,7 +320,7 @@ impl Renderer { if result == ptr::null() { Err(get_error()) } else { - unsafe { Ok(Texture::from_ll(result)) } + unsafe { Ok(Texture::from_ll(self, result)) } } } @@ -364,7 +347,7 @@ impl Renderer { if result == ptr::null() { Err(get_error()) } else { - unsafe { Ok(Texture::from_ll(result)) } + unsafe { Ok(Texture::from_ll(self, result)) } } } } @@ -372,15 +355,15 @@ impl Renderer { /// Drawing functionality for the render context. pub struct RenderDrawer<'renderer> { raw: *const ll::SDL_Renderer, - _borrow: RefMut<'renderer, ()> + is_renderer_alive: &'renderer Rc> } /// Render target methods for the drawer impl<'renderer> RenderDrawer<'renderer> { - fn new<'l>(raw: *const ll::SDL_Renderer, borrow: RefMut<'l, ()>) -> RenderDrawer<'l> { + fn new<'l>(raw: *const ll::SDL_Renderer, is_renderer_alive: &'l Rc>) -> RenderDrawer<'l> { RenderDrawer { raw: raw, - _borrow: borrow + is_renderer_alive: is_renderer_alive } } @@ -392,11 +375,11 @@ impl<'renderer> RenderDrawer<'renderer> { /// Gets the render target handle. /// /// Returns `None` if the window does not support the use of render targets. - pub fn render_target<'a>(&'a mut self) -> Option> { + pub fn render_target(&mut self) -> Option { if self.render_target_supported() { Some(RenderTarget { raw: self.raw, - _marker_renderer: PhantomData, + is_renderer_alive: self.is_renderer_alive }) } else { None @@ -659,6 +642,8 @@ impl<'renderer> RenderDrawer<'renderer> { /// Panics if drawing fails for any reason (e.g. driver failure), /// or if the provided texture does not belong to the renderer. pub fn copy(&mut self, texture: &Texture, src: Option, dst: Option) { + texture.check_renderer(); + let ret = unsafe { ll::SDL_RenderCopy( self.raw, @@ -694,6 +679,8 @@ impl<'renderer> RenderDrawer<'renderer> { /// if the provided texture does not belong to the renderer, /// or if the driver does not support RenderCopyEx. pub fn copy_ex(&mut self, texture: &Texture, src: Option, dst: Option, angle: f64, center: Option, (flip_horizontal, flip_vertical): (bool, bool)) { + texture.check_renderer(); + let flip = match (flip_horizontal, flip_vertical) { (false, false) => ll::SDL_FLIP_NONE, (true, false) => ll::SDL_FLIP_HORIZONTAL, @@ -768,7 +755,7 @@ impl<'renderer> RenderDrawer<'renderer> { /// use sdl2::render::{RenderDrawer, Texture}; /// /// // Draw a red rectangle to a new texture -/// fn draw_to_texture<'renderer>(drawer: &'renderer mut RenderDrawer<'renderer>) -> Texture<'renderer> { +/// fn draw_to_texture(drawer: &mut RenderDrawer) -> Texture { /// drawer.render_target() /// .expect("This platform doesn't support render targets") /// .create_and_set(PixelFormatEnum::RGBA8888, 512, 512); @@ -784,21 +771,24 @@ impl<'renderer> RenderDrawer<'renderer> { /// ``` pub struct RenderTarget<'renderer> { raw: *const ll::SDL_Renderer, - _marker_renderer: PhantomData<&'renderer ()> + is_renderer_alive: &'renderer Rc> } impl<'renderer> RenderTarget<'renderer> { /// Resets the render target to the default render target. /// /// The old render target is returned if the function is successful. - pub fn reset(&mut self) -> SdlResult>> { + pub fn reset(&mut self) -> SdlResult> { unsafe { let old_texture_raw = ll::SDL_GetRenderTarget(self.raw); if ll::SDL_SetRenderTarget(self.raw, ptr::null()) == 0 { Ok(match old_texture_raw.is_null() { true => None, - false => Some(Texture::from_ll(old_texture_raw)) + false => Some(Texture { + raw: old_texture_raw, + is_renderer_alive: self.is_renderer_alive.clone() + }) }) } else { Err(get_error()) @@ -810,15 +800,20 @@ impl<'renderer> RenderTarget<'renderer> { /// The texture must be created with the texture access: `sdl2::render::TextureAccess::Target`. /// /// The old render target is returned if the function is successful. - pub fn set(&mut self, texture: Texture) -> SdlResult>> { + pub fn set(&mut self, texture: Texture) -> SdlResult> { + texture.check_renderer(); + unsafe { let old_texture_raw = ll::SDL_GetRenderTarget(self.raw); if ll::SDL_SetRenderTarget(self.raw, texture.raw) == 0 { - mem::forget(texture); + texture.forget(); Ok(match old_texture_raw.is_null() { true => None, - false => Some(Texture::from_ll(old_texture_raw)) + false => Some(Texture { + raw: old_texture_raw, + is_renderer_alive: self.is_renderer_alive.clone() + }) }) } else { Err(get_error()) @@ -829,7 +824,7 @@ impl<'renderer> RenderTarget<'renderer> { /// Creates a new texture and sets it as the render target. /// /// The old render target is returned if the function is successful. - pub fn create_and_set(&mut self, format: pixels::PixelFormatEnum, width: i32, height: i32) -> SdlResult>> { + pub fn create_and_set(&mut self, format: pixels::PixelFormatEnum, width: i32, height: i32) -> SdlResult> { let new_texture_raw = unsafe { let access = ll::SDL_TEXTUREACCESS_TARGET; ll::SDL_CreateTexture(self.raw, format as uint32_t, access as c_int, width as c_int, height as c_int) @@ -844,7 +839,10 @@ impl<'renderer> RenderTarget<'renderer> { if ll::SDL_SetRenderTarget(self.raw, new_texture_raw) == 0 { Ok(match old_texture_raw.is_null() { true => None, - false => Some(Texture::from_ll(old_texture_raw)) + false => Some(Texture { + raw: old_texture_raw, + is_renderer_alive: self.is_renderer_alive.clone() + }) }) } else { Err(get_error()) @@ -852,22 +850,6 @@ impl<'renderer> RenderTarget<'renderer> { } } } - - /// Gets the current render target. - /// Returns None if the default render target is set. - pub fn get(&mut self) -> Option { - let texture_raw = unsafe { ll::SDL_GetRenderTarget(self.raw) }; - - if texture_raw == ptr::null() { - None - } else { - Some(Texture { - raw: texture_raw, - owned: false, - _marker: PhantomData - }) - } - } } #[derive(Copy, Clone)] @@ -880,29 +862,49 @@ pub struct TextureQuery { /// A texture for a rendering context. /// -/// Textures are owned by and cannot live longer than the parent `Renderer`. -/// Each texture is bound to the `'renderer` contravariant lifetime. -pub struct Texture<'renderer> { +/// Every Texture is owned by a Renderer. +/// If a Texture is accessed after the corresponding Renderer is dropped, then +/// the program will panic (clarification: will not crash). +/// +/// A Texture can be safely dropped before or after the Renderer is dropped. +pub struct Texture { raw: *const ll::SDL_Texture, - owned: bool, - /// Textures cannot live longer than the Renderer it was born from: 'a - /// All SDL textures contain an internal reference to a Renderer - _marker: PhantomData<&'renderer ()> + is_renderer_alive: Rc> } -impl<'renderer> Drop for Texture<'renderer> { +impl Drop for Texture { fn drop(&mut self) { - if self.owned { - unsafe { + unsafe { + if *self.is_renderer_alive.get() { ll::SDL_DestroyTexture(self.raw); } } } } -impl<'renderer> Texture<'renderer> { +impl Texture { + #[inline] + fn check_renderer(&self) { + let alive = unsafe { *self.is_renderer_alive.get() }; + if !alive { + panic!("renderer has been destroyed; cannot use Texture"); + } + } + + /// Doesn't free the Texture, but decrements its `is_renderer_alive` box. + fn forget(self) { + unsafe { + let _is_renderer_alive: Rc> = mem::transmute_copy(&self.is_renderer_alive); + mem::forget(self); + + // is_renderer_alive gets deref'd + } + } + /// Queries the attributes of the texture. pub fn query(&self) -> TextureQuery { + self.check_renderer(); + let format: uint32_t = 0; let access: c_int = 0; let width: c_int = 0; @@ -924,6 +926,8 @@ impl<'renderer> Texture<'renderer> { /// Sets an additional color value multiplied into render copy operations. pub fn set_color_mod(&mut self, red: u8, green: u8, blue: u8) { + self.check_renderer(); + let ret = unsafe { ll::SDL_SetTextureColorMod(self.raw, red, green, blue) }; if ret != 0 { @@ -933,6 +937,8 @@ impl<'renderer> Texture<'renderer> { /// Gets the additional color value multiplied into render copy operations. pub fn get_color_mod(&self) -> (u8, u8, u8) { + self.check_renderer(); + let r = 0; let g = 0; let b = 0; @@ -945,6 +951,8 @@ impl<'renderer> Texture<'renderer> { /// Sets an additional alpha value multiplied into render copy operations. pub fn set_alpha_mod(&mut self, alpha: u8) { + self.check_renderer(); + let ret = unsafe { ll::SDL_SetTextureAlphaMod(self.raw, alpha) }; if ret != 0 { @@ -954,6 +962,8 @@ impl<'renderer> Texture<'renderer> { /// Gets the additional alpha value multiplied into render copy operations. pub fn get_alpha_mod(&self) -> u8 { + self.check_renderer(); + let alpha = 0; let ret = unsafe { ll::SDL_GetTextureAlphaMod(self.raw, &alpha) }; @@ -964,6 +974,8 @@ impl<'renderer> Texture<'renderer> { /// Sets the blend mode for a texture, used by `RenderDrawer::copy()`. pub fn set_blend_mode(&mut self, blend: BlendMode) { + self.check_renderer(); + let ret = unsafe { ll::SDL_SetTextureBlendMode(self.raw, FromPrimitive::from_i64(blend as i64).unwrap()) }; if ret != 0 { @@ -973,6 +985,8 @@ impl<'renderer> Texture<'renderer> { /// Gets the blend mode used for texture copy operations. pub fn get_blend_mode(&self) -> BlendMode { + self.check_renderer(); + let blend = 0; let ret = unsafe { ll::SDL_GetTextureBlendMode(self.raw, &blend) }; @@ -988,6 +1002,8 @@ impl<'renderer> Texture<'renderer> { /// /// * If `rect` is `None`, the entire texture is updated. pub fn update(&mut self, rect: Option, pixel_data: &[u8], pitch: i32) -> SdlResult<()> { + self.check_renderer(); + let ret = unsafe { let rect_raw_ptr = match rect { Some(ref rect) => rect as *const _, @@ -1023,6 +1039,8 @@ impl<'renderer> Texture<'renderer> { /// Updates a rectangle within a planar YV12 or IYUV texture with new pixel data. pub fn update_yuv(&mut self, rect: Option, y_plane: &[u8], y_pitch: i32, u_plane: &[u8], u_pitch: i32, v_plane: &[u8], v_pitch: i32) -> SdlResult<()> { + self.check_renderer(); + let rect_raw_ptr = match rect { Some(ref rect) => rect as *const _, None => ptr::null() @@ -1083,6 +1101,8 @@ impl<'renderer> Texture<'renderer> { pub fn with_lock(&mut self, rect: Option, func: F) -> SdlResult where F: FnOnce(&mut [u8], usize) -> R { + self.check_renderer(); + // Call to SDL to populate pixel data let loaded = unsafe { let q = self.query(); @@ -1119,6 +1139,8 @@ impl<'renderer> Texture<'renderer> { /// Binds an OpenGL/ES/ES2 texture to the current /// context for use with when rendering OpenGL primitives directly. pub unsafe fn gl_bind_texture(&mut self) -> (f32, f32) { + self.check_renderer(); + let texw = 0.0; let texh = 0.0; @@ -1131,6 +1153,8 @@ impl<'renderer> Texture<'renderer> { /// Unbinds an OpenGL/ES/ES2 texture from the current context. pub unsafe fn gl_unbind_texture(&mut self) { + self.check_renderer(); + if ll::SDL_GL_UnbindTexture(self.raw) != 0 { panic!("OpenGL texture unbinding not supported"); } @@ -1138,6 +1162,8 @@ impl<'renderer> Texture<'renderer> { /// Binds and unbinds an OpenGL/ES/ES2 texture from the current context. pub fn gl_with_bind R>(&mut self, f: F) -> R { + self.check_renderer(); + unsafe { let texw = 0.0; let texh = 0.0; @@ -1157,20 +1183,10 @@ impl<'renderer> Texture<'renderer> { } } - pub unsafe fn from_ll<'l>(raw: *const ll::SDL_Texture) -> Texture<'l> { + pub unsafe fn from_ll(renderer: &Renderer, raw: *const ll::SDL_Texture) -> Texture { Texture { raw: raw, - owned: true, - _marker: PhantomData - } - } - - #[unstable="Will likely be removed with ownership reform"] - pub unsafe fn from_ll_unowned<'l>(raw: *const ll::SDL_Texture) -> Texture<'l> { - Texture { - raw: raw, - owned: false, - _marker: PhantomData + is_renderer_alive: renderer.is_alive.clone() } } From 7d27c53c54bc1ef1522799805e0beb441e7807b5 Mon Sep 17 00:00:00 2001 From: Dan Spencer Date: Thu, 9 Apr 2015 20:20:06 -0600 Subject: [PATCH 2/4] Window safety: Ensure thread, event-pump safety All Window properties must be accessed using `Window::properties`, which requires a reference to the event pump. If the event pump is not being used (mutably), then the caller can obtain a shared reference to it, which means the event pump cannot mutate the Window. Renderer also has a way to access window properties, using `Renderer::window::properties`. Additionally, Window initialization requires an `Sdl` reference. Because `Sdl` is `!Send` and `!Sync`, obtaining a `Sdl` reference is only possible on the same thread. --- examples/demo.rs | 2 +- examples/renderer-texture.rs | 2 +- examples/renderer-yuv.rs | 2 +- src/sdl2/render.rs | 16 +++- src/sdl2/video.rs | 178 +++++++++++++++++++++++++---------- 5 files changed, 146 insertions(+), 54 deletions(-) diff --git a/examples/demo.rs b/examples/demo.rs index bb18bce3..a22355f5 100644 --- a/examples/demo.rs +++ b/examples/demo.rs @@ -8,7 +8,7 @@ use sdl2::keycode::KeyCode; pub fn main() { let sdl_context = sdl2::init(sdl2::INIT_VIDEO).unwrap(); - let window = match Window::new("rust-sdl2 demo: Video", WindowPos::PosCentered, WindowPos::PosCentered, 800, 600, OPENGL) { + let window = match Window::new(&sdl_context, "rust-sdl2 demo: Video", WindowPos::PosCentered, WindowPos::PosCentered, 800, 600, OPENGL) { Ok(window) => window, Err(err) => panic!("failed to create window: {}", err) }; diff --git a/examples/renderer-texture.rs b/examples/renderer-texture.rs index 1ef766f0..3ba5bba6 100644 --- a/examples/renderer-texture.rs +++ b/examples/renderer-texture.rs @@ -9,7 +9,7 @@ use sdl2::keycode::KeyCode; pub fn main() { let sdl_context = sdl2::init(sdl2::INIT_VIDEO).unwrap(); - let window = match Window::new("rust-sdl2 demo: Renderer + Texture", WindowPos::PosCentered, WindowPos::PosCentered, 800, 600, OPENGL) { + let window = match Window::new(&sdl_context, "rust-sdl2 demo: Renderer + Texture", WindowPos::PosCentered, WindowPos::PosCentered, 800, 600, OPENGL) { Ok(window) => window, Err(err) => panic!("failed to create window: {}", err) }; diff --git a/examples/renderer-yuv.rs b/examples/renderer-yuv.rs index 03e835cf..33b2ede9 100644 --- a/examples/renderer-yuv.rs +++ b/examples/renderer-yuv.rs @@ -9,7 +9,7 @@ use sdl2::keycode::KeyCode; pub fn main() { let sdl_context = sdl2::init(sdl2::INIT_VIDEO).unwrap(); - let window = match Window::new("rust-sdl2 demo: YUV", WindowPos::PosCentered, WindowPos::PosCentered, 800, 600, SHOWN) { + let window = match Window::new(&sdl_context, "rust-sdl2 demo: YUV", WindowPos::PosCentered, WindowPos::PosCentered, 800, 600, SHOWN) { Ok(window) => window, Err(err) => panic!("failed to create window: {}", err) }; diff --git a/src/sdl2/render.rs b/src/sdl2/render.rs index 5ca3eef6..af6f8a05 100644 --- a/src/sdl2/render.rs +++ b/src/sdl2/render.rs @@ -28,8 +28,10 @@ //! None of the draw methods in `RenderDrawer` are expected to fail. //! If they do, a panic is raised and the program is aborted. +use Sdl; +use event::EventPump; use video; -use video::Window; +use video::{Window, WindowProperties}; use surface; use surface::Surface; use pixels; @@ -182,7 +184,7 @@ impl Renderer { } /// Creates a window and default renderer. - pub fn new_with_window(width: i32, height: i32, window_flags: video::WindowFlags) -> SdlResult { + pub fn new_with_window(_sdl: &Sdl, width: i32, height: i32, window_flags: video::WindowFlags) -> SdlResult { use sys::video::SDL_Window; let raw_window: *const SDL_Window = ptr::null(); @@ -243,6 +245,16 @@ impl Renderer { } } + /// Accesses the Window properties, such as the position, size and title of a Window. + /// Returns None if the renderer is not associated with a Window. + pub fn window_properties<'b>(&'b mut self, event: &'b EventPump) -> Option> + { + match self.parent.as_mut() { + Some(&mut RendererParent::Window(ref mut window)) => Some(window.properties(event)), + _ => None + } + } + #[inline] pub fn unwrap_parent(mut self) -> RendererParent { use std::mem; diff --git a/src/sdl2/video.rs b/src/sdl2/video.rs index 7e5db25b..a875f99b 100644 --- a/src/sdl2/video.rs +++ b/src/sdl2/video.rs @@ -1,11 +1,14 @@ use libc::{c_int, c_float, uint32_t}; use std::ffi::{CStr, CString, NulError}; +use std::marker::PhantomData; use std::ptr; use std::vec::Vec; +use event::EventPump; use rect::Rect; use surface::Surface; use pixels; +use Sdl; use SdlResult; use num::FromPrimitive; @@ -165,8 +168,6 @@ impl Drop for GLContext { } } -#[derive(PartialEq)] -#[allow(raw_pointer_derive)] pub struct Window { raw: *const ll::SDL_Window, owned: bool @@ -196,8 +197,35 @@ impl Drop for Window { } } +/// Contains accessors to a `Window`'s properties. +pub struct WindowProperties<'a> { + raw: *const ll::SDL_Window, + _marker: PhantomData<&'a ()> +} + impl Window { - pub fn new(title: &str, x: WindowPos, y: WindowPos, width: i32, height: i32, window_flags: WindowFlags) -> SdlResult { + /// Creates a new Window. + /// + /// Note: a reference to the SDL context is required to ensure that the + /// Window is being created on the same thread as the SDL main thread + /// (`Sdl` cannot be moved or referenced across other threads). + /// + /// # Example + /// ```no_run + /// use sdl2::video::{Window, WindowPos}; + /// + /// let sdl_context = sdl2::init(sdl2::INIT_EVERYTHING).unwrap(); + /// + /// let window = Window::new(&sdl_context, "My SDL window", WindowPos::PosCentered, WindowPos::PosCentered, 800, 600, sdl2::video::SHOWN).unwrap(); + /// ``` + pub fn new(sdl: &Sdl, title: &str, x: WindowPos, y: WindowPos, width: i32, height: i32, window_flags: WindowFlags) -> SdlResult { + Window::new_with_init(sdl, title, x, y, width, height, window_flags, |_| { Ok(()) }) + } + + /// Creates a new Window, and initializes the Window with other properties. + pub fn new_with_init<'a, F: 'a>(_sdl: &Sdl, title: &str, x: WindowPos, y: WindowPos, width: i32, height: i32, window_flags: WindowFlags, init: F) -> SdlResult + where F: FnOnce(WindowProperties<'a>) -> SdlResult<()> + { unsafe { let buff = CString::new(title).unwrap().as_ptr(); let raw = ll::SDL_CreateWindow( @@ -212,13 +240,63 @@ impl Window { if raw == ptr::null() { Err(get_error()) } else { + try!(init(WindowProperties { + raw: raw, + _marker: PhantomData + })); + Ok(Window{ raw: raw, owned: true }) } } } - pub fn from_id(id: u32) -> SdlResult { - let raw = unsafe { ll::SDL_GetWindowFromID(id) }; + /// Accesses the Window properties, such as the position, size and title of a Window. + /// + /// In order to access a Window's properties, it must be guaranteed that the + /// event loop is not running. + /// This is why a reference to the application's `EventPump` is required + /// (a shared `EventPump` reference is only obtainable if it's not being mutated). + /// Event pumping could otherwise mutate a Window's properties without your consent! + /// + /// # Example + /// ```no_run + /// use sdl2::video::{Window, WindowPos}; + /// + /// let mut sdl_context = sdl2::init(sdl2::INIT_EVERYTHING).unwrap(); + /// let mut window = Window::new(&sdl_context, "My SDL window", WindowPos::PosCentered, WindowPos::PosCentered, 800, 600, sdl2::video::SHOWN).unwrap(); + /// let mut event_pump = sdl_context.event_pump(); + /// + /// loop { + /// let mut pos = None; + /// + /// for event in event_pump.poll_iter() { + /// use sdl2::event::Event; + /// match event { + /// Event::MouseMotion { x, y, .. } => { pos = Some((x, y)); }, + /// _ => () + /// } + /// } + /// + /// if let Some((x, y)) = pos { + /// // Set the window title + /// window.properties(&event_pump).set_title(&format!("{}, {}", x, y)); + /// } + /// } + /// ``` + pub fn properties<'a>(&'a mut self, _event: &'a EventPump) -> WindowProperties<'a> { + WindowProperties { + raw: self.raw, + _marker: PhantomData + } + } + + /// Get a Window from a stored ID. + /// + /// Warning: This function is unsafe! + /// It may introduce aliased Window values if a Window of the same ID is + /// already being used as a variable in the application. + pub unsafe fn from_id(id: u32) -> SdlResult { + let raw = ll::SDL_GetWindowFromID(id); if raw == ptr::null() { Err(get_error()) } else { @@ -226,6 +304,29 @@ impl Window { } } + pub fn get_id(&self) -> u32 { + unsafe { ll::SDL_GetWindowID(self.raw) } + } + + pub fn gl_create_context(&self) -> SdlResult { + let result = unsafe { ll::SDL_GL_CreateContext(self.raw) }; + if result == ptr::null() { + Err(get_error()) + } else { + Ok(GLContext{raw: result, owned: true}) + } + } + + pub fn gl_make_current(&self, context: &GLContext) -> bool { + unsafe { ll::SDL_GL_MakeCurrent(self.raw, context.raw) == 0 } + } + + pub fn gl_swap_window(&self) { + unsafe { ll::SDL_GL_SwapWindow(self.raw) } + } +} + +impl<'a> WindowProperties<'a> { pub fn get_display_index(&self) -> SdlResult { let result = unsafe { ll::SDL_GetWindowDisplayIndex(self.raw) }; if result < 0 { @@ -235,7 +336,7 @@ impl Window { } } - pub fn set_display_mode(&self, display_mode: Option) -> bool { + pub fn set_display_mode(&mut self, display_mode: Option) -> bool { return unsafe { ll::SDL_SetWindowDisplayMode( self.raw, @@ -268,10 +369,6 @@ impl Window { unsafe{ FromPrimitive::from_u64(ll::SDL_GetWindowPixelFormat(self.raw) as u64).unwrap() } } - pub fn get_id(&self) -> u32 { - unsafe { ll::SDL_GetWindowID(self.raw) } - } - pub fn get_flags(&self) -> WindowFlags { unsafe { let raw = ll::SDL_GetWindowFlags(self.raw); @@ -279,7 +376,7 @@ impl Window { } } - pub fn set_title(&self, title: &str) -> Result<(), NulError>{ + pub fn set_title(&mut self, title: &str) -> Result<(), NulError>{ let buff = match CString::new(title.as_bytes()) { Ok(s) => s.as_ptr(), @@ -296,14 +393,14 @@ impl Window { } } - pub fn set_icon(&self, icon: &Surface) { + pub fn set_icon(&mut self, icon: &Surface) { unsafe { ll::SDL_SetWindowIcon(self.raw, icon.raw()) } } //pub fn SDL_SetWindowData(window: *SDL_Window, name: *c_char, userdata: *c_void) -> *c_void; //TODO: Figure out what this does //pub fn SDL_GetWindowData(window: *SDL_Window, name: *c_char) -> *c_void; - pub fn set_position(&self, x: WindowPos, y: WindowPos) { + pub fn set_position(&mut self, x: WindowPos, y: WindowPos) { unsafe { ll::SDL_SetWindowPosition(self.raw, unwrap_windowpos(x), unwrap_windowpos(y)) } } @@ -314,7 +411,7 @@ impl Window { (x as i32, y as i32) } - pub fn set_size(&self, w: i32, h: i32) { + pub fn set_size(&mut self, w: i32, h: i32) { unsafe { ll::SDL_SetWindowSize(self.raw, w as c_int, h as c_int) } } @@ -332,7 +429,7 @@ impl Window { (w as i32, h as i32) } - pub fn set_minimum_size(&self, w: i32, h: i32) { + pub fn set_minimum_size(&mut self, w: i32, h: i32) { unsafe { ll::SDL_SetWindowMinimumSize(self.raw, w as c_int, h as c_int) } } @@ -343,7 +440,7 @@ impl Window { (w as i32, h as i32) } - pub fn set_maximum_size(&self, w: i32, h: i32) { + pub fn set_maximum_size(&mut self, w: i32, h: i32) { unsafe { ll::SDL_SetWindowMaximumSize(self.raw, w as c_int, h as c_int) } } @@ -354,39 +451,39 @@ impl Window { (w as i32, h as i32) } - pub fn set_bordered(&self, bordered: bool) { + pub fn set_bordered(&mut self, bordered: bool) { unsafe { ll::SDL_SetWindowBordered(self.raw, if bordered { 1 } else { 0 }) } } - pub fn show(&self) { + pub fn show(&mut self) { unsafe { ll::SDL_ShowWindow(self.raw) } } - pub fn hide(&self) { + pub fn hide(&mut self) { unsafe { ll::SDL_HideWindow(self.raw) } } - pub fn raise(&self) { + pub fn raise(&mut self) { unsafe { ll::SDL_RaiseWindow(self.raw) } } - pub fn maximize(&self) { + pub fn maximize(&mut self) { unsafe { ll::SDL_MaximizeWindow(self.raw) } } - pub fn minimize(&self) { + pub fn minimize(&mut self) { unsafe { ll::SDL_MinimizeWindow(self.raw) } } - pub fn restore(&self) { + pub fn restore(&mut self) { unsafe { ll::SDL_RestoreWindow(self.raw) } } - pub fn set_fullscreen(&self, fullscreen_type: FullscreenType) -> bool { + pub fn set_fullscreen(&mut self, fullscreen_type: FullscreenType) -> bool { unsafe { ll::SDL_SetWindowFullscreen(self.raw, fullscreen_type as uint32_t) == 0 } } - pub fn get_surface(&self) -> SdlResult { + pub fn get_surface(&mut self) -> SdlResult { let raw = unsafe { ll::SDL_GetWindowSurface(self.raw) }; if raw == ptr::null() { @@ -404,7 +501,7 @@ impl Window { unsafe { ll::SDL_UpdateWindowSurfaceRects(self.raw, rects.as_ptr(), rects.len() as c_int) == 0} } - pub fn set_grab(&self, grabbed: bool) { + pub fn set_grab(&mut self, grabbed: bool) { unsafe { ll::SDL_SetWindowGrab(self.raw, if grabbed { 1 } else { 0 }) } } @@ -412,7 +509,7 @@ impl Window { unsafe { ll::SDL_GetWindowGrab(self.raw) == 1 } } - pub fn set_brightness(&self, brightness: f64) -> bool { + pub fn set_brightness(&mut self, brightness: f64) -> bool { unsafe { ll::SDL_SetWindowBrightness(self.raw, brightness as c_float) == 0 } } @@ -420,7 +517,7 @@ impl Window { unsafe { ll::SDL_GetWindowBrightness(self.raw) as f64 } } - pub fn set_gamma_ramp(&self, red: Option<&[u16; 256]>, green: Option<&[u16; 256]>, blue: Option<&[u16; 256]>) -> bool { + pub fn set_gamma_ramp(&mut self, red: Option<&[u16; 256]>, green: Option<&[u16; 256]>, blue: Option<&[u16; 256]>) -> bool { unsafe { let unwrapped_red = match red { Some(values) => values.as_ptr(), @@ -449,23 +546,6 @@ impl Window { Err(get_error()) } } - - pub fn gl_create_context(&self) -> SdlResult { - let result = unsafe { ll::SDL_GL_CreateContext(self.raw) }; - if result == ptr::null() { - Err(get_error()) - } else { - Ok(GLContext{raw: result, owned: true}) - } - } - - pub fn gl_make_current(&self, context: &GLContext) -> bool { - unsafe { ll::SDL_GL_MakeCurrent(self.raw, context.raw) == 0 } - } - - pub fn gl_swap_window(&self) { - unsafe { ll::SDL_GL_SwapWindow(self.raw) } - } } pub fn get_num_video_drivers() -> SdlResult { @@ -648,8 +728,8 @@ pub fn gl_get_attribute(attr: GLAttr) -> SdlResult { } } -pub fn gl_get_current_window() -> SdlResult { - let raw = unsafe { ll::SDL_GL_GetCurrentWindow() }; +pub unsafe fn gl_get_current_window() -> SdlResult { + let raw = ll::SDL_GL_GetCurrentWindow(); if raw == ptr::null() { Err(get_error()) } else { @@ -657,8 +737,8 @@ pub fn gl_get_current_window() -> SdlResult { } } -pub fn gl_get_current_context() -> SdlResult { - let raw = unsafe { ll::SDL_GL_GetCurrentContext() }; +pub unsafe fn gl_get_current_context() -> SdlResult { + let raw = ll::SDL_GL_GetCurrentContext(); if raw == ptr::null() { Err(get_error()) } else { From 74656f465177afd2f2fa780e06f195c842a64fff Mon Sep 17 00:00:00 2001 From: Dan Spencer Date: Thu, 9 Apr 2015 20:57:58 -0600 Subject: [PATCH 3/4] Add lifetimes to `Surface` and `Renderer`. * `Surface` requires a lifetime because of the `Surface::from_data()` method. The method does not copy the buffer passed to it, and thus must live no longer than the buffer. * `Renderer` now takes a lifetime in accordance with `Surface`'s new lifetime requirement. --- src/sdl2/render.rs | 24 +++++++++--------- src/sdl2/surface.rs | 61 +++++++++++++++++++++++++++++++-------------- 2 files changed, 54 insertions(+), 31 deletions(-) diff --git a/src/sdl2/render.rs b/src/sdl2/render.rs index af6f8a05..4c38c227 100644 --- a/src/sdl2/render.rs +++ b/src/sdl2/render.rs @@ -141,19 +141,19 @@ impl RendererInfo { } } -pub enum RendererParent { - Surface(Surface), +pub enum RendererParent<'a> { + Surface(Surface<'a>), Window(Window) } /// 2D rendering context -pub struct Renderer { +pub struct Renderer<'a> { raw: *const ll::SDL_Renderer, - parent: Option, + parent: Option>, is_alive: Rc> } -impl Drop for Renderer { +impl<'a> Drop for Renderer<'a> { fn drop(&mut self) { unsafe { *self.is_alive.get() = false; @@ -162,9 +162,9 @@ impl Drop for Renderer { } } -impl Renderer { +impl<'a> Renderer<'a> { /// Creates a 2D rendering context for a window. - pub fn from_window(window: Window, index: RenderDriverIndex, renderer_flags: RendererFlags) -> SdlResult { + pub fn from_window(window: Window, index: RenderDriverIndex, renderer_flags: RendererFlags) -> SdlResult> { let index = match index { RenderDriverIndex::Auto => -1, RenderDriverIndex::Index(x) => x @@ -184,7 +184,7 @@ impl Renderer { } /// Creates a window and default renderer. - pub fn new_with_window(_sdl: &Sdl, width: i32, height: i32, window_flags: video::WindowFlags) -> SdlResult { + pub fn new_with_window(_sdl: &Sdl, width: i32, height: i32, window_flags: video::WindowFlags) -> SdlResult> { use sys::video::SDL_Window; let raw_window: *const SDL_Window = ptr::null(); @@ -201,7 +201,7 @@ impl Renderer { } /// Creates a 2D software rendering context for a surface. - pub fn from_surface(surface: surface::Surface) -> SdlResult { + pub fn from_surface(surface: surface::Surface<'a>) -> SdlResult> { let raw_renderer = unsafe { ll::SDL_CreateSoftwareRenderer(surface.raw()) }; if raw_renderer != ptr::null() { unsafe { @@ -256,7 +256,7 @@ impl Renderer { } #[inline] - pub fn unwrap_parent(mut self) -> RendererParent { + pub fn unwrap_parent(mut self) -> RendererParent<'a> { use std::mem; mem::replace(&mut self.parent, None).unwrap() } @@ -270,7 +270,7 @@ impl Renderer { } #[inline] - pub fn unwrap_parent_as_surface(self) -> Option { + pub fn unwrap_parent_as_surface(self) -> Option> { match self.unwrap_parent() { RendererParent::Surface(surface) => Some(surface), _ => None @@ -310,7 +310,7 @@ impl Renderer { } /// Texture-creating methods for the renderer -impl Renderer { +impl<'a> Renderer<'a> { /// Creates a texture for a rendering context. /// /// `size` is the width and height of the texture. diff --git a/src/sdl2/surface.rs b/src/sdl2/surface.rs index e0700a36..da96ab6d 100644 --- a/src/sdl2/surface.rs +++ b/src/sdl2/surface.rs @@ -1,3 +1,4 @@ +use std::marker::PhantomData; use std::mem; use std::path::Path; use rect::Rect; @@ -21,14 +22,13 @@ bitflags! { } } -#[derive(PartialEq)] -#[allow(raw_pointer_derive, missing_copy_implementations)] -pub struct Surface { +pub struct Surface<'a> { raw: *const ll::SDL_Surface, - owned: bool + owned: bool, + _marker: PhantomData<&'a ()> } -impl Drop for Surface { +impl<'a> Drop for Surface<'a> { fn drop(&mut self) { if self.owned { unsafe { @@ -38,13 +38,21 @@ impl Drop for Surface { } } -impl_raw_accessors!((Surface, *const ll::SDL_Surface)); -impl_owned_accessors!((Surface, owned)); -impl_raw_constructor!((Surface, Surface (raw: *const ll::SDL_Surface, owned: bool))); +impl<'a> Surface<'a> { + pub unsafe fn raw(&self) -> *const ll::SDL_Surface { self.raw } + + pub unsafe fn owned(&self) -> bool { self.owned } + + pub unsafe fn from_ll<'b>(raw: *const ll::SDL_Surface, owned: bool) -> Surface<'b> { + Surface { + raw: raw, + owned: owned, + _marker: PhantomData + } + } -impl Surface { pub fn new(surface_flags: SurfaceFlag, width: i32, height: i32, bpp: i32, - rmask: u32, gmask: u32, bmask: u32, amask: u32) -> SdlResult { + rmask: u32, gmask: u32, bmask: u32, amask: u32) -> SdlResult> { unsafe { let raw = ll::SDL_CreateRGBSurface(surface_flags.bits(), width as c_int, height as c_int, bpp as c_int, rmask, gmask, bmask, amask); @@ -52,13 +60,17 @@ impl Surface { if raw == ptr::null() { Err(get_error()) } else { - Ok(Surface { raw: raw, owned: true }) + Ok(Surface { + raw: raw, + owned: true, + _marker: PhantomData + }) } } } - pub fn from_data(data: &mut [u8], width: i32, height: i32, bpp: i32, pitch: i32, - rmask: u32, gmask: u32, bmask: u32, amask: u32) -> SdlResult { + pub fn from_data(data: &'a mut [u8], width: i32, height: i32, bpp: i32, pitch: i32, + rmask: u32, gmask: u32, bmask: u32, amask: u32) -> SdlResult> { unsafe { let raw = ll::SDL_CreateRGBSurfaceFrom( @@ -68,7 +80,11 @@ impl Surface { if raw == ptr::null() { Err(get_error()) } else { - Ok(Surface { raw: raw, owned: true }) + Ok(Surface { + raw: raw, + owned: true, + _marker: PhantomData + }) } } } @@ -126,13 +142,20 @@ impl Surface { unsafe { ll::SDL_UnlockSurface(self.raw); } } - pub fn from_bmp(path: &Path) -> SdlResult { + pub fn from_bmp(path: &Path) -> SdlResult> { let raw = unsafe { ll::SDL_LoadBMP_RW(try!(rwops::RWops::from_file(path, "rb")).raw(), 0) }; - if raw.is_null() { Err(get_error()) } - else { Ok(Surface{raw: raw, owned: true}) } + if raw.is_null() { + Err(get_error()) + } else { + Ok(Surface { + raw: raw, + owned: true, + _marker: PhantomData + }) + } } pub fn save_bmp(&self, path: &Path) -> SdlResult<()> { @@ -307,7 +330,7 @@ impl Surface { rect } - pub fn convert(&self, format: &pixels::PixelFormat) -> SdlResult { + pub fn convert(&self, format: &pixels::PixelFormat) -> SdlResult> { // SDL_ConvertSurface takes a flag as the last parameter, which should be 0 by the docs. let surface_ptr = unsafe { ll::SDL_ConvertSurface(self.raw, format.raw(), 0u32) }; @@ -318,7 +341,7 @@ impl Surface { } } - pub fn convert_format(&self, format: pixels::PixelFormatEnum) -> SdlResult { + pub fn convert_format(&self, format: pixels::PixelFormatEnum) -> SdlResult> { let surface_ptr = unsafe { ll::SDL_ConvertSurfaceFormat(self.raw, format as uint32_t, 0u32) }; if surface_ptr == ptr::null() { From 997e243523620d6505ee75a67d3b4f80276f2dab Mon Sep 17 00:00:00 2001 From: Dan Spencer Date: Sat, 11 Apr 2015 06:03:46 -0600 Subject: [PATCH 4/4] Some cleanup in video.rs; mostly returning SdlResult instead of bool Convert functions that return a `bool` into returning `SdlResult<()>`. WindowProperties::get_title() returns a string slice instead of a newly allocated String. Fix `Window::get_display_mode()` --- src/sdl2/video.rs | 120 +++++++++++++++++++++++++++++----------------- 1 file changed, 75 insertions(+), 45 deletions(-) diff --git a/src/sdl2/video.rs b/src/sdl2/video.rs index a875f99b..833fddbb 100644 --- a/src/sdl2/video.rs +++ b/src/sdl2/video.rs @@ -1,6 +1,7 @@ use libc::{c_int, c_float, uint32_t}; use std::ffi::{CStr, CString, NulError}; use std::marker::PhantomData; +use std::mem; use std::ptr; use std::vec::Vec; @@ -61,17 +62,6 @@ bitflags! { } } -fn empty_sdl_display_mode() -> ll::SDL_DisplayMode { - ll::SDL_DisplayMode { - format: 0, - w: 0, - h: 0, - refresh_rate: 0, - driverdata: ptr::null() - } -} - -#[allow(missing_copy_implementations)] #[derive(Clone, PartialEq)] pub struct DisplayMode { pub format: u32, @@ -81,7 +71,6 @@ pub struct DisplayMode { } impl DisplayMode { - pub fn new(format: u32, w: i32, h: i32, refresh_rate: i32) -> DisplayMode { DisplayMode { format: format, @@ -317,8 +306,14 @@ impl Window { } } - pub fn gl_make_current(&self, context: &GLContext) -> bool { - unsafe { ll::SDL_GL_MakeCurrent(self.raw, context.raw) == 0 } + pub fn gl_make_current(&self, context: &GLContext) -> SdlResult<()> { + unsafe { + if ll::SDL_GL_MakeCurrent(self.raw, context.raw) == 0 { + Ok(()) + } else { + Err(get_error()) + } + } } pub fn gl_swap_window(&self) { @@ -336,25 +331,30 @@ impl<'a> WindowProperties<'a> { } } - pub fn set_display_mode(&mut self, display_mode: Option) -> bool { - return unsafe { - ll::SDL_SetWindowDisplayMode( + pub fn set_display_mode(&mut self, display_mode: Option) -> SdlResult<()> { + unsafe { + let result = ll::SDL_SetWindowDisplayMode( self.raw, match display_mode { Some(ref mode) => &mode.to_ll() as *const _, None => ptr::null() } - ) == 0 + ); + if result < 0 { + Err(get_error()) + } else { + Ok(()) + } } } - pub fn get_display_mode(&self, display_mode: &DisplayMode) -> SdlResult { - let dm = empty_sdl_display_mode(); + pub fn get_display_mode(&self) -> SdlResult { + let dm = unsafe { mem::uninitialized() }; let result = unsafe { ll::SDL_GetWindowDisplayMode( self.raw, - &display_mode.to_ll() + &dm ) == 0 }; @@ -376,20 +376,21 @@ impl<'a> WindowProperties<'a> { } } - pub fn set_title(&mut self, title: &str) -> Result<(), NulError>{ - let buff = - match CString::new(title.as_bytes()) { - Ok(s) => s.as_ptr(), - Err(e) => return Err(e), - }; + pub fn set_title(&mut self, title: &str) -> Result<(), NulError> { + let buff = try!(CString::new(title)).as_ptr(); unsafe { ll::SDL_SetWindowTitle(self.raw, buff); } Ok(()) } - pub fn get_title(&self) -> String { + pub fn get_title(&self) -> &str { + use std::ffi::CStr; + use std::str; + unsafe { let buf = ll::SDL_GetWindowTitle(self.raw); - String::from_utf8_lossy(CStr::from_ptr(buf).to_bytes()).to_string() + + // The window title must be encoded in UTF-8. + str::from_utf8(CStr::from_ptr(buf).to_bytes()).unwrap() } } @@ -479,8 +480,14 @@ impl<'a> WindowProperties<'a> { unsafe { ll::SDL_RestoreWindow(self.raw) } } - pub fn set_fullscreen(&mut self, fullscreen_type: FullscreenType) -> bool { - unsafe { ll::SDL_SetWindowFullscreen(self.raw, fullscreen_type as uint32_t) == 0 } + pub fn set_fullscreen(&mut self, fullscreen_type: FullscreenType) -> SdlResult<()> { + unsafe { + if ll::SDL_SetWindowFullscreen(self.raw, fullscreen_type as uint32_t) == 0 { + Ok(()) + } else { + Err(get_error()) + } + } } pub fn get_surface(&mut self) -> SdlResult { @@ -493,12 +500,24 @@ impl<'a> WindowProperties<'a> { } } - pub fn update_surface(&self) -> bool { - unsafe { ll::SDL_UpdateWindowSurface(self.raw) == 0 } + pub fn update_surface(&self) -> SdlResult<()> { + unsafe { + if ll::SDL_UpdateWindowSurface(self.raw) == 0 { + Ok(()) + } else { + Err(get_error()) + } + } } - pub fn update_surface_rects(&self, rects: &[Rect]) -> bool { - unsafe { ll::SDL_UpdateWindowSurfaceRects(self.raw, rects.as_ptr(), rects.len() as c_int) == 0} + pub fn update_surface_rects(&self, rects: &[Rect]) -> SdlResult<()> { + unsafe { + if ll::SDL_UpdateWindowSurfaceRects(self.raw, rects.as_ptr(), rects.len() as c_int) == 0 { + Ok(()) + } else { + Err(get_error()) + } + } } pub fn set_grab(&mut self, grabbed: bool) { @@ -509,15 +528,21 @@ impl<'a> WindowProperties<'a> { unsafe { ll::SDL_GetWindowGrab(self.raw) == 1 } } - pub fn set_brightness(&mut self, brightness: f64) -> bool { - unsafe { ll::SDL_SetWindowBrightness(self.raw, brightness as c_float) == 0 } + pub fn set_brightness(&mut self, brightness: f64) -> SdlResult<()> { + unsafe { + if ll::SDL_SetWindowBrightness(self.raw, brightness as c_float) == 0 { + Ok(()) + } else { + Err(get_error()) + } + } } pub fn get_brightness(&self) -> f64 { unsafe { ll::SDL_GetWindowBrightness(self.raw) as f64 } } - pub fn set_gamma_ramp(&mut self, red: Option<&[u16; 256]>, green: Option<&[u16; 256]>, blue: Option<&[u16; 256]>) -> bool { + pub fn set_gamma_ramp(&mut self, red: Option<&[u16; 256]>, green: Option<&[u16; 256]>, blue: Option<&[u16; 256]>) -> SdlResult<()> { unsafe { let unwrapped_red = match red { Some(values) => values.as_ptr(), @@ -531,7 +556,12 @@ impl<'a> WindowProperties<'a> { Some(values) => values.as_ptr(), None => ptr::null() }; - ll::SDL_SetWindowGammaRamp(self.raw, unwrapped_red, unwrapped_green, unwrapped_blue) == 0 + + if ll::SDL_SetWindowGammaRamp(self.raw, unwrapped_red, unwrapped_green, unwrapped_blue) == 0 { + Ok(()) + } else { + Err(get_error()) + } } } @@ -621,7 +651,7 @@ pub fn get_num_display_modes(display_index: i32) -> SdlResult { } pub fn get_display_mode(display_index: i32, mode_index: i32) -> SdlResult { - let dm = empty_sdl_display_mode(); + let dm = unsafe { mem::uninitialized() }; let result = unsafe { ll::SDL_GetDisplayMode(display_index as c_int, mode_index as c_int, &dm) == 0}; if result { @@ -632,7 +662,7 @@ pub fn get_display_mode(display_index: i32, mode_index: i32) -> SdlResult SdlResult { - let dm = empty_sdl_display_mode(); + let dm = unsafe { mem::uninitialized() }; let result = unsafe { ll::SDL_GetDesktopDisplayMode(display_index as c_int, &dm) == 0}; if result { @@ -643,7 +673,7 @@ pub fn get_desktop_display_mode(display_index: i32) -> SdlResult { } pub fn get_current_display_mode(display_index: i32) -> SdlResult { - let dm = empty_sdl_display_mode(); + let dm = unsafe { mem::uninitialized() }; let result = unsafe { ll::SDL_GetCurrentDisplayMode(display_index as c_int, &dm) == 0}; if result { @@ -655,14 +685,14 @@ pub fn get_current_display_mode(display_index: i32) -> SdlResult { pub fn get_closest_display_mode(display_index: i32, mode: &DisplayMode) -> SdlResult { let input = mode.to_ll(); - let out = empty_sdl_display_mode(); + let dm = unsafe { mem::uninitialized() }; - let result = unsafe { ll::SDL_GetClosestDisplayMode(display_index as c_int, &input, &out) }; + let result = unsafe { ll::SDL_GetClosestDisplayMode(display_index as c_int, &input, &dm) }; if result == ptr::null() { Err(get_error()) } else { - Ok(DisplayMode::from_ll(&out)) + Ok(DisplayMode::from_ll(&dm)) } }