Re: [waffle] [PATCH 04/10] cmake: add autodetection for waffle_has_egl, glx...
On Sat, May 31, 2014 at 03:22:02AM +0100, Emil Velikov wrote: Silence the pkg_check_modules and check set the default options depending on the packages found. This is a good idea and will make Waffle easier to configure for everyone. There are a few problems though. If configuration fails because the user tried to enable a platform for which his system lacks the dependencies, then post-patch CMake no longer gives sufficient explanation why it failed. For example, if -Dwaffle_has_gbm=1 fails, CMake informs the user thta gbm requirements are not met but does explain what those requirements are. And, since waffle_pkg_config is now silenced by QUIET, CMake's earlier output provides no clue. To avoid providing the user a silent mystery, this block +if(waffle_has_wayland) +if(NOT wayland-client_FOUND OR + NOT wayland-egl_FOUND OR NOT egl_FOUND) +message(FATAL_ERROR wayland requirements are not met.) +endif() +endif() should explain which requirements, including the version if any, were not met. For example, if -Dwaffle_has_wayland=1 but the system lacks wayland-egl=9.1, then CMake should print something like wayland dependency is missing: wayland-egl=9.1 or wayland requirement wayland-egl=9.1 is not met Why did you decide to silece waffle_pkg_config with QUIET? I feel that it's useful to get notification for each item that CMake probes for. ___ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle
Re: [waffle] [PATCH 05/10] waffle: remove found_platform from waffle_init_parse_attrib_list
On Sat, May 31, 2014 at 03:22:03AM +0100, Emil Velikov wrote: Whenever a platform is missing a case statement, the default will kick in throwing an error and exiting the function. Ah, but that's not what 'found_platform' is checking for... -if (!found_platform) { -wcore_errorf(WAFFLE_ERROR_BAD_ATTRIBUTE, - attribute list is missing WAFFLE_PLATFORM); -return false; -} ... waffle_init() uses 'found_platform' to check if the user provided WAFFLE_PLATFORM. I admit that waffle_init_parse_attrib_list() is a clumsily written function. It was one of the very first functions in Waffle's codebase. ___ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle
Re: [waffle] [PATCH 08/10] cgl: avoid leaking the PixelFormat
On Sat, May 31, 2014 at 03:22:06AM +0100, Emil Velikov wrote: According to apple developer page, starting with OS X v10.5 the pixelformat is reference counted. The object is created at CGLChoosePixelFormat and should be unrefeferenced via CGLReleasePixelFormat/CGLDestroyPixelFormat. The latter two are identical accoring to the documentation. Signed-off-by: Emil Velikov emil.l.veli...@gmail.com --- src/waffle/cgl/cgl_config.m | 4 1 file changed, 4 insertions(+) All CGL patches in this series look good to me. However, I'll defer committing them until I've tested them on my MacBook. ___ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle
Re: [waffle] [PATCH 09/10] x11: correctly set the screen number of the current display
On Sat, May 31, 2014 at 03:22:07AM +0100, Emil Velikov wrote: Signed-off-by: Emil Velikov emil.l.veli...@gmail.com --- src/waffle/x11/x11_display.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/waffle/x11/x11_display.c b/src/waffle/x11/x11_display.c index 43b6ef0..8a3524d 100644 --- a/src/waffle/x11/x11_display.c +++ b/src/waffle/x11/x11_display.c @@ -48,8 +48,7 @@ x11_display_init(struct x11_display *self, const char *name) return false; } -// FIXME: Don't assume screen is 0. -self-screen = 0; +self-screen = DefaultScreen(self-xlib); return true; } Reviewed-by: Chad Versace chad.vers...@linux.intel.com ___ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle
Re: [waffle] [PATCH 10/10] glx: glx_context_create_native returns GLXContext not bool
On Sat, May 31, 2014 at 03:22:08AM +0100, Emil Velikov wrote: Signed-off-by: Emil Velikov emil.l.veli...@gmail.com --- src/waffle/glx/glx_context.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/waffle/glx/glx_context.c b/src/waffle/glx/glx_context.c index c7a7d91..62573dc 100644 --- a/src/waffle/glx/glx_context.c +++ b/src/waffle/glx/glx_context.c @@ -167,7 +167,7 @@ glx_context_create_native(struct glx_config *config, ok = glx_context_fill_attrib_list(config, attrib_list); if (!ok) -return false; +return NULL; ctx = wrapped_glXCreateContextAttribsARB(platform, dpy-x11.xlib, -- Reviewed-by: Chad Versace chad.vers...@linux.intel.com ___ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle
Re: [waffle] [PATCH 0/3] Memory leak fixes
On Sun, Jun 01, 2014 at 03:11:59PM +0100, Emil Velikov wrote: Spotted while searching for a severe memory corruption under wgl. Did not manage to find it yet, although notices these bad boys. Awesome. Patch 1 is rb'd and committed to master. I will case the corruption bug over the next few days + cleanupsplit the patches hopefully I will see what's going wrong, or perhaps someone will be kind enough to point out what I'm doing wrong :) AFAICS the user is supposed to explicitly release the current context using waffle_make_current(dry, NULL, NULL), is that correct ? I'll get back to you on that question tomorrow. I don't trust my judgement this late into the night ;) ___ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle
Re: [waffle] [GSOC2014] Current design/implementation + the infamous corruption
On Thu, Jun 05, 2014 at 05:49:04PM +0100, Emil Velikov wrote: On 05/06/14 03:19, Emil Velikov wrote: waffle_window_create(config, w, h) + waffle_window_resize() // Reuse the window... and boom. Any suggestions on // how to resolve this, or should we mandate that // this is not a valid usage of waffle ? There is no use case in piglit + waffle{examples/utils} where one would create multiple windows for a single config. I might just add a lovely assert and don't worry about this too much. Don't worry too much about getting mutliple windows to work. Last I checked, multiple window support was broken in the Android backend too. It's perfectly ok to aim for imperfect good-enough during Waffle's initial Windows support. ___ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle
Re: [waffle] [GSOC2014] Current design/implementation + the infamous corruption
On Fri, June 6, 2014 9:45 am, Chad Versace wrote: On Thu, Jun 05, 2014 at 05:49:04PM +0100, Emil Velikov wrote: On 05/06/14 03:19, Emil Velikov wrote: waffle_window_create(config, w, h) + waffle_window_resize() // Reuse the window... and boom. Any suggestions on // how to resolve this, or should we mandate that // this is not a valid usage of waffle ? There is no use case in piglit + waffle{examples/utils} where one would create multiple windows for a single config. I might just add a lovely assert and don't worry about this too much. Don't worry too much about getting mutliple windows to work. Last I checked, multiple window support was broken in the Android backend too. It's perfectly ok to aim for imperfect good-enough during Waffle's initial Windows support. When making Android version of Waffle on top of Ice Cream Sandwich I saw process becoming unstable when asking for second surface control object (read: window) within same process. The problem was on Android SurfaceFlinger side thus I decided just to ignore the problem of multiple windows. On Android Waffle does what is asked from it and if the process segfaults because of this I left it up to the programmer to figure how to behave on chosen OS. Though, asserting this might be good idea to highlight what went wrong. /Juha-Pekka ___ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle
Re: [waffle] [PATCH 05/10] waffle: remove found_platform from waffle_init_parse_attrib_list
On 06/06/14 07:25, Chad Versace wrote: On Sat, May 31, 2014 at 03:22:03AM +0100, Emil Velikov wrote: Whenever a platform is missing a case statement, the default will kick in throwing an error and exiting the function. Ah, but that's not what 'found_platform' is checking for... -if (!found_platform) { -wcore_errorf(WAFFLE_ERROR_BAD_ATTRIBUTE, - attribute list is missing WAFFLE_PLATFORM); -return false; -} ... waffle_init() uses 'found_platform' to check if the user provided WAFFLE_PLATFORM. AFAICS that is handled by the default switch case. I admit that waffle_init_parse_attrib_list() is a clumsily written function. It was one of the very first functions in Waffle's codebase. IMHO the code bails out if we've missed (partially or fully) any platform and/or if the user has provided a invalid WAFFLE_PLATFORM. Thus we will reach the if (!found_platform)... only when the variable is already set (via the CASE_DEFINED_PLATFORM macro). Perhaps this code is targeting some elaborate use-case which I'm failing to see here? -Emil ___ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle
Re: [waffle] [PATCH 04/10] cmake: add autodetection for waffle_has_egl, glx...
On 06/06/14 07:18, Chad Versace wrote: On Sat, May 31, 2014 at 03:22:02AM +0100, Emil Velikov wrote: Silence the pkg_check_modules and check set the default options depending on the packages found. This is a good idea and will make Waffle easier to configure for everyone. There are a few problems though. If configuration fails because the user tried to enable a platform for which his system lacks the dependencies, then post-patch CMake no longer gives sufficient explanation why it failed. For example, if -Dwaffle_has_gbm=1 fails, CMake informs the user thta gbm requirements are not met but does explain what those requirements are. And, since waffle_pkg_config is now silenced by QUIET, CMake's earlier output provides no clue. To avoid providing the user a silent mystery, this block +if(waffle_has_wayland) +if(NOT wayland-client_FOUND OR + NOT wayland-egl_FOUND OR NOT egl_FOUND) +message(FATAL_ERROR wayland requirements are not met.) +endif() +endif() should explain which requirements, including the version if any, were not met. For example, if -Dwaffle_has_wayland=1 but the system lacks wayland-egl=9.1, then CMake should print something like wayland dependency is missing: wayland-egl=9.1 or wayland requirement wayland-egl=9.1 is not met This would add a few extra lines, although it would be very nice to have. Why did you decide to silece waffle_pkg_config with QUIET? I feel that it's useful to get notification for each item that CMake probes for. Was under the impression that when REQUIRED|QUIET is missing cmake defaults to the former, which errors out when the package is missing. Seems like I got confused with another cmake command. Updated patch will follow shortly. -Emil ___ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle
Re: [waffle] [PATCH 01/10] pkg/archlinux: Update to a dual (+multilib) package
On 06/06/14 06:55, Chad Versace wrote: On Sat, May 31, 2014 at 03:21:59AM +0100, Emil Velikov wrote: Include both 64bit and multilib binaries when building on x86-64 platform. This saves us deeping track of version numbers and interdependencies in case of a split package. v2: Rebase and bump pkgrel Signed-off-by: Emil Velikov emil.l.veli...@gmail.com --- pkg/archlinux/waffle-1.3.0/PKGBUILD | 86 +++-- 1 file changed, 73 insertions(+), 13 deletions(-) The x86_64 package shouldn't depend on lib32 packages. Not everyone needs or wants to enable the multilib repo, and waffle shouldn't force them to do that. If you're concerned about the lib32-waffle PKGBUILD drifting out-of-sync with the base waffle PKGBUILD, maybe you could cleverly use the bash 'source' directive and string substitution. To be honest enabling multilib repo was the first thing I've done since I got introduced to Arch :) Seems like I got the wrong impression with what is meant with keeping them in sync. Will amend things appropriately. -Emil ___ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle
Re: [waffle] [GSOC2014] Current design/implementation + the infamous corruption
- Original Message - On 05/06/14 03:19, Emil Velikov wrote: Hi all, Over the last few days I have been though some head-scratching and re-writing things, in order to resolve an interesting memory corruption to no avail :'( Before explaining more about the issue here is some info about the current design + where to get the patches. Let me know if you would like me to send them to the ML. Branch GSOC2014 over at https://urldefense.proofpoint.com/v1/url?u=https://github.com/evelikov/wafflek=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0Ar=NMr9uy2iTjWVixC0wOcYCWEIYhfo80qKwRgdodpoDzA%3D%0Am=a2j0YKhUfDCeKnrHMRWtbX2GGkBhvFdRuL%2BmYYiIN7g%3D%0As=653132e086e89788df300b121854441a78291782135936411a4a6254e6959c6a Archlinux users can use the pkgbuild script in pkg/archlinux/mingw-w64-waffle. Design by example - gl_sample.c (i.e. how to use waffle) waffle_init() waffle_display_connect() + RegisterClass()// Registers the window class used as a template // by Windows to create the window // Create a default/root window, config and context // as wglGetProcAddress needs to be executed under // current context. Missed case: waffle_get_proc_address() // if (wglGetCurrentContext() == NULL) { // using_display_context = true; // wglMakeCurrent(display-hdc, display-hglrc) // } // ok = wglGetProcAddress() // if (using_display_context == true) // wglMakeCurrent(NULL, NULL) // return ok; // Unbinding the current context may be an // overkill although will help with unintentional // misuse of waffle's API. // NOTE: Will need to change waffle's internals // in order to get access to wcore_display. // The API will need a change to include the // display waffle_get_proc_address(dpy, glHam) waffle_config_choose(dpy...) + CreateWindow() // Create a window and prepend it to the // wgl_display::windows list. Now that I look at it, I'm not entirely sure why I needed a list of all windows in wgl_display. Seems like I can drop that. + ChoosePixelFormat() + SetPixelFormat() // Set the pixelformat and link the current window // to the wgl_config waffle_context_create(config...) + CreateContext(hDC... ) // At this point we need access to the wgl_window, // as hWnd/hDC (window_handle/device_context handle) // is essential. // This is where wgl_config::window comes into play. waffle_window_create(config, w, h) + wgl_window_resize()// Reuse the window, adjusting it's dimensions, as // w, h used at creation time are include the window // border. AFAICK there is no way to determine the // correct window dimensions prior to creation. ... waffle_window_create(config, w, h) + waffle_window_resize() // Reuse the window... and boom. Any suggestions on // how to resolve this, or should we mandate that // this is not a valid usage of waffle ? There is no use case in piglit + waffle{examples/utils} where one would create multiple windows for a single config. I might just add a lovely assert and don't worry about this too much. With the above resolved I will take a look at handling gl profiles and alike. And now back to the memory corruption: Resolved, thanks to Jon Turney. The functions obtained by waffle_dl_sym() are part of the Windows API, which uses different calling convention (stdcall vs cdecl) which causes the heap corruption. Good catch. Annotating properly makes the gl_basic test work like a charm. That's great progres! AFAICS waffle_get_proc_address is in the same boat. Will need to test. Cheers, Emil Jose ___ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle
[waffle] [PATCHv2 04/10] cmake: add autodetection for waffle_has_egl, glx...
Silence the pkg_check_modules and check set the default options depending on the packages found. Error out if the user has selected an option and it's requirements are not met. v2: - Do not silence pkg_check_modules. - Explicitly list the failing requirements. Signed-off-by: Emil Velikov emil.l.veli...@gmail.com --- The patch turned out a bit longer that I would like to, although I cannot see any sane way of splitting it up :'( At least it nicely prints which dependency is missing, so that there should be little-to-no head-bashing from people building waffle. -Emil CMakeLists.txt | 2 +- Options.cmake | 32 -- cmake/Modules/WaffleFindDependencies.cmake | 30 ++--- cmake/Modules/WaffleValidateOptions.cmake | 70 ++ 4 files changed, 113 insertions(+), 21 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b32b3d1..cf19929 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -30,12 +30,12 @@ cmake_minimum_required(VERSION 2.8) list(APPEND CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR}/cmake/Modules) include(WaffleDefineOS) +include(WaffleFindDependencies) include(Options.cmake) include(WaffleDefineInternalOptions) include(WaffleValidateOptions) include(WaffleDefineVersion) include(WaffleDefineCompilerFlags) -include(WaffleFindDependencies) include(GNUInstallDirs) if(waffle_build_tests) diff --git a/Options.cmake b/Options.cmake index 7f694e3..c316070 100644 --- a/Options.cmake +++ b/Options.cmake @@ -1,9 +1,33 @@ if(waffle_on_linux) +if(gl_FOUND AND x11-xcb_FOUND) +set(glx_default ON) +else() +set(glx_default OFF) +endif() + +if(wayland-client_FOUND AND wayland-egl_FOUND AND egl_FOUND) +set(wayland_default ON) +else() +set(wayland_default OFF) +endif() + +if(x11-xcb_FOUND AND egl_FOUND) +set(x11_egl_default ON) +else() +set(x11_egl_default OFF) +endif() + +if(gbm_FOUND AND libudev_FOUND AND egl_FOUND) +set(gbm_default ON) +else() +set(gbm_default OFF) +endif() + # On Linux, you must enable at least one of the below options. -option(waffle_has_glx Build support for GLX OFF) -option(waffle_has_wayland Build support for Wayland OFF) -option(waffle_has_x11_egl Build support for X11/EGL OFF) -option(waffle_has_gbm Build support for GBM OFF) +option(waffle_has_glx Build support for GLX ${glx_default}) +option(waffle_has_wayland Build support for Wayland ${wayland_default}) +option(waffle_has_x11_egl Build support for X11/EGL ${x11_egl_default}) +option(waffle_has_gbm Build support for GBM ${gbm_default}) endif() option(waffle_build_tests Build tests ON) diff --git a/cmake/Modules/WaffleFindDependencies.cmake b/cmake/Modules/WaffleFindDependencies.cmake index 527e01f..9245772 100644 --- a/cmake/Modules/WaffleFindDependencies.cmake +++ b/cmake/Modules/WaffleFindDependencies.cmake @@ -61,24 +61,22 @@ if(waffle_on_mac) find_library(CORE_FOUNDATION_FRAMEWORK CoreFoundation REQUIRED) endif() -if(waffle_has_egl) -waffle_pkg_config(egl REQUIRED egl) -endif() -if(waffle_has_glx) -waffle_pkg_config(gl REQUIRED gl) -endif() +if(waffle_on_linux) +# waffle_has_egl +waffle_pkg_config(egl egl) -if(waffle_has_wayland) -waffle_pkg_config(wayland-client REQUIRED wayland-client=1) -waffle_pkg_config(wayland-egl REQUIRED wayland-egl=9.1) -endif() +# waffle_has_glx +waffle_pkg_config(gl gl) -if(waffle_has_x11) -waffle_pkg_config(x11-xcb REQUIRED x11-xcb) -endif() +# waffle_has_wayland +waffle_pkg_config(wayland-client wayland-client=1) +waffle_pkg_config(wayland-egl wayland-egl=9.1) + +# waffle_has_x11 +waffle_pkg_config(x11-xcb x11-xcb) -if(waffle_has_gbm) -waffle_pkg_config(gbm REQUIRED gbm) -waffle_pkg_config(libudev REQUIRED libudev) +# waffle_has_gbm +waffle_pkg_config(gbm gbm) +waffle_pkg_config(libudev libudev) endif() diff --git a/cmake/Modules/WaffleValidateOptions.cmake b/cmake/Modules/WaffleValidateOptions.cmake index 03eb4ba..d9fc299 100644 --- a/cmake/Modules/WaffleValidateOptions.cmake +++ b/cmake/Modules/WaffleValidateOptions.cmake @@ -52,6 +52,76 @@ if(waffle_on_linux) waffle_has_glx, waffle_has_wayland, waffle_has_x11_egl, waffle_has_gbm.) endif() +if(waffle_has_gbm) +if(NOT gbm_FOUND) +set(gbm_missing_deps +${gbm_missing_deps} gbm +) +endif() +if(NOT libudev_FOUND) +set(gbm_missing_deps +${gbm_missing_deps} libudev +) +endif() +if(NOT egl_FOUND) +set(gbm_missing_deps +${gbm_missing_deps} egl +) +endif() +if(gbm_missing_deps) +message(FATAL_ERROR gbm dependency is missing: ${gbm_missing_deps}) +
Re: [waffle] [GSOC2014] Current design/implementation + the infamous corruption
On 06/06/14 14:00, Jose Fonseca wrote: - Original Message - On 05/06/14 03:19, Emil Velikov wrote: Hi all, Over the last few days I have been though some head-scratching and re-writing things, in order to resolve an interesting memory corruption to no avail :'( Before explaining more about the issue here is some info about the current design + where to get the patches. Let me know if you would like me to send them to the ML. Branch GSOC2014 over at https://urldefense.proofpoint.com/v1/url?u=https://github.com/evelikov/wafflek=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0Ar=NMr9uy2iTjWVixC0wOcYCWEIYhfo80qKwRgdodpoDzA%3D%0Am=a2j0YKhUfDCeKnrHMRWtbX2GGkBhvFdRuL%2BmYYiIN7g%3D%0As=653132e086e89788df300b121854441a78291782135936411a4a6254e6959c6a Archlinux users can use the pkgbuild script in pkg/archlinux/mingw-w64-waffle. Design by example - gl_sample.c (i.e. how to use waffle) waffle_init() waffle_display_connect() + RegisterClass()// Registers the window class used as a template // by Windows to create the window // Create a default/root window, config and context // as wglGetProcAddress needs to be executed under // current context. Missed case: waffle_get_proc_address() // if (wglGetCurrentContext() == NULL) { // using_display_context = true; // wglMakeCurrent(display-hdc, display-hglrc) // } // ok = wglGetProcAddress() // if (using_display_context == true) // wglMakeCurrent(NULL, NULL) // return ok; // Unbinding the current context may be an // overkill although will help with unintentional // misuse of waffle's API. // NOTE: Will need to change waffle's internals // in order to get access to wcore_display. // The API will need a change to include the // display waffle_get_proc_address(dpy, glHam) waffle_config_choose(dpy...) + CreateWindow() // Create a window and prepend it to the // wgl_display::windows list. Now that I look at it, I'm not entirely sure why I needed a list of all windows in wgl_display. Seems like I can drop that. + ChoosePixelFormat() + SetPixelFormat() // Set the pixelformat and link the current window // to the wgl_config waffle_context_create(config...) + CreateContext(hDC... ) // At this point we need access to the wgl_window, // as hWnd/hDC (window_handle/device_context handle) // is essential. // This is where wgl_config::window comes into play. waffle_window_create(config, w, h) + wgl_window_resize()// Reuse the window, adjusting it's dimensions, as // w, h used at creation time are include the window // border. AFAICK there is no way to determine the // correct window dimensions prior to creation. ... waffle_window_create(config, w, h) + waffle_window_resize() // Reuse the window... and boom. Any suggestions on // how to resolve this, or should we mandate that // this is not a valid usage of waffle ? There is no use case in piglit + waffle{examples/utils} where one would create multiple windows for a single config. I might just add a lovely assert and don't worry about this too much. With the above resolved I will take a look at handling gl profiles and alike. And now back to the memory corruption: Resolved, thanks to Jon Turney. The functions obtained by waffle_dl_sym() are part of the Windows API, which uses different calling convention (stdcall vs cdecl) which causes the heap corruption. Good catch. The mess was so deep that I've managed to bring the whole Windows down on two occasions :) Annotating properly makes the gl_basic test work like a charm. That's great progres! Indeed I was over the moon when I saw the window contents flip through the different colors. Kind of stuck at the next step though :\ Jose, gents, Would you know how to convert redsize [1] to redBits/redShift [2] ? The former format is used by ChoosePixelFormat under glx, egl and cgl which do a lot of dancing at libGL/libEGL level, while windows delegates that to the caller and expects the latter format. I've been through mesa/src/glx and it's kind of making my head spin. I'm going to see if I can make more