<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> On 07. aug. 2018 11:31, Dave Airlie wrote: <blockquote type="cite" cite="mid:CAPM=9twdPXcZbKvA2AwYh34h77qAqPQUko3H52gaqnL5rpLi-Q@mail.gmail.com"> <div dir="auto"> <div> <div class="gmail_quote"> <div dir="ltr">On Tue., 7 Aug. 2018, 18:44 Erik Faye-Lund, <<a href="mailto:erik.faye-lund@collabora.com" moz-do-not-send="true">erik.faye-lund@collabora.com</a>> wrote: </div> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 07. aug. 2018 02:51, Dave Airlie wrote: > On 7 August 2018 at 04:38, Erik Faye-Lund <<a href="mailto:erik.faye-lund@collabora.com" target="_blank" rel="noreferrer" moz-do-not-send="true">erik.faye-lund@collabora.com</a>> wrote: >> OpenGL ES 3.1 doesn't support using glUniform1i() for binding >> images, so let's use layout qualifiers instead. This should in >> theory be slightly more performant as well, as we do less API >> calls. > I like this idea, I think we can get rid of a bunch of stuff with > explicit bindings, > > However I think explicit layout bindings on GL require a shader extension, > > At least on desktop GL a lot of tests fail after I apply this. > > Dave. Hmm, that's odd. I don't think there's any extensions that should be required by this, but I'll take a look. Thanks for pointing this out. </blockquote> </div> </div> <div dir="auto"> </div> <div dir="auto">Our baseline glsl version is 130 btw </div> <div dir="auto"> </div> </div> </blockquote> Right. I think I got a relevant error here: vrend_compile_shader: context error reported 8 "Xorg" Illegal shader 0 shader failed to compile 0:9(1): error: the "binding" qualifier only applies to uniform blocks, storage blocks, opaque variables, or arrays thereof GLSL: #version 330 #extension GL_ARB_compute_shader : require #extension GL_ARB_shader_storage_buffer_object : require #extension GL_ARB_shader_bit_encoding : require #extension GL_ARB_shader_image_load_store : require layout (local_size_x = 2, local_size_y = 4, local_size_z = 1) in; vec4 temp0[3]; uint ssbo_addr_temp; layout(binding=0, r32ui) uniform uimage2D csimg0; layout (binding = 6, std430) buffer csssbo6 { uint csssbocontents6[]; }; void main(void) { temp0[0].xy = vec2(uintBitsToFloat((uvec4(gl_WorkGroupID.x, gl_WorkGroupID.y, gl_WorkGroupID.z, gl_WorkGroupID.z) * (uvec4(2U,4U,1U,1U)) + uvec4(gl_LocalInvocationID.x, gl_LocalInvocationID.y, gl_LocalInvocationID.z, gl_LocalInvocationID.z)).xy)); temp0[1].x = float(uintBitsToFloat((uvec4(uvec4(gl_NumWorkGroups.x, gl_NumWorkGroups.x, gl_NumWorkGroups.x, gl_NumWorkGroups.x)) * uvec4((uvec4(2U,2U,2U,2U))))).x); temp0[2].x = uintBitsToFloat(imageLoad(csimg0, ivec2(floatBitsToInt( temp0[0].xyxx ))).x); temp0[0].x = float(uintBitsToFloat((floatBitsToUint(temp0[0].yyyy) * floatBitsToUint(temp0[1].xxxx) + floatBitsToUint(temp0[0].xxxx)).x)); temp0[0].x = float(uintBitsToFloat((uvec4(floatBitsToUint(temp0[0].xxxx)) * uvec4((uvec4(16U,16U,16U,16U))))).x); csssbocontents6[uint(floatBitsToUint( temp0[0].xxxx ))>>2] = floatBitsToUint( temp0[2].xxxx ).x; } Checking the mesa source that's from: ---8<--- } else if ((state->is_version(420, 310) || state->ARB_shading_language_420pack_enable) && base_type->is_image()) { assert(ctx->Const.MaxImageUnits <= MAX_IMAGE_UNITS); if (max_index >= ctx->Const.MaxImageUnits) { _mesa_glsl_error(loc, state, "Image binding %d exceeds the " "maximum number of image units (%d)", max_index, ctx->Const.MaxImageUnits); return; } } else { _mesa_glsl_error(loc, state, "the \"binding\" qualifier only applies to uniform " "blocks, storage blocks, opaque variables, or arrays " "thereof"); return; } ---8<--- ... Yeah, seems we need GLSL 420 (or ARB_shading_language_420pack_enable) to use this construct. So, question is... Should we a) make GLSL 420 a hard requirement for images, or b) keep the old approach if on GL 4.1 (or ealier) + GL_ARB_shader_image_load_store? I guess a) is the "nicer" thing to do, but it makes the support burden a bit worse. </body> </html>