Bug 59648

Summary: [SNB/IVB/HSW Bisected]Piglit spec/ARB_uniform_buffer/object_layout-std140-base-size-and-alignment fails
Product: Mesa Reporter: lu hua <huax.lu>
Component: glsl-compilerAssignee: Matt Turner <mattst88>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: medium CC: idr, xunx.fang
Version: unspecified   
Hardware: All   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Bug Depends on:    
Bug Blocks: 67224    
Attachments: patch

Description lu hua 2013-01-21 03:28:36 UTC
System Environment:
--------------------------
Arch:           i386
Platform:       sandybridge
Libdrm:		(master)libdrm-2.4.41-3-g303ca37e722e68900cb7eb43ddbef8069b0c711b
Mesa:		(master)f9f5c92f734c517c1bd486f5a3b1931ea6f871a5
Xserver:(master)xorg-server-1.13.99.901-2-g6703a7c7cf1a349c137e247a0c8eb462ff7b07be
Xf86_video_intel:(master)2.20.18-15-gc9263f192e2f85dd961bc1c4e9ca8180db874517
Cairo:		(master)ed2fa6b16b03fccc3e21598cdb9157cbcebd1d37
Libva:		(staging)21649988d6b532cc96f633db017d1e4369f640e9
Libva_intel_driver:(staging)d206b47a6ac86c089149ecd71b01eea6ebda5796
Kernel:	(drm-intel-nightly) 862f37d8092266e70907e6060fc8f6f96c7a23e7

Bug detailed description:
-------------------------
It fails on  sandybridge, ivybridge and haswell with mesa master branch. It works well on 9.0 branch.
Case spec_ARB_uniform_buffer_object_getactiveuniformblockiv-uniform-block-data-size also fails and has same bisect commit.

Bisect shows:bb47a4d081c383a2c42555c2bbde2f5d8ee2412c is the first bad commit
commit bb47a4d081c383a2c42555c2bbde2f5d8ee2412c
Author:     Ian Romanick <ian.d.romanick@intel.com>
AuthorDate: Wed Jan 16 12:01:50 2013 -0800
Commit:     Ian Romanick <ian.d.romanick@intel.com>
CommitDate: Fri Jan 18 17:35:32 2013 -0800

    glsl: Reject row_major and column_major on non-matrix types

    About both row_major and column_major layout qualifiers, the GLSL spec
    says:

        "It only affects the layout of matrices."

    However, the OpenGL ES 3.0 conformance tests have taken this to mean it
    is an error use it elsewhere.  This seems logical given that
    'layout(row_major) vec4 foo' is probably not what the programmer meant.

    The only catch is dealing with structures that contain matrices.  Layout
    qualifiers cannot be applied directly to fields of structures, so the
    only way to affect the layout of the fields is to apply a qualifier to
    the structure declaration itself.  There is ongoing debate about this
    within Khronos, and it seems to be settling in favor of allowing the
    qualifiers on structures.  I light of this, I have chosen to allow the
    qualifiers on structures but emit a warning since the usage may not be
    portable.

    Fixes gles3conform test
    uniform_buffer_object_layouts_not_for_matrix_type and causes no
    regressions.


output:
type        row_major   offset  expected offset     size expected size
float               n        4 4                       4 4
Failed to compile fragment shader: 0:4(27): error: uniform block layout qualifiers row_major and column_major can only be applied to matrix and structure types

Failed to compile shader:
#version 140
layout(std140) uniform ubo {
        float pad;
        layout(row_major)  float u;
        float size_test;
};

void main() {
        gl_FragColor = vec4(pad);
}
PIGLIT: {'result': 'fail' }


Reproduce steps:
----------------
1. xinit
2. ./bin/arb_uniform_buffer_object-layout-std140-base-size-and-alignment -auto -fbo
Comment 1 Ian Romanick 2013-01-29 00:55:15 UTC

*** This bug has been marked as a duplicate of bug 59957 ***
Comment 2 fangxun 2013-02-18 08:21:10 UTC
With latest piglit, it still fails on mesa master and 9.1 branch.

Output:
type        row_major   offset  expected offset     size expected size
float               n        4 4                       4 4
float               y        4 4                       4 4
vec2                n        8 8                       8 8
vec2                y        8 8                       8 8
vec3                n       16 16                     12 12
vec3                y       16 16                     12 12
vec4                n       16 16                     16 16
vec4                y       16 16                     16 16
int                 n        4 4                       4 4
int                 y        4 4                       4 4
ivec2               n        8 8                       8 8
ivec2               y        8 8                       8 8
ivec3               n       16 16                     12 12
ivec3               y       16 16                     12 12
ivec4               n       16 16                     16 16
ivec4               y       16 16                     16 16
uint                n        4 4                       4 4
uint                y        4 4                       4 4
uvec2               n        8 8                       8 8
uvec2               y        8 8                       8 8
uvec3               n       16 16                     12 12
uvec3               y       16 16                     12 12
uvec4               n       16 16                     16 16
uvec4               y       16 16                     16 16
Failed to compile fragment shader: 0:9(30): error: Operands to arithmetic operators must
 be numeric
0:9(41): error: Operands to arithmetic operators must be numeric
0:9(17): error: cannot construct `vec4' from a non-numeric data type

Failed to compile shader:
#version 140
layout(std140) uniform ubo {
        float pad;
         bool u;
        float size_test;
};

void main() {
        gl_FragColor = vec4(pad + u + size_test);
}
PIGLIT: {'result': 'fail' }
Comment 3 Ian Romanick 2013-02-19 21:08:26 UTC
It's obvious that the test is broken... you can't add a bool and a vec4.  I'll have to fix it harder.
Comment 4 Eric Anholt 2013-02-21 01:36:44 UTC
commit e656145aed68c680b8a9cfb0d07b74d7928a607c
Author: Eric Anholt <eric@anholt.net>
Date:   Tue Feb 19 12:00:02 2013 -0800

    layout-std140-base-size-and-alignment: Fix running the test.
Comment 5 lu hua 2013-02-26 05:46:49 UTC
spec/ARB_uniform_buffer/object_layout-std140-base-size-and-alignment fixed.

spec/ARB_uniform_buffer/object_getactiveuniformblockiv-uniform-block-data-size still fails.
output:
type                  row_major  DATA_SIZE   expected
float                         n         16         16
Failed to compile fragment shader: 0:0(0): error: uniform block layout qualifiers row_major and column_major can only be applied to matrix and structure types

Failed to compile shader:
#version 140
layout(std140) uniform ubo {
        float align_test;
        layout(row_major) float u;
};

void main() {
        gl_FragColor = vec4(align_test);
}
PIGLIT: {'result': 'fail' }
Comment 6 Ian Romanick 2013-07-23 18:15:36 UTC
Updated versions of both desktop GLSL and OpenGL ES 3.0 GLSL spec allow these layout qualifiers on more types.  The GLSL 4.40 spec says:

    "The row_major and column_major qualifiers affect the layout of
    matrices, including all matrices contained in structures and arrays
    they are applied to, to all depths of nesting. These qualifiers can
    be applied to other types, but will have no effect."

Mesa should be updated.
Comment 7 Matt Turner 2013-07-25 19:11:42 UTC
Created attachment 83004 [details] [review]
patch

(In reply to comment #6)
> Updated versions of both desktop GLSL and OpenGL ES 3.0 GLSL spec allow
> these layout qualifiers on more types.  The GLSL 4.40 spec says:
> 
>     "The row_major and column_major qualifiers affect the layout of
>     matrices, including all matrices contained in structures and arrays
>     they are applied to, to all depths of nesting. These qualifiers can
>     be applied to other types, but will have no effect."
> 
> Mesa should be updated.

Should we consider this a clarification? If so, see attached patch.

Is the updated ES 3.0 GLSL spec not released yet? 3.00.4 does not contain this text.
Comment 8 Ian Romanick 2013-07-27 05:47:35 UTC
(In reply to comment #7)
> Should we consider this a clarification? If so, see attached patch.
> 
> Is the updated ES 3.0 GLSL spec not released yet? 3.00.4 does not contain
> this text.

I'll double check.  It's always possible that the ES group decided something different and I missed it.
Comment 9 Ian Romanick 2013-07-29 17:56:54 UTC
(In reply to comment #7)
> Created attachment 83004 [details] [review] [review]
> patch
> 
> (In reply to comment #6)
> > Updated versions of both desktop GLSL and OpenGL ES 3.0 GLSL spec allow
> > these layout qualifiers on more types.  The GLSL 4.40 spec says:
> > 
> >     "The row_major and column_major qualifiers affect the layout of
> >     matrices, including all matrices contained in structures and arrays
> >     they are applied to, to all depths of nesting. These qualifiers can
> >     be applied to other types, but will have no effect."
> > 
> > Mesa should be updated.
> 
> Should we consider this a clarification? If so, see attached patch.

Still trying to sort this out.

> Is the updated ES 3.0 GLSL spec not released yet? 3.00.4 does not contain
> this text.

I think I'd like to keep the warnings.  There are still shipping compilers, especially for ES3, that will reject this usage.  The warning check can be simplified to just !type->is_matrix(), and never generate the error... at least on desktop.
Comment 10 Matt Turner 2013-08-15 18:27:29 UTC
Patches on the list.
Comment 11 Matt Turner 2013-08-22 09:35:24 UTC
Committed.

1a45db97058d63b742993d0fcf0e5df44e2aa1c6
77373e020ecc1b156802a583745dc6ce16e91c9e
921ef55a72dd07a6db0f170767d4a278c46d9ae6
dded321f92e4727584a98b71d7aaa15d4f01fb24
Comment 12 lu hua 2013-08-23 01:57:58 UTC
Verified.Fixed.

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.