[PATCH] Revert "include: Use the ARRAY_SIZE() macro in debug.h."

[Axel Davy]

On 27/11/2018 19:30, Henri Verbeet wrote:
> On Tue, 27 Nov 2018 at 20:31, Axel Davy <davyaxel0 at gmail.com> wrote:
>> On 27/11/2018 17:23, Henri Verbeet wrote:
>>> My main question would be "Why do you need that?" For example, event
>>> processing should normally already be done by the application.
>>> Somewhat similarly, __wine_present_make_wine_drawable() may make sense
>>> if Wine would need to store some extra information along with the
>>> native pixmap, but I don't think we do.
>> Processing present events is not done by the application.
>>
> Sure.
>
>> You have to set up a listener for the window to specify you want to
>> listen to them,
>> and use a callback. You use your own xcb queue, thus only you control
>> when to treat the events.
>>
>> If you don't use a callback, you cannot know if/when the drawable was
>> presented, and if released or not (which is pretty important to know).
>>
>> If you let nine/other user of the wine_present API get a callback, it
>> will be pretty hard to make generic. Imagine another similar window
>> system API doesn't work with callback ?
>>
>> If you handle the callback in wine_present, you are forced to store
>> somewhere the presented/released information when handling the callback,
>> which is why I suggested __wine_present_make_wine_drawable.
>>
> I don't think "released" is a Present term. (Do you mean "idle"?) If

Yes, that's what I meant. It's equivalent (it means the server is done
with your buffer and won't use it ever again).

> you're doing buffer swaps on a separate thread anyway though, we can
> also just block in __wine_present_native_drawable(), instead of

You mean wait the buffer you send gets released/presented ?

That would require one thread per presentation.

It could be done I guess (though it's not what nine does currently).

Can you think of any current window system (MacOS ?) that would benefit 
from this approach ?

> spinning on __wine_present_is_drawable_released(). If it needs to be
> asynchronous though, a callback on completion/idle really isn't that
> Present specific. If the platform only provides a synchronous API,
> Wine can just call the notification callback itself on completion.

Sure you can implement a specific callback that would call a generic 
callback.
It's an implementation choice indeed. I believe it would make the code 
more complex to do that way though for the user of the API (need to 
handle the multithreading part of the callback).

We made a few mistakes for Nine that caused the code to be a bit more 
complex that what it could have been.

The API I suggested would, I believe, avoid some of these mistakes and 
results in the simplest code.

Please note I have some experience in the area, as I also contributed to 
the egl wayland backend,
the glx dri3 backend and wrote the first complete XWayland Present 
implementation,

which I hope will help get that generic API right (but I don't pretend 
to have the truth there).

> (And note that if you ignored the msc, you could implement
> __wine_present_native_drawable() using a regular blit as well.)
You wouldn't get tear free then :-). You can get that even without vsync 
if you want - basically triple buffering (we have option for that).
>
> In terms of other platforms, we have Android/ChromeOS, Wayland, and
> MacOS. Some of those could conceivably run Nine.