[Mesa-dev] [PATCH 12/15] mesa: Add a gles3 error condition for sized internalformat in glCopyTexImage*()

[Anuj Phogat]

Courtney, Should I consider this r-b you?


On Wed, Jun 11, 2014 at 11:34 AM, Courtney Goeltzenleuchter <
courtney at lunarg.com> wrote:

> Very good. I thought there might be something limiting the inputs but
> wasn't immediately obvious.
>
>
> On Wed, Jun 11, 2014 at 12:09 PM, 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)
>> >>
>> >>
>> >> 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]
>> >>
>> >>
>> >>>
>> >>> +       || (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
>> >>
>>
>
>
>
> --
> Courtney Goeltzenleuchter
> LunarG
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140731/597c95a1/attachment-0001.html>