Brian, Did you get a chance to look at my other patch as well: ([PATCH 2/2] _mesa_meta_GenerateMipmap: Generate separate shaders for glsl 120 / 130) ? Any more thoughts on including the code for 3D textures and integer-textures ? I posted my views on this in an earlier e-mail. I'll be happy to make any suggested changes in my patches.
On Wed, Sep 5, 2012 at 10:55 AM, Anuj Phogat <anuj.pho...@gmail.com> wrote: > On Wed, Sep 5, 2012 at 7:58 AM, Brian Paul <bri...@vmware.com> wrote: >> >> On 09/04/2012 08:42 PM, Anuj Phogat wrote: >>> >>> glsl path of _mesa_meta_GenerateMipmap() function would require different >>> fragment >>> shaders depending on the texture target. This patch adds the code to >>> generate >>> appropriate fragment shader programs at run time. >>> Fixes https://bugs.freedesktop.org/show_bug.cgi?id=54296 >>> >>> NOTE: This is a candidate for stable branches. >>> >>> Signed-off-by: Anuj Phogat<anuj.pho...@gmail.com> >>> --- >>> src/mesa/drivers/common/meta.c | 100 >>> +++++++++++++++++++++++++++++++++++---- >>> 1 files changed, 89 insertions(+), 11 deletions(-) >>> >>> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c >>> index 36672a7..7d701f4 100644 >>> --- a/src/mesa/drivers/common/meta.c >>> +++ b/src/mesa/drivers/common/meta.c >>> @@ -286,6 +286,15 @@ struct gen_mipmap_state >>> GLuint IntegerShaderProg; >>> }; >>> >>> +/** >>> + * State for GLSL texture sampler which is used to generate fragment >>> + * shader in _mesa_meta_generate_mipmap(). >>> + */ >>> +struct glsl_sampler { >>> + const char *type; >>> + const char *func; >>> + const char *texcoords; >>> +}; >>> >>> /** >>> * State for texture decompression >>> @@ -2974,7 +2983,7 @@ setup_texture_coords(GLenum faceTarget, >>> >>> static void >>> setup_ff_generate_mipmap(struct gl_context *ctx, >>> - struct gen_mipmap_state *mipmap) >>> + struct gen_mipmap_state *mipmap) >>> { >>> struct vertex { >>> GLfloat x, y, tex[3]; >>> @@ -3004,12 +3013,53 @@ setup_ff_generate_mipmap(struct gl_context *ctx, >>> >>> >>> static void >>> +setup_texture_sampler(GLenum target, struct glsl_sampler *sampler) >>> +{ >>> + switch(target) { >>> + case GL_TEXTURE_1D: >>> + sampler->type = "sampler1D"; >>> + sampler->func = "texture1D"; >>> + sampler->texcoords = "texCoords.x"; >>> + break; >>> + case GL_TEXTURE_2D: >>> + sampler->type = "sampler2D"; >>> + sampler->func = "texture2D"; >>> + sampler->texcoords = "texCoords.xy"; >>> + break; >>> + case GL_TEXTURE_3D: >>> + sampler->type = "sampler3D"; >>> + sampler->func = "texture3D"; >>> + sampler->texcoords = "texCoords"; >>> + break; >>> + case GL_TEXTURE_CUBE_MAP: >>> + sampler->type = "samplerCube"; >>> + sampler->func = "textureCube"; >>> + sampler->texcoords = "texCoords"; >>> + break; >>> + case GL_TEXTURE_1D_ARRAY: >>> + sampler->type = "sampler1DARRAY"; >>> + sampler->texcoords = "texCoords.xy"; >>> + break; >>> + case GL_TEXTURE_2D_ARRAY: >>> + sampler->type = "sampler2DARRAY"; >>> + sampler->texcoords = "texCoords"; >>> + break; >> >> >> Don't you need to set sampler->func for the ARARY cases? > Yes. I have added the support for array textures in my 2nd patch. > Refer to: [PATCH 2/2] _mesa_meta_GenerateMipmap: Generate separate > shaders for glsl 120 / 130 >> Also note that we don't support mipmap generation for 3D textures yet (it's >> a sw fallback) and I don't recall ever testing 1D/2D ARRAY mipmap generation. > 1D/2D array textures are allowed in glGenerateMipmap(). I'll find some > time to add a piglit test case to verify their support. > It might be useful to keep the code for 3D textures, in case we want > to support them. > >>> + default: >>> + /* unexpected texture target */ >>> + return; >>> + } >>> +} >>> + >>> + >>> +static void >>> setup_glsl_generate_mipmap(struct gl_context *ctx, >>> - struct gen_mipmap_state *mipmap) >>> + struct gen_mipmap_state *mipmap, >>> + GLenum target) >>> { >>> struct vertex { >>> GLfloat x, y, tex[3]; >>> }; >>> + struct glsl_sampler sampler; >>> >>> static const char *vs_source = >>> "attribute vec2 position;\n" >>> @@ -3020,14 +3070,17 @@ setup_glsl_generate_mipmap(struct gl_context *ctx, >>> " texCoords = textureCoords;\n" >>> " gl_Position = vec4(position, 0.0, 1.0);\n" >>> "}\n"; >>> - static const char *fs_source = >>> - "uniform sampler2D tex2d;\n" >>> + static const char *fs_template = >>> + "#define SAMPLER_TYPE %s\n" >>> + "#define SAMPLER_FUNCTION %s\n" >>> + "#define TEX_COORDS %s\n" >>> + "uniform SAMPLER_TYPE texSampler;\n" >>> "varying vec3 texCoords;\n" >>> "void main()\n" >>> "{\n" >>> - " gl_FragColor = texture2D(tex2d, texCoords.xy);\n" >>> + " gl_FragColor = SAMPLER_FUNCTION(texSampler, TEX_COORDS);\n" >>> "}\n"; >>> - >>> + >>> static const char *vs_int_source = >>> "#version 130\n" >>> "in vec2 position;\n" >>> @@ -3036,18 +3089,22 @@ setup_glsl_generate_mipmap(struct gl_context *ctx, >>> "void main()\n" >>> "{\n" >>> " texCoords = textureCoords;\n" >>> - " gl_Position = gl_Vertex;\n" >>> + " gl_Position = vec4(position, 0.0, 1.0);\n" >>> "}\n"; >>> - static const char *fs_int_source = >>> + static const char *fs_int_template = >>> "#version 130\n" >>> - "uniform isampler2D tex2d;\n" >>> + "#define SAMPLER_TYPE i%s\n" >>> + "#define TEX_COORDS %s\n" >>> + "uniform SAMPLER_TYPE texSampler;\n" >>> "in vec3 texCoords;\n" >>> "out ivec4 out_color;\n" >>> "\n" >>> "void main()\n" >>> "{\n" >>> - " out_color = texture(tex2d, texCoords.xy);\n" >>> + " out_color = texture(texSampler, TEX_COORDS);\n" >>> "}\n"; >>> + char *fs_source, *fs_int_source; >>> + unsigned fs_alloc_len; >>> GLuint vs, fs; >>> >>> /* Check if already initialized */ >>> @@ -3067,6 +3124,15 @@ setup_glsl_generate_mipmap(struct gl_context *ctx, >>> _mesa_VertexAttribPointerARB(1, 3, GL_FLOAT, GL_FALSE, >>> sizeof(struct vertex), OFFSET(tex)); >>> >>> + /* Generate a fragment shader program appropriate for the texture >>> target */ >>> + setup_texture_sampler(target,&sampler); >>> + fs_alloc_len = strlen(fs_template) + strlen(sampler.type) + >>> + strlen(sampler.func) + strlen(sampler.texcoords) + 1; >>> + fs_source = (char *) malloc(fs_alloc_len); >>> + >>> + sprintf(fs_source, fs_template, >>> + sampler.type, sampler.func, sampler.texcoords); >>> + >>> vs = compile_shader_with_debug(ctx, GL_VERTEX_SHADER, vs_source); >>> fs = compile_shader_with_debug(ctx, GL_FRAGMENT_SHADER, fs_source); >>> >>> @@ -3080,9 +3146,20 @@ setup_glsl_generate_mipmap(struct gl_context *ctx, >>> _mesa_EnableVertexAttribArrayARB(0); >>> _mesa_EnableVertexAttribArrayARB(1); >>> link_program_with_debug(ctx, mipmap->ShaderProg); >>> + free(fs_source); >>> >>> if ((_mesa_is_desktop_gl(ctx)&& ctx->Const.GLSLVersion>= 130) || >>> >>> _mesa_is_gles3(ctx)){ >>> + /* Generate a fragment shader program appropriate for the texture >>> + * target >>> + */ >>> + fs_alloc_len = strlen(fs_int_template) + strlen(sampler.type) + >>> + strlen(sampler.texcoords) + 1; >>> + fs_int_source = (char *) malloc(fs_alloc_len); >>> + >>> + sprintf(fs_int_source, fs_int_template, >>> + sampler.type, sampler.texcoords); >>> + >> >> >> I think the ARB is leaning toward disallowing automatic mipmap generation >> for integer-valued textures. So you might put that part on hold. >> > Code for the integer textures already exists in > _mesa_meta_GenerateMipmap() but it doesn't support all texture targets > at the moment. We might push this code to complete the missing > support, in case ARB hasn't yet reached any conclusion on integer > textures. All the code for integer textures in > _mesa_meta_GenerateMipmap() can be later removed if ARB decides to > disallow them. >> >> >>> vs = compile_shader_with_debug(ctx, GL_VERTEX_SHADER, >>> vs_int_source); >>> fs = compile_shader_with_debug(ctx, GL_FRAGMENT_SHADER, >>> fs_int_source); >>> >>> @@ -3099,6 +3176,7 @@ setup_glsl_generate_mipmap(struct gl_context *ctx, >>> * BindFragDataLocation to 0. >>> */ >>> link_program_with_debug(ctx, mipmap->IntegerShaderProg); >>> + free(fs_int_source); >>> } >>> } >>> >>> @@ -3172,7 +3250,7 @@ _mesa_meta_GenerateMipmap(struct gl_context *ctx, >>> GLenum target, >>> * GenerateMipmap function. >>> */ >>> if (use_glsl_version) { >>> - setup_glsl_generate_mipmap(ctx, mipmap); >>> + setup_glsl_generate_mipmap(ctx, mipmap, target); >>> >>> if (texObj->_IsIntegerFormat) >>> _mesa_UseProgramObjectARB(mipmap->IntegerShaderProg); >> >> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
