Summary: | gl_BaseVertex should always be zero when the draw command has no <basevertex> parameter | ||
---|---|---|---|
Product: | Mesa | Reporter: | Antia Puentes <apuentes> |
Component: | Drivers/DRI/i965 | Assignee: | Antia Puentes <apuentes> |
Status: | RESOLVED FIXED | QA Contact: | Intel 3D Bugs Mailing List <intel-3d-bugs> |
Severity: | normal | ||
Priority: | medium | CC: | apuentes, idr, kenneth |
Version: | git | ||
Hardware: | Other | ||
OS: | All | ||
Whiteboard: | |||
i915 platform: | i915 features: | ||
Bug Depends on: | |||
Bug Blocks: | 102590 |
Description
Antia Puentes
2017-09-12 16:16:14 UTC
I believe Antia is working on a fix. (In reply to Kenneth Graunke from comment #1) > I believe Antia is working on a fix. I have a patch that fixes the tests. I am currently testing for regressions, if there isn't any I will send it shortly. The patch also fixes: KHR-GL45.shader_draw_parameters_tests.ShaderMultiDrawArraysIndirectCountParameters which was failing for the same reason. This failure was unnoticed when I filled the bug because at the time we were not supporting ARB_indirect_parameters. (In reply to Antia Puentes from comment #2) > (In reply to Kenneth Graunke from comment #1) > > I believe Antia is working on a fix. > > I have a patch that fixes the tests. I am currently testing for regressions, > if there isn't any I will send it shortly. The patch produced regressions related to gl_VertexID. Specifically in the tests which involve gl*DrawArrays*, pass a value different from zero as argument of the <first> parameter of the draw command, and check if gl_VertexID is correct. According to the spec, for non-indexed draw calls: "The vertex ID of the ith element transferred is <first> + i." Mesa's compiler lowers the gl_VertexID as gl_VertexID(base zero) + gl_BaseVertex, which is the way expected for indexed draw calls. For non-indexed draw calls, this currently works because we use the <first> argument as gl_BaseVertex. If we give gl_BaseVertex a zero value for gl*DrawArrays* as the spec says, the gl_VertexID will be incorrect. Not sure how to fix the gl_VertexID issue, it looks like the i965 does not have a way to add a base value during the vertex-id generation. Ian, any thoughts? This just feels wrong at this point...I thought the intention was that gl_VertexID - gl_BaseVertex would be the zero-based vertex ID. But if gl_VertexID is <first + i>, and gl_BaseVertex is required to be 0 for DrawArrays, then...there is no GLSL variable to get <first>, so there is no way to get the zero-based index. It seems like gl_BaseVertex really ought to be <first> for DrawArrays. glDrawArrays behaves as if you did unsigned indices[count]; for (i = 0; i < count; i++) indices[i] = first + count; glDrawElements(mode, count, GL_UNSIGNED_INT, indices); The language in the spec is a bit more complicated than that, IIRC, but it should be equivalent. I think gl_BaseVertex=0 and gl_VertexID=first+i is consistent with that. Fixed by: commit 3a1df14a7b5c1652aa72eb6cf43e69ab447c6273 Author: Antia Puentes <apuentes@igalia.com> Date: Sat Apr 28 14:09:22 2018 +0200 intel: activate the gl_BaseVertex lowering Surplus code related to the basevertex is removed. The Vertex Elements contain now: * VE 1: <firstvertex, BaseInstance, VertexID, InstanceID> * VE 2: <DrawID, is_indexed_draw, 0, 0> Also fixes unreachable message. Fixes OpenGL CTS tests: * KHR-GL46.shader_draw_parameters_tests.ShaderDrawArraysInstancedParameters * KHR-GL46.shader_draw_parameters_tests.ShaderMultiDrawArraysParameters * KHR-GL46.shader_draw_parameters_tests.MultiDrawArraysIndirectCountParameters * KHR-GL46.shader_draw_parameters_tests.ShaderDrawArraysParameters * KHR-GL46.shader_draw_parameters_tests.ShaderMultiDrawArraysIndirectParameters Fixes Piglit tests: * arb_shader_draw_parameters-drawid-indirect baseinstance * arb_shader_draw_parameters-basevertex Reviewed-by: Jason Ekstrand <jason@jlekstrand.net> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102678 |
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.