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

[Emil Velikov]

On 23/12/14 01:11, Chad Versace 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.
>  
True, that's why I've mentioned "future work... as the attribs list
grows". Absolutely no objections of keeping it as is.

>> 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?
> 
Guilty as charged :-) Yet again my comment was meant as a side tip, than
anything serious.

Cheers,
Emil