On Wed, Jun 11, 2014 at 11:09 AM, Anuj Phogat <anuj.pho...@gmail.com> wrote:
> Adding mesa-dev to the conversation.
>
> On Wed, Jun 11, 2014 at 10:52 AM, Anuj Phogat <anuj.pho...@gmail.com>
> wrote:
> >
> > On Wed, Jun 11, 2014 at 10:28 AM, Courtney Goeltzenleuchter
> > <court...@lunarg.com> wrote:
> >>
> >> On Fri, Jun 6, 2014 at 5:57 PM, Anuj Phogat <anuj.pho...@gmail.com>
> wrote:
> >>>
> >>> Fixes many failures in gles3 Khronos CTS test: packed_pixels
> >>>
> >>> Cc: <mesa-sta...@lists.freedesktop.org>
> >>> Signed-off-by: Anuj Phogat <anuj.pho...@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.
> >>
> >>
> >> 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?
> >>
> >>
> >>>
> >>> + || (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-sta...@lists.freedesktop.org
> >>> http://lists.freedesktop.org/mailman/listinfo/mesa-stable
> >>
> >>
> >> --
> >> Courtney Goeltzenleuchter
> >> LunarG
> >>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
