Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

[Chad Versace]

On Thu 14 Apr 2016, Michel Dänzer wrote:
> From: Michel Dänzer 
> 
> Fixes build failure due to wl_proxy_marshal_constructor_versioned being
> unresolved when building against current wayland.
> 
> This API was introduced in wayland 1.9.91 by commit 557032e3 ("Track
> protocol object versions inside wl_proxy."). The waffle code doesn't
> reference wl_proxy_marshal_constructor_versioned directly but
> indirectly via wayland-scanner.
> 
> v2:
> * Add paragraph about how wl_proxy_marshal_constructor_versioned was
>   introduced. (Emil Velikov)
> * Only resolve wl_proxy_marshal_constructor_versioned with wayland >=
>   1.9.91.
> 
> Signed-off-by: Michel Dänzer 
> ---
>  src/waffle/wayland/wayland_wrapper.c | 5 +
>  src/waffle/wayland/wayland_wrapper.h | 8 
>  2 files changed, 13 insertions(+)

Michel, thanks for the patch. It's merged to master.

I squashed a small fix into your patch: `#include `
was needed to make your version check work.
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

[Chad Versace]

On Mon 20 Jun 2016, Chad Versace wrote:
> On Sun 17 Apr 2016, Emil Velikov wrote:
> > On 17 April 2016 at 01:41, Jason Ekstrand  wrote:
> > > On Sat, Apr 16, 2016 at 4:12 PM, Emil Velikov 
> > > wrote:
> > >>
> > >> On 16 April 2016 at 22:06, Jason Ekstrand  wrote:
> 
> > >> >> >> In either case, I think checking wayland-client-protocol.h into the
> > >> >> >> repo is
> > >> >> >> a must.
> > >> >> >
> > >> >> > I'm convinced.
> > >> >> Unfortunately I'm not ;'-(
> > >> >
> > >> >
> > >> > Are you now?
> > >> >
> > >> Not quite I'm afraid.
> > >>
> > >> As a queue, I'm doing to (slightly) beat the SDL drum.
> > >> SDL caters for everyone's needs and has a much wider exposure than
> > >> waffle. I'm suggesting the exact same approach like the one they opted
> > >> for ;-)
> > >
> > >
> > > I doubt its the "exact" same or they'd be having build breaks too.
> > They do actually [1]. The person who fixed it is familiar wayland
> > developer [2] yet he choose the same approach as me ;-)
> > 
> > > If you
> > > want to provide a sort of glue layer to an application, your suggestion of
> > > "optional" entrypoints is probably exactly what you want.
> > Indeed. Thank you.
> > 
> > >  However, waffle
> > > itself needs to call something and it either needs to be smart enough to
> > > call the right thing depending on the libwayland version it just dlopened 
> > > or
> > > it needs to just always call old ones.
> > >
> > The cases of waffle being "dumb" (not being smart enough) are so
> > infrequent, that it beats the added overhead that importing the header
> > will bring.
> > 
> > Thanks for the discussion. Hopefully you're seeing things from my POV ;-)
> > Emil
> > 
> > [1] 
> > https://github.com/spurious/SDL-mirror/tree/master/src/video/wayland/SDL_wayland{dyn.{c,h},sym.h}
> > [2] 
> > https://github.com/spurious/SDL-mirror/commit/737415012588a2636794292129a2d39f2a28fe3c
> 
> Both Jason's and Emil's approaches seem valid to me. And my preference
> flip-flops every few minutes as I read the thread.
> 
> In April, I was strongly convinced of Jason's position. Now I'm leaning
> slightly toward's Emil's.
> 
> I want to look more closely at SDL's approach (Emil, thanks for the
> links), and then make a final decision. But it's late in the day for me
> and my brain is done. Exhausted brains can't be trusted, so the decision
> will have to wait until morning.

Everyone, thanks for the lengthy discussion. The winner is... Michel's
patch v2, which is basically Emil's and SDL's position.

I decided against importing any Wayland headers, because the Wayland
headers actually contain a lot of inline function *definitions*. When
upstream Wayland applies bugfixes and improvements to those functions,
by not using imported headers Waffle automatically receives the bugfixes
and improvements simply by being rebuilt; this seems to be the intent of
the Wayland authors for client projects. If Waffle were to use imported
headers then, to receive the same improvements, someone (likely me)
would need to diligently keep the imported headers up-to-date.

As a bonus, Michel's patch is considerably smaller and requires less
maintenance than an import-some-headers patch.

And Michel's patch provides correct behavior, at least in my opinion:

- If a user or distro builds libwaffle against wayland < 1.10, then
  that same libwaffle will continue to work with wayland >= 1.10.

- If a user or distro builds libwaffle against wayland == 1.10, then
  the libwaffle will correctly emit an informative error message and
  fail if it dlopens a libwayland-client < 1.10, thanks to the 'goto
  error' in src/waffle/wayland/waylan_wrapper.c:RETRIEVE_WL_CLIENT_SYMBOL.
  Specifically, the libwaffle will not crash or do undefined
  behavior; it gracefully emits an error and fails responsibly.
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] long-standing wgl pixel format issue

[Brian Paul]

On 06/20/2016 05:48 PM, Emil Velikov wrote:

On 20 June 2016 at 15:46, Brian Paul  wrote:

On 06/17/2016 07:59 PM, Emil Velikov wrote:


On 17 June 2016 at 16:53, Brian Paul  wrote:



I spent a few hours yesterday pulling out my hair trying to understand
why
the piglit fbo-mipmap-copypix test was failing on Windows.  But it was
only
failing when I ran it directly.  It passed when I ran it via
piglit-run.py

The key difference was the -fbo option.  With -fbo the test used an RGBA8
framebuffer but without -fbo the test used an RGB565 framebuffer.

So why is a 565 framebuffer being used?  It boils down to the fact that
wglChoosePixelFormatARB() does not work like glXChooseFBConfig().

  From the glXChooseFBConfig man page:
"""
GLX_RED_SIZE, GLX_GREEN_SIZE, GLX_BLUE_SIZE, GLX_ALPHA_SIZE

   Each attribute, if present, must be followed by a nonnegative
minimum
size
  specification or GLX_DONT_CARE.
  The largest available total RGBA color buffer size (sum of
GLX_RED_SIZE,
  GLX_GREEN_SIZE, GLX_BLUE_SIZE, and GLX_ALPHA_SIZE)
  of at least the minimum size specified for each color component is
preferred.
"""

So if you specify GLX_RED_SIZE, BLUE_SIZE, etc to be 1 and there are both
RGB565 and RGBA8 formats available, the _later_ (the largest) will be
chosen.

But the wglChoosePixelFormatARB docs say:
"""
Some attribute values must match the pixel format value exactly when
  the attribute is specified while others specify a minimum criteria,
  meaning that the pixel format value must meet or exceed the
  specified value.

   Attribute  TypeMatch Criteria
   WGL_RED_BITS_ARB   integer minimum
   WGL_GREEN_BITS_ARB integer minimum
   WGL_BLUE_BITS_ARB  integer minimum
   WGL_ALPHA_BITS_ARB integer minimum
"""

So if you specify WGL_RED/GREEN/BLUE_BITS_ARB to be 1 and there are both
RGB565 and RGBA8 formats available, the _former_ may be chosen.  Note
that
some WGL apps use WGL_COLOR_BITS_ARB=24 and avoid this.

Piglit's call to piglit_wfl_framework_init() uses an attribute list with
WAFFLE_RED/GREEN/BLUE_SIZE = 1 and that winds up going directly to
wglChoosePixelFormatARB and glXChooseFBConfig so this difference in
behavior
effects the window's pixel format.


Thanks for this Brian and apologies I did not spot these differences
as I was writing the WGL backend.

Here's a bit more comprehensive list, listing all the waffle backends
and attributes.

GLX/EGL:
Largest - red, green, blue, alpha plus their accum counterparts + depth
Smallest - buffer, stencil

If requested size is zero - "largest" become "smallest" (but it's not
said it will be zero), "smallest" become "zero".

CGL
One that "most closely matches the specified size is preferred"

WGL/NaCL
"At least", meaning that there's not definition if it's the "smallest"
or "largest". Furthermore there's not mention that it will give you
the smallest if you specify 0 :-\



The Waffle docs for waffle_config_choose() say:

"""
WAFFLE_RED_SIZE
WAFFLE_GREEN_SIZE
WAFFLE_BLUE_SIZE
WAFFLE_ALPHA_SIZE
WAFFLE_DEPTH_SIZE
WAFFLE_STENCIL_SIZE

  The default value for each size attribute is 0. Valid values are the
non-negative integers and WAFFLE_DONT_CARE. If the requested size for a
channel is 0, then any surface created with the config will lack that
channel. If the requested size for a channel is positive, then the number
of
bits in that channel for any surface created with the config will be at
least the requested size.
"""

There's some ambiguity here because if several different pixel formats
(such
as RGB565 and RGBA8) both meet the WAFFLE_RED/GREEN/BLUE_SIZE minimums,
which should be preferred?

I can fix my Windows Piglit issue by changing Piglit's
choose_config_attribs() function to specify WAFFLE_RED/GREEN/BLUE_SIZE=8
instead of 1, but that's not a final solution.


I propose:

1. The Waffle docs should be clarified to specify whether the largest or
smallest color format should be used when several meet the WAFFLE_*_SIZE
minimums.  My suggesting is "smallest", like WGL.

2. The Waffle code for either GLX or WGL should be modified to follow
that
part of the spec.  Following my suggestion, the GLX format chooser code
would need to be modified.

3. The Piglit code to specify the Waffle pixel format should be updated,
probably replacing '1' with '8' as above.  And maybe falling back to the
former if the later fails (though I doubt anyone runs piglit on less than
a
24-bit display nowadays).

4. If Waffle wants to get fancy, we could consider new attributes like
WAFFLE_MIN_RED_SIZE, WAFFLE_MAX_RED_SIZE and WAFFLE_EXACT_RED_SIZE to
provide more control over format selection.  But I think my suggestion in
(1) would avoid this for now.

Thoughts?


I'm somewhat inclined that the GLX/EGL behaviour might be the better
choice. Then again I don't might if people choose another route -
always 

Re: [waffle] [PATCH 00/13] Core waffle cleanups

[Emil Velikov]

On 16 May 2016 at 11:57, Emil Velikov  wrote:
> On 16 May 2016 at 11:54, Emil Velikov  wrote:
>> Hi all,
>>
>> While looking at the gbm/egl I've noticed a few interesting bits.
>>  - We do NULL checking for values that are guaranteed by API to be
>> non-NULL.
>>  - wcore_*_init does not need a return type, plus in some places we were
>> not calling it in the correct time.
>>  - wcore_*_teardown is a simple wrapper around assert, which (at the
>> time the function should be called) is too late/not needed.
>>
>> So this series simplifies these, giving us a nice -350 line count ;-)
>>
>> The whole thing can be found in
>> https://github.com/evelikov/waffle/tree/for-upstream/core-cleanups
>>
> For some reason git send-email seems to be choking on patches 08/13
> and 09/13. Please check those out via the above repo or let me know if
> you'd prefer them in other format.
>
I might have gone overboard (too much) folding the error label(s) in
09/13 "core: remove wcore_*_init() return type". I can split those up
if people prefer.

-Emil
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle