[waffle] [PATCH 00/11] Add new public func waffle_window_create2()

[Chad Versace]

On 12/23/2014 04:22 PM, Frank Henigman wrote:
> On Mon, Dec 22, 2014 at 8:11 PM, Chad Versace <chad.versace at intel.com> wrote:
>> On 12/21/2014 01:41 PM, Emil Velikov wrote:
>>> On 16 December 2014 at 08:18, Chad Versace <chad.versace at linux.intel.com> wrote:
>>>> Today, waffle_window() has only two parameters: width and height.
>>>>
>>>> Frank Henigman wants to extend Waffle's GBM backend with the ability to
>>>> post window contents to the display. Multiple methods exist for posting
>>>> content to the screen with the drm API, and that method should be
>>>> configurable per waffle_window. Therefore, we need to be able to pass
>>>> additional attributes to waffle_window_create().
>>>>
>>>> It would also be nice to specify at time of creation that the
>>>> waffle_window should be full screen. Again, we need to pass additional
>>>> attributes to waffle_window_create().
>>>>
>>>> The new function waffle_window_create2() is conceptually equivalent to
>>>> the original waffle_window_create() with the addition of an attrib_list
>>>> parameter.  The only supported attributes are currently
>>>> WAFFLE_WINDOW_WIDTH and WAFFLE_WINDOW_HEIGHT.
>>>>
>>>> I tested the new function on GLX, X11/EGL, Wayland, and GBM.
>>>>
>>>> I did not even test the build on Windows, Android, and Mac. Before merging this
>>>> series, I will ensure it doesn't break the build on those platforms. I don't
>>>> have the ability to actually test on Android or Windows, though.
>>>>
>>>> This patch series is available on my personal branch 'waffle_window_create2'.
>>>>
>>> Hi Chad,
>>>
>>> Overall it looks very good imho. I've managed to spot only a few of
>>> trivial bits.
>>> Just a small note for future work - popping an attribute at a time,
>>> will become noticeable as the attribs list grows.
>>
>> Just as you, I winced when I wrote the code that popped one attribute
>> at a time. I really wanted to write it better, but I repeated to myself
>> the mantra "pre-optimization is the root of all evil". The pop-one-at-a-time
>> code is very easy to read in comparison to walk-the-list-once-and-pop-as-we-go
>> code, so I think it's the better choice, at least until the attribute lists
>> grow substantially longer.
>>
>>> Also if you're not a fan of adding numbers at the end of function
>>> names you might use waffle_window_create_{with,from}_attibs
>>
>> I weighed the two naming conventions. I thought that, if we ever
>> bump the signature for waffle_window_create again, then we might
>> not be able to invent a creatively descriptive but short name
>> for the new function. But good ole #3 is always there for us.
>>
>> Also, I tried typing waffle_window_create_attribs and waffle_window_create2
>> several times, and I just couldn't bring myself to make people to type
>> a function name so lengthily Java-like.
>>
>> I assume you're not a fan of using 2?
> 
> Another option is to break compatibility and rename the existing
> function waffle_window_create(_size,0,_old,_orig) and reuse the nice
> clean name for the nice new function.  It seems an easy enough fix for
> people updating their waffle version.  I'll do the chrome os test
> code, and piglit seems to have just one call to waffle_window_create.

Updating code is mostly painless. The real, ongoing pain comes because
some environments will need to maintain in parallel an installation of each
incompatible version of Waffle.

> Are there many other users I don't know about?

Yes, there are more users. I believe that the bulk of projects that use
Waffle are invisible, little single-use tools or little experiments written by
developers for themselves. Those developers deserve a stable ABI too :)

Here are the serious projects I know of that use Waffle
in some way: the Psychophysics Toolbox, apitrace, piglit, Regal. Little projects I
know of: Mark Janes's tool "grips" and some tool called "gamemonitor". Also,
I occasionally get Android patches from Linaro. I never asked them what they use
Waffle for.

...

When Waffle breaks compatibility, one requirement is that the old and the
new versions are parallel installable. For a monolithically developed OS, like
Chrome OS, parallel installability probably isn't needed. It's everyone else
on the regular distros that need the parallel installability. Fedora and Redhat
package Waffle and Piglit, as does Debian.

And graphics developers (and distro package maintainers) (and continuous integration
people too) will curse when they need two Waffles installed, one for
piglit at master and another for piglit at master~1.

I'm not ready yet to proceed through the steps and conversations needed to ensure
that Waffle is safely parallel installable in everyone's environment and to mitigate any
damage in Piglit.

(... maybe Linux's overuse of shared libraries is the thing to blame here, but I digress...)

> Seems like short term pain for long term gain, to me.

To me, each break in compatibility seems to inflict widely distributed
pain to people and projects I've never encountered. Therefore, I want
to avoid frequent compatibility breaks and instead queue of several incompatible
changes for one large break. Otherwise, Waffle becomes a black sheep like libpng.

> If you buy that, why not also change all the "int32_t attribs[]"
> signatures to intptr_t, and not keep around two versions of those
> functions.

I think it best if we deprecate all the int32_t functions and offer replacement
functions such as waffle_window_create2(). Except the waffle_attrib_list*()
-- deprecate and maintain them but offer no replacement.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 884 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/waffle/attachments/20141231/4b79370e/attachment-0001.sig>