I guess I should follow up with that I don't really care how the patch ends up or which six colors of the bikeshed we end up choosing. I just want you to be aware of why that patch fixes the bug, why Eric is saying that we don't need MAX_LEVEL, and why GL is surprisingly difficult to get right.
Patch, as-is, is Reviewed-by: Jasper St. Pierre <jstpie...@mecheye.net> On Fri, Jan 9, 2015 at 6:20 PM, Jasper St. Pierre <jstpie...@mecheye.net> wrote: > Well, either MAX_LEVEL / BASE_LEVEL or MIN_FILTER / MAG_FILTER will fix it. > > Basically, a texture is only considered complete [0] when all of its > usable mipmap levels have been defined. Either you set MAX_LEVEL to say it > has 0 mipmaps (by default MAX_LEVEL is 1000), or you set MIN_FILTER / > MAG_FILTER to say it doesn't use any mipmaps when sampling from it. > > Another solution is to use glTexStorage2D to specify the number of mipmaps > directly when defining internal texture storage. > > [0] See 8.17, "TEXTURE COMPLETENESS", in the GL specification: > https://www.opengl.org/registry/#apispecs > > On Fri, Jan 9, 2015 at 5:39 PM, Keith Packard <kei...@keithp.com> wrote: > >> Axel Davy <axel.d...@ens.fr> writes: >> >> > Actually we went up with a solution, the final patch wasn't sent to ml >> > for a reason >> > I don't know or remember. >> > >> > Recently someone else complaint of the bug and I linked him to a >> > preliminary patch we had, >> > and it fixed it for him. >> > The preliminary patch was >> > http://markus.members.selfnet.de/xorg/xwayland.patch >> > >> >> That's this patch: >> >> diff --git a/glamor/glamor_egl.c b/glamor/glamor_egl.c >> index 54af275..b127b96 100644 >> --- a/glamor/glamor_egl.c >> +++ b/glamor/glamor_egl.c >> @@ -170,6 +170,7 @@ glamor_create_texture_from_image(ScreenPtr screen, >> glBindTexture(GL_TEXTURE_2D, *texture); >> glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); >> glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); >> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL, 0); >> >> glEGLImageTargetTexture2DOES(GL_TEXTURE_2D, image); >> glBindTexture(GL_TEXTURE_2D, 0); >> diff --git a/hw/xwayland/xwayland-glamor.c b/hw/xwayland/xwayland-glamor.c >> index 4be883f..7c2bd08 100644 >> --- a/hw/xwayland/xwayland-glamor.c >> +++ b/hw/xwayland/xwayland-glamor.c >> @@ -137,6 +137,10 @@ xwl_glamor_create_pixmap_for_bo(ScreenPtr screen, >> struct gbm_bo *bo, int depth) >> >> glGenTextures(1, &xwl_pixmap->texture); >> glBindTexture(GL_TEXTURE_2D, xwl_pixmap->texture); >> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST); >> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); >> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAX_LEVEL, 0); >> + >> glEGLImageTargetTexture2DOES(GL_TEXTURE_2D, xwl_pixmap->image); >> glBindTexture(GL_TEXTURE_2D, 0); >> >> Eric says that we probably need the MIN_FILTER and MAG_FILTER bits, or >> you'll probably get black, but the MAX_LEVEL changes aren't necessary. >> >> > I think there was a more recent one, but I couldn't find it. >> >> Let me know. >> >> -- >> -keith >> >> _______________________________________________ >> xorg-devel@lists.x.org: X.Org development >> Archives: http://lists.x.org/archives/xorg-devel >> Info: http://lists.x.org/mailman/listinfo/xorg-devel >> > > > > -- > Jasper > -- Jasper
_______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel