factor out frame-to-surface code in SDL, use it to convert pixel format before upscaling in frame-to-canvas

ferretro_components/Cargo.toml

@@ -12,7 +12,7 @@ ferretro_base = { path = "../ferretro_base" }
 libloading = "0.5"
 num_enum = "0.4"
 ffmpeg-next = { version = "4.3.8", optional = true }
-sdl2 = { version = "0.32", optional = true, features = ["gfx"] }
+sdl2 = { version = "0.35.1", optional = true, features = ["gfx"] }
 gl = { version = "0.14", optional = true }
 crossbeam-channel = { version = "0.4", optional = true }
 tempfile = "3"

ferretro_components/src/provided/sdl2/canvas.rs

@@ -4,17 +4,14 @@ use std::ffi::CStr;
 use std::path::Path;
 
 use sdl2::Sdl;
-use sdl2::rect::Rect;
 use sdl2::render::WindowCanvas;
 use sdl2::video::FullscreenType;
 
 pub(crate) fn paint_frame_on_canvas(frame: &VideoFrame, canvas: &mut WindowCanvas) -> bool {
-    if let Some((pixel_data, pitch)) = frame.data_pitch_as_bytes() {
+    if let Ok(surf) = frame_to_surface(frame) {
         let (cw, ch) = canvas.output_size().unwrap();
-        let (width, height) = frame.dimensions();
-        let pixel_format = frame.pixel_format().map(sdl2_pixfmt);
 
-        let should_intscale = if cw >= width && ch >= height {
+        let should_intscale = if cw >= surf.width() && ch >= surf.height() {
             sdl2::sys::SDL_bool::SDL_TRUE
         } else {
             sdl2::sys::SDL_bool::SDL_FALSE
@@ -23,15 +20,14 @@ pub(crate) fn paint_frame_on_canvas(frame: &VideoFrame, canvas: &mut WindowCanva
             sdl2::sys::SDL_RenderSetIntegerScale(canvas.raw(), should_intscale);
         }
 
-        if let Ok(mut tex) = canvas
+        let format = sdl2::pixels::PixelFormat::try_from(canvas.default_pixel_format()).unwrap();
+        let surface = surf.convert(&format).unwrap();
+
+        if let Ok(tex) = canvas
             .texture_creator()
-            .create_texture_static(pixel_format, width, height)
+            .create_texture_from_surface(surface)
         {
-            let rect = Rect::new(0, 0, width, height);
+            canvas.copy(&tex, None, None).unwrap();
-            if tex.update(rect, pixel_data, pitch).is_ok() {
-                canvas.clear();
-                canvas.copy(&tex, None, None).unwrap();
-            }
         }
         canvas.present();
         true
@@ -48,16 +44,33 @@ pub(crate) fn sdl2_pixfmt(retro_fmt: PixelFormat) -> sdl2::pixels::PixelFormatEn
     }
 }
 
+pub(crate) fn frame_to_surface<'a>(frame: &'a VideoFrame) -> crate::base::Result<sdl2::surface::Surface<'a>> {
+    let (bytes, pitch) = frame.data_pitch_as_bytes().unwrap();
+    let (width, height) = frame.dimensions();
+    let pixel_format = frame
+        .pixel_format()
+        .map(sdl2_pixfmt)
+        .ok_or("Unknown pixel format")?;
+    // dirty, but must be &mut for SDL API.
+    // safe as long as we don't offer a &mut Surface in the public API.
+    let data = unsafe {
+        core::slice::from_raw_parts_mut(bytes.as_ptr() as *mut u8, bytes.len())
+    };
+    sdl2::surface::Surface::from_data(data, width, height, pitch as u32, pixel_format)
+        .map_err(Into::into)
+}
+
 /// Creates a root window in SDL2, then displays each 2D video frame provided by the core
 /// by converting it to a [sdl2::render::Texture] and copying it to the window's canvas.
 ///
 /// This component has no public interface and manages the SDL2 window on its own.
 pub struct SimpleSdl2CanvasComponent {
     canvas: WindowCanvas,
+    win_size: (u32, u32),
 }
 
 impl SimpleSdl2CanvasComponent {
-    pub fn new(sdl_context: &mut Sdl, ft: FullscreenType) -> Result<Self, Box<dyn std::error::Error>> {
+    pub fn new(sdl_context: &mut Sdl, ft: FullscreenType) -> crate::base::Result<Self> {
         // default to old libsnes 256x224 window size until load_game (prereq of get_system_av_info)
         let mut window = sdl_context
             .video()?
@@ -72,8 +85,11 @@ impl SimpleSdl2CanvasComponent {
             .into_canvas()
             .build()?;
 
+        let win_size = canvas.output_size().unwrap();
+
         Ok(SimpleSdl2CanvasComponent {
             canvas,
+            win_size,
         })
     }
 }
@@ -95,6 +111,11 @@ impl RetroComponent for SimpleSdl2CanvasComponent {
 
 impl RetroCallbacks for SimpleSdl2CanvasComponent {
     fn video_refresh(&mut self, frame: &VideoFrame) {
+        let output_size = self.canvas.output_size().unwrap();
+        if output_size != self.win_size {
+            self.canvas.clear();
+            self.win_size = output_size;
+        }
         paint_frame_on_canvas(frame, &mut self.canvas);
     }
 

ferretro_components/src/provided/sdl2/opengl.rs

@@ -21,6 +21,7 @@ use super::canvas::paint_frame_on_canvas;
 #[allow(non_snake_case)]
 pub struct SimpleSdl2OpenglComponent {
     canvas: WindowCanvas,
+    win_size: (u32, u32),
     window_fbo: c_uint,
     hw_context_reset_fn: Option<HwContextResetFn>,
     hw_context_destroy_fn: Option<HwContextResetFn>,
@@ -59,8 +60,11 @@ impl SimpleSdl2OpenglComponent {
         // SDL_CreateRenderer, called by CanvasBuilder::build, creates a new GL Context.
         let window_fbo = unsafe { let mut fbo =  0; glGetIntegerv(gl::FRAMEBUFFER_BINDING, &mut fbo); fbo };
 
+        let win_size = canvas.output_size().unwrap();
+
         Ok(SimpleSdl2OpenglComponent {
             canvas,
+            win_size,
             window_fbo,
             hw_context_reset_fn: None,
             hw_context_destroy_fn: None,
@@ -110,7 +114,14 @@ impl RetroCallbacks for SimpleSdl2OpenglComponent {
                 unsafe { (self.glClear)(gl::COLOR_BUFFER_BIT); }
             }
             VideoFrame::Duplicate { .. } => {}
-            _ => { paint_frame_on_canvas(frame, &mut self.canvas); },
+            _ => {
+                let output_size = self.canvas.output_size().unwrap();
+                if output_size != self.win_size {
+                    self.canvas.clear();
+                    self.win_size = output_size;
+                }
+                paint_frame_on_canvas(frame, &mut self.canvas);
+            },
         }
     }
 

ferretro_components/src/provided/sdl2/surface.rs

@@ -1,4 +1,5 @@
 use crate::prelude::*;
+use crate::provided::sdl2::canvas::frame_to_surface;
 
 use sdl2::surface::Surface;
 
@@ -9,7 +10,7 @@ pub struct Sdl2SurfaceComponent {
 }
 
 impl Sdl2SurfaceComponent {
-    pub fn new() -> Result<Self, Box<dyn std::error::Error + Send + Sync>> {
+    pub fn new() -> crate::base::Result<Self> {
         let pixel_format = sdl2::pixels::PixelFormatEnum::ARGB1555;
 
         // automatically replaced in video_refresh whenever size doesn't match
@@ -39,26 +40,14 @@ impl Sdl2SurfaceComponent {
 impl RetroComponent for Sdl2SurfaceComponent {}
 impl RetroCallbacks for Sdl2SurfaceComponent {
     fn video_refresh(&mut self, frame: &VideoFrame) {
-        match frame {
+        if let Ok(surf) = frame_to_surface(frame) {
-            VideoFrame::XRGB1555 { width, height, .. }
+            if self.surface.size() != surf.size() {
-            | VideoFrame::RGB565 { width, height, .. }
+                let (width, height) = surf.size();
-            | VideoFrame::XRGB8888 { width, height, .. } => {
+                if let Ok(new) = Surface::new(width, height, self.pixel_format) {
-                // dirty, but must be &mut for SDL API.
+                    self.surface = new;
-                // safe as long as we don't offer a &mut Surface in the API.
-                let (bytes, pitch) = frame.data_pitch_as_bytes().unwrap();
-                let data = unsafe {
-                    core::slice::from_raw_parts_mut(bytes.as_ptr() as *mut u8, bytes.len())
-                };
-                if let Ok(surf) = Surface::from_data(data, *width, *height, pitch as u32, self.pixel_format) {
-                    if self.surface.size() != (*width, *height) {
-                        if let Ok(new) = Surface::new(*width, *height, self.pixel_format) {
-                            self.surface = new;
-                        }
-                    }
-                    let _ = surf.blit(None, &mut self.surface, None);
                 }
             }
-            _ => {}
+            let _ = surf.blit(None, &mut self.surface, None);
         }
     }