From 90f8b062d36d13cb94bf4b41d2619730bde689b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 15 Oct 2015 18:23:59 +0200 Subject: [PATCH 1/6] platform/glutin: Allow failure on glutin platform and minor nits We should probably change `Err(&str)` to another type of error... --- src/lib.rs | 2 ++ src/platform/with_glutin/mod.rs | 39 +++++++++++++++++++-------------- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index b676742..e75c1b9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,6 +13,8 @@ extern crate x11; extern crate cgl; #[cfg(target_os="macos")] extern crate core_foundation; +#[cfg(target_os="windows")] +extern crate glutin; mod platform; pub use platform::{NativeGLContext, NativeGLContextMethods}; diff --git a/src/platform/with_glutin/mod.rs b/src/platform/with_glutin/mod.rs index 0aa5964..a90c04a 100644 --- a/src/platform/with_glutin/mod.rs +++ b/src/platform/with_glutin/mod.rs @@ -1,42 +1,47 @@ -extern crate glutin; - use platform::NativeGLContextMethods; +use glutin::{HeadlessContext, HeadlessRendererBuilder}; + #[cfg(feature="texture_surface")] use layers::platform::surface::NativeDisplay; #[cfg(not(feature="texture_surface"))] struct NativeDisplay; +#[cfg(not(feature="texture_surface"))] +impl NativeDisplay { + fn new() -> NativeDisplay { + NativeDisplay + } +} + + pub struct NativeGLContext { - context: glutin::HeadlessContext, + context: HeadlessContext, display: NativeDisplay, } impl NativeGLContextMethods for NativeGLContext { - fn get_proc_address(addr: &str) -> *const () { - unsafe { - 0 as *const () - } + fn get_proc_address(_addr: &str) -> *const () { + 0 as *const () } fn create_headless() -> Result { - let display = NativeDisplay; - return Ok(NativeGLContext { - context: glutin::HeadlessRendererBuilder::new(128, 128).build().unwrap(), - display: display, - }); + let builder = HeadlessRendererBuilder::new(128, 128); + let glutin_context = try!(builder.build().or(Err("Glutin Headless context creation error"))); + + Ok(NativeGLContext { + context: glutin_context, + display: NativeDisplay::new(), + }) } fn is_current(&self) -> bool { - return self.context.is_current(); + self.context.is_current() } fn make_current(&self) -> Result<(), &'static str> { unsafe { - match self.context.make_current() { - Ok(()) => Ok(()), - Err(_) => Err("MakeCurrent failed") - } + self.context.make_current().or(Err("MakeCurrent failed")) } } From 024f44923e8d4ff6a4b59933e0cb6828a76986ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 29 Oct 2015 00:21:03 +0100 Subject: [PATCH 2/6] platform/glutin: Use servo-specific glutin on win32, and remove outdated comments --- Cargo.toml | 6 ++++-- src/platform/mod.rs | 9 --------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 060a1ad..16978e9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,5 +46,7 @@ features = ["xlib"] version = "^2.0.1" features = ["xlib"] -[target.x86_64-pc-windows-gnu.dependencies] -glutin = "*" +[target.x86_64-pc-windows-gnu.dependencies.glutin] +git = "https://github.com/servo/glutin" +branch = "servo" + diff --git a/src/platform/mod.rs b/src/platform/mod.rs index 57a4145..c86362b 100644 --- a/src/platform/mod.rs +++ b/src/platform/mod.rs @@ -31,15 +31,6 @@ pub mod with_egl; #[cfg(target_os="android")] pub use self::with_egl::NativeGLContext; -// TODO(ecoal95): Get a machine to test with mac and -// get android building: -// -// #[cfg(not(target_os="linux"))] -// pub mod with_egl; -// -// #[cfg(not(target_os="linux"))] -// pub use platform::with_egl::NativeGLContext; - #[cfg(target_os="windows")] pub mod with_glutin; From 79fd07b649cd0de660f0198136c567e0eb5b0a68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 29 Oct 2015 00:26:59 +0100 Subject: [PATCH 3/6] platform/unimplemented: Use unimplemented platform Also implements the missing methods. Don't know when it was removed on the first place. --- src/platform/mod.rs | 6 ++++++ src/platform/not_implemented/native_gl_context.rs | 6 +++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/platform/mod.rs b/src/platform/mod.rs index c86362b..ef3ba22 100644 --- a/src/platform/mod.rs +++ b/src/platform/mod.rs @@ -36,3 +36,9 @@ pub mod with_glutin; #[cfg(target_os="windows")] pub use self::with_glutin::NativeGLContext; + +#[cfg(not(any(target_os="linux", target_os="macos", target_os="android", target_os="windows")))] +pub mod not_implemented; + +#[cfg(not(any(target_os="linux", target_os="macos", target_os="android", target_os="windows")))] +pub use self::not_implemented::NativeGLContext; diff --git a/src/platform/not_implemented/native_gl_context.rs b/src/platform/not_implemented/native_gl_context.rs index 70ebec1..b0412b8 100644 --- a/src/platform/not_implemented/native_gl_context.rs +++ b/src/platform/not_implemented/native_gl_context.rs @@ -6,7 +6,7 @@ pub struct NativeGLContext; use layers::platform::surface::NativeGraphicsMetadata; impl NativeGLContextMethods for NativeGLContext { - fn get_proc_address(addr: &str) -> *const () { + fn get_proc_address(_addr: &str) -> *const () { 0 as *const () } @@ -22,6 +22,10 @@ impl NativeGLContextMethods for NativeGLContext { Err("Not implemented (yet)") } + fn unbind(&self) -> Result<(), &'static str> { + unimplemented!() + } + #[cfg(feature="texture_surface")] fn get_metadata(&self) -> NativeGraphicsMetadata { unimplemented!() From 38710f2e445e49ab4c9777d23b5d805224d4d807 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 29 Oct 2015 00:29:37 +0100 Subject: [PATCH 4/6] build: Silence warnings from gl_generator code --- src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib.rs b/src/lib.rs index e75c1b9..f175f21 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -41,6 +41,7 @@ pub use gl_formats::GLFormats; extern crate log; #[cfg(target_os="linux")] +#[allow(improper_ctypes)] mod glx { include!(concat!(env!("OUT_DIR"), "/glx_bindings.rs")); } From 226da656d7b3fa8c35cc9c77a86d9862f5da3f4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 29 Oct 2015 02:02:41 +0100 Subject: [PATCH 5/6] draw_buffer: Fix texture_flip_and_target usage and tests This commit updates the tests to the newest rust-layers. --- src/draw_buffer.rs | 6 ++++- src/tests.rs | 63 +++++++++++++++++++++------------------------- 2 files changed, 33 insertions(+), 36 deletions(-) diff --git a/src/draw_buffer.rs b/src/draw_buffer.rs index b9c2ed9..df3e195 100644 --- a/src/draw_buffer.rs +++ b/src/draw_buffer.rs @@ -262,13 +262,17 @@ impl DrawBufferHelpers for DrawBuffer { #[cfg(feature="texture_surface")] ColorAttachmentType::TextureWithSurface => { // TODO(ecoal95): check if this is correct - let (flip, target) = Texture::texture_flip_and_target(true); + let (flip, target) = Texture::texture_flip_and_target(false); let mut texture = Texture::new(target, Size2D::new(self.size.width as usize, self.size.height as usize)); texture.flip = flip; let surface_wrapper = LayersSurfaceWrapper::new(context.get_display(), self.size); surface_wrapper.bind_to_texture(&texture); + unsafe { + debug_assert!(gl::GetError() == gl::NO_ERROR); + } + Some(ColorAttachment::TextureWithSurface(surface_wrapper, texture)) } }; diff --git a/src/tests.rs b/src/tests.rs index af4810a..87e2166 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -8,9 +8,6 @@ use ColorAttachmentType; #[cfg(feature="texture_surface")] use layers::texturegl::Texture; -#[cfg(feature="texture_surface")] -use layers::platform::surface::NativeCompositingGraphicsContext; - #[cfg(target_os="macos")] #[link(name="OpenGL", kind="framework")] extern {} @@ -36,10 +33,8 @@ fn load_gl() { fn test_gl_context(context: &GLContext) { context.make_current().unwrap(); - unsafe { - gl::ClearColor(1.0, 0.0, 0.0, 1.0); - gl::Clear(gl::COLOR_BUFFER_BIT); - } + gl::clear_color(1.0, 0.0, 0.0, 1.0); + gl::clear(gl::COLOR_BUFFER_BIT); let size = context.draw_buffer_size().unwrap(); @@ -49,13 +44,15 @@ fn test_gl_context(context: &GLContext) { test_pixels(&pixels); } -fn test_pixels(pixels: &Vec) { +fn test_pixels(pixels: &[u8]) { + let mut idx = 0; for pixel in pixels.chunks(4) { - println!("{:?}", pixel); + println!("{}: {:?}", idx, pixel); assert!(pixel[0] == 255); assert!(pixel[1] == 0); assert!(pixel[2] == 0); assert!(pixel[3] == 255); + idx += 1; } } @@ -71,19 +68,6 @@ fn test_texture_color_attachment() { test_gl_context(&GLContext::create_offscreen_with_color_attachment(Size2D::new(256, 256), GLContextAttributes::default(), ColorAttachmentType::Texture).unwrap()) } -#[cfg(target_os="linux")] -#[cfg(feature="texture_surface")] -fn get_compositing_context(gl_context: &GLContext) -> NativeCompositingGraphicsContext { - NativeCompositingGraphicsContext::from_display(gl_context.get_metadata().display) -} - -#[cfg(not(target_os="linux"))] -#[cfg(feature="texture_surface")] -fn get_compositing_context(_: &GLContext) -> NativeCompositingGraphicsContext { - NativeCompositingGraphicsContext::new() -} - - #[test] #[cfg(feature="texture_surface")] fn test_texture_surface_color_attachment() { @@ -93,27 +77,36 @@ fn test_texture_surface_color_attachment() { test_gl_context(&ctx); + // Get the bound texture and check we're painting on it + let texture = ctx.borrow_draw_buffer().unwrap().borrow_bound_layers_texture().unwrap(); + + // Bind the texture, get its pixels in rgba format and test + // if it has the surface contents + let _bound = texture.bind(); + + let mut vec = vec![0u8; (size.width * size.height * 4) as usize]; + + unsafe { + gl::GetTexImage(texture.target.as_gl_target(), 0, gl::RGBA as u32, gl::UNSIGNED_BYTE, vec.as_mut_ptr() as *mut _); + assert!(gl::GetError() == gl::NO_ERROR); + } + + test_pixels(&vec); + // Pick up the (in theory) painted surface - // And bind it to a new Texture + // And bind it to a new Texture, to check if we actually painted on it let surface = ctx.borrow_draw_buffer().unwrap().borrow_bound_surface().unwrap(); - let (flip, target) = Texture::texture_flip_and_target(true); + let (flip, target) = Texture::texture_flip_and_target(false); let mut texture = Texture::new(target, Size2D::new(size.width as usize, size.height as usize)); texture.flip = flip; + surface.bind_to_texture(&ctx.get_display(), &texture); - let compositing_context = get_compositing_context(&ctx); - - surface.bind_to_texture(&compositing_context, &texture); - - // Bind the texture, get its pixels in rgba format and test - // if it has the surface contents let _bound = texture.bind(); - let mut vec : Vec = vec![]; - - vec.reserve((size.width * size.height * 4) as usize); + let mut vec = vec![0u8; (size.width * size.height * 4) as usize]; unsafe { - gl::TexImage2D(texture.target.as_gl_target(), 0, gl::RGBA8 as i32, size.width, size.height, 0, gl::RGBA, gl::UNSIGNED_BYTE, vec.as_mut_ptr() as *mut _); - vec.set_len((size.width * size.height * 4) as usize); + gl::GetTexImage(texture.target.as_gl_target(), 0, gl::RGBA as u32, gl::UNSIGNED_BYTE, vec.as_mut_ptr() as *mut _); + assert!(gl::GetError() == gl::NO_ERROR); } test_pixels(&vec); From 4b02652c7f6eb9669184a6a0398d15d0f8746783 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 29 Oct 2015 02:06:13 +0100 Subject: [PATCH 6/6] test: Add test with features to travis --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 464a65a..3795d24 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,3 +22,4 @@ script: - cargo build --features texture_surface --verbose # FIXME(ecoal95): Travis' ubuntu doesn't have the neccessary graphics stack and it fails - if [ "$TRAVIS_OS_NAME" != "linux" ]; then cargo test --verbose; fi + - if [ "$TRAVIS_OS_NAME" != "linux" ]; then cargo test --features texture_surface --verbose; fi