Several WGL cleanups

src/api/wgl/mod.rs

@@ -24,6 +24,9 @@ use gdi32;
 mod make_current_guard;
 mod gl;
 
+/// A WGL context.
+///
+/// Note: should be destroyed before its window.
 pub struct Context {
     context: ContextWrapper,
 
@@ -50,6 +53,7 @@ impl Drop for WindowWrapper {
     }
 }
 
+/// Wraps around a context so that it is destroyed when necessary.
 struct ContextWrapper(winapi::HGLRC);
 
 impl Drop for ContextWrapper {
@@ -64,11 +68,14 @@ impl Context {
     /// Attempt to build a new WGL context on a window.
     ///
     /// The window must **not** have had `SetPixelFormat` called on it.
-    pub fn new(builder: &BuilderAttribs<'static>, window: winapi::HWND,
+    ///
+    /// # Unsafety
+    ///
+    /// The `window` must continue to exist as long as the resulting `Context` exists.
+    pub unsafe fn new(builder: &BuilderAttribs<'static>, window: winapi::HWND,
                       builder_sharelists: Option<winapi::HGLRC>)
                       -> Result<Context, CreationError>
     {
-        unsafe {
         let hdc = user32::GetDC(window);
         if hdc.is_null() {
             let err = Err(CreationError::OsError(format!("GetDC function failed: {}",
@@ -76,76 +83,30 @@ impl Context {
             return err;
         }
 
-            // the first step is to create a dummy window and a dummy context which we will use
+        // loading the functions that are not guaranteed to be supported
-            // to load the pointers to some functions in the OpenGL driver in `extra_functions`
+        let extra_functions = try!(load_extra_functions(window));
-            let extra_functions = {
-                let (ex_style, style) = (winapi::WS_EX_APPWINDOW, winapi::WS_POPUP |
-                                         winapi::WS_CLIPSIBLINGS | winapi::WS_CLIPCHILDREN);
 
-                // creating a dummy invisible window
+        // getting the list of the supported extensions
-                let dummy_window = {
+        let extensions = if extra_functions.GetExtensionsStringARB.is_loaded() {
-                    let rect = {
+            let data = extra_functions.GetExtensionsStringARB(hdc as *const _);
-                        let mut placement: winapi::WINDOWPLACEMENT = unsafe { mem::zeroed() };
+            let data = CStr::from_ptr(data).to_bytes().to_vec();
-                        placement.length = mem::size_of::<winapi::WINDOWPLACEMENT>() as winapi::UINT;
+            String::from_utf8(data).unwrap()
-                        if unsafe { user32::GetWindowPlacement(window, &mut placement) } == 0 {
-                            panic!();
-                        }
-                        placement.rcNormalPosition
-                    };
 
-                    let mut class_name = [0u16; 128];
+        } else if extra_functions.GetExtensionsStringEXT.is_loaded() {
-                    if user32::GetClassNameW(window, class_name.as_mut_ptr(), 128) == 0 {
+            let data = extra_functions.GetExtensionsStringEXT();
-                        return Err(CreationError::OsError(format!("GetClassNameW function failed: {}",
+            let data = CStr::from_ptr(data).to_bytes().to_vec();
-                                                          format!("{}", io::Error::last_os_error()))));
+            String::from_utf8(data).unwrap()
-                    }
 
-                    let handle = user32::CreateWindowExW(ex_style, class_name.as_ptr(),
+        } else {
-                                                         b"dummy window\0".as_ptr() as *const _, style,
+            format!("")
-                                                         winapi::CW_USEDEFAULT, winapi::CW_USEDEFAULT,
-                                                         rect.right - rect.left, rect.bottom - rect.top,
-                                                         ptr::null_mut(), ptr::null_mut(),
-                                                         kernel32::GetModuleHandleW(ptr::null()),
-                                                         ptr::null_mut());
-
-                    if handle.is_null() {
-                        return Err(CreationError::OsError(format!("CreateWindowEx function failed: {}",
-                                                          format!("{}", io::Error::last_os_error()))));
-                    }
-
-                    let hdc = user32::GetDC(handle);
-                    if hdc.is_null() {
-                        let err = Err(CreationError::OsError(format!("GetDC function failed: {}",
-                                                           format!("{}", io::Error::last_os_error()))));
-                        return err;
-                    }
-
-                    WindowWrapper(handle, hdc)
-                };
-
-                // getting the pixel format that we will use and setting it
-                {
-                    let formats = enumerate_native_pixel_formats(dummy_window.1);
-                    let id = try!(choose_dummy_pixel_format(formats.into_iter()));
-                    try!(set_pixel_format(dummy_window.1, id));
-                }
-
-                // creating the dummy OpenGL context and making it current
-                let dummy_context = try!(create_context(None, dummy_window.0, dummy_window.1, None));
-                let _current_context = try!(CurrentContextGuard::make_current(dummy_window.1,
-                                                                              dummy_context.0));
-
-                // loading the extra WGL functions
-                gl::wgl_extra::Wgl::load_with(|addr| {
-                    let addr = CString::new(addr.as_bytes()).unwrap();
-                    let addr = addr.as_ptr();
-                    gl::wgl::GetProcAddress(addr) as *const libc::c_void
-                })
         };
 
         // calling SetPixelFormat
         let pixel_format = {
-                let formats = if extra_functions.GetPixelFormatAttribivARB.is_loaded() {
+            let formats = if extensions.split(' ').find(|&i| i == "WGL_ARB_pixel_format")
-                    let f = enumerate_arb_pixel_formats(&extra_functions, hdc);
+                                                  .is_some()
+            {
+                let f = enumerate_arb_pixel_formats(&extra_functions, &extensions, hdc);
                 if f.is_empty() {
                     enumerate_native_pixel_formats(hdc)
                 } else {
@@ -155,20 +116,22 @@ impl Context {
                 enumerate_native_pixel_formats(hdc)
             };
 
-                let (id, f) = try!(builder.choose_pixel_format(formats.into_iter().map(|(a, b)| (b, a))));
+            let (id, f) = try!(builder.choose_pixel_format(formats));
             try!(set_pixel_format(hdc, id));
             f
         };
 
         // creating the OpenGL context
-            let context = try!(create_context(Some((&extra_functions, builder)), window, hdc,
+        let context = try!(create_context(Some((&extra_functions, builder, &extensions)),
-                                              builder_sharelists));
+                                          window, hdc, builder_sharelists));
 
         // loading the opengl32 module
         let gl_library = try!(load_opengl32_dll());
 
         // handling vsync
         if builder.vsync {
+            // contrary to most extensions, it is permitted to discover the presence of
+            // `WGL_EXT_swap_control` by seeing if the function pointer is available
             if extra_functions.SwapIntervalEXT.is_loaded() {
                 let _guard = try!(CurrentContextGuard::make_current(hdc, context.0));
 
@@ -185,7 +148,6 @@ impl Context {
             pixel_format: pixel_format,
         })
     }
-    }
 
     /// Returns the raw HGLRC.
     pub fn get_hglrc(&self) -> winapi::HGLRC {
@@ -233,14 +195,21 @@ impl GlContext for Context {
 unsafe impl Send for Context {}
 unsafe impl Sync for Context {}
 
-unsafe fn create_context(extra: Option<(&gl::wgl_extra::Wgl, &BuilderAttribs<'static>)>,
+/// Creates an OpenGL context.
+///
+/// If `extra` is `Some`, this function will attempt to use the latest WGL functions to create the
+/// context.
+///
+/// Otherwise, only the basic API will be used and the chances of `CreationError::NotSupported`
+/// being returned increase.
+unsafe fn create_context(extra: Option<(&gl::wgl_extra::Wgl, &BuilderAttribs<'static>, &str)>,
                          _: winapi::HWND, hdc: winapi::HDC, share: Option<winapi::HGLRC>)
                          -> Result<ContextWrapper, CreationError>
 {
     let share = share.unwrap_or(ptr::null_mut());
 
-    let ctxt = if let Some((extra_functions, builder)) = extra {
+    let ctxt = if let Some((extra_functions, builder, extensions)) = extra {
-        if extra_functions.CreateContextAttribsARB.is_loaded() {
+        if extensions.split(' ').find(|&i| i == "WGL_ARB_create_context").is_some() {
             let mut attributes = Vec::new();
 
             match builder.gl_version {
@@ -252,8 +221,8 @@ unsafe fn create_context(extra: Option<(&gl::wgl_extra::Wgl, &BuilderAttribs<'st
                     attributes.push(minor as libc::c_int);
                 },
                 GlRequest::Specific(Api::OpenGlEs, (major, minor)) => {
-                    if is_extension_supported(extra_functions, hdc,
+                    if extensions.split(' ').find(|&i| i == "WGL_EXT_create_context_es2_profile")
-                                              "WGL_EXT_create_context_es2_profile")
+                                            .is_some()
                     {
                         attributes.push(gl::wgl_extra::CONTEXT_PROFILE_MASK_ARB as libc::c_int);
                         attributes.push(gl::wgl_extra::CONTEXT_ES2_PROFILE_BIT_EXT as libc::c_int);
@@ -276,8 +245,7 @@ unsafe fn create_context(extra: Option<(&gl::wgl_extra::Wgl, &BuilderAttribs<'st
             }
 
             if let Some(profile) = builder.gl_profile {
-                if is_extension_supported(extra_functions, hdc,
+                if extensions.split(' ').find(|&i| i == "WGL_ARB_create_context_profile").is_some()
-                                          "WGL_ARB_create_context_profile")
                 {
                     let flag = match profile {
                         GlProfile::Compatibility =>
@@ -329,7 +297,10 @@ unsafe fn create_context(extra: Option<(&gl::wgl_extra::Wgl, &BuilderAttribs<'st
     Ok(ContextWrapper(ctxt as winapi::HGLRC))
 }
 
-unsafe fn enumerate_native_pixel_formats(hdc: winapi::HDC) -> Vec<(PixelFormat, libc::c_int)> {
+/// Enumerates the list of pixel formats without using WGL.
+///
+/// Gives less precise results than `enumerate_arb_pixel_formats`.
+unsafe fn enumerate_native_pixel_formats(hdc: winapi::HDC) -> Vec<(libc::c_int, PixelFormat)> {
     let size_of_pxfmtdescr = mem::size_of::<winapi::PIXELFORMATDESCRIPTOR>() as u32;
     let num = gdi32::DescribePixelFormat(hdc, 1, size_of_pxfmtdescr, ptr::null_mut());
 
@@ -354,7 +325,7 @@ unsafe fn enumerate_native_pixel_formats(hdc: winapi::HDC) -> Vec<(PixelFormat,
             continue;
         }
 
-        result.push((PixelFormat {
+        result.push((index, PixelFormat {
             hardware_accelerated: (output.dwFlags & winapi::PFD_GENERIC_FORMAT) == 0,
             color_bits: output.cRedBits + output.cGreenBits + output.cBlueBits,
             alpha_bits: output.cAlphaBits,
@@ -364,14 +335,17 @@ unsafe fn enumerate_native_pixel_formats(hdc: winapi::HDC) -> Vec<(PixelFormat,
             double_buffer: (output.dwFlags & winapi::PFD_DOUBLEBUFFER) != 0,
             multisampling: None,
             srgb: false,
-        }, index));
+        }));
     }
 
     result
 }
 
-unsafe fn enumerate_arb_pixel_formats(extra: &gl::wgl_extra::Wgl, hdc: winapi::HDC)
+/// Enumerates the list of pixel formats by using extra WGL functions.
-                                      -> Vec<(PixelFormat, libc::c_int)>
+///
+/// Gives more precise results than `enumerate_native_pixel_formats`.
+unsafe fn enumerate_arb_pixel_formats(extra: &gl::wgl_extra::Wgl, extensions: &str,
+                                      hdc: winapi::HDC) -> Vec<(libc::c_int, PixelFormat)>
 {
     let get_info = |index: u32, attrib: u32| {
         let mut value = mem::uninitialized();
@@ -403,7 +377,7 @@ unsafe fn enumerate_arb_pixel_formats(extra: &gl::wgl_extra::Wgl, hdc: winapi::H
             continue;
         }
 
-        result.push((PixelFormat {
+        result.push((index as libc::c_int, PixelFormat {
             hardware_accelerated: true,
             color_bits: get_info(index, gl::wgl_extra::RED_BITS_ARB) as u8 + 
                         get_info(index, gl::wgl_extra::GREEN_BITS_ARB) as u8 +
@@ -414,7 +388,7 @@ unsafe fn enumerate_arb_pixel_formats(extra: &gl::wgl_extra::Wgl, hdc: winapi::H
             stereoscopy: get_info(index, gl::wgl_extra::STEREO_ARB) != 0,
             double_buffer: get_info(index, gl::wgl_extra::DOUBLE_BUFFER_ARB) != 0,
             multisampling: {
-                if is_extension_supported(extra, hdc, "WGL_ARB_multisample") {
+                if extensions.split(' ').find(|&i| i == "WGL_ARB_multisample").is_some() {
                     match get_info(index, gl::wgl_extra::SAMPLES_ARB) {
                         0 => None,
                         a => Some(a as u16),
@@ -423,19 +397,20 @@ unsafe fn enumerate_arb_pixel_formats(extra: &gl::wgl_extra::Wgl, hdc: winapi::H
                     None
                 }
             },
-            srgb: if is_extension_supported(extra, hdc, "WGL_ARB_framebuffer_sRGB") {
+            srgb: if extensions.split(' ').find(|&i| i == "WGL_ARB_framebuffer_sRGB").is_some() {
                 get_info(index, gl::wgl_extra::FRAMEBUFFER_SRGB_CAPABLE_ARB) != 0
-            } else if is_extension_supported(extra, hdc, "WGL_EXT_framebuffer_sRGB") {
+            } else if extensions.split(' ').find(|&i| i == "WGL_EXT_framebuffer_sRGB").is_some() {
                 get_info(index, gl::wgl_extra::FRAMEBUFFER_SRGB_CAPABLE_EXT) != 0
             } else {
                 false
             },
-        }, index as libc::c_int));
+        }));
     }
 
     result
 }
 
+/// Calls `SetPixelFormat` on a window.
 unsafe fn set_pixel_format(hdc: winapi::HDC, id: libc::c_int) -> Result<(), CreationError> {
     let mut output: winapi::PIXELFORMATDESCRIPTOR = mem::zeroed();
 
@@ -454,6 +429,7 @@ unsafe fn set_pixel_format(hdc: winapi::HDC, id: libc::c_int) -> Result<(), Crea
     Ok(())
 }
 
+/// Loads the `opengl32.dll` library.
 unsafe fn load_opengl32_dll() -> Result<winapi::HMODULE, CreationError> {
     let name = OsStr::new("opengl32.dll").encode_wide().chain(Some(0).into_iter())
                                          .collect::<Vec<_>>();
@@ -468,32 +444,86 @@ unsafe fn load_opengl32_dll() -> Result<winapi::HMODULE, CreationError> {
     Ok(lib)
 }
 
-unsafe fn is_extension_supported(extra: &gl::wgl_extra::Wgl, hdc: winapi::HDC,
+/// Loads the WGL functions that are not guaranteed to be supported.
-                                 extension: &str) -> bool
+///
-{
+/// The `window` must be passed because the driver can vary depending on the window's
-    let extensions = if extra.GetExtensionsStringARB.is_loaded() {
+/// characteristics.
-        let data = extra.GetExtensionsStringARB(hdc as *const _);
+unsafe fn load_extra_functions(window: winapi::HWND) -> Result<gl::wgl_extra::Wgl, CreationError> {
-        let data = CStr::from_ptr(data).to_bytes().to_vec();
+    let (ex_style, style) = (winapi::WS_EX_APPWINDOW, winapi::WS_POPUP |
-        String::from_utf8(data).unwrap()
+                             winapi::WS_CLIPSIBLINGS | winapi::WS_CLIPCHILDREN);
 
-    } else if extra.GetExtensionsStringEXT.is_loaded() {
+    // creating a dummy invisible window
-        let data = extra.GetExtensionsStringEXT();
+    let dummy_window = {
-        let data = CStr::from_ptr(data).to_bytes().to_vec();
+        // getting the rect of the real window
-        String::from_utf8(data).unwrap()
+        let rect = {
-
+            let mut placement: winapi::WINDOWPLACEMENT = mem::zeroed();
-    } else {
+            placement.length = mem::size_of::<winapi::WINDOWPLACEMENT>() as winapi::UINT;
-        return false;
+            if user32::GetWindowPlacement(window, &mut placement) == 0 {
+                panic!();
+            }
+            placement.rcNormalPosition
         };
 
-    extensions.split(" ").find(|&e| e == extension).is_some()
+        // getting the class name of the real window
+        let mut class_name = [0u16; 128];
+        if user32::GetClassNameW(window, class_name.as_mut_ptr(), 128) == 0 {
+            return Err(CreationError::OsError(format!("GetClassNameW function failed: {}",
+                                              format!("{}", io::Error::last_os_error()))));
+        }
+
+        // this dummy window should match the real one enough to get the same OpenGL driver
+        let win = user32::CreateWindowExW(ex_style, class_name.as_ptr(),
+                                          b"dummy window\0".as_ptr() as *const _, style,
+                                          winapi::CW_USEDEFAULT, winapi::CW_USEDEFAULT,
+                                          rect.right - rect.left,
+                                          rect.bottom - rect.top,
+                                          ptr::null_mut(), ptr::null_mut(),
+                                          kernel32::GetModuleHandleW(ptr::null()),
+                                          ptr::null_mut());
+
+        if win.is_null() {
+            return Err(CreationError::OsError(format!("CreateWindowEx function failed: {}",
+                                              format!("{}", io::Error::last_os_error()))));
+        }
+
+        let hdc = user32::GetDC(win);
+        if hdc.is_null() {
+            let err = Err(CreationError::OsError(format!("GetDC function failed: {}",
+                                               format!("{}", io::Error::last_os_error()))));
+            return err;
+        }
+
+        WindowWrapper(win, hdc)
+    };
+
+    // getting the pixel format that we will use and setting it
+    {
+        let formats = enumerate_native_pixel_formats(dummy_window.1);
+        let id = try!(choose_dummy_pixel_format(formats.into_iter()));
+        try!(set_pixel_format(dummy_window.1, id));
+    }
+
+    // creating the dummy OpenGL context and making it current
+    let dummy_context = try!(create_context(None, dummy_window.0, dummy_window.1, None));
+    let _current_context = try!(CurrentContextGuard::make_current(dummy_window.1,
+                                                                  dummy_context.0));
+
+    // loading the extra WGL functions
+    Ok(gl::wgl_extra::Wgl::load_with(|addr| {
+        let addr = CString::new(addr.as_bytes()).unwrap();
+        let addr = addr.as_ptr();
+        gl::wgl::GetProcAddress(addr) as *const libc::c_void
+    }))
 }
 
+/// Given a list of pixel formats, this function chooses one that is likely to be provided by
+/// the main video driver of the system.
 fn choose_dummy_pixel_format<I>(iter: I) -> Result<libc::c_int, CreationError>
-                                where I: Iterator<Item=(PixelFormat, libc::c_int)>
+                                where I: Iterator<Item=(libc::c_int, PixelFormat)>
 {
     let mut backup_id = None;
 
-    for (format, id) in iter {
+    for (id, format) in iter {
         if backup_id.is_none() {
             backup_id = Some(id);
         }

src/lib.rs

@@ -352,7 +352,7 @@ impl<'a> BuilderAttribs<'a> {
     }
 
     fn choose_pixel_format<T, I>(&self, iter: I) -> Result<(T, PixelFormat), CreationError>
-                                 where I: Iterator<Item=(T, PixelFormat)>, T: Clone
+                                 where I: IntoIterator<Item=(T, PixelFormat)>, T: Clone
     {
         let mut current_result = None;
         let mut current_software_result = None;