On Wed May 31 20:42:17 2023 +0000, Rémi Bernon wrote:
I think NULL should not be accepted here. I can see that the code is adapted pretty much pristine from winex11, but in my opinion it is not necessarily a good reference. Having a new driver should be an occasion to write better code, not one to duplicate (potentially not great) code. The winex11 driver works well, yes, but it is also extremely difficult to refactor. It would be great not to spread that difficulty. For instance, the window data creation pattern `if (!get && !create)` looks awkward and racy at first as it is unguarded. It is only working because of how `WindowPosChanging` is called, and I don't think we should rely on this.
Seems very sensible. I will move to a more traditional/explicit locking pattern, getting rid of `wayland_win_data_release()` completely and making `wayland_win_data_get()` not perform any internal locking. For example for the data creation could look like (in pseudo-code) :
```c lock; if (!get && !create) { unlock; return; } ... unlock; ```
Let me know if you had some other approach in mind.