Bug 84677 - Triangle disappears with glPolygonMode GL_LINE
Summary: Triangle disappears with glPolygonMode GL_LINE
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: 10.2
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Neil Roberts
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2014-10-05 00:46 UTC by Ben Foppa
Modified: 2015-11-30 18:19 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
main.c demonstrating the issue (2.90 KB, text/plain)
2014-10-05 00:46 UTC, Ben Foppa
Details

Description Ben Foppa 2014-10-05 00:46:53 UTC
Created attachment 107340 [details]
main.c demonstrating the issue

A triangle that renders fine with GL_FILL disappears when rendered with GL_LINE; turning off DRI causes GL_LINE to work as expected.
Comment 1 Ben Foppa 2014-10-05 00:48:18 UTC
shaders accompanying main.c:

fragment shader:

    #version 330 core

    out vec4 frag_color;

    void main() {
      frag_color = vec4(1, 1, 0, 1); 
    }

vertex shader:

    #version 330 core

    void main() {
      if(gl_VertexID == 0) {
        gl_Position = vec4(-1, -1, 0, 1);
      } else if(gl_VertexID == 1) {
        gl_Position = vec4(1, -1, 0, 1);
      } else if(gl_VertexID == 2) {
        gl_Position = vec4(-1, 1, 0, 1);
      }
    }
Comment 2 Tapani Pälli 2014-10-06 05:31:52 UTC
On quick inspection it looks this bug is about gl_VertexID not working correctly. If I create a VBO with triangle points in the test case, triangle gets rendered correctly.
Comment 3 Neil Roberts 2014-10-22 18:06:22 UTC
gl_VertexID is implemented by adding in an extra vertex attribute in brw_emit_vertices. It has a special source which says to set the value of one of the components to the vertex ID. However the problem seems to be that when you set the polygon mode to something other than fill then that same function also sneaks in another attribute to upload the edge flags. This gets added before the attribute for the vertex ID so I think it is no longer in the correct register when the vertex shader tries to read it.

A comment in the function says that the attribute for the edge flags has to be the last one. I guess that implies the order of these attributes should be swapped around. Indeed it seems to fix the bug if I just swap them over. However I'm not confident whether the edge flags are actually being uploaded correctly because I've only tested it with a single triangle and it could be reading the edge flags from the wrong attribute and it just coincidentally looks ok.

There is a warning in the gen8 version of the function (gen8_emit_vertices) that says something about needing to reorder the attributes when VID is used with edge flags. This was added by Ken so maybe he has some idea about what needs to be done here.

I will try experimenting with more complex geometry tomorrow to determine if the edge flags are working correctly when the order is swapped.
Comment 4 Neil Roberts 2014-10-23 19:03:03 UTC
I can't see any reason why swapping the attributes wouldn't work so I've posted the patch to the mailing list here:

http://lists.freedesktop.org/archives/mesa-dev/2014-October/069706.html

I've also posted a Piglit test here:

http://lists.freedesktop.org/archives/piglit/2014-October/013052.html
Comment 5 Ben Foppa 2014-10-23 20:02:36 UTC
Thanks a lot for your help!
Comment 6 Matt Turner 2015-03-05 00:42:04 UTC
Ugh. Looks like this got lost.

I'd be nice if we could fix this up (for Gen8+ as well, I just confirmed that the piglit test fails) for 10.5.
Comment 7 Neil Roberts 2015-03-11 16:46:49 UTC
I don't have any Broadwell hardware to test on but I tried it with Skylake after doing the change for gen8_emit_vertices but for some reason it doesn't fix the bug. It looks like neither the edge flags nor the vertex ID are working because the test case draws all 6 points in the same location. I confirmed this by setting the blend mode to additive. It's a bit surprising because it really looks like the same problem should apply on Gen8+. It'd be great if someone could test it on Broadwell.

In the meantime I don't think it would do any harm to land the existing patch as is and then fix Gen8+ later if someone wants to review it. It would be good to get Ken's opinion as he wrote a comment in gen8_emit_vertices about needing to swap the attributes so he may know more about this.
Comment 8 Neil Roberts 2015-07-13 17:12:31 UTC
I think I've figured out what was going wrong in BDW and I've posted a v2 of the patch here:

http://patchwork.freedesktop.org/patch/54324/
Comment 9 Neil Roberts 2015-11-30 18:19:17 UTC
The patch has been pushed to master and later released in 10.6.6

http://cgit.freedesktop.org/mesa/mesa/commit/?id=fb02b4ec482762ccf2a9fedf24fe6f


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.