Initial OpenGL cleanup pass

not great but it's "better", I suppose.

crates/retro_frontend/src/libretro_callbacks.rs

@@ -108,11 +108,11 @@ pub(crate) unsafe extern "C" fn environment_callback(
 			let hw_render = (data as *mut HwRenderCallback).as_mut().unwrap();
 
 			let hw_render_context_type =
-				HwContextType::from_uint(hw_render.context_type).expect("FUCK");
+				HwContextType::from_uint(hw_render.context_type).expect("Uh oh!");
 
 			if hw_render_context_type != HwContextType::OpenGL && hw_render_context_type != HwContextType::OpenGLCore {
 				error!(
-					"Core is trying to request an context type we don't support ({:?})",
+					"Core is trying to request an context type we don't support ({:?}), failing",
 					hw_render_context_type
 				);
 				return false;
@@ -128,6 +128,9 @@ pub(crate) unsafe extern "C" fn environment_callback(
 			// reset context
 			(hw_render.context_reset)();
 
+			// Once we have initalized HW rendering any data here doesn't matter and isn't needed.
+			(*FRONTEND).converted_pixel_buffer.clear();
+
 			return true;
 		}
 

crates/retrovnc/src/egl.rs

@@ -1,4 +1,5 @@
 #[allow(non_camel_case_types)]
+#[allow(unused_imports)]
 mod egl_impl {
 	pub type khronos_utime_nanoseconds_t = khronos_uint64_t;
 	pub type khronos_uint64_t = u64;

crates/retrovnc/src/main.rs

@@ -1,4 +1,4 @@
-use std::{net::Ipv4Addr, path::Path, ptr::addr_of_mut};
+use std::{net::Ipv4Addr, path::Path};
 
 use anyhow::Result;
 
@@ -11,16 +11,15 @@ use tracing_subscriber::FmtSubscriber;
 
 use clap::{arg, command, value_parser};
 
-
+/// Called by OpenGL. We use this to dump errors.
-
 extern "system" fn opengl_message_callback(
 	source: gl::types::GLenum,
 	_type: gl::types::GLenum,
 	id: gl::types::GLuint,
-	severity: gl::types::GLenum,
+	_severity: gl::types::GLenum,
-	length: gl::types::GLsizei,
+	_length: gl::types::GLsizei,
 	message: *const gl::types::GLchar,
-	user: *mut std::ffi::c_void,
+	_user: *mut std::ffi::c_void,
 ) {
 	unsafe {
 		let message = std::ffi::CStr::from_ptr(message);
@@ -47,11 +46,12 @@ struct App {
 
 	hw_render: bool,
 
-	// opengl state shits
+	// OpenGL object IDs
 	texture_id: gl::types::GLuint,
 	renderbuffer_id: gl::types::GLuint,
 	fbo_id: gl::types::GLuint,
 
+	/// Cached readback buffer.
 	readback_buffer: Vec<u32>,
 }
 
@@ -121,14 +121,13 @@ impl App {
 	}
 
 	fn hw_gl_resize(&mut self, width: u32, height: u32) {
-		// TODO: if requested, add depth renderbuffer.
+		// TODO: cleanup "delete" codepaths so we aren't duplicating as much.
 		unsafe {
 			if self.fbo_id == 0 {
 				gl::GenFramebuffers(1, std::ptr::addr_of_mut!(self.fbo_id));
 				gl::BindFramebuffer(gl::FRAMEBUFFER, self.fbo_id);
 			} else {
 				gl::DeleteFramebuffers(1, std::ptr::addr_of_mut!(self.fbo_id));
-
 				gl::GenFramebuffers(1, std::ptr::addr_of_mut!(self.fbo_id));
 				gl::BindFramebuffer(gl::FRAMEBUFFER, self.fbo_id);
 			}
@@ -154,17 +153,22 @@ impl App {
 				std::ptr::null(),
 			);
 
-			if self.renderbuffer_id != 0 {
+			if self.renderbuffer_id == 0 {
+				gl::GenRenderbuffers(1, std::ptr::addr_of_mut!(self.renderbuffer_id));
+				gl::BindRenderbuffer(gl::RENDERBUFFER, self.renderbuffer_id);
+			} else {
+				gl::DeleteRenderbuffers(1, std::ptr::addr_of_mut!(self.renderbuffer_id));
 				gl::GenRenderbuffers(1, std::ptr::addr_of_mut!(self.renderbuffer_id));
 				gl::BindRenderbuffer(gl::RENDERBUFFER, self.renderbuffer_id);
-				gl::RenderbufferStorage(
-					gl::RENDERBUFFER,
-					gl::DEPTH_COMPONENT,
-					width as i32,
-					height as i32,
-				);
 			}
 
+			gl::RenderbufferStorage(
+				gl::RENDERBUFFER,
+				gl::DEPTH_COMPONENT,
+				width as i32,
+				height as i32,
+			);
+
 			gl::FramebufferTexture2D(
 				gl::FRAMEBUFFER,
 				gl::COLOR_ATTACHMENT0,
@@ -212,12 +216,9 @@ impl App {
 
 impl FrontendInterface for App {
 	fn video_resize(&mut self, width: u32, height: u32) {
-		//let width = width * 2;
-		//let height = height * 2;
-
 		tracing::info!("Resized to {width}x{height}");
 
-		// Resize OpenGL
+		// Resize OpenGL resources if we need to.
 		if self.hw_render {
 			self.hw_gl_resize(width, height);
 		}
@@ -236,7 +237,7 @@ impl FrontendInterface for App {
 		unsafe {
 			gl::BindFramebuffer(gl::FRAMEBUFFER, self.fbo_id);
 
-			// I know this sucks but hey
+			// I know this sucks but it works for this case.
 			gl::ReadPixels(
 				0,
 				0,
@@ -247,7 +248,8 @@ impl FrontendInterface for App {
 				self.readback_buffer.as_mut_ptr() as *mut std::ffi::c_void,
 			);
 
-			self.rfb_server.update_buffer(&self.readback_buffer[..], dimensions.0, true);
+			self.rfb_server
+				.update_buffer(&self.readback_buffer[..], dimensions.0, true);
 
 			gl::BindFramebuffer(gl::FRAMEBUFFER, 0);
 		}
@@ -269,7 +271,7 @@ impl FrontendInterface for App {
 
 	fn hw_gl_init(&mut self) -> HwGlInitData {
 		if self.hw_render {
-			panic!("Nope!");
+			panic!("Cannot initalize HW rendering more than once");
 		}
 
 		let display = unsafe {
@@ -288,24 +290,19 @@ impl FrontendInterface for App {
 
 			// load OpenGL functions (using EGL loader. We should probably check the one extension exists)
 			gl::load_with(|s| {
-				let str = std::ffi::CString::new(s).expect("you MOTHERFUCKER");
+				let str = std::ffi::CString::new(s).expect("Uhh huh.");
 				std::mem::transmute(egl::GetProcAddress(str.as_ptr()))
 			});
 
-			tracing::info!("before query devices");
-
 			(query_devices_ext)(
 				NR_DEVICES_MAX as i32,
 				devices.as_mut_ptr(),
 				std::ptr::addr_of_mut!(nr_devices_real),
 			);
 
-			tracing::info!("after query devices");
 			(get_platform_display_ext)(egl::PLATFORM_DEVICE_EXT, devices[0], std::ptr::null())
 		};
 
-		tracing::info!("made EGL display");
-
 		let context = unsafe {
 			const EGL_CONFIG_ATTRIBUTES: [egl::types::EGLenum; 13] = [
 				egl::SURFACE_TYPE,
@@ -354,21 +351,16 @@ impl FrontendInterface for App {
 			gl::Enable(gl::DEBUG_OUTPUT);
 			gl::DebugMessageCallback(Some(opengl_message_callback), std::ptr::null());
 
+			// Resize to initial dimensions (this will create an FBO as well)
 			let dimensions = self.get_frontend().get_size();
-
 			self.hw_gl_resize(dimensions.0, dimensions.1);
-
-			gl::BindFramebuffer(gl::FRAMEBUFFER, 0);
 		}
 
 		self.hw_render = true;
 
 		return unsafe {
 			HwGlInitData {
-				get_proc_address: std::mem::transmute::<
+				get_proc_address: std::mem::transmute(egl::GetProcAddress as *mut std::ffi::c_void),
-					_,
-					retro_frontend::frontend::GlGetProcAddress,
-				>(egl::GetProcAddress as usize),
 			}
 		};
 	}
@@ -386,6 +378,7 @@ fn main() -> Result<()> {
 		.arg(arg!(--core <VALUE>).required(true))
 		// Not that it matters, but this is only really required for cores that require
 		// content to be loaded; that's most cores, but libretro does support the difference.
+		// TODO: A core will tell us if it requires content, if it's not provided we can yell and exit.
 		.arg(arg!(--rom <VALUE>).required(false))
 		.arg(
 			arg!(--rfb_listen <ADDRESS>)

crates/retrovnc/src/rfb.rs

@@ -99,7 +99,7 @@ impl RfbServer {
 		}
 	}
 
-	pub fn update_buffer(&mut self, slice: &[u32], pitch: u32, flipped: bool) {
+	pub fn update_buffer(&mut self, slice: &[u32], pitch: u32, from_opengl: bool) {
 		//self.framebuffer.copy_from_slice(&slice);
 
 		let has_disconnected_pitch = pitch != self.width as u32;
@@ -108,18 +108,20 @@ impl RfbServer {
 		//	info!("DEBUG: Pitch is not the same as width {}, {}", pitch, self.width);
 		//}
 
-		// copy a line at a time (test)
+		// If this frame came from OpenGL we need to flip the image around
-
+		// so it is right side up (from our perspective).
-		if flipped {
+		if from_opengl {
 			let mut scanlines: Vec<&[u32]> = Vec::with_capacity(self.height as usize);
 
+			// Push scanline slices in reverse order (which will actually flip them to the right orientation)
 			for y in (0..self.height).rev() {
 				let src_line_off = (y as u32 * pitch) as usize;
 				let src_slice = &slice[src_line_off..src_line_off + self.width as usize];
 				scanlines.push(src_slice);
 			}
 
-			for y in (0..self.height) {
+			// Draw them
+			for y in 0..self.height {
 				let src_line_off = (y as u32 * pitch) as usize;
 				let mut dest_line_off = src_line_off;
 
@@ -133,13 +135,12 @@ impl RfbServer {
 
 				dest_slice.copy_from_slice(scanlines[y as usize]);
 
-				// swap scanline to BGRA (compatibility hack. Should make this optional!)
+				// swap the scanline pixels to BGRA order (compatibility hack. Should make this optional!)
 				for pix in dest_slice {
-					let a = ((*pix & 0xff000000) >> 24);
+					let a = (*pix & 0xff000000) >> 24;
-					let b = ((*pix & 0x00ff0000) >> 16);
+					let b = (*pix & 0x00ff0000) >> 16;
-					let g = ((*pix & 0x0000ff00) >> 8);
+					let g = (*pix & 0x0000ff00) >> 8;
-					let r = (*pix & 0x000000ff);
+					let r = *pix & 0x000000ff;
-
 					*pix = a << 24 | r << 16 | g << 8 | b;
 				}
 			}