[Mesa-dev] Review request: [PATCH] Fix grabbing the wrong variant if glDrawPixels is called

[Ilia Mirkin]

Hi Matias,

You may want to review

https://www.mesa3d.org/submittingpatches.html

Particularly the "mailing patches" bit of it.

Cheers,

  -ilia


On Fri, Jul 7, 2017 at 11:41 AM, Matias N. Goldberg
<dark_sylinc at yahoo.com.ar> wrote:
> Hi!
>
> I just subscribed to this dev list.
>
>
> I wrote this patch (copy at the end of this email)
>
> https://bugs.freedesktop.org/attachment.cgi?id=132462&action=edit
>
> in order to fix bug Bug 101596 - Blender renders black UI elements
> (https://bugs.freedesktop.org/show_bug.cgi?id=101596)
>
> Note that this bug may not only affect Mesa.
>
>
> I am asking for this patch to be reviewed for inclusion in Mesa.
>
>
> Thanks
>
> Matias
>
>
> From 3db888f8645acd5d41b689ee6522d465bcf71044 Mon Sep 17 00:00:00 2001
> Message-Id:
> <3db888f8645acd5d41b689ee6522d465bcf71044.1499274200.git.dark_sylinc at yahoo.com.ar>
> From: "Matias N. Goldberg" <dark_sylinc at yahoo.com.ar>
> Date: Wed, 5 Jul 2017 14:02:50 -0300
> Subject: [PATCH] Fix grabbing the wrong variant if glDrawPixels is called
>
> By design pixel shaders can have up to 3 variants:
> * The standard one.
> * glDrawPixels variant.
> * glBitmap variant.
> However "shader_has_one_variant" ignores this fact, and therefore
> st_update_fp would select the wrong variant if glDrawPixels or glBitmap
> was ever called.
>
> This patch fixes the problem. If the standard variant has been created,
> calling glDrawPixels or glBitmap will append the variant to the second
> entry of the linked list, so that st_update_fp still selects the right
> one if shader_has_one_variant is set.
>
> If the standard variant hasn't been created yet and glDrawPixel/Bitmap
> has been called, st_update_fp will will see this and take the slow path
> instead. The standard variant will then be added at the front of the
> linked list, so that the next time the fast path is taken.
>
> Blender in particular is hit by this bug.
>
> Fixes https://bugs.freedesktop.org/show_bug.cgi?id=101596
> ---
>  src/mesa/state_tracker/st_atom_shader.c |  4 +++-
>  src/mesa/state_tracker/st_program.c     | 23 ++++++++++++++++++++---
>  2 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_atom_shader.c
> b/src/mesa/state_tracker/st_atom_shader.c
> index c1869d323b..07cf54f555 100644
> --- a/src/mesa/state_tracker/st_atom_shader.c
> +++ b/src/mesa/state_tracker/st_atom_shader.c
> @@ -108,7 +108,9 @@ st_update_fp( struct st_context *st )
>     if (st->shader_has_one_variant[MESA_SHADER_FRAGMENT] &&
>         !stfp->ati_fs && /* ATI_fragment_shader always has multiple variants
> */
>         !stfp->Base.ExternalSamplersUsed && /* external samplers need
> variants */
> -       stfp->variants) {
> +       stfp->variants &&
> +       !stfp->variants->key.drawpixels &&
> +       !stfp->variants->key.bitmap ) {
>        shader = stfp->variants->driver_shader;
>     } else {
>        memset(&key, 0, sizeof(key));
> diff --git a/src/mesa/state_tracker/st_program.c
> b/src/mesa/state_tracker/st_program.c
> index 6de61741dc..86faf5982d 100644
> --- a/src/mesa/state_tracker/st_program.c
> +++ b/src/mesa/state_tracker/st_program.c
> @@ -1322,9 +1322,26 @@ st_get_fp_variant(struct st_context *st,
>        /* create new */
>        fpv = st_create_fp_variant(st, stfp, key);
>        if (fpv) {
> -         /* insert into list */
> -         fpv->next = stfp->variants;
> -         stfp->variants = fpv;
> +         if( key->bitmap || key->drawpixels ) {
> +            /* Regular variants should always come before the
> +               bitmap & drawpixels variants, (unless there
> +               are no regular variants) so that
> +               st_update_fp can take a fast path when
> +               shader_has_one_variant is set.
> +            */
> +            /* insert into list */
> +            if( !stfp->variants ) {
> +               fpv->next = stfp->variants;
> +               stfp->variants = fpv;
> +            } else {
> +               fpv->next = stfp->variants->next;
> +               stfp->variants->next = fpv;
> +            }
> +         } else {
> +            /* insert into list */
> +            fpv->next = stfp->variants;
> +            stfp->variants = fpv;
> +         }
>        }
>     }
>
> --
> 2.11.0
>
>
> IMPORTANT: The information contained in this email may be commercially
> sensitive and/or legally privileged. It is intended solely for the person(s)
> to whom it is addressed. If the reader of this message is not the intended
> recipient, you are on notice of its status and hereby notified that your
> access is unauthorized, and any review, dissemination, distribution,
> disclose or copying of this message including any attachments is strictly
> prohibited. Please notify the sender immediately by reply e-mail and then
> delete this message from your system.
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>