xserver: Branch 'master' - 3 commits

Keith Packard keithp at kemper.freedesktop.org
Thu May 26 16:19:32 UTC 2016


 glamor/glamor.c                  |   36 ++++++++++++++++++++++++++++++++++++
 glamor/glamor_composite_glyphs.c |    3 +--
 glamor/glamor_copy.c             |    8 ++++----
 glamor/glamor_dash.c             |    3 +--
 glamor/glamor_fbo.c              |    4 +---
 glamor/glamor_priv.h             |   28 ++++++++++++++++++++++++++++
 glamor/glamor_program.c          |    8 +++++++-
 glamor/glamor_render.c           |   28 +++++++++++++++++++++-------
 glamor/glamor_spans.c            |    3 +--
 glamor/glamor_transfer.c         |    3 +--
 glamor/glamor_transform.c        |   12 ++++++++----
 glamor/glamor_transform.h        |    4 +++-
 12 files changed, 112 insertions(+), 28 deletions(-)

New commits:
commit 181a4bd0cc436f89582408196038ff37032f9bac
Author: Keith Packard <keithp at keithp.com>
Date:   Fri May 13 16:19:38 2016 -0700

    glamor: Preserve GL_RED bits in R channel when destination is GL_RED [v2]
    
    A1 and A8 pixmaps are usually stored in the Red channel to conform
    with more recent GL versions. When using these pixmaps as mask values,
    that works great. When using these pixmaps as source values, then the
    value we want depends on what the destination looks like.
    
    For RGBA or RGB destinations, then we want to use the Red channel
    for A values and leave RGB all set to zero.
    
    For A destinations, then we want to leave the R values in the Red
    channel so that they end up in the Red channel of the output.
    
    This patch adds a helper function, glamor_bind_texture, which performs
    the glBindTexture call along with setting the swizzle parameter
    correctly for the Red channel. The swizzle parameter for the Alpha
    channel doesn't depend on the destination as it's safe to leave it
    always swizzled from the Red channel.
    
    This fixes incorrect rendering in firefox for this page:
    
    	https://gfycat.com/HoarseCheapAmericankestrel
    
    while not breaking rendering for this page:
    
    	https://feedly.com
    
    v2: Add change accidentally left in patch for missing
        glDisable(GL_COLOR_LOGIC_OP).
        Found by Emil Velikov <emil.l.velikov at gmail.com>
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=63397
    Signed-off-by: Keith Packard <keithp at keithp.com>
    Tested-by: Michel Dänzer <michel.daenzer at amd.com>

diff --git a/glamor/glamor.c b/glamor/glamor.c
index 477bc0e..62b5c3a 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -140,6 +140,42 @@ glamor_get_pixmap_texture(PixmapPtr pixmap)
     return pixmap_priv->fbo->tex;
 }
 
+void
+glamor_bind_texture(glamor_screen_private *glamor_priv, GLenum texture,
+                    glamor_pixmap_fbo *fbo, Bool destination_red)
+{
+    glActiveTexture(texture);
+    glBindTexture(GL_TEXTURE_2D, fbo->tex);
+
+    /* If we're pulling data from a GL_RED texture, then whether we
+     * want to make it an A,0,0,0 result or a 0,0,0,R result depends
+     * on whether the destination is also a GL_RED texture.
+     *
+     * For GL_RED destinations, we need to leave the bits in the R
+     * channel. For all other destinations, we need to clear out the R
+     * channel so that it returns zero for R, G and B.
+     *
+     * Note that we're leaving the SWIZZLE_A value alone; for GL_RED
+     * destinations, that means we'll actually be returning R,0,0,R,
+     * but it doesn't matter as the bits in the alpha channel aren't
+     * going anywhere.
+     */
+
+    /* Is the operand a GL_RED fbo?
+     */
+
+    if (glamor_fbo_red_is_alpha(glamor_priv, fbo)) {
+
+        /* If destination is also GL_RED, then preserve the bits in
+         * the R channel */
+
+        if (destination_red)
+            glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_SWIZZLE_R, GL_RED);
+        else
+            glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_SWIZZLE_R, GL_ZERO);
+    }
+}
+
 PixmapPtr
 glamor_create_pixmap(ScreenPtr screen, int w, int h, int depth,
                      unsigned int usage)
diff --git a/glamor/glamor_composite_glyphs.c b/glamor/glamor_composite_glyphs.c
index f51ff6d..50fd026 100644
--- a/glamor/glamor_composite_glyphs.c
+++ b/glamor/glamor_composite_glyphs.c
@@ -246,8 +246,7 @@ glamor_glyphs_flush(CARD8 op, PicturePtr src, PicturePtr dst,
     glamor_put_vbo_space(drawable->pScreen);
 
     glEnable(GL_SCISSOR_TEST);
-    glActiveTexture(GL_TEXTURE1);
-    glBindTexture(GL_TEXTURE_2D, atlas_fbo->tex);
+    glamor_bind_texture(glamor_priv, GL_TEXTURE1, atlas_fbo, FALSE);
 
     for (;;) {
         if (!glamor_use_program_render(prog, op, src, dst))
diff --git a/glamor/glamor_copy.c b/glamor/glamor_copy.c
index 5fed89f..3501a0d 100644
--- a/glamor/glamor_copy.c
+++ b/glamor/glamor_copy.c
@@ -38,8 +38,8 @@ use_copyarea(PixmapPtr dst, GCPtr gc, glamor_program *prog, void *arg)
     struct copy_args *args = arg;
     glamor_pixmap_fbo *src = args->src;
 
-    glActiveTexture(GL_TEXTURE0);
-    glBindTexture(GL_TEXTURE_2D, src->tex);
+    glamor_bind_texture(glamor_get_screen_private(dst->drawable.pScreen),
+                        GL_TEXTURE0, src, TRUE);
 
     glUniform2f(prog->fill_offset_uniform, args->dx, args->dy);
     glUniform2f(prog->fill_size_inv_uniform, 1.0f/src->width, 1.0f/src->height);
@@ -67,8 +67,8 @@ use_copyplane(PixmapPtr dst, GCPtr gc, glamor_program *prog, void *arg)
     struct copy_args *args = arg;
     glamor_pixmap_fbo *src = args->src;
 
-    glActiveTexture(GL_TEXTURE0);
-    glBindTexture(GL_TEXTURE_2D, src->tex);
+    glamor_bind_texture(glamor_get_screen_private(dst->drawable.pScreen),
+                        GL_TEXTURE0, src, TRUE);
 
     glUniform2f(prog->fill_offset_uniform, args->dx, args->dy);
     glUniform2f(prog->fill_size_inv_uniform, 1.0f/src->width, 1.0f/src->height);
diff --git a/glamor/glamor_dash.c b/glamor/glamor_dash.c
index a6a11c1..3c19dba 100644
--- a/glamor/glamor_dash.c
+++ b/glamor/glamor_dash.c
@@ -188,8 +188,7 @@ glamor_dash_setup(DrawablePtr drawable, GCPtr gc)
 
     /* Set the dash pattern as texture 1 */
 
-    glActiveTexture(GL_TEXTURE1);
-    glBindTexture(GL_TEXTURE_2D, dash_priv->fbo->tex);
+    glamor_bind_texture(glamor_priv, GL_TEXTURE1, dash_priv->fbo, FALSE);
     glUniform1i(prog->dash_uniform, 1);
     glUniform1f(prog->dash_length_uniform, dash_pixmap->drawable.width);
 
diff --git a/glamor/glamor_fbo.c b/glamor/glamor_fbo.c
index f4f8749..a531f60 100644
--- a/glamor/glamor_fbo.c
+++ b/glamor/glamor_fbo.c
@@ -352,10 +352,8 @@ _glamor_create_tex(glamor_screen_private *glamor_priv,
     glBindTexture(GL_TEXTURE_2D, tex);
     glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
     glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
-    if (format == glamor_priv->one_channel_format && format == GL_RED) {
-        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_SWIZZLE_R, GL_ZERO);
+    if (format == glamor_priv->one_channel_format && format == GL_RED)
         glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_SWIZZLE_A, GL_RED);
-    }
     glamor_priv->suppress_gl_out_of_memory_logging = true;
     glTexImage2D(GL_TEXTURE_2D, 0, format, w, h, 0,
                  format, GL_UNSIGNED_BYTE, NULL);
diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index 8f994ea..c34eb84 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -573,6 +573,34 @@ void glamor_fini_pixmap_fbo(ScreenPtr screen);
 Bool glamor_pixmap_fbo_fixup(ScreenPtr screen, PixmapPtr pixmap);
 void glamor_fbo_expire(glamor_screen_private *glamor_priv);
 
+/* Return whether 'picture' is alpha-only */
+static inline Bool glamor_picture_is_alpha(PicturePtr picture)
+{
+    return picture->format == PICT_a1 || picture->format == PICT_a8;
+}
+
+/* Return whether 'fbo' is storing alpha bits in the red channel */
+static inline Bool
+glamor_fbo_red_is_alpha(glamor_screen_private *glamor_priv, glamor_pixmap_fbo *fbo)
+{
+    /* True when the format is GL_RED (that can only happen when our one channel format is GL_RED */
+    return fbo->format == GL_RED;
+}
+
+/* Return whether 'picture' is storing alpha bits in the red channel */
+static inline Bool
+glamor_picture_red_is_alpha(PicturePtr picture)
+{
+    /* True when the picture is alpha only and the screen is using GL_RED for alpha pictures */
+    return glamor_picture_is_alpha(picture) &&
+        glamor_get_screen_private(picture->pDrawable->pScreen)->one_channel_format == GL_RED;
+}
+
+void glamor_bind_texture(glamor_screen_private *glamor_priv,
+                         GLenum texture,
+                         glamor_pixmap_fbo *fbo,
+                         Bool destination_red);
+
 glamor_pixmap_fbo *glamor_create_fbo_array(glamor_screen_private *glamor_priv,
                                            int w, int h, GLenum format,
                                            int flag, int block_w, int block_h,
diff --git a/glamor/glamor_program.c b/glamor/glamor_program.c
index 322d198..dec116c 100644
--- a/glamor/glamor_program.c
+++ b/glamor/glamor_program.c
@@ -531,6 +531,7 @@ use_source_picture(CARD8 op, PicturePtr src, PicturePtr dst, glamor_program *pro
     glamor_set_blend(op, prog->alpha, dst);
 
     return glamor_set_texture((PixmapPtr) src->pDrawable,
+                              glamor_picture_red_is_alpha(dst),
                               0, 0,
                               prog->fill_offset_uniform,
                               prog->fill_size_inv_uniform);
@@ -549,7 +550,8 @@ use_source_1x1_picture(CARD8 op, PicturePtr src, PicturePtr dst, glamor_program
 {
     glamor_set_blend(op, prog->alpha, dst);
 
-    return glamor_set_texture_pixmap((PixmapPtr) src->pDrawable);
+    return glamor_set_texture_pixmap((PixmapPtr) src->pDrawable,
+                                     glamor_picture_red_is_alpha(dst));
 }
 
 static const glamor_facet glamor_source_1x1_picture = {
diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c
index a67965e..9c5cca6 100644
--- a/glamor/glamor_render.c
+++ b/glamor/glamor_render.c
@@ -512,15 +512,24 @@ static void
 glamor_set_composite_texture(glamor_screen_private *glamor_priv, int unit,
                              PicturePtr picture,
                              PixmapPtr pixmap,
-                             GLuint wh_location, GLuint repeat_location)
+                             GLuint wh_location, GLuint repeat_location,
+                             glamor_pixmap_private *dest_priv)
 {
     glamor_pixmap_private *pixmap_priv = glamor_get_pixmap_private(pixmap);
+    glamor_pixmap_fbo *fbo = pixmap_priv->fbo;
     float wh[4];
     int repeat_type;
 
     glamor_make_current(glamor_priv);
-    glActiveTexture(GL_TEXTURE0 + unit);
-    glBindTexture(GL_TEXTURE_2D, pixmap_priv->fbo->tex);
+
+    /* The red channel swizzling doesn't depend on whether we're using
+     * 'fbo' as source or mask as we must have the same answer in case
+     * the same fbo is being used for both. That means the mask
+     * channel will sometimes get red bits in the R channel, and
+     * sometimes get zero bits in the R channel, which is harmless.
+     */
+    glamor_bind_texture(glamor_priv, GL_TEXTURE0 + unit, fbo,
+                        glamor_fbo_red_is_alpha(glamor_priv, dest_priv->fbo));
     repeat_type = picture->repeatType;
     switch (picture->repeatType) {
     case RepeatNone:
@@ -1075,7 +1084,8 @@ glamor_composite_set_shader_blend(glamor_screen_private *glamor_priv,
         glamor_set_composite_texture(glamor_priv, 0,
                                      shader->source,
                                      shader->source_pixmap, shader->source_wh,
-                                     shader->source_repeat_mode);
+                                     shader->source_repeat_mode,
+                                     dest_priv);
     }
 
     if (key->mask != SHADER_MASK_NONE) {
@@ -1087,7 +1097,8 @@ glamor_composite_set_shader_blend(glamor_screen_private *glamor_priv,
             glamor_set_composite_texture(glamor_priv, 1,
                                          shader->mask,
                                          shader->mask_pixmap, shader->mask_wh,
-                                         shader->mask_repeat_mode);
+                                         shader->mask_repeat_mode,
+                                         dest_priv);
         }
     }
 
diff --git a/glamor/glamor_spans.c b/glamor/glamor_spans.c
index 89a9c51..5217d04 100644
--- a/glamor/glamor_spans.c
+++ b/glamor/glamor_spans.c
@@ -294,8 +294,7 @@ glamor_set_spans_gl(DrawablePtr drawable, GCPtr gc, char *src,
         BoxPtr              box = glamor_pixmap_box_at(pixmap_priv, box_index);
         glamor_pixmap_fbo  *fbo = glamor_pixmap_fbo_at(pixmap_priv, box_index);
 
-        glActiveTexture(GL_TEXTURE0);
-        glBindTexture(GL_TEXTURE_2D, fbo->tex);
+        glamor_bind_texture(glamor_priv, GL_TEXTURE0, fbo, TRUE);
 
         s = src;
         for (n = 0; n < numPoints; n++) {
diff --git a/glamor/glamor_transfer.c b/glamor/glamor_transfer.c
index ed81195..d788d06 100644
--- a/glamor/glamor_transfer.c
+++ b/glamor/glamor_transfer.c
@@ -83,8 +83,7 @@ glamor_upload_boxes(PixmapPtr pixmap, BoxPtr in_boxes, int in_nbox,
         BoxPtr                  boxes = in_boxes;
         int                     nbox = in_nbox;
 
-        glActiveTexture(GL_TEXTURE0);
-        glBindTexture(GL_TEXTURE_2D, fbo->tex);
+        glamor_bind_texture(glamor_priv, GL_TEXTURE0, fbo, TRUE);
 
         while (nbox--) {
 
diff --git a/glamor/glamor_transform.c b/glamor/glamor_transform.c
index fc96fd6..eff500c 100644
--- a/glamor/glamor_transform.c
+++ b/glamor/glamor_transform.c
@@ -158,7 +158,7 @@ glamor_set_solid(PixmapPtr      pixmap,
 }
 
 Bool
-glamor_set_texture_pixmap(PixmapPtr texture)
+glamor_set_texture_pixmap(PixmapPtr texture, Bool destination_red)
 {
     glamor_pixmap_private *texture_priv;
 
@@ -170,8 +170,9 @@ glamor_set_texture_pixmap(PixmapPtr texture)
     if (glamor_pixmap_priv_is_large(texture_priv))
         return FALSE;
 
-    glActiveTexture(GL_TEXTURE0);
-    glBindTexture(GL_TEXTURE_2D, texture_priv->fbo->tex);
+    glamor_bind_texture(glamor_get_screen_private(texture->drawable.pScreen),
+                        GL_TEXTURE0,
+                        texture_priv->fbo, destination_red);
 
     /* we're not setting the sampler uniform here as we always use
      * GL_TEXTURE0, and the default value for uniforms is zero. So,
@@ -182,12 +183,13 @@ glamor_set_texture_pixmap(PixmapPtr texture)
 
 Bool
 glamor_set_texture(PixmapPtr    texture,
+                   Bool         destination_red,
                    int          off_x,
                    int          off_y,
                    GLint        offset_uniform,
                    GLint        size_inv_uniform)
 {
-    if (!glamor_set_texture_pixmap(texture))
+    if (!glamor_set_texture_pixmap(texture, destination_red))
         return FALSE;
 
     glUniform2f(offset_uniform, off_x, off_y);
@@ -208,6 +210,7 @@ glamor_set_tiled(PixmapPtr      pixmap,
         return FALSE;
 
     return glamor_set_texture(gc->tile.pixmap,
+                              TRUE,
                               -gc->patOrg.x,
                               -gc->patOrg.y,
                               offset_uniform,
@@ -289,6 +292,7 @@ glamor_set_stippled(PixmapPtr      pixmap,
         return FALSE;
 
     return glamor_set_texture(stipple,
+                              FALSE,
                               -gc->patOrg.x,
                               -gc->patOrg.y,
                               offset_uniform,
diff --git a/glamor/glamor_transform.h b/glamor/glamor_transform.h
index 5a520eb..70d2c16 100644
--- a/glamor/glamor_transform.h
+++ b/glamor/glamor_transform.h
@@ -48,10 +48,12 @@ glamor_set_color(PixmapPtr      pixmap,
 }
 
 Bool
-glamor_set_texture_pixmap(PixmapPtr    texture);
+glamor_set_texture_pixmap(PixmapPtr     texture,
+                          Bool          destination_red);
 
 Bool
 glamor_set_texture(PixmapPtr    texture,
+                   Bool         destination_red,
                    int          off_x,
                    int          off_y,
                    GLint        offset_uniform,
commit b07bc700b3cf2f5c8912fc5b9e0dad2baf395525
Author: Keith Packard <keithp at keithp.com>
Date:   Sat May 14 08:22:17 2016 -0700

    glamor: glamor_make_current sooner in glamor_composite_with_shader
    
    glamor_make_current is supposed to be called before any GL APIs.
    
    Signed-off-by: Keith Packard <keithp at keithp.com>
    Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>

diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c
index aa3a566..a67965e 100644
--- a/glamor/glamor_render.c
+++ b/glamor/glamor_render.c
@@ -1154,12 +1154,12 @@ glamor_composite_with_shader(CARD8 op,
         }
     }
 
+    glamor_make_current(glamor_priv);
+
     glamor_set_destination_pixmap_priv_nc(glamor_priv, dest_pixmap, dest_pixmap_priv);
     glamor_composite_set_shader_blend(glamor_priv, dest_pixmap_priv, &key, shader, &op_info);
     glamor_set_alu(screen, GXcopy);
 
-    glamor_make_current(glamor_priv);
-
     glamor_priv->has_source_coords = key.source != SHADER_SOURCE_SOLID;
     glamor_priv->has_mask_coords = (key.mask != SHADER_MASK_NONE &&
                                     key.mask != SHADER_MASK_SOLID);
commit 743b6f231e999d8b2909228412266dc13cc433c5
Author: Keith Packard <keithp at keithp.com>
Date:   Fri May 13 04:25:43 2016 -0700

    glamor: Disable logic ops when doing compositing [v4]
    
    If the logic op gets left enabled, it overrides the blending
    operation, causing incorrect contents on the display.
    
    v2: Disable only on non-ES2, but disable even for PictOpSrc
    
    v3: Found another place this is needed in
        glamor_composite_set_shader_blend
    
    v4: Remove change dependent on new glamor_set_composite_texture
        API. This belongs in a different patch.
        Found by Emil Velikov <emil.l.velikov at gmail.com>
    
    Signed-off-by: Keith Packard <keithp at keithp.com>
    Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>

diff --git a/glamor/glamor_program.c b/glamor/glamor_program.c
index 0a94de6..322d198 100644
--- a/glamor/glamor_program.c
+++ b/glamor/glamor_program.c
@@ -445,6 +445,7 @@ static struct blendinfo composite_op_info[] = {
 static void
 glamor_set_blend(CARD8 op, glamor_program_alpha alpha, PicturePtr dst)
 {
+    glamor_screen_private *glamor_priv = glamor_get_screen_private(dst->pDrawable->pScreen);
     GLenum src_blend, dst_blend;
     struct blendinfo *op_info;
 
@@ -459,6 +460,9 @@ glamor_set_blend(CARD8 op, glamor_program_alpha alpha, PicturePtr dst)
         break;
     }
 
+    if (glamor_priv->gl_flavor != GLAMOR_GL_ES2)
+        glDisable(GL_COLOR_LOGIC_OP);
+
     if (op == PictOpSrc)
         return;
 
diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c
index 88781d9..aa3a566 100644
--- a/glamor/glamor_render.c
+++ b/glamor/glamor_render.c
@@ -1091,6 +1091,9 @@ glamor_composite_set_shader_blend(glamor_screen_private *glamor_priv,
         }
     }
 
+    if (glamor_priv->gl_flavor != GLAMOR_GL_ES2)
+        glDisable(GL_COLOR_LOGIC_OP);
+
     if (op_info->source_blend == GL_ONE && op_info->dest_blend == GL_ZERO) {
         glDisable(GL_BLEND);
     }


More information about the xorg-commit mailing list