Bug 90961

Summary: [Regression, bisected] spec.ext_transform_feedback.structs_gles3 tests fail
Product: Mesa Reporter: Mark Janes <mark.a.janes>
Component: Drivers/DRI/i965Assignee: Ian Romanick <idr>
Status: RESOLVED FIXED QA Contact: Intel 3D Bugs Mailing List <intel-3d-bugs>
Severity: normal    
Priority: medium CC: mark.a.janes, t_arceri
Version: git   
Hardware: x86-64 (AMD64)   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:

Description Mark Janes 2015-06-13 00:17:01 UTC
The series 3d78bdea3155ff3f19a782e0eb3a55612bfd8dd0 - 0d2068a92d74f421960947e589cf56a2b125035f caused 29 piglit tests to regress.

Tim Arceri feels like the tests are probably wrong:

          From section 4.3.6 of the GLSL ES 3.0 spec: "Vertex output
          variables output per-vertex data and are declared using the
          out storage qualifier or the centroid out storage qualifier.
          They can only be float, floating-point vectors, matrices,
          signed or unsigned integers or integer vectors, or arrays or
          structures of any these."

We need a second opinion on the language and the tests, before deleting or fixing the tests.  The test list:

    piglit.spec.ext_transform_feedback.structs_gles3 array-struct error
    piglit.spec.ext_transform_feedback.structs_gles3 array-struct get
    piglit.spec.ext_transform_feedback.structs_gles3 array-struct run
    piglit.spec.ext_transform_feedback.structs_gles3 array-struct run-no-fs
    piglit.spec.ext_transform_feedback.structs_gles3 array-struct-array-elem error
    piglit.spec.ext_transform_feedback.structs_gles3 array-struct-array-elem get
    piglit.spec.ext_transform_feedback.structs_gles3 array-struct-array-elem run
    piglit.spec.ext_transform_feedback.structs_gles3 array-struct-array-elem run-no-fs
    piglit.spec.ext_transform_feedback.structs_gles3 array-struct-array-struct error
    piglit.spec.ext_transform_feedback.structs_gles3 array-struct-array-struct get
    piglit.spec.ext_transform_feedback.structs_gles3 array-struct-array-struct run
    piglit.spec.ext_transform_feedback.structs_gles3 array-struct-array-struct run-no-fs
    piglit.spec.ext_transform_feedback.structs_gles3 array-struct-whole-array error
    piglit.spec.ext_transform_feedback.structs_gles3 array-struct-whole-array get
    piglit.spec.ext_transform_feedback.structs_gles3 array-struct-whole-array run
    piglit.spec.ext_transform_feedback.structs_gles3 array-struct-whole-array run-no-fs
    piglit.spec.ext_transform_feedback.structs_gles3 struct-array-elem error
    piglit.spec.ext_transform_feedback.structs_gles3 struct-array-elem get
    piglit.spec.ext_transform_feedback.structs_gles3 struct-array-elem run
    piglit.spec.ext_transform_feedback.structs_gles3 struct-array-elem run-no-fs
    piglit.spec.ext_transform_feedback.structs_gles3 struct-struct error
    piglit.spec.ext_transform_feedback.structs_gles3 struct-struct get
    piglit.spec.ext_transform_feedback.structs_gles3 struct-struct run
    piglit.spec.ext_transform_feedback.structs_gles3 struct-struct run-no-fs
    piglit.spec.ext_transform_feedback.structs_gles3 struct-whole-array error
    piglit.spec.ext_transform_feedback.structs_gles3 struct-whole-array get
    piglit.spec.ext_transform_feedback.structs_gles3 struct-whole-array run
    piglit.spec.ext_transform_feedback.structs_gles3 struct-whole-array run-no-fs
    piglit.spec.glsl-es-3_00.execution.varying-struct-arrays
Comment 1 Timothy Arceri 2015-06-13 01:35:54 UTC
I've submitted a patch for piglit to stop running some of the test under gles here:

http://lists.freedesktop.org/archives/piglit/2015-June/016235.html
Comment 2 Timothy Arceri 2015-06-13 01:51:27 UTC
It seems desktop specs before 4.40 used the same line as the ES 3.0 spec, but the test don't fail to compile on closed source Nvidia drivers (although they do seem to fail execution).

However arrays of structs or structs that contain arrays or structs are clearly allowed from 4.40 on.

My head hurts, Ian please help.
Comment 3 Ian Romanick 2015-06-15 18:39:30 UTC
(In reply to Timothy Arceri from comment #2)
> It seems desktop specs before 4.40 used the same line as the ES 3.0 spec,
> but the test don't fail to compile on closed source Nvidia drivers (although
> they do seem to fail execution).
> 
> However arrays of structs or structs that contain arrays or structs are
> clearly allowed from 4.40 on.
> 
> My head hurts, Ian please help.

Basically, the ES group interpreted the line differently than the desktop group.  There was discussion about this while bringing arrays-of-arrays to ES, and that precipitated the clarification in the desktop spec.

Frankly, unless there is a conformance test or a dEQP test that requires the restricted behavior, I'd rather not start enforcing it.  Since the code is already landed, it's probably not worth the effort to undo it.  My best guess is that we should just remove the gles3 tests that require out-of-spec behavior.

Also... if a patch series is known to cause a big pile of tests to fail (you did run piglit before pushing, right?), that needs to be resolved first.
Comment 4 Timothy Arceri 2015-06-16 10:39:31 UTC
(In reply to Ian Romanick from comment #3)
> Basically, the ES group interpreted the line differently than the desktop
> group.  There was discussion about this while bringing arrays-of-arrays to
> ES, and that precipitated the clarification in the desktop spec.

Thanks for clearing this up Ian.

> 
> Frankly, unless there is a conformance test or a dEQP test that requires the
> restricted behavior, I'd rather not start enforcing it.  Since the code is
> already landed, it's probably not worth the effort to undo it.

I added it because it seemed related to arrays of arrays, if you can't have an array of structs or nested structs then you can't have arrays of arrays of nested structs.

I don't mind undoing it if you think its a good idea.

>  My best
> guess is that we should just remove the gles3 tests that require out-of-spec
> behavior.

On the up side all the tests have a desktop version so not much is lost.

> 
> Also... if a patch series is known to cause a big pile of tests to fail (you
> did run piglit before pushing, right?), that needs to be resolved first.

Yeah I didn't know I'd cased so many tests to fail until Mark pointed it out on IRC. Turns out I didn't have ES enabled in my piglit build.
Comment 5 Timothy Arceri 2015-06-18 08:46:39 UTC
Piglit fixed with this commit f85bcce2be1f729d1d6b3f2b5738b17635a569c2

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.