<div dir="ltr">On 21 November 2013 05:21, Marek Olšák <span dir="ltr"><<a href="mailto:maraeo@gmail.com" target="_blank">maraeo@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
I currently work on a simpler solution for Gallium. I'm using<br>
AMD_vertex_shader_layer and this vertex shader (I wrote it in TGSI<br>
though):<br>
<br>
void main()<br>
{<br>
gl_Position = gl_Vertex;<br>
gl_TexCoord[0] = MultiTexCoord0; // clear color<br>
gl_Layer = gl_InstanceID;<br>
}<br>
<br>
Then I render the quad with DrawArraysInstanced and set the instance<br>
count = NumLayers. That way each instance (each quad) is sent to a<br>
different layer and everything is cleared in one draw call.<br>
<br>
Drivers not supporting AMD_vertex_shader_layer will have to use a GS though.<br></blockquote><div><br></div><div>Your approach potentially has better performance, but I'm concerned about risk, because I'm hoping to get this patch cherry-picked into 10.0 (which is due to be released in a week). Currently GLSL clear uses 4 different shaders:<br>
<br><div>1. A vertex shader that just sets gl_Position (compilable for either desktop GLSL 1.10 or GLSL ES 1.00)<br></div><div>2. A vertex shader that just sets gl_Position (compilable for either desktop GLSL 1.30 or GLSL ES 3.00)<br>
</div>3. A fragment shader that outputs floating point color (compilable for either desktop GLSL or GLSL ES)<br><div>4. A fragment shader that outputs integer color (compilable for either desktop GLSL 1.30 or GLSL ES 3.00)<br>
<br></div><div>My proposal adds a fifth:<br></div><br><div>5. A geometry shader that sets gl_Position and gl_Layer<br><br></div><div>If we want to use gl_InstanceID and AMD_vertex_shader_layer, we would have to add two more:<br>
</div><br>6. A vertex shader that sets gl_Position and gl_Layer (for drivers that support AMD_vertex_shader_layer)<br></div><div>7. A vertex shader that sets gl_Position and passes through gl_InstanceID to the GS (for drivers that support GS but not AMD_vertex_shader_layer)<br>
</div><br></div><div class="gmail_quote">Testing on the Meta clear path is already pretty thin (at least for i965) because most i965 clears can be done by faster mechanisms (fast color clear, fast depth clear, or the "blorp" color clear). I'm worried about introducing 3 new shaders to a poorly-tested Meta path this late in the release process.<br>
<br></div><div class="gmail_quote">Anyone want to weigh in on this subject from a risk perspective?<br><br></div><div>I'm also wondering how important performance really is in this patch. I suspect it's not a big issue for i965, since i965 usually does clears using faster mechanisms. It's not an issue for i915, since i915 doesn't support geometry shaders. Are there other drivers that use Meta other than those two? If not, I'm tempted to leave this patch as is, based on the reasoning that since Meta is a fallback mechanism, correctness is more important than performance.<br>
<br></div><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class=""><font color="#888888"><br>
Marek<br>
</font></span><div class=""><div class="h5"><br>
<br>
On Wed, Nov 20, 2013 at 5:47 AM, Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> wrote:<br>
> From section 4.4.7 (Layered Framebuffers) of the GLSL 3.2 spec:<br>
><br>
> When the Clear or ClearBuffer* commands are used to clear a<br>
> layered framebuffer attachment, all layers of the attachment are<br>
> cleared.<br>
><br>
> This patch fixes meta clears to properly clear all layers of a layered<br>
> framebuffer attachment. We accomplish this by adding a geometry<br>
> shader to the meta clear program which sets gl_Layer to a uniform<br>
> value. When clearing a layered framebuffer, we execute in a loop,<br>
> setting the uniform to point to each layer in turn.<br>
><br>
> Cc: "10.0" <<a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.freedesktop.org</a>><br>
> ---<br>
> src/mesa/drivers/common/meta.c | 51 +++++++++++++++++++++++++++++++++++++++---<br>
> 1 file changed, 48 insertions(+), 3 deletions(-)<br>
><br>
> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c<br>
> index 99b02ba..a6d5098 100644<br>
> --- a/src/mesa/drivers/common/meta.c<br>
> +++ b/src/mesa/drivers/common/meta.c<br>
> @@ -241,9 +241,11 @@ struct clear_state<br>
> GLuint VBO;<br>
> GLuint ShaderProg;<br>
> GLint ColorLocation;<br>
> + GLint LayerLocation;<br>
><br>
> GLuint IntegerShaderProg;<br>
> GLint IntegerColorLocation;<br>
> + GLint IntegerLayerLocation;<br>
> };<br>
><br>
><br>
> @@ -2145,6 +2147,19 @@ meta_glsl_clear_init(struct gl_context *ctx, struct clear_state *clear)<br>
> "{\n"<br>
> " gl_Position = position;\n"<br>
> "}\n";<br>
> + const char *gs_source =<br>
> + "#version 150\n"<br>
> + "layout(triangles) in;\n"<br>
> + "layout(triangle_strip, max_vertices = 4) out;\n"<br>
> + "uniform int layer;\n"<br>
> + "void main()\n"<br>
> + "{\n"<br>
> + " for (int i = 0; i < 3; i++) {\n"<br>
> + " gl_Layer = layer;\n"<br>
> + " gl_Position = gl_in[i].gl_Position;\n"<br>
> + " EmitVertex();\n"<br>
> + " }\n"<br>
> + "}\n";<br>
> const char *fs_source =<br>
> "#ifdef GL_ES\n"<br>
> "precision highp float;\n"<br>
> @@ -2154,7 +2169,7 @@ meta_glsl_clear_init(struct gl_context *ctx, struct clear_state *clear)<br>
> "{\n"<br>
> " gl_FragColor = color;\n"<br>
> "}\n";<br>
> - GLuint vs, fs;<br>
> + GLuint vs, gs = 0, fs;<br>
> bool has_integer_textures;<br>
><br>
> if (clear->ArrayObj != 0)<br>
> @@ -2176,6 +2191,12 @@ meta_glsl_clear_init(struct gl_context *ctx, struct clear_state *clear)<br>
> _mesa_ShaderSource(vs, 1, &vs_source, NULL);<br>
> _mesa_CompileShader(vs);<br>
><br>
> + if (_mesa_has_geometry_shaders(ctx)) {<br>
> + gs = _mesa_CreateShaderObjectARB(GL_GEOMETRY_SHADER);<br>
> + _mesa_ShaderSource(gs, 1, &gs_source, NULL);<br>
> + _mesa_CompileShader(gs);<br>
> + }<br>
> +<br>
> fs = _mesa_CreateShaderObjectARB(GL_FRAGMENT_SHADER);<br>
> _mesa_ShaderSource(fs, 1, &fs_source, NULL);<br>
> _mesa_CompileShader(fs);<br>
> @@ -2183,6 +2204,8 @@ meta_glsl_clear_init(struct gl_context *ctx, struct clear_state *clear)<br>
> clear->ShaderProg = _mesa_CreateProgramObjectARB();<br>
> _mesa_AttachShader(clear->ShaderProg, fs);<br>
> _mesa_DeleteObjectARB(fs);<br>
> + if (gs != 0)<br>
> + _mesa_AttachShader(clear->ShaderProg, gs);<br>
> _mesa_AttachShader(clear->ShaderProg, vs);<br>
> _mesa_DeleteObjectARB(vs);<br>
> _mesa_BindAttribLocation(clear->ShaderProg, 0, "position");<br>
> @@ -2190,6 +2213,10 @@ meta_glsl_clear_init(struct gl_context *ctx, struct clear_state *clear)<br>
><br>
> clear->ColorLocation = _mesa_GetUniformLocation(clear->ShaderProg,<br>
> "color");<br>
> + if (gs != 0) {<br>
> + clear->LayerLocation = _mesa_GetUniformLocation(clear->ShaderProg,<br>
> + "layer");<br>
> + }<br>
><br>
> has_integer_textures = _mesa_is_gles3(ctx) ||<br>
> (_mesa_is_desktop_gl(ctx) && ctx->Const.GLSLVersion >= 130);<br>
> @@ -2227,6 +2254,8 @@ meta_glsl_clear_init(struct gl_context *ctx, struct clear_state *clear)<br>
> clear->IntegerShaderProg = _mesa_CreateProgramObjectARB();<br>
> _mesa_AttachShader(clear->IntegerShaderProg, fs);<br>
> _mesa_DeleteObjectARB(fs);<br>
> + if (gs != 0)<br>
> + _mesa_AttachShader(clear->IntegerShaderProg, gs);<br>
> _mesa_AttachShader(clear->IntegerShaderProg, vs);<br>
> _mesa_DeleteObjectARB(vs);<br>
> _mesa_BindAttribLocation(clear->IntegerShaderProg, 0, "position");<br>
> @@ -2240,7 +2269,13 @@ meta_glsl_clear_init(struct gl_context *ctx, struct clear_state *clear)<br>
><br>
> clear->IntegerColorLocation =<br>
> _mesa_GetUniformLocation(clear->IntegerShaderProg, "color");<br>
> + if (gs != 0) {<br>
> + clear->IntegerLayerLocation =<br>
> + _mesa_GetUniformLocation(clear->IntegerShaderProg, "layer");<br>
> + }<br>
> }<br>
> + if (gs != 0)<br>
> + _mesa_DeleteObjectARB(gs);<br>
> }<br>
><br>
> static void<br>
> @@ -2371,8 +2406,18 @@ _mesa_meta_glsl_Clear(struct gl_context *ctx, GLbitfield buffers)<br>
> _mesa_BufferData(GL_ARRAY_BUFFER_ARB, sizeof(verts), verts,<br>
> GL_DYNAMIC_DRAW_ARB);<br>
><br>
> - /* draw quad */<br>
> - _mesa_DrawArrays(GL_TRIANGLE_FAN, 0, 4);<br>
> + /* draw quad(s) */<br>
> + if (fb->Layered) {<br>
> + for (unsigned layer = 0; layer < fb->NumLayers; layer++) {<br>
> + if (fb->_IntegerColor)<br>
> + _mesa_Uniform1i(clear->IntegerLayerLocation, layer);<br>
> + else<br>
> + _mesa_Uniform1i(clear->LayerLocation, layer);<br>
> + _mesa_DrawArrays(GL_TRIANGLE_FAN, 0, 4);<br>
> + }<br>
> + } else {<br>
> + _mesa_DrawArrays(GL_TRIANGLE_FAN, 0, 4);<br>
> + }<br>
><br>
> _mesa_meta_end(ctx);<br>
> }<br>
> --<br>
> 1.8.4.2<br>
><br>
</div></div><div class=""><div class="h5">> _______________________________________________<br>
> mesa-dev mailing list<br>
> <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</div></div></blockquote></div><br></div></div>