> > > [...] > > > > > > the idea here looks fine to me. > > > > > > However, I've been wondering, surely there was a reason why it wasn't > > > coded like this in the first place? > > > > > > Could you check if this patch causes Xwayland to create lots of > > > wl_buffers it will never attach and commit to a wl_surface? If it does, > > > that was probably the reason. > > > > Was that evaluated?
I guess the risk here is that we might end up creating useless wl_buffers. Still, the benefit of closing the fd as soon as possible is greater than the risk of creating too many unused wl_buffer, so I think it's worth it. (Not convinced? try running 10 instances of "gtk3-demo --run=cursors" in Wayland) > > AFAICS, when using glamor, the only code path that still uses the > > xwl_shm_*_pixmap() is the cursor code, so even with glamor, running several > > X11 apps which create several cursors each will quickly cause Xwayland to > > reach the limit of file descriptors. > > > > > If that is true, then one has to weigh between creating wl_buffers > > > immediately vs. creating them only when actually needed for a > > > wl_surface. > > > > > > Maybe Daniel knows? CC'ing also Olivier for his information. > > > > I cannot tell why this was done the way it is, but I think this patch does > > improve things for e.g. downstream bug > > https://bugzilla.redhat.com/show_bug.cgi?id=1373451 That particular bug is possibly a leak of cursors in Qt, but still, there is a weakness in Xwayland. so, weighing the pros and cons of this patch, FWIW: Reviewed-by: Olivier Fourdan <ofour...@redhat.com> Unless someone else is opposed to this patch, I'm in... Cheers, Olivier _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
