[waffle] [GSOC2014] Current design/implementation + the infamous corruption

Jose Fonseca jfonseca at vmware.com
Fri Jun 6 10:45:52 PDT 2014



----- Original Message -----
> 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/waffle&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=NMr9uy2iTjWVixC0wOcYCWEIYhfo80qKwRgdodpoDzA%3D%0A&m=a2j0YKhUfDCeKnrHMRWtbX2GGkBhvFdRuL%2BmYYiIN7g%3D%0A&s=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] ?

I believe that redBits = redsize. And you don't need to fill in redShift (just leave it 0.)

> 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 sense from mesa/src/egl.
> 
> AFAICS I can use wglChoosePixelFormatARB/wglChoosePixelFormat(GL) over
> ChoosePixelFormat (GDI), although I have seen a handful recommendations
> against it - black window, excessive flickering etc.
>
> IMHO the former should help although it's not really documented on MSDN :\

You don't want to use wglChoosePixelFormat (GDI32.DLL's ChoosePixelFormat will LoadLibrary(OpenGL32.dll and call wglChoosePixelFormat).

wglChoosePixelFormatARB is a pain too -- like any WGL extension, you need first to bind a temporary context/pixelformat so you can get it -- so it is meaningless unless you really need to specify some additional attribute that's not supported by ChoosePixelFormat.

> 
> 
> Any suggestions and links on the topic would be appreciated :)

FWIW you can see what apitrace does in https://github.com/apitrace/apitrace/blob/master/retrace/glws_wgl.cpp#L218

Jose

> 
> Cheers,
> Emil
> 
> 
> 
> 
> [1]
> https://urldefense.proofpoint.com/v1/url?u=https://github.com/waffle-gl/waffle/blob/master/src/waffle/core/wcore_config_attrs.h%23L32&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=NMr9uy2iTjWVixC0wOcYCWEIYhfo80qKwRgdodpoDzA%3D%0A&m=3DeRAN9DRhiyORWWYmuDpskvb0WN1pVt8gHpXg0iI2E%3D%0A&s=493477568f62a3ded6c273fc4611dd8cee83789c89968c5fa4b075e606002a0a
> 
> [2]
> https://urldefense.proofpoint.com/v1/url?u=http://msdn.microsoft.com/en-gb/library/windows/desktop/dd368826%28v%3Dvs.85%29.aspx&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=NMr9uy2iTjWVixC0wOcYCWEIYhfo80qKwRgdodpoDzA%3D%0A&m=3DeRAN9DRhiyORWWYmuDpskvb0WN1pVt8gHpXg0iI2E%3D%0A&s=203e2af4cf4b8387b8b7feecbaa44440bce733e51487719418438368af9c814f
> 
> 
> >>
> >> AFAICS waffle_get_proc_address is in the same boat. Will need to test.
> >>
> >> Cheers,
> >> Emil
> >>
> > 
> > Jose
> > 
> 
> _______________________________________________
> waffle mailing list
> waffle at lists.freedesktop.org
> https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/waffle&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=NMr9uy2iTjWVixC0wOcYCWEIYhfo80qKwRgdodpoDzA%3D%0A&m=3DeRAN9DRhiyORWWYmuDpskvb0WN1pVt8gHpXg0iI2E%3D%0A&s=7303eefd8267b23c98944650cfeebe7517cfc2f2192a483f387c3bf97f35d546
> 


More information about the waffle mailing list