[Mesa-dev] [PATCH 12/15] mesa: Add a gles3 error condition for sized internalformat in glCopyTexImage*()
Anuj Phogat
anuj.phogat at gmail.com
Fri Aug 1 18:10:34 PDT 2014
On Fri, Aug 1, 2014 at 3:03 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>
> On Wed, Jun 11, 2014 at 11:09 AM, Anuj Phogat <anuj.phogat at gmail.com> wrote:
>>
>> Adding mesa-dev to the conversation.
>>
>> On Wed, Jun 11, 2014 at 10:52 AM, Anuj Phogat <anuj.phogat at gmail.com>
>> wrote:
>> >
>> > On Wed, Jun 11, 2014 at 10:28 AM, Courtney Goeltzenleuchter
>> > <courtney at lunarg.com> wrote:
>> >>
>> >> On Fri, Jun 6, 2014 at 5:57 PM, Anuj Phogat <anuj.phogat at gmail.com>
>> >> wrote:
>> >>>
>> >>> Fixes many failures in gles3 Khronos CTS test: packed_pixels
>> >>>
>> >>> Cc: <mesa-stable at lists.freedesktop.org>
>> >>> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
>> >>> ---
>> >>> src/mesa/main/teximage.c | 43
>> >>> +++++++++++++++++++++++++++++++++++++++++++
>> >>> 1 file changed, 43 insertions(+)
>> >>>
>> >>> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
>> >>> index 6474dba..c926a2f 100644
>> >>> --- a/src/mesa/main/teximage.c
>> >>> +++ b/src/mesa/main/teximage.c
>> >>> @@ -3582,6 +3582,25 @@ copytexsubimage_by_slice(struct gl_context
>> >>> *ctx,
>> >>> }
>> >>> }
>> >>>
>> >>> +static GLboolean
>> >>> +formats_differ_in_component_sizes (mesa_format f1,
>> >>> + mesa_format f2)
>> >>> +{
>> >>> + GLint f1_r_bits = _mesa_get_format_bits(f1, GL_RED_BITS);
>> >>> + GLint f1_g_bits = _mesa_get_format_bits(f1, GL_GREEN_BITS);
>> >>> + GLint f1_b_bits = _mesa_get_format_bits(f1, GL_BLUE_BITS);
>> >>> +
>> >>> + GLint f2_r_bits = _mesa_get_format_bits(f2, GL_RED_BITS);
>> >>> + GLint f2_g_bits = _mesa_get_format_bits(f2, GL_GREEN_BITS);
>> >>> + GLint f2_b_bits = _mesa_get_format_bits(f2, GL_BLUE_BITS);
>> >>> +
>> >>> + if ((f1_r_bits && f2_r_bits && f1_r_bits != f2_r_bits)
>
>
> What about alpha? We should also do something sensible if the destination
> texture is luminance or luminance_alpha.
>
Check for the alpha bits is redundant in case of GL ES 3.0:
Table 3.2 and 3.3 on Page 112 for OpenGL ES 3.0 spec lists no internalformats
with same RGB component sizes but different alpha component sizes. Also,
there are no sized GL_ALPHA or GL_LUMINACE_ALPHA internalformats listed.
!_mesa_is_enum_format_unsized(internalFormat) check will fail for
GL_ALPHA, GL_LUMINANCE and GL_LUMINACE_ALPHA internalformats
and formats_differ_in_component_sizes() will never get called.
Following rule from spec applies in that case:
"If internalformat is unsized, the internal format of the new texel array is the
effective internal format of the source buffer, and this is also the new texel
array’s effective internal format."
It took me a while to figure out what I was thinking when I wrote this patch.
If you agree to above explanation, I'll add a short comment in
formats_differ_in_component_sizes() for missing alpha check and will also
change the name of function to es_formats_differ_in_component_sizes()?
>>
>> >>
>> >>
>> >> I'm curious, why wouldn't a format with f1_r_bits = 0 and f2_r_bits !=
>> >> 0
>> >> and everything else the same, not be considered different formats?
>> >
>> > If we include that condition, it will be equivalent using:
>> > if(f1_r_bits != f2_r_bits || f1_g_bits != f2_g_bits || f1_b_bits !=
>> > f2_b_bits)
>> >
>> > This won't work because gles allows dropping the components in internal
>> > formats but doesn't allow adding new components. This check in
>> > copytexture_error_check() takes care of this case when one component is
>> > missing:
>> >
>> > [snip]
>> > if (_mesa_base_format_component_count(baseFormat) >
>> > _mesa_base_format_component_count(rb_base_format)) {
>> > valid = false;
>> > }
>> > [snip]
>
>
> Also, this isn't 100% sufficient. What about if the framebuffer is GL_RGB
> but the texture is GL_ALPHA? This check will pass but it's clearly a bad
> combination. Is there something dealing with this case?
>
Yes. This case is handled by formats_differ_in_component_sizes() check
in copyteximage().
>>
>> >>
>> >>
>> >>>
>> >>> + || (f1_g_bits && f2_g_bits && f1_g_bits != f2_g_bits)
>> >>> + || (f1_b_bits && f2_b_bits && f1_b_bits != f2_b_bits))
>> >>> + return GL_TRUE;
>> >>> +
>> >>> + return GL_FALSE;
>> >>> +}
>> >>>
>> >>> /**
>> >>> * Implement the glCopyTexImage1/2D() functions.
>> >>> @@ -3595,6 +3614,7 @@ copyteximage(struct gl_context *ctx, GLuint
>> >>> dims,
>> >>> struct gl_texture_image *texImage;
>> >>> const GLuint face = _mesa_tex_target_to_face(target);
>> >>> mesa_format texFormat;
>> >>> + struct gl_renderbuffer *rb;
>> >>>
>> >>> FLUSH_VERTICES(ctx, 0);
>> >>>
>> >>> @@ -3624,6 +3644,29 @@ copyteximage(struct gl_context *ctx, GLuint
>> >>> dims,
>> >>>
>> >>> texFormat = _mesa_choose_texture_format(ctx, texObj, target,
>> >>> level,
>> >>> internalFormat, GL_NONE,
>> >>> GL_NONE);
>> >>> +
>> >>> + rb = _mesa_get_read_renderbuffer_for_format(ctx, internalFormat);
>> >>> +
>> >>> + /* From Page 139 of OpenGL ES 3.0 spec:
>> >>> + * "If internalformat is sized, the internal format of the new
>> >>> texel
>> >>> + * array is internalformat, and this is also the new texel
>> >>> array’s
>> >>> + * effective internal format. If the component sizes of
>> >>> internalformat
>> >>> + * do not exactly match the corresponding component sizes of
>> >>> the
>> >>> source
>> >>> + * buffer’s effective internal format, described below, an
>> >>> + * INVALID_OPERATION error is generated. If internalformat is
>> >>> unsized,
>> >>> + * the internal format of the new texel array is the effective
>> >>> internal
>> >>> + * format of the source buffer, and this is also the new texel
>> >>> array’s
>> >>> + * effective internal format.
>> >>> + */
>> >>> + if (_mesa_is_gles3(ctx)
>> >>> + && !_mesa_is_enum_format_unsized(internalFormat)
>> >>> + && formats_differ_in_component_sizes (texFormat, rb->Format))
>> >>> {
>> >>> + _mesa_error(ctx, GL_INVALID_OPERATION,
>> >>> + "glCopyTexImage%uD(componenet size changed in"
>> >>> + " internal format)", dims);
>> >>> + return;
>> >>> + }
>> >>> +
>> >>> assert(texFormat != MESA_FORMAT_NONE);
>> >>>
>> >>> if (!ctx->Driver.TestProxyTexImage(ctx, proxy_target(target),
>> >>> --
>> >>> 1.8.3.1
>> >>>
>> >>> _______________________________________________
>> >>> mesa-stable mailing list
>> >>> mesa-stable at lists.freedesktop.org
>> >>> http://lists.freedesktop.org/mailman/listinfo/mesa-stable
>> >>
>> >>
>> >> --
>> >> Courtney Goeltzenleuchter
>> >> LunarG
>> >>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
More information about the mesa-dev
mailing list