On Fri, Aug 21, 2015 at 04:09:20PM +0300, Pekka Paalanen wrote: > On Wed, 19 Aug 2015 11:15:21 -0500 > Derek Foreman <[email protected]> wrote: > > > On 26/01/15 04:19 AM, Jonas Ådahl wrote: > > > mesa supports EGLSwapInterval 0 now, so lets remove this hack. As a > > > bonus we don't conflict with the XDG shell protocol that doesn't allow > > > committing a null-buffer, which was a side effect of this hack. > > Hmm, but the code removed here is *not* committing a null buffer. > Instead, it is doing a commit without an attach. Surely that is legal > even with an xdg_surface?
There is a check in mutter which terminates a client if a surface is committed without a buffer. If a client has never attached a buffer to a surface, this could happen, and is what happened here. Though now that I think about it should be allowed, since one might want to negotiate initial state before attaching an initial buffer, and one needs to commit the state before that. *adds item to todo list*. What probably shouldn't be allowed is to attach a NULL buffer and then commit it. That is what the intention with the check in mutter is I assume. So I guess the commit message part about the bonus there is incorrect. > > Btw. did we have any patches to xdg-shell spec to say that you cannot > attach and commit a null buffer? ISTR it has been discussed, yes, but I > couldn't see it in the spec on a quick look. Some patch in the queue? It is undefined behavior. mutter defines it as illegal and weston doesn't. Yes, it should be specified IMO. I don't know about such a patch. > > > Unreviewed for far too long. :( > > > > The question I guess is whether toytoolkit needs to maintain this hack > > forever because mesa used to have a problem with this. As far as I can > > tell from reading the EGL spec, eglSwapInterval(0) has been defined for > > as long as eglSwapInterval() has existed (EGL 1.1)... > > > > You've stated your preference - and I'm convinced. I don't think > > toytoolkit apps should be a reference for avoiding old mesa bugs. I'm > > also not hugely concerned about other broken EGL stacks... (if you're > > making a custom IVI with a nasty driver stack, you probably shouldn't be > > using toytoolkit at all, and if you are you can re-invent this nastiness.) > > > > Besides, the comments even say we should kill it now. > > > > All that said, > > Reviewed-by: Derek Foreman <[email protected]> > > > > Can we kill this hack? :) > > Yeah, I think we should. Let's see if anyone complains afterwards. > > Btw. I added a reference to > e9297f8e7ee09fa39b1d4293fad6e97705ccff21 which was the patch adding > this hack, it contains an elaborate explanation. > > > > Signed-off-by: Jonas Ådahl <[email protected]> > > > --- > > > clients/window.c | 32 -------------------------------- > > > 1 file changed, 32 deletions(-) > > I was very close to pushing this, when I realized that only subsurfaces > client actually sets eglSwapInterval(0). Testing nested.c is also a bit > difficult because it requires cairo-glesv2, and it seems my distro has > dropped the possibility of that option from Cairo. > > Looking at nested.c closer, I suspect it might be ok as is. The > sub-surface renderer does not call eglSwapbuffers on sub-surfaces > (right?). As far as I can see, we just get a wl_buffer from EGL, then attach and commit it. No going through EGL for the wl_surface interaction. And we don't block on the frame callback either for that matter. Jonas > > Pushed: > 1a912a9..decc965 master -> master > > > Thanks, > pq _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
