Improve error handling with OpenGL functions

src/buffer.rs

@@ -38,14 +38,6 @@ impl<'a> Buffer<'a> {
                              (vertex_data.len() * mem::size_of::<f32>()) as context::gl::types::GLsizeiptr,
                              vertex_data.as_ptr() as *const _,
                              context::gl::STATIC_DRAW);
-
-            //TODO call this only once
-            // VAOs are not OpenGL ES 2.0 compatible, but are required for rendering with a core context.
-            if cx.gl.BindVertexArray.is_loaded() {
-                let mut vao = mem::uninitialized();
-                cx.gl.GenVertexArrays(1, &mut vao);
-                cx.gl.BindVertexArray(vao);
-            }
         }
 
         Buffer {

src/context.rs

@@ -1,4 +1,6 @@
 use glutin;
+use std::mem;
+use std::ffi::CStr;
 
 pub(crate) mod gl {
     #![allow(unknown_lints)]
@@ -15,7 +17,7 @@ pub struct Context {
 impl ::std::fmt::Debug for Context {
     fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
         let version = unsafe {
-            let data = ::std::ffi::CStr::from_ptr(self.gl.GetString(gl::VERSION) as *const _).to_bytes().to_vec();
+            let data = CStr::from_ptr(self.gl.GetString(gl::VERSION) as *const _).to_bytes().to_vec();
             String::from_utf8(data).unwrap_or_else(|_| "".into())
         };
         write!(f, "Context {{ version: {:?} }}", version)
@@ -31,7 +33,17 @@ macro_rules! check_gl_errors {
 impl Context {
     pub fn from_window(window: &glutin::Window) -> Context {
         let gl = gl::Gl::load_with(|ptr| window.get_proc_address(ptr) as *const _);
-        let cx = Context {gl: gl};
+        let cx = Context { gl: gl };
+
+        // Initialize a vertex array object (VAO) if the current OpenGL context supports it. VAOs are
+        // not OpenGL ES 2.0 compatible, but are required for rendering with a core context.
+        if cx.gl.BindVertexArray.is_loaded() {
+            unsafe {
+                let mut vao = mem::uninitialized();
+                cx.gl.GenVertexArrays(1, &mut vao);
+                cx.gl.BindVertexArray(vao);
+            }
+        }
 
         info!("OpenGL version: {}", cx.gl_version());
         debug!("MAX_TEXTURE_SIZE: {}", cx.max_texture_size());
@@ -41,7 +53,7 @@ impl Context {
 
     pub fn gl_version(&self) -> String {
         unsafe {
-            let data = ::std::ffi::CStr::from_ptr(self.gl.GetString(gl::VERSION) as *const _).to_bytes().to_vec();
+            let data = CStr::from_ptr(self.gl.GetString(gl::VERSION) as *const _).to_bytes().to_vec();
             String::from_utf8(data).unwrap_or_else(|_| "".into())
         }
     }
@@ -54,30 +66,42 @@ impl Context {
         }
     }
 
-    pub unsafe fn check_errors(&self, file: &str, line: u32) {
+    pub fn check_errors(&self, file: &str, line: u32) {
+        let mut fail = false;
+
         loop {
-            match self.gl.GetError() {
+            match unsafe { self.gl.GetError() } {
                 gl::NO_ERROR => break,
                 gl::INVALID_VALUE => {
                     error!("{}:{}, invalid value error", file, line);
+                    fail = true;
                 },
                 gl::INVALID_ENUM => {
                     error!("{}:{}, invalid enum error", file, line);
+                    fail = true;
                 },
                 gl::INVALID_OPERATION => {
                     error!("{}:{}, invalid operation error", file, line);
+                    fail = true;
                 },
                 gl::INVALID_FRAMEBUFFER_OPERATION => {
                     error!("{}:{}, invalid framebuffer operation error", file, line);
+                    fail = true;
                 },
                 gl::OUT_OF_MEMORY => {
                     error!("{}:{}, out of memory error", file, line);
+                    fail = true;
                 },
                 x => {
                     error!("{}:{}, unknown error {}", file, line, x);
+                    fail = true;
                 },
             }
         }
+
+        if fail {
+            panic!("OpenGL error");
+        }
     }
 
     pub fn clear_color(&self, color: (f32, f32, f32, f32)) {
@@ -86,4 +110,15 @@ impl Context {
             self.gl.Clear(gl::COLOR_BUFFER_BIT);
         }
     }
+
+    pub fn set_viewport(&self, x: i32, y: i32, width: u32, height: u32) {
+        unsafe {
+            self.gl.Viewport(
+                x,
+                y,
+                width as gl::types::GLsizei,
+                height as gl::types::GLsizei,
+            );
+        }
+    }
 }

src/map_view_gl.rs

@@ -1,4 +1,3 @@
-use ::context;
 use ::std::ffi::CStr;
 use buffer::{Buffer, DrawMode};
 use context::Context;
@@ -32,76 +31,65 @@ impl<'a> MapViewGl<'a> {
         ) -> MapViewGl
         where F: Fn() + Sync + Send + 'static,
     {
-        unsafe {
-            let mut program = Program::from_paths(cx, "shader/map.vert", "shader/map.frag");
-            check_gl_errors!(cx);
-
-            let tile_size = 256;
-
-            let atlas_size = {
-                let default_size = 2048;
-                let max_size = cx.max_texture_size() as u32;
-                if default_size <= max_size {
-                    default_size
-                } else {
-                    if tile_size * 3 > max_size {
-                        error!("maximal tile size ({}) is too small", max_size);
-                    }
+        let mut program = Program::from_paths(cx, "shader/map.vert", "shader/map.frag").unwrap();
+        check_gl_errors!(cx);
+
+        let tile_size = 256;
 
-                    max_size
+        let atlas_size = {
+            let default_size = 2048;
+            let max_size = cx.max_texture_size() as u32;
+            if default_size <= max_size {
+                default_size
+            } else {
+                if tile_size * 3 > max_size {
+                    error!("maximal tile size ({}) is too small", max_size);
                 }
-            };
 
-            let tex = Texture::empty(cx, atlas_size, atlas_size, TextureFormat::Rgb8);
-            check_gl_errors!(cx);
+                max_size
+            }
+        };
 
-            let buf = Buffer::new(cx, &[], 0);
+        let tex = Texture::empty(cx, atlas_size, atlas_size, TextureFormat::Rgb8);
+        check_gl_errors!(cx);
 
-            check_gl_errors!(cx);
+        let buf = Buffer::new(cx, &[], 0);
+        check_gl_errors!(cx);
 
-            program.add_texture(&tex, CStr::from_bytes_with_nul(b"tex_map\0").unwrap());
-            check_gl_errors!(cx);
+        program.add_texture(&tex, CStr::from_bytes_with_nul(b"tex_map\0").unwrap());
+        check_gl_errors!(cx);
 
-            program.add_attribute(CStr::from_bytes_with_nul(b"position\0").unwrap(), 2, 8, 0);
-            check_gl_errors!(cx);
-            program.add_attribute(CStr::from_bytes_with_nul(b"tex_coord\0").unwrap(), 2, 8, 2);
-            check_gl_errors!(cx);
-            program.add_attribute(CStr::from_bytes_with_nul(b"tex_minmax\0").unwrap(), 4, 8, 4);
-            check_gl_errors!(cx);
+        program.add_attribute(CStr::from_bytes_with_nul(b"position\0").unwrap(), 2, 8, 0);
+        program.add_attribute(CStr::from_bytes_with_nul(b"tex_coord\0").unwrap(), 2, 8, 2);
+        program.add_attribute(CStr::from_bytes_with_nul(b"tex_minmax\0").unwrap(), 4, 8, 4);
+        check_gl_errors!(cx);
 
-            program.before_render();
+        program.before_render();
 
-            let mut map_view = MapView::new(f64::from(initial_size.0), f64::from(initial_size.1), tile_size);
+        let mut map_view = MapView::new(f64::from(initial_size.0), f64::from(initial_size.1), tile_size);
 
-            // set initial zoom
-            {
-                let min_dimension = f64::from(initial_size.0.min(initial_size.1));
-                let zoom = (min_dimension / f64::from(tile_size)).log2().ceil();
-                map_view.set_zoom(zoom);
-            }
+        // set initial zoom
+        {
+            let min_dimension = f64::from(initial_size.0.min(initial_size.1));
+            let zoom = (min_dimension / f64::from(tile_size)).log2().ceil();
+            map_view.set_zoom(zoom);
+        }
 
-            MapViewGl {
-                cx: cx,
-                program: program,
-                buf: buf,
-                viewport_size: initial_size,
-                map_view: map_view,
-                tile_cache: TileCache::new(move |_tile| update_func(), use_network),
-                tile_atlas: TileAtlas::new(tex, 256, use_async),
-            }
+        MapViewGl {
+            cx: cx,
+            program: program,
+            buf: buf,
+            viewport_size: initial_size,
+            map_view: map_view,
+            tile_cache: TileCache::new(move |_tile| update_func(), use_network),
+            tile_atlas: TileAtlas::new(tex, 256, use_async),
         }
     }
 
     pub fn set_viewport_size(&mut self, width: u32, height: u32) {
         self.viewport_size = (width, height);
         self.map_view.set_size(f64::from(width), f64::from(height));
-        unsafe {
-            self.cx.gl.Viewport(
-                0,
-                0,
-                width as context::gl::types::GLsizei,
-                height as context::gl::types::GLsizei);
-        }
+        self.cx.set_viewport(0, 0, width, height);
     }
 
     pub fn increase_atlas_size(&mut self) -> Result<(), ()> {

src/program.rs

@@ -1,5 +1,6 @@
 use ::context;
 use context::Context;
+use std::error::Error;
 use std::ffi::CStr;
 use std::fs::File;
 use std::io::BufReader;
@@ -8,6 +9,7 @@ use std::mem;
 use std::path::Path;
 use texture::{Texture, TextureId};
 
+
 #[derive(Clone, Debug)]
 pub struct Program<'a> {
     cx: &'a ::context::Context,
@@ -24,110 +26,118 @@ pub struct ProgramId {
 }
 
 impl<'a> Program<'a> {
-    pub unsafe fn from_paths<P: AsRef<Path>>(cx: &'a Context, vert_path: P, frag_path: P) -> Program<'a> {
+    pub fn from_paths<P: AsRef<Path>>(cx: &'a Context, vert_path: P, frag_path: P) -> Result<Program<'a>, String> {
         let vert_src = {
-            let file = File::open(&vert_path).unwrap();
+            let file = File::open(&vert_path)
+                .map_err(|e| e.description().to_string())?;
             let mut reader = BufReader::new(file);
             let mut buf: Vec<u8> = vec![];
-            reader.read_to_end(&mut buf).unwrap();
+            reader.read_to_end(&mut buf)
+                .map_err(|e| e.description().to_string())?;
             buf
         };
 
         let frag_src = {
-            let file = File::open(&frag_path).unwrap();
+            let file = File::open(&frag_path)
+                .map_err(|e| e.description().to_string())?;
             let mut reader = BufReader::new(file);
             let mut buf: Vec<u8> = vec![];
-            reader.read_to_end(&mut buf).unwrap();
+            reader.read_to_end(&mut buf)
+                .map_err(|e| e.description().to_string())?;
             buf
         };
 
         Self::new(cx, &vert_src, &frag_src)
     }
 
-    pub unsafe fn new(cx: &'a Context, vert_src: &[u8], frag_src: &[u8]) -> Program<'a> {
-        let vert_obj = {
-            let vert_obj = cx.gl.CreateShader(context::gl::VERTEX_SHADER);
-            let vert_len = vert_src.len() as i32;
-            cx.gl.ShaderSource(
-                vert_obj,
-                1,
-                [vert_src.as_ptr() as *const _].as_ptr(),
-                &vert_len as *const _);
-            cx.gl.CompileShader(vert_obj);
-            check_compile_errors(cx, vert_obj);
-            check_gl_errors!(cx);
-            vert_obj
-        };
-
-        let frag_obj = {
-            let frag_obj = cx.gl.CreateShader(context::gl::FRAGMENT_SHADER);
-            let frag_len = frag_src.len() as i32;
-            cx.gl.ShaderSource(
-                frag_obj,
-                1,
-                [frag_src.as_ptr() as *const _].as_ptr(),
-                &frag_len as *const _);
-            cx.gl.CompileShader(frag_obj);
-            check_compile_errors(cx, frag_obj);
-            check_gl_errors!(cx);
-            frag_obj
-        };
-
-        let program_obj = {
-            let prog = cx.gl.CreateProgram();
-            cx.gl.AttachShader(prog, vert_obj);
-            cx.gl.AttachShader(prog, frag_obj);
-            cx.gl.LinkProgram(prog);
-            check_link_errors(cx, prog);
-
-            cx.gl.UseProgram(prog);
-            check_gl_errors!(cx);
-            prog
-        };
-
-        Program {
-            cx: cx,
-            vert_obj: vert_obj,
-            frag_obj: frag_obj,
-            program_obj: program_obj,
-            tex_ids: vec![],
-            tex_locations: vec![],
+    pub fn new(cx: &'a Context, vert_src: &[u8], frag_src: &[u8]) -> Result<Program<'a>, String> {
+        unsafe {
+            let vert_obj = {
+                let vert_obj = cx.gl.CreateShader(context::gl::VERTEX_SHADER);
+                let vert_len = vert_src.len() as i32;
+                cx.gl.ShaderSource(
+                    vert_obj,
+                    1,
+                    [vert_src.as_ptr() as *const _].as_ptr(),
+                    &vert_len as *const _);
+                cx.gl.CompileShader(vert_obj);
+                check_compile_errors(cx, vert_obj)?;
+                check_gl_errors!(cx);
+                vert_obj
+            };
+
+            let frag_obj = {
+                let frag_obj = cx.gl.CreateShader(context::gl::FRAGMENT_SHADER);
+                let frag_len = frag_src.len() as i32;
+                cx.gl.ShaderSource(
+                    frag_obj,
+                    1,
+                    [frag_src.as_ptr() as *const _].as_ptr(),
+                    &frag_len as *const _);
+                cx.gl.CompileShader(frag_obj);
+                check_compile_errors(cx, frag_obj)?;
+                check_gl_errors!(cx);
+                frag_obj
+            };
+
+            let program_obj = {
+                let prog = cx.gl.CreateProgram();
+                cx.gl.AttachShader(prog, vert_obj);
+                cx.gl.AttachShader(prog, frag_obj);
+                cx.gl.LinkProgram(prog);
+                check_link_errors(cx, prog)?;
+
+                cx.gl.UseProgram(prog);
+                check_gl_errors!(cx);
+                prog
+            };
+
+            Ok(Program {
+                cx: cx,
+                vert_obj: vert_obj,
+                frag_obj: frag_obj,
+                program_obj: program_obj,
+                tex_ids: vec![],
+                tex_locations: vec![],
+            })
         }
     }
 
-    pub unsafe fn add_texture(&mut self, texture: &Texture, uniform_name: &CStr) {
+    pub fn add_texture(&mut self, texture: &Texture, uniform_name: &CStr) {
         //TODO store reference to texture
-        let tex_loc = self.cx.gl.GetUniformLocation(self.program_obj, uniform_name.as_ptr() as *const _);
-        check_gl_errors!(self.cx);
-
-        self.tex_ids.push(texture.id());
-        self.tex_locations.push(tex_loc);
+        unsafe {
+            let tex_loc = self.cx.gl.GetUniformLocation(self.program_obj, uniform_name.as_ptr() as *const _);
+            check_gl_errors!(self.cx);
 
+            self.tex_ids.push(texture.id());
+            self.tex_locations.push(tex_loc);
+        }
     }
 
-    pub unsafe fn add_attribute(&mut self, name: &CStr, number_components: u32, stride: usize, offset: usize) {
-        let attrib_id = self.cx.gl.GetAttribLocation(self.program_obj, name.as_ptr() as *const _);
-        check_gl_errors!(self.cx);
-        self.cx.gl.VertexAttribPointer(
-            attrib_id as u32,
-            number_components as i32, // size
-            context::gl::FLOAT, // type
-            0, // normalized
-            (stride * mem::size_of::<f32>()) as context::gl::types::GLsizei,
-            (offset * mem::size_of::<f32>()) as *const () as *const _);
-        check_gl_errors!(self.cx);
-        self.cx.gl.EnableVertexAttribArray(attrib_id as u32);
+    pub fn add_attribute(&mut self, name: &CStr, number_components: u32, stride: usize, offset: usize) {
+        unsafe {
+            let attrib_id = self.cx.gl.GetAttribLocation(self.program_obj, name.as_ptr() as *const _);
+            self.cx.gl.VertexAttribPointer(
+                attrib_id as u32,
+                number_components as i32, // size
+                context::gl::FLOAT, // type
+                0, // normalized
+                (stride * mem::size_of::<f32>()) as context::gl::types::GLsizei,
+                (offset * mem::size_of::<f32>()) as *const () as *const _);
+            self.cx.gl.EnableVertexAttribArray(attrib_id as u32);
+        }
         check_gl_errors!(self.cx);
     }
 
-    pub unsafe fn before_render(&self) {
-        check_gl_errors!(self.cx);
-        //self.cx.gl.UseProgram(self.program_obj);
-        //TODO check max texture number
-        for (i, (tex_id, &tex_loc)) in self.tex_ids.iter().zip(&self.tex_locations).enumerate() {
-            self.cx.gl.ActiveTexture(context::gl::TEXTURE0 + i as u32);
-            self.cx.gl.BindTexture(context::gl::TEXTURE_2D, tex_id.id);
-            self.cx.gl.Uniform1i(tex_loc, i as i32);
+    pub fn before_render(&self) {
+        unsafe {
+            //self.cx.gl.UseProgram(self.program_obj);
+            //TODO check max texture number
+            for (i, (tex_id, &tex_loc)) in self.tex_ids.iter().zip(&self.tex_locations).enumerate() {
+                self.cx.gl.ActiveTexture(context::gl::TEXTURE0 + i as u32);
+                self.cx.gl.BindTexture(context::gl::TEXTURE_2D, tex_id.id);
+                self.cx.gl.Uniform1i(tex_loc, i as i32);
+            }
         }
     }
 
@@ -138,66 +148,51 @@ impl<'a> Program<'a> {
     }
 }
 
-unsafe fn check_link_errors(cx: &Context, program_obj: u32)
-{
-    let mut link_success: i32 = mem::uninitialized();
-
-    cx.gl.GetProgramiv(program_obj, context::gl::LINK_STATUS, &mut link_success);
-
-    if link_success == 0 {
-
-        match cx.gl.GetError() {
-            context::gl::NO_ERROR => (),
-            context::gl::INVALID_VALUE => {
-                error!("invalid value");
-                return;
-            },
-            context::gl::INVALID_OPERATION => {
-                error!("invalid operation");
-                return;
-            },
-            _ => {
-                error!("unknown error");
-                return;
-            }
-        };
-
-        let mut error_log_size: i32 = mem::uninitialized();
+fn check_link_errors(cx: &Context, program_obj: u32) -> Result<(), String> {
+    unsafe {
+        let mut link_success: i32 = mem::uninitialized();
 
-        cx.gl.GetProgramiv(program_obj, context::gl::INFO_LOG_LENGTH, &mut error_log_size);
+        cx.gl.GetProgramiv(program_obj, context::gl::LINK_STATUS, &mut link_success);
 
-        let mut error_log: Vec<u8> = Vec::with_capacity(error_log_size as usize);
+        if link_success == 0 {
+            let mut error_log_size: i32 = mem::uninitialized();
+            cx.gl.GetProgramiv(program_obj, context::gl::INFO_LOG_LENGTH, &mut error_log_size);
 
-        cx.gl.GetProgramInfoLog(program_obj, error_log_size, &mut error_log_size,
-                             error_log.as_mut_ptr() as *mut context::gl::types::GLchar);
+            let mut error_log: Vec<u8> = Vec::with_capacity(error_log_size as usize);
+            cx.gl.GetProgramInfoLog(program_obj, error_log_size, &mut error_log_size,
+                                    error_log.as_mut_ptr() as *mut context::gl::types::GLchar);
 
-        error_log.set_len(error_log_size as usize);
+            error_log.set_len(error_log_size as usize);
 
-        let msg = String::from_utf8(error_log).unwrap();
-        error!("{}", msg);
+            Err(String::from_utf8_lossy(&error_log).into())
+        } else {
+            Ok(())
+        }
     }
 }
 
-unsafe fn check_compile_errors(cx: &Context, shader_obj: u32) {
-    // checking compilation success by reading a flag on the shader
-    let compilation_success = {
-        let mut compilation_success: i32 = mem::uninitialized();
-        cx.gl.GetShaderiv(shader_obj, context::gl::COMPILE_STATUS, &mut compilation_success);
-        compilation_success
-    };
-
-    if compilation_success != 1 {
-        // compilation error
-        let mut error_log_size: i32 = mem::uninitialized();
-        cx.gl.GetShaderiv(shader_obj, context::gl::INFO_LOG_LENGTH, &mut error_log_size);
-        let mut error_log: Vec<u8> = Vec::with_capacity(error_log_size as usize);
-
-        cx.gl.GetShaderInfoLog(shader_obj, error_log_size, &mut error_log_size,
-                                 error_log.as_mut_ptr() as *mut _);
-        error_log.set_len(error_log_size as usize);
-
-        if let Ok(msg) = String::from_utf8(error_log) {
-            error!("{}", msg);
+fn check_compile_errors(cx: &Context, shader_obj: u32) -> Result<(), String> {
+    unsafe {
+        // checking compilation success by reading a flag on the shader
+        let compilation_success = {
+            let mut compilation_success: i32 = mem::uninitialized();
+            cx.gl.GetShaderiv(shader_obj, context::gl::COMPILE_STATUS, &mut compilation_success);
+            compilation_success
+        };
+
+        if compilation_success != 1 {
+            // compilation error
+            let mut error_log_size: i32 = mem::uninitialized();
+            cx.gl.GetShaderiv(shader_obj, context::gl::INFO_LOG_LENGTH, &mut error_log_size);
+            let mut error_log: Vec<u8> = Vec::with_capacity(error_log_size as usize);
+
+            cx.gl.GetShaderInfoLog(shader_obj, error_log_size, &mut error_log_size,
+                                     error_log.as_mut_ptr() as *mut _);
+            error_log.set_len(error_log_size as usize);
+
+            Err(String::from_utf8_lossy(&error_log).into())
+        } else {
+            Ok(())
         }
     }
 }