Bug 93813

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.