Created attachment 121191 [details] Test case When GL_CLIP_ORIGIN is set to GL_UPPER_LEFT using glClipControl, the vertical range of pixels that are written to during rendering does not match that which was requested with glViewport unless the y offset is 0. It appears that during rendering the y offset is always 0, and the height of the viewport is the requested height minus the requested y offset (or 0 if that is negative). However the GL_VIEWPORT values returned by glGet are what was originally requested. This was seen on both Intel Haswell on Ubuntu and AMD R9 270 on Fedora. I have attached a test case. Examining the texture it produces with apitrace shows it wrote to pixels 0 and 1 instead of pixels 1, 2, and 3 of a texture. It should print "0, 1", but on affected machines it prints "1, 0". These values are the contents of pixel 0 and 2. 0 is the cleared value before the draw, and 1 written in the draw (which uses a viewport of height 3 with y offset 1, so only covers the second test pixel).
Without trying to determine whether mesa or your test right, this "fixes" your test: diff --git a/src/mesa/main/viewport.c b/src/mesa/main/viewport.c index 7d891429..2881a75 100644 --- a/src/mesa/main/viewport.c +++ b/src/mesa/main/viewport.c @@ -456,7 +456,7 @@ _mesa_get_viewport_xform(struct gl_context *ctx, unsigned i, translate[0] = half_width + x; if (ctx->Transform.ClipOrigin == GL_UPPER_LEFT) { scale[1] = -half_height; - translate[1] = half_height - y; + translate[1] = half_height + y; } else { scale[1] = half_height; translate[1] = half_height + y;
And FWIW this is what st/nine does (which uses the upper_left/zero-to-one convention): pvport.scale[0] = (float)vport->Width * 0.5f; pvport.scale[1] = (float)vport->Height * -0.5f; pvport.scale[2] = vport->MaxZ - vport->MinZ; pvport.translate[0] = (float)vport->Width * 0.5f + (float)vport->X; pvport.translate[1] = (float)vport->Height * 0.5f + (float)vport->Y; pvport.translate[2] = vport->MinZ; So I'm suspecting that it's mesa that's wrong. Really need to draw this out on paper and compare to the spec.
A bit of potentially-relevant spec quote from ARB_clip_control: 12) Does setting the clip control origin to GL_UPPER_LEFT change the origin of the window coordinate system use for commands such as glViewport, glScissor, glWindowPos2i, and glReadPixels? RESOLVED: No. The (x,y) window space location passed to these commands have the (0,0) origin at the lower left corner of the window, independent of the state of the clip control origin. Still unsure whether that rules in favor of Mesa or of the test case :) Hopefully someone more knowledgeable can weigh in.
Hi, I can't remember why I thought there must be a minus. Today this looks rather wrong to me ... So I think that the proposed change to mesa is a good one. Also I have checked out the provided test case on nvidias binary driver which also aligns with James expected behaviour. I believe that the piglit tests don't uses non zero y values for the viewport. May be we want to add a test there also. Greetings Mathias
(In reply to Ilia Mirkin from comment #3) > A bit of potentially-relevant spec quote from ARB_clip_control: > > 12) Does setting the clip control origin to GL_UPPER_LEFT change the > origin of the window coordinate system use for commands such as > glViewport, glScissor, glWindowPos2i, and glReadPixels? > > RESOLVED: No. > > The (x,y) window space location passed to these commands have the > (0,0) origin at the lower left corner of the window, independent > of the state of the clip control origin. > > Still unsure whether that rules in favor of Mesa or of the test case :) > Hopefully someone more knowledgeable can weigh in. Hi Ilia, First of all, let me warn that I'm not an expert on this extension. I appreciate your perseverance getting to the bottom at this though. In fact, we (at VMware) actually ran into issues using this extension, but the person most knowledgeable about it is currently on PTO. FWIW, I looked into this, and the way I interpret it, the text of issue 12) is actually conclusive evidence that Mesa is *wrong*: - The attached draws a red rectangle covering the whole viewport, as xyzw coordinates before clipping will be -1.0, -1.0, 0.5, 1.0 3.0, -1.0, 0.5, 1.0 -1.0, 3.0, 0.5, 1.0 3.0, 3.0, 0.5, 1.0 hence the expected rendering is that all pixels inside viewport will be red, all outside will be black (the clear color) - Issue 12) states that the _window coordinate system_ for glViewport should be the same - Hence, we should get the _exact_ same rendering, whether we comment or not the glClipControl(GL_UPPER_LEFT, GL_ZERO_TO_ONE) call in the attached example. That is, it should doesn't matter where's the clip origin, as the primitive completely cover the viewport, and all fragments have the same color. - But that's not what's happening: - if one comments out glClipControl one gets the viewport pixels painted red - if one does not comment the glClipControl only some viewport pixels are painted red, but some are painted black, which really shouldn't happen. For reference, this are a few modifications I did to the attached sample to better understand it: --- fdo121191.c.orig 2016-02-09 11:37:33.122145252 +0000 +++ fdo121191.c 2016-02-09 11:40:23.597820907 +0000 @@ -68,7 +68,8 @@ typedef void (APIENTRYP PFNGLCLIPCONTROLPROC) (GLenum origin, GLenum depth); const auto glClipControl = reinterpret_cast<PFNGLCLIPCONTROLPROC>( SDL_GL_GetProcAddress("glClipControl")); - glClipControl(GL_UPPER_LEFT, GL_ZERO_TO_ONE); + // XXX: We should get the same rendering regardless we have 1 or 0 here + if (1) glClipControl(GL_UPPER_LEFT, GL_ZERO_TO_ONE); GLuint texture; glGenTextures(1, &texture); @@ -160,7 +161,12 @@ assert(glGetError() == GL_NO_ERROR); - std::cout << colours[0] << ", " << colours[2] << "\n"; + for (unsigned y = 0; y < textureSizeY; ++y) { + for (unsigned x = 0; x < textureSizeX; ++x) { + std::cout << colours[y*textureSizeX + x] << "\t"; + } + std::cout << "\n"; + } // This was outside the viewport, so should be black from the clear assert(colours[0] == 0.0f); // This was inside the viewport, so should be red from the draw
(In reply to Mathias Fröhlich from comment #4) > I believe that the piglit tests don't uses non zero y values for > the viewport. May be we want to add a test there also. Definitely. And in fact James example seems an excellent way to verify it.
I think Ilia's fix in comment #1 is correct. BTW, the piglit clip-control test currently passes with Mesa but fails with nvidia's driver. I'll try to take a closer look.
Mathias, Jose, and Brian -- thanks to all of you for taking a look. Sounds like everyone's in agreement that Mesa is wrong. I'll get the ball rolling on throwing something into piglit and sending out my patch from comment #1 as a proper patch.
Brian, I had the same experience with the nvidia driver back when I wrote the test some time ago. But if I recall correctly, then the nvidia driver does agree with the test in any of these tested state combinations. But the piglit test wildly switches between the state combinations in between draws which probably exposes an internal update problem in the blob. May be you will prove me wrong, but keep that in mind. I believe in the end I could get the expected from the binary blob by only changing clip control at the beginning of a new frame or something similar. Greetings Mathias
Should be fixed with commit fe14110f359b0665cb0c09aa14f13a5ebb33b1bc There's also a new piglit test for glClipControl + glViewport.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.