changed minimal opengl version to 3.1

[christiankerl]

this should maximize compatibility with older mesa versions. just tested on nvidia gpu, could someone try this on intel/mesa? downgrading to opengl 3.0 would require to remove the use of rectangle textures (this would be some more work...)

[xlz]

What about adding glfwSetErrorCallback, and glGetError at places?

[christiankerl]

@xlz where? why? is it related to the lower version or just to improve error handling?

[xlz]

To improve diagnosis for common situations like the output being black, since there isn't much information printed about internal OpenGL errors.

I can make a separate PR for this later, but since you already have an OpenGL PR going on, I thought this would be an easy addition.

glfwseterrorcallback is called before glfwinit.
In principle glgeterror is needed after every OpenGL call, but I think only those prone to errors (shaders, between frames) need this.

[floe]

@christiankerl couldn't you just use GL_EXT_texture_rectangle instead of GL_ARB_texture_rectangle? However, I'm not sure if it's worth the hassle, 3.1 is already very widely supported AFAICT.

BTW, unfortunately this still doesn't change anything on Intel GPU under Linux: all depth windows remain black :-/ @xlz's suggestion of liberal glGetError usage is probably the only way to deal with this at some point.

[xlz]

After adding a bunch of glGetError, the first error is

glTexImage2D(GL_TEXTURE_RECTANGLE, 0, 0x8234, 352, 4240, 0, 0x8d94, 0x1403, NULL): invalid value

Intel Mesa has GL_MAX_RECTANGLE_TEXTURE_SIZE=4096, but this is asking for 4240. All Linux/Intel cards have GL_MAX_RECTANGLE_TEXTURE_SIZE<=4096. Windows Intel drivers do have 8192 size, but Intel Mesa limits it to 4096 because

commit 954dfba12986f578f2d8461818f9e9ac1f8f2b41
Author: Keith Packard <[email protected]>
Date:   Fri Jan 30 21:51:32 2009 -0800

    i965: bump texture limit to 4kx4k

    Rendering and textures are limited to 8kx8k, but mesa limits things to
    4kx4k, and magic guard band stuff may break on 8kx8k drawing. This is safe
    though, and makes compiz work on bigger screens.

    Signed-off-by: Keith Packard <[email protected]>

I think we can drop the 10th frame and allocate 352x3816 (424*9).

[xlz]

The second error is

FBO incomplete: Unsupported HW texture/renderbuffer format attached: MESA_FORMAT_RGB_FLOAT32

The solution is to change F32C3 to F32C4 for stage1_data and filter1_data.

[xlz]

The last issue is black output with Ubuntu 14.04.

3.3 (Core Profile) Mesa 10.1.3 does not work.

3.3 (Core Profile) Mesa 10.7.0-devel (git-86a74e9 2015-06-03 trusty-oibaf-ppa) works.

Mesa 10.1.3 also works with this change in stage1.fs:

-  bvec3 invalid_pixel = bvec3(!valid_pixel);
+  vec3 invalid_pixel = vec3(!valid_pixel);

   A    = mix(vec3(ab0.x, ab1.x, ab2.x), vec3(0.0), invalid_pixel);

A is zero with bvec3. GLSL mix() doesn't say bvec3 is wrong though.

[christiankerl]

so with all these changes it works on mesa 10.1.3 on intel gpu? I will try to update the PR

[xlz]

Yes. I had it working with Mesa 10.1.3 all the changes.

The first one: allocate(352, 424*9) and std:copy(..., buffer_size/10*9, input_data...)

The second one, s/F32C3/F32C4/.

But the last one is only a workaround. I have not found the root cause. I think there will be a bit performance penalty to use vec3 instead of bvec3.

[xlz]

The root cause of the last issue is this (fixed in this commit):

commit 2e51dc838be177a09f60958da7d1d904f1038d9c
Author: Matt Turner <[email protected]>
Date:   Fri Aug 8 21:00:31 2014 -0700

    i965: Use ~0 to represent true on Gen >= 6.

    total instructions in shared programs: 4292303 -> 4288650 (-0.09%)
    instructions in affected programs:     299670 -> 296017 (-1.22%)

    Reviewed-by: Anuj Phogat <[email protected]>

diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
index 12f898a..216b788 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -483,7 +483,32 @@ brw_initialize_context_constants(struct brw_context *brw)
       ctx->Const.QuadsFollowProvokingVertexConvention = false;

    ctx->Const.NativeIntegers = true;
-   ctx->Const.UniformBooleanTrue = 1;
+
+   /* Regarding the CMP instruction, the Ivybridge PRM says:
+    *
+    *   "For each enabled channel 0b or 1b is assigned to the appropriate flag
+    *    bit and 0/all zeros or all ones (e.g, byte 0xFF, word 0xFFFF, DWord
+    *    0xFFFFFFFF) is assigned to dst."
+    *
+    * but PRMs for earlier generations say
+    *
+    *   "In dword format, one GRF may store up to 8 results. When the register
+    *    is used later as a vector of Booleans, as only LSB at each channel
+    *    contains meaning [sic] data, software should make sure all higher bits
+    *    are masked out (e.g. by 'and-ing' an [sic] 0x01 constant)."
+    *
+    * We select the representation of a true boolean uniform to match what the
+    * CMP instruction returns.
+    *
+    * The Sandybridge BSpec's description of the CMP instruction matches that
+    * of the Ivybridge PRM. (The description in the Sandybridge PRM is seems
+    * to have not been updated from Ironlake). Its CMP instruction behaves like
+    * Ivybridge and newer.
+    */
+   if (brw->gen >= 6)
+      ctx->Const.UniformBooleanTrue = ~0;
+   else
+      ctx->Const.UniformBooleanTrue = 1;

[floe]

Closing in favor of #368.