Re: [PATCH xserver] autogen: Set a default subject prefix for patches
Adam Jackson writes: > Per discussion at XDC2015, we want this so we can easily distinguish > which module a patch is for. There's no way to set this in the > server-side config, so setting a default at autogen time is about the > best we can do. > > Signed-off-by: Adam Jackson > --- > autogen.sh | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/autogen.sh b/autogen.sh > index aee4beb..4b1b523 100755 > --- a/autogen.sh > +++ b/autogen.sh > @@ -12,3 +12,6 @@ cd "$ORIGDIR" || exit $? > if test -z "$NOCONFIGURE"; then > exec "$srcdir"/configure "$@" > fi > + > +git config --local --get format.subjectPrefix || > +git config --local format.subjectPrefix "PATCH xserver" > -- This doesn't work unless you have NOCONFIGURE set, because of "exec". If you move it up above that block, it's: Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 2/8] glamor/xv: add vbo support
Dave Airlie writes: > From: Dave Airlie > > This converts the Xv code to using VBOs instead of > client ptrs. This is necessary to move towards using > the core profile later. > > Signed-off-by: Dave Airlie > --- > glamor/glamor_xv.c | 31 +-- > 1 file changed, 21 insertions(+), 10 deletions(-) > > diff --git a/glamor/glamor_xv.c b/glamor/glamor_xv.c > index 85e6528..d9db574 100644 > --- a/glamor/glamor_xv.c > +++ b/glamor/glamor_xv.c > @@ -245,7 +245,6 @@ glamor_xv_render(glamor_port_private *port_priv) > PixmapPtr pixmap = port_priv->pPixmap; > glamor_pixmap_private *pixmap_priv = glamor_get_pixmap_private(pixmap); > glamor_pixmap_private *src_pixmap_priv[3]; > -float vertices[32], texcoords[8]; > BoxPtr box = REGION_RECTS(&port_priv->clip); > int nBox = REGION_NUM_RECTS(&port_priv->clip); > int dst_x_off, dst_y_off; > @@ -260,6 +259,8 @@ glamor_xv_render(glamor_port_private *port_priv) > float bright, cont, gamma; > int ref = port_priv->transform_index; > GLint uloc, sampler_loc; > +GLfloat *v; > +char *vbo_offset; > > if (!glamor_priv->xv_prog) > glamor_init_xv_shader(screen); > @@ -335,16 +336,13 @@ glamor_xv_render(glamor_port_private *port_priv) > sampler_loc = glGetUniformLocation(glamor_priv->xv_prog, "v_sampler"); > glUniform1i(sampler_loc, 2); > > -glVertexAttribPointer(GLAMOR_VERTEX_SOURCE, 2, > - GL_FLOAT, GL_FALSE, > - 2 * sizeof(float), texcoords); > +glEnableVertexAttribArray(GLAMOR_VERTEX_POS); > glEnableVertexAttribArray(GLAMOR_VERTEX_SOURCE); > > -glVertexAttribPointer(GLAMOR_VERTEX_POS, 2, GL_FLOAT, > - GL_FALSE, 2 * sizeof(float), vertices); > - > -glEnableVertexAttribArray(GLAMOR_VERTEX_POS); > glEnable(GL_SCISSOR_TEST); > + > +v = glamor_get_vbo_space(screen, 16 * sizeof(GLfloat) * nBox, > &vbo_offset); > + > for (i = 0; i < nBox; i++) { > float off_x = box[i].x1 - port_priv->drw_x; > float off_y = box[i].y1 - port_priv->drw_y; > @@ -352,6 +350,7 @@ glamor_xv_render(glamor_port_private *port_priv) > float diff_y = (float) port_priv->src_h / (float) port_priv->dst_h; > float srcx, srcy, srcw, srch; > int dstx, dsty, dstw, dsth; > +GLfloat *ptr = v + (i * 16); > > dstx = box[i].x1 + dst_x_off; > dsty = box[i].y1 + dst_y_off; > @@ -369,7 +368,7 @@ glamor_xv_render(glamor_port_private *port_priv) > dsty, > dstx + dstw, > dsty + dsth * 2, > - vertices); > + ptr); > > glamor_set_normalize_tcoords(src_pixmap_priv[0], > src_xscale[0], > @@ -378,16 +377,28 @@ glamor_xv_render(glamor_port_private *port_priv) > srcy, > srcx + srcw, > srcy + srch * 2, > - texcoords); > + ptr + 8); > + > + > +glVertexAttribPointer(GLAMOR_VERTEX_POS, 2, > + GL_FLOAT, GL_FALSE, > + 2 * sizeof(float), vbo_offset); > + > +glVertexAttribPointer(GLAMOR_VERTEX_SOURCE, 2, > + GL_FLOAT, GL_FALSE, > + 2 * sizeof(float), vbo_offset + 8 * > sizeof(GLfloat)); > > glScissor(dstx, dsty, dstw, dsth); > glDrawArrays(GL_TRIANGLE_FAN, 0, 3); > +vbo_offset += 16 * sizeof(GLfloat); You could move the pointer setup out of the loop, s/0/i * 4/ in glDrawArrays(), and then drop the vbo_offset math, I think. With that changed, the first two are: Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 3/8] glamor: Use vertex arrays
Dave Airlie writes: > From: Keith Packard > > Core contexts require the use of vertex arrays, so switch both glamor > and ephyr/glamor over. I had been thinking of doing serious VAO usage, where each path set up its VAO so that the driver could precompute state (like what you did in GLX here). This is way more incremental, and I like it a lot. Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 4/8] glamor: Use GL_RED instead of GL_ALPHA if we have texture_swizzle
Dave Airlie writes: > From: Keith Packard > > GL_RED is supported by core profiles while GL_ALPHA is not; use GL_RED > for one channel objects (depth 1 to 8), and then swizzle them into the > alpha channel when used as a mask. > > [airlied: updated to master, add swizzle to composited glyphs and xv paths] > > Signed-off-by: Keith Packard > Signed-off-by: Dave Airlie > --- > glamor/glamor.c | 4 > glamor/glamor_composite_glyphs.c | 5 + > glamor/glamor_fbo.c | 2 ++ > glamor/glamor_picture.c | 22 ++ > glamor/glamor_priv.h | 2 ++ > glamor/glamor_render.c | 9 + > glamor/glamor_transfer.c | 2 +- > glamor/glamor_utils.h| 4 +++- > glamor/glamor_xv.c | 12 > 9 files changed, 52 insertions(+), 10 deletions(-) > > diff --git a/glamor/glamor.c b/glamor/glamor.c > index 8828ad3..7fa3a46 100644 > --- a/glamor/glamor.c > +++ b/glamor/glamor.c > @@ -596,6 +596,10 @@ glamor_init(ScreenPtr screen, unsigned int flags) > glamor_priv->max_fbo_size = MAX_FBO_SIZE; > #endif > > +glamor_priv->one_channel_format = GL_ALPHA; > +if (epoxy_has_gl_extension("GL_ARB_texture_rg") && > epoxy_has_gl_extension("GL_ARB_texture_swizzle")) > +glamor_priv->one_channel_format = GL_RED; > + > glamor_set_debug_level(&glamor_debug_level); > > glamor_priv->saved_procs.create_screen_resources = > diff --git a/glamor/glamor_composite_glyphs.c > b/glamor/glamor_composite_glyphs.c > index fb31340..5f0fda5 100644 > --- a/glamor/glamor_composite_glyphs.c > +++ b/glamor/glamor_composite_glyphs.c > @@ -247,6 +247,11 @@ glamor_glyphs_flush(CARD8 op, PicturePtr src, PicturePtr > dst, > glActiveTexture(GL_TEXTURE1); > glBindTexture(GL_TEXTURE_2D, atlas_fbo->tex); > > +if (glamor_priv->one_channel_format == GL_RED && > +atlas->atlas->drawable.depth <= 8) > +{ > +glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_SWIZZLE_A, GL_RED); > +} > for (;;) { > if (!glamor_use_program_render(prog, op, src, dst)) > break; Since these TexParameters are stored in the texture object, I think what we really want is when we make an 8-bit FBO, at that point set SWIZZLE_A to RED and SWIZZLE_RED to 0. (If we don't do RED to 0, I expect this will cause Render regressions). Then we don't have to do these expensive TexParameters at draw time. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 5/8] glamor: add core profile support.
Dave Airlie writes: > From: Dave Airlie > > This adds a new flag to glamor_init to denote the context is > core profile. > > This flag is used to disable quads for rendering. > > Signed-off-by: Dave Airlie > --- > glamor/glamor.c | 3 ++- > glamor/glamor.h | 3 ++- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/glamor/glamor.c b/glamor/glamor.c > index 7fa3a46..a2bd687 100644 > --- a/glamor/glamor.c > +++ b/glamor/glamor.c > @@ -578,7 +578,8 @@ glamor_init(ScreenPtr screen, unsigned int flags) > > glamor_setup_debug_output(screen); > > -glamor_priv->use_quads = (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP); > +glamor_priv->use_quads = (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP) > && !(glamor_priv->flags & GLAMOR_USE_CORE_PROFILE); > + Instead of having the context creator pass us a flag, couldn't we just have bool core_context = gl_version >= 32 && !epoxy_has_gl_extension("GL_ARB_compatibility")? signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 6/8] ephyr: Create 3.3 core profile context if possible (v2)
Dave Airlie writes: > From: Keith Packard > > On desktop GL, Ask for a 3.3 core profile context if that's available, > otherwise create a generic context. > > v2: tell glamor the profile is a core one. > > Signed-off-by: Keith Packard > Signed-off-by: Dave Airlie > --- > hw/kdrive/ephyr/ephyr_glamor_glx.c | 19 +-- > hw/kdrive/ephyr/ephyr_glamor_glx.h | 2 +- > hw/kdrive/ephyr/hostx.c| 7 +-- > 3 files changed, 23 insertions(+), 5 deletions(-) > > diff --git a/hw/kdrive/ephyr/ephyr_glamor_glx.c > b/hw/kdrive/ephyr/ephyr_glamor_glx.c > index 30c5245..674e7f5 100644 > --- a/hw/kdrive/ephyr/ephyr_glamor_glx.c > +++ b/hw/kdrive/ephyr/ephyr_glamor_glx.c > @@ -277,7 +277,7 @@ ephyr_glamor_process_event(xcb_generic_event_t *xev) > } > > struct ephyr_glamor * > -ephyr_glamor_glx_screen_init(xcb_window_t win) > +ephyr_glamor_glx_screen_init(xcb_window_t win, Bool *profile_is_core) > { > static const float position[] = { > -1, -1, > @@ -295,6 +295,7 @@ ephyr_glamor_glx_screen_init(xcb_window_t win) > struct ephyr_glamor *glamor; > GLXWindow glx_win; > > +*profile_is_core = FALSE; > glamor = calloc(1, sizeof(struct ephyr_glamor)); > if (!glamor) { > FatalError("malloc"); > @@ -319,7 +320,21 @@ ephyr_glamor_glx_screen_init(xcb_window_t win) > "GLX_EXT_create_context_es2_profile\n"); > } > } else { > -ctx = glXCreateContext(dpy, visual_info, NULL, True); > +static const int context_attribs[] = { > +GLX_CONTEXT_PROFILE_MASK_ARB, > +GLX_CONTEXT_CORE_PROFILE_BIT_ARB, > +GLX_CONTEXT_MAJOR_VERSION_ARB, > +3, > +GLX_CONTEXT_MINOR_VERSION_ARB, > +3, > +0, > +}; > +ctx = glXCreateContextAttribsARB(dpy, fb_config, NULL, True, > + context_attribs); > +if (!ctx) > +ctx = glXCreateContext(dpy, visual_info, NULL, True); > +else > +*profile_is_core = TRUE; > } Before using the new function, we need to check epoxy_has_glx_extension("GLX_ARB_create_context") signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: glamor core profile support
r<#secure method=pgpmime mode=sign> Marek Olšák writes: > On Tue, Jan 19, 2016 at 1:56 AM, Dave Airlie wrote: >> This series implements support for glamor in ephyr/EGL/Xwayland >> to use GL core profile when it can. >> >> This required 4 main changes: >> a) stop using client ptrs everywhere, I found 3 places left, >> gradient, picture and xv where we use these. >> b) start using VAOs >> c) use GL_RED instead of GL_ALPHA >> d) stop using GL_QUADS. > > Won't this negatively affect performance on drivers which can do quads > natively? You've probably got instancing, though, and most of our high-vertex-rate code will use that with triangle strips to reduce vertex attribute data anyway (see glamor_text.c). ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] glamor: don't do copy if we have 0 boxes to copy.
Dave Airlie writes: > From: Dave Airlie > > This happens if you run twm + mplayer + xclock and drag > the clock over the mplayer. If we don't catch it, we cause > an illegal draw elements command to be passed to GL. What exactly is the error? I was thinking it would be something about count == 0, but that's not an error apparently. If there is one about count == 0, shouldn't we just have glamor_gldrawarrays_quads_using_indices() do the short circuit so the callers don't have to worry about it? signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v2 xserver] glamor: Disable debugging messages other than GL API errors
Michel Dänzer writes: > From: Michel Dänzer > > According to Nicolai Hähnle, the relevant specification says "All > messages are initially enabled unless their assigned severity is > DEBUG_SEVERITY_LOW", so we need to explicitly disable the messages we > don't want to get. Failing that, we were accidentally logging e.g. > shader stats intended for shader-db. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93659 > Tested-by: Laurent Carlier > Reviewed-by: Emil Velikov > Signed-off-by: Michel Dänzer > --- > > v2: Added Bugzilla/Tested-by/Reviewed-by tags Merged: commit 1db6de7b6a6ee240eb50a13fe1fa1e135d7cb93b Author: Michel Dänzer Date: Tue Jan 12 15:42:47 2016 +0900 glamor: Disable debugging messages other than GL API errors signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 3/8] glamor: Use vertex arrays
Dave Airlie writes: > From: Keith Packard > > Core contexts require the use of vertex arrays, so switch both glamor > and ephyr/glamor over. > > Signed-off-by: Keith Packard > Signed-off-by: Dave Airlie commit message should s/vertex arrays/vertex array objects/. With that changed, patches other than the GL_RED one are: Reviewed-by: Eric Anholt I think GL_RED's going to have rendering bugs, and I'm doing a piglit-xts run now to check. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 1/2] glamor: Fix copy-like Render operations between 15 and 16 depth.
Please cherry-pick this to active stable branches. Reading and writing to 16-depth pixmaps using PICT_x1r5g5b5 ends up failing, unless you're doing a straight copy at the same bpp where the misinterpretation matches on both sides. Fixes rendercheck/blend/over and renderhceck/blend/src in piglit. Signed-off-by: Eric Anholt --- glamor/glamor_render.c | 8 1 file changed, 8 insertions(+) diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c index d8574ec..92b6b0c 100644 --- a/glamor/glamor_render.c +++ b/glamor/glamor_render.c @@ -512,6 +512,14 @@ static int compatible_formats(CARD8 op, PicturePtr dst, PicturePtr src) { if (op == PictOpSrc) { +/* We can't do direct copies between different depths at 16bpp + * because r,g,b are allocated to different bits. + */ +if (dst->pDrawable->bitsPerPixel == 16 && +dst->pDrawable->depth != src->pDrawable->depth) { +return 0; +} + if (src->format == dst->format) return 1; -- 2.6.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 2/2] glamor: Drop the composite_with_copy path entirely.
I originally inherited this from the EXA code, without determining whether it was really needed. Regular composite should end up doing the same thing, since it's all just shaders anyway. To the extent that it doesn't, we should fix composite. Signed-off-by: Eric Anholt --- glamor/glamor_render.c | 92 -- 1 file changed, 92 deletions(-) diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c index 92b6b0c..1b226aa 100644 --- a/glamor/glamor_render.c +++ b/glamor/glamor_render.c @@ -508,41 +508,6 @@ glamor_set_composite_solid(float *color, GLint uniform_location) glUniform4fv(uniform_location, 1, color); } -static int -compatible_formats(CARD8 op, PicturePtr dst, PicturePtr src) -{ -if (op == PictOpSrc) { -/* We can't do direct copies between different depths at 16bpp - * because r,g,b are allocated to different bits. - */ -if (dst->pDrawable->bitsPerPixel == 16 && -dst->pDrawable->depth != src->pDrawable->depth) { -return 0; -} - -if (src->format == dst->format) -return 1; - -if (src->format == PICT_a8r8g8b8 && dst->format == PICT_x8r8g8b8) -return 1; - -if (src->format == PICT_a8b8g8r8 && dst->format == PICT_x8b8g8r8) -return 1; -} -else if (op == PictOpOver) { -if (src->alphaMap || dst->alphaMap) -return 0; - -if (src->format != dst->format) -return 0; - -if (src->format == PICT_x8r8g8b8 || src->format == PICT_x8b8g8r8) -return 1; -} - -return 0; -} - static char glamor_get_picture_location(PicturePtr picture) { @@ -564,54 +529,6 @@ glamor_get_picture_location(PicturePtr picture) return glamor_get_drawable_location(picture->pDrawable); } -static Bool -glamor_composite_with_copy(CARD8 op, - PicturePtr source, - PicturePtr dest, - INT16 x_source, - INT16 y_source, - INT16 x_dest, INT16 y_dest, RegionPtr region) -{ -int ret = FALSE; - -if (!source->pDrawable) -return FALSE; - -if (!compatible_formats(op, dest, source)) -return FALSE; - -if (source->repeat || source->transform) { -return FALSE; -} - -x_dest += dest->pDrawable->x; -y_dest += dest->pDrawable->y; -x_source += source->pDrawable->x; -y_source += source->pDrawable->y; -if (PICT_FORMAT_A(source->format) == 0) { -/* Fallback if we sample outside the source so that we - * swizzle the correct clear color for out-of-bounds texels. - */ -if (region->extents.x1 + x_source - x_dest < 0) -goto cleanup_region; -if (region->extents.x2 + x_source - x_dest > source->pDrawable->width) -goto cleanup_region; - -if (region->extents.y1 + y_source - y_dest < 0) -goto cleanup_region; -if (region->extents.y2 + y_source - y_dest > source->pDrawable->height) -goto cleanup_region; -} -glamor_copy(source->pDrawable, -dest->pDrawable, NULL, -RegionRects(region), RegionNumRects(region), -x_source - x_dest, y_source - y_dest, -FALSE, FALSE, 0, NULL); -ret = TRUE; - cleanup_region: -return ret; -} - static void * glamor_setup_composite_vbo(ScreenPtr screen, int n_verts) { @@ -1451,15 +1368,6 @@ glamor_composite_clipped_region(CARD8 op, } } -if (!mask && temp_src) { -if (glamor_composite_with_copy(op, temp_src, dest, - x_temp_src, y_temp_src, - x_dest, y_dest, region)) { -ok = TRUE; -goto out; -} -} - if (temp_src_pixmap == dest_pixmap) { glamor_fallback("source and dest pixmaps are the same\n"); goto out; -- 2.6.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 0/3] Fixups for GL_RED patch.
Here are my fixes for the regressions in the glamor-core-profile series. The first commit would go just before the GL_RED change, then I'd squash the other two into GL_RED. The series is now in glamor-core-profile of my fdo tree. Eric Anholt (3): glamor: Drop duplicated GLAMOR_DEFAULT_PRECISIONs in render accel. squash: glamor: Fix rendering to a8 textures in core profile. squash: glamor: Fix Render blending for alpha-to-red. glamor/glamor_priv.h | 10 - glamor/glamor_render.c | 101 ++--- 2 files changed, 87 insertions(+), 24 deletions(-) -- 2.6.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 2/3] squash: glamor: Fix rendering to a8 textures in core profile.
Signed-off-by: Eric Anholt --- glamor/glamor_priv.h | 10 +- glamor/glamor_render.c | 50 +++--- 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h index 558ed63..f1eed5b 100644 --- a/glamor/glamor_priv.h +++ b/glamor/glamor_priv.h @@ -116,10 +116,17 @@ enum shader_in { SHADER_IN_COUNT, }; +enum shader_dest_swizzle { +SHADER_DEST_SWIZZLE_DEFAULT, +SHADER_DEST_SWIZZLE_ALPHA_TO_RED, +SHADER_DEST_SWIZZLE_COUNT, +}; + struct shader_key { enum shader_source source; enum shader_mask mask; enum shader_in in; +enum shader_dest_swizzle dest_swizzle; }; struct blendinfo { @@ -285,7 +292,8 @@ typedef struct glamor_screen_private { int render_nr_quads; glamor_composite_shader composite_shader[SHADER_SOURCE_COUNT] [SHADER_MASK_COUNT] -[SHADER_IN_COUNT]; +[SHADER_IN_COUNT] +[SHADER_DEST_SWIZZLE_COUNT]; /* shaders to restore a texture to another texture. */ GLint finish_access_prog[2]; diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c index 7a50fe9..c36b345 100644 --- a/glamor/glamor_render.c +++ b/glamor/glamor_render.c @@ -177,25 +177,38 @@ glamor_create_composite_fs(struct shader_key *key) " return rel_sampler(mask_sampler, mask_texture,\n" " mask_wh, mask_repeat_mode, 1);\n" "}\n"; + +const char *dest_swizzle_default = +"vec4 dest_swizzle(vec4 color)\n" +"{" +" return color;" +"}"; +const char *dest_swizzle_alpha_to_red = +"vec4 dest_swizzle(vec4 color)\n" +"{" +" float undef;\n" +" return vec4(color.a, undef, undef, undef);" +"}"; + const char *in_source_only = "void main()\n" "{\n" -" gl_FragColor = get_source();\n" +" gl_FragColor = dest_swizzle(get_source());\n" "}\n"; const char *in_normal = "void main()\n" "{\n" -" gl_FragColor = get_source() * get_mask().a;\n" +" gl_FragColor = dest_swizzle(get_source() * get_mask().a);\n" "}\n"; const char *in_ca_source = "void main()\n" "{\n" -" gl_FragColor = get_source() * get_mask();\n" +" gl_FragColor = dest_swizzle(get_source() * get_mask());\n" "}\n"; const char *in_ca_alpha = "void main()\n" "{\n" -" gl_FragColor = get_source().a * get_mask();\n" +" gl_FragColor = dest_swizzle(get_source().a * get_mask());\n" "}\n"; const char *in_ca_dual_blend = "out vec4 color0;\n" @@ -214,6 +227,7 @@ glamor_create_composite_fs(struct shader_key *key) const char *in; const char *header; const char *header_norm = ""; +const char *dest_swizzle; GLuint prog; switch (key->source) { @@ -246,6 +260,21 @@ glamor_create_composite_fs(struct shader_key *key) FatalError("Bad composite shader mask"); } +/* If we're storing to an a8 texture but our texture format is + * GL_RED because of a core context, then we need to make sure to + * put the alpha into the red channel. + */ +switch (key->dest_swizzle) { +case SHADER_DEST_SWIZZLE_DEFAULT: +dest_swizzle = dest_swizzle_default; +break; +case SHADER_DEST_SWIZZLE_ALPHA_TO_RED: +dest_swizzle = dest_swizzle_alpha_to_red; +break; +default: +FatalError("Bad composite shader dest swizzle"); +} + header = header_norm; switch (key->in) { case SHADER_IN_SOURCE_ONLY: @@ -271,8 +300,8 @@ glamor_create_composite_fs(struct shader_key *key) XNFasprintf(&source, "%s" GLAMOR_DEFAULT_PRECISION -"%s%s%s%s%s%s", header, repeat_define, relocate_texture, -rel_sampler, source_fetch, mask_fetch, in); +"%s%s%s%s%s%s%s", header, repeat_define, relocate_texture, +rel_sampler, source_fetch, mask_fetch, dest_swizzle, in); prog = glamor_compile_glsl_prog(GL_FRAGMENT_SHADER, source); free(source); @@ -386,7 +415,7 @@ glamor_lookup_composite_shader(ScreenPtr screen, struct glamor_screen_private *glamor_priv = glamor_get_screen_private(screen); glamor_composite_shader *shader; -shader = &glamor_priv->composite_shader[key->source][key->mask][key->in]; +shade
[PATCH xserver 3/3] squash: glamor: Fix Render blending for alpha-to-red.
Signed-off-by: Eric Anholt --- glamor/glamor_render.c | 36 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c index c36b345..51718d1 100644 --- a/glamor/glamor_render.c +++ b/glamor/glamor_render.c @@ -215,8 +215,8 @@ glamor_create_composite_fs(struct shader_key *key) "out vec4 color1;\n" "void main()\n" "{\n" -" color0 = get_source() * get_mask();\n" -" color1 = get_source().a * get_mask();\n" +" color0 = dest_swizzle(get_source() * get_mask());\n" +" color1 = dest_swizzle(get_source().a * get_mask());\n" "}\n"; const char *header_ca_dual_blend = "#version 130\n"; @@ -422,11 +422,29 @@ glamor_lookup_composite_shader(ScreenPtr screen, struct return shader; } +static GLenum +glamor_translate_blend_alpha_to_red(GLenum blend) +{ +switch (blend) { +case GL_SRC_ALPHA: +return GL_SRC_COLOR; +case GL_DST_ALPHA: +return GL_DST_COLOR; +case GL_ONE_MINUS_SRC_ALPHA: +return GL_ONE_MINUS_SRC_COLOR; +case GL_ONE_MINUS_DST_ALPHA: +return GL_ONE_MINUS_DST_COLOR; +default: +return blend; +} +} + static Bool glamor_set_composite_op(ScreenPtr screen, CARD8 op, struct blendinfo *op_info_result, PicturePtr dest, PicturePtr mask, -enum ca_state ca_state) +enum ca_state ca_state, +struct shader_key *key) { GLenum source_blend, dest_blend; struct blendinfo *op_info; @@ -473,6 +491,14 @@ glamor_set_composite_op(ScreenPtr screen, } } +/* If we're outputting our alpha to the red channel, then any + * reads of alpha for blending need to come from the red channel. + */ +if (key->dest_swizzle == SHADER_DEST_SWIZZLE_ALPHA_TO_RED) { +source_blend = glamor_translate_blend_alpha_to_red(source_blend); +dest_blend = glamor_translate_blend_alpha_to_red(dest_blend); +} + op_info_result->source_blend = source_blend; op_info_result->dest_blend = dest_blend; op_info_result->source_alpha = op_info->source_alpha; @@ -1006,8 +1032,10 @@ glamor_composite_choose_shader(CARD8 op, goto fail; } -if (!glamor_set_composite_op(screen, op, op_info, dest, mask, ca_state)) +if (!glamor_set_composite_op(screen, op, op_info, dest, mask, ca_state, + &key)) { goto fail; +} *shader = glamor_lookup_composite_shader(screen, &key); if ((*shader)->prog == 0) { -- 2.6.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 1/3] glamor: Drop duplicated GLAMOR_DEFAULT_PRECISIONs in render accel.
We only need it once at the top of the shader, so just put it there. Signed-off-by: Eric Anholt --- glamor/glamor_render.c | 17 - 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c index e4beafc..7a50fe9 100644 --- a/glamor/glamor_render.c +++ b/glamor/glamor_render.c @@ -72,7 +72,6 @@ glamor_create_composite_fs(struct shader_key *key) "uniform int source_repeat_mode;\n" "uniform int mask_repeat_mode;\n"; const char *relocate_texture = -GLAMOR_DEFAULT_PRECISION "vec2 rel_tex_coord(vec2 texture, vec4 wh, int repeat) \n" "{\n" " vec2 rel_tex; \n" @@ -119,14 +118,12 @@ glamor_create_composite_fs(struct shader_key *key) "}\n"; const char *source_solid_fetch = -GLAMOR_DEFAULT_PRECISION "uniform vec4 source;\n" "vec4 get_source()\n" "{\n" " return source;\n" "}\n"; const char *source_alpha_pixmap_fetch = -GLAMOR_DEFAULT_PRECISION "varying vec2 source_texture;\n" "uniform sampler2D source_sampler;\n" "uniform vec4 source_wh;" @@ -139,7 +136,6 @@ glamor_create_composite_fs(struct shader_key *key) " source_wh, source_repeat_mode, 0);\n" "}\n"; const char *source_pixmap_fetch = -GLAMOR_DEFAULT_PRECISION "varying vec2 source_texture;\n" "uniform sampler2D source_sampler;\n" "uniform vec4 source_wh;\n" @@ -152,14 +148,12 @@ glamor_create_composite_fs(struct shader_key *key) " source_wh, source_repeat_mode, 1);\n" "}\n"; const char *mask_solid_fetch = -GLAMOR_DEFAULT_PRECISION "uniform vec4 mask;\n" "vec4 get_mask()\n" "{\n" " return mask;\n" "}\n"; const char *mask_alpha_pixmap_fetch = -GLAMOR_DEFAULT_PRECISION "varying vec2 mask_texture;\n" "uniform sampler2D mask_sampler;\n" "uniform vec4 mask_wh;\n" @@ -172,7 +166,6 @@ glamor_create_composite_fs(struct shader_key *key) " mask_wh, mask_repeat_mode, 0);\n" "}\n"; const char *mask_pixmap_fetch = -GLAMOR_DEFAULT_PRECISION "varying vec2 mask_texture;\n" "uniform sampler2D mask_sampler;\n" "uniform vec4 mask_wh;\n" @@ -185,31 +178,26 @@ glamor_create_composite_fs(struct shader_key *key) " mask_wh, mask_repeat_mode, 1);\n" "}\n"; const char *in_source_only = -GLAMOR_DEFAULT_PRECISION "void main()\n" "{\n" " gl_FragColor = get_source();\n" "}\n"; const char *in_normal = -GLAMOR_DEFAULT_PRECISION "void main()\n" "{\n" " gl_FragColor = get_source() * get_mask().a;\n" "}\n"; const char *in_ca_source = -GLAMOR_DEFAULT_PRECISION "void main()\n" "{\n" " gl_FragColor = get_source() * get_mask();\n" "}\n"; const char *in_ca_alpha = -GLAMOR_DEFAULT_PRECISION "void main()\n" "{\n" " gl_FragColor = get_source().a * get_mask();\n" "}\n"; const char *in_ca_dual_blend = -GLAMOR_DEFAULT_PRECISION "out vec4 color0;\n" "out vec4 color1;\n" "void main()\n" @@ -280,7 +268,10 @@ glamor_create_composite_fs(struct shader_key *key) FatalError("Bad composite IN type"); } -XNFasprintf(&source, "%s%s%s%s%s%s%s", header, repeat_define, relocate_texture, +XNFasprintf(&source, +"%s" +GLAMOR_DEFAULT_PRECISION +"%s%s%s%s%s%s", header, repeat_define, relocate_texture, rel_sampler, source_fetch, mask_fetch, in); prog = glamor_compile_glsl_prog(GL_FRAGMENT_SHADER, source); -- 2.6.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 1/2] glamor: Fix copy-like Render operations between 15 and 16 depth.
Keith Packard writes: > Eric Anholt writes: > >> if (op == PictOpSrc) { >> +/* We can't do direct copies between different depths at 16bpp >> + * because r,g,b are allocated to different bits. >> + */ >> +if (dst->pDrawable->bitsPerPixel == 16 && >> +dst->pDrawable->depth != src->pDrawable->depth) { >> +return 0; >> +} >> + > > How can this pass the following check then? The render format is > supposed to completely define the component layout within a pixel? Yes, it's supposed to, but we don't actually store 16bpp anything in glamor, because we suck. We do getimage/putimage in and out of our 32bpp storage using two different 16bpp channel layouts depending on the depth. So, it's all a terrible mess, and trying to represent Render operations on top of it is basically hopeless (this is why the general composite path doesn't support anything for 16bpp). We need texstorage. To more clearly outline how things were broken, let's say I upload some contents to a 15 depth pixmap: xrgb which gets splatted out to a 32bpp rgba actual storage then do a Render Src from this as x1r5g5b5 to a dest with x1r5g5b5, but my dest is 16 depth, hitting this path. It's still stored as 32bpp rgba, so that's: then I download with getimage, and since it's 16bpp glamor_transfer.c gives us back: rggb Wait! I uploaded xrgb and did what should have been a bit-for-bit copy (except the x is undefined), and my rs and gs got shifted! signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 3/5] xephyr: Remove DRI1
Adam Jackson writes: > This only worked if the backend server supported DRI1, which is > stunningly unlikely these days. Patches 1-3 are: Reviewed-by: Eric Anholt I have an old branch around for doing DRI3 under Xephyr, but I'm happy to resurrect what I need when I get around to that. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH RESEND] xfree86: prune duplicate monitor modes.
Michel Dänzer writes: > From: Leo Liu > > same monitor modes added causing memory leak > when looping `xrandr --prop'. > > Signed-off-by: Leo Liu > Signed-off-by: Michel Dänzer > --- > > More than two years later... Can somebody pick this up? > > hw/xfree86/modes/xf86EdidModes.c | 22 ++ > 1 file changed, 22 insertions(+) > > diff --git a/hw/xfree86/modes/xf86EdidModes.c > b/hw/xfree86/modes/xf86EdidModes.c > index 4ee862d..261780b 100644 > --- a/hw/xfree86/modes/xf86EdidModes.c > +++ b/hw/xfree86/modes/xf86EdidModes.c > @@ -1143,6 +1143,27 @@ handle_detailed_monset(struct > detailed_monitor_section *det_mon, void *data) > } > } > +static void > +xf86PruneDuplicateMonitorModes(MonPtr Monitor) > +{ > +DisplayModePtr master, clone, next; > + > +for (master = Monitor->Modes; > + master && master != Monitor->Last; > + master = master->next) { > +for (clone = master->next; > + clone && clone != Monitor->Modes; > + clone = next) { > +next = clone->next; > +if (xf86ModesEqual (master, clone)) { > +if (Monitor->Last == clone) > +Monitor->Last = clone->prev; > +xf86DeleteMode (&Monitor->Modes, clone); > +} > +} > +} > +} > + > /* > * Fill out MonPtr with xf86MonPtr information. > */ > @@ -1204,5 +1225,6 @@ xf86EdidMonitorSet(int scrnIndex, MonPtr Monitor, > xf86MonPtr DDC) > Monitor->Modes = Modes; > Monitor->Last = Mode; > } > +xf86PruneDuplicateMonitorModes(Monitor); > } > } It looks like xf86EdidMonitorSet() is just appending the new DDC modes to the monitor. If you switch monitors, I think you get the DDC modes From both. Shouldn't we just be deleting the old M_T_DRIVER modes right before appending? signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 09/19] glamor: Simplify XV vertex setup.
We were clipping the drawn rectangle to each clip box, then expanding the box to a big triangle to avoid tearing, then drawing each triangle to the destination through a scissor. If we're using a scissor for clipping, though, then we don't need to clip the drawn primitive on the CPU in the first place. Signed-off-by: Eric Anholt --- glamor/glamor_xv.c | 68 -- 1 file changed, 30 insertions(+), 38 deletions(-) diff --git a/glamor/glamor_xv.c b/glamor/glamor_xv.c index 5d31fee..2593d47 100644 --- a/glamor/glamor_xv.c +++ b/glamor/glamor_xv.c @@ -343,45 +343,36 @@ glamor_xv_render(glamor_port_private *port_priv) glEnable(GL_SCISSOR_TEST); -v = glamor_get_vbo_space(screen, 16 * sizeof(GLfloat) * nBox, &vbo_offset); +v = glamor_get_vbo_space(screen, 3 * 4 * sizeof(GLfloat), &vbo_offset); -for (i = 0; i < nBox; i++) { -float off_x = box[i].x1 - port_priv->drw_x; -float off_y = box[i].y1 - port_priv->drw_y; -float diff_x = (float) port_priv->src_w / (float) port_priv->dst_w; -float diff_y = (float) port_priv->src_h / (float) port_priv->dst_h; -float srcx, srcy, srcw, srch; -int dstx, dsty, dstw, dsth; -GLfloat *vptr = v + (i * 8); -GLfloat *tptr = vptr + (8 * nBox); +/* Set up a single primitive covering the area being drawn. We'll + * clip it to port_priv->clip using GL scissors instead of just + * emitting a GL_QUAD per box, because this way we hopefully avoid + * diagonal tearing between the two trangles used to rasterize a + * GL_QUAD. + */ +i = 0; +v[i++] = v_from_x_coord_x(dst_xscale, port_priv->drw_x + dst_x_off); +v[i++] = v_from_x_coord_y(dst_yscale, port_priv->drw_y + dst_y_off); -dstx = box[i].x1 + dst_x_off; -dsty = box[i].y1 + dst_y_off; -dstw = box[i].x2 - box[i].x1; -dsth = box[i].y2 - box[i].y1; +v[i++] = v_from_x_coord_x(dst_xscale, port_priv->drw_x + dst_x_off + + port_priv->dst_w * 2); +v[i++] = v_from_x_coord_y(dst_yscale, port_priv->drw_y + dst_y_off); -srcx = port_priv->src_x + off_x * diff_x; -srcy = port_priv->src_y + off_y * diff_y; -srcw = (port_priv->src_w * dstw) / (float) port_priv->dst_w; -srch = (port_priv->src_h * dsth) / (float) port_priv->dst_h; - -glamor_set_normalize_vcoords(pixmap_priv, - dst_xscale, dst_yscale, - dstx - dstw, - dsty, - dstx + dstw, - dsty + dsth * 2, - vptr); - -glamor_set_normalize_tcoords(src_pixmap_priv[0], - src_xscale[0], - src_yscale[0], - srcx - srcw, - srcy, - srcx + srcw, - srcy + srch * 2, - tptr); -} +v[i++] = v_from_x_coord_x(dst_xscale, port_priv->drw_x + dst_x_off); +v[i++] = v_from_x_coord_y(dst_yscale, port_priv->drw_y + dst_y_off + + port_priv->dst_h * 2); + +v[i++] = t_from_x_coord_x(src_xscale[0], port_priv->src_x); +v[i++] = t_from_x_coord_y(src_yscale[0], port_priv->src_y); + +v[i++] = t_from_x_coord_x(src_xscale[0], port_priv->src_x + + port_priv->src_w * 2); +v[i++] = t_from_x_coord_y(src_yscale[0], port_priv->src_y); + +v[i++] = t_from_x_coord_x(src_xscale[0], port_priv->src_x); +v[i++] = t_from_x_coord_y(src_yscale[0], port_priv->src_y + + port_priv->src_h * 2); glVertexAttribPointer(GLAMOR_VERTEX_POS, 2, GL_FLOAT, GL_FALSE, @@ -389,10 +380,11 @@ glamor_xv_render(glamor_port_private *port_priv) glVertexAttribPointer(GLAMOR_VERTEX_SOURCE, 2, GL_FLOAT, GL_FALSE, - 2 * sizeof(float), vbo_offset + (nBox * 8 * sizeof(GLfloat))); + 2 * sizeof(float), vbo_offset + 6 * sizeof(GLfloat)); glamor_put_vbo_space(screen); +/* Now draw our big triangle, clipped to each of the clip boxes. */ for (i = 0; i < nBox; i++) { int dstx, dsty, dstw, dsth; @@ -402,7 +394,7 @@ glamor_xv_render(glamor_port_private *port_priv) dsth = box[i].y2 - box[i].y1; glScissor(dstx, dsty, dstw, dsth); -glDrawArrays(GL_TRIANGLE_FAN, i * 4, 3); +glDrawArrays(GL_TRIANGLE_FAN, 0, 3); } glDisable(GL_SCISSOR_TEST); -- 2.6.4 ___ xorg-devel@lists.x.or
[PATCH xserver 02/19] glamor: Label programs before linking them.
i965 does most of its compiling at link time, so our debug output for its shaders didn't have the name on. Signed-off-by: Eric Anholt --- glamor/glamor_core.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/glamor/glamor_core.c b/glamor/glamor_core.c index 0104b88..b9948b5 100644 --- a/glamor/glamor_core.c +++ b/glamor/glamor_core.c @@ -87,6 +87,17 @@ glamor_link_glsl_prog(ScreenPtr screen, GLint prog, const char *format, ...) GLint ok; glamor_screen_private *glamor_priv = glamor_get_screen_private(screen); +if (glamor_priv->has_khr_debug) { +char *label; +va_list va; + +va_start(va, format); +XNFvasprintf(&label, format, va); +glObjectLabel(GL_PROGRAM, prog, -1, label); +free(label); +va_end(va); +} + glLinkProgram(prog); glGetProgramiv(prog, GL_LINK_STATUS, &ok); if (!ok) { @@ -100,17 +111,6 @@ glamor_link_glsl_prog(ScreenPtr screen, GLint prog, const char *format, ...) ErrorF("Failed to link: %s\n", info); FatalError("GLSL link failure\n"); } - -if (glamor_priv->has_khr_debug) { -char *label; -va_list va; - -va_start(va, format); -XNFvasprintf(&label, format, va); -glObjectLabel(GL_PROGRAM, prog, -1, label); -free(label); -va_end(va); -} } /* -- 2.6.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 18/19] glamor: Cut down a bunch of conditional handling for RepeatFix.
For hardware that doesn't do actual jumps for conditionals (i915, current vc4 driver), this reduces the number of texture fetches performed (assuming the driver isn't really smart about noticing that the same sampler is used on each side of an if). No performance difference on i965 with x11perf -magpixwin100 (n=40). Signed-off-by: Eric Anholt --- glamor/glamor_render.c | 50 +++--- 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c index ed425f5..da45920 100644 --- a/glamor/glamor_render.c +++ b/glamor/glamor_render.c @@ -105,19 +105,18 @@ glamor_create_composite_fs(struct shader_key *key) /* The texture and the pixmap size is not match eaxctly, so can't sample it directly. * rel_sampler will recalculate the texture coords.*/ const char *rel_sampler = -" vec4 rel_sampler(sampler2D tex_image, vec2 tex, vec4 wh, int repeat, int set_alpha)\n" +" vec4 rel_sampler(sampler2D tex_image, vec2 tex, vec4 wh, int repeat)\n" "{\n" -" tex = rel_tex_coord(tex, wh, repeat);\n" -" if (repeat == RepeatFix + RepeatNone) {\n" -" if (!(tex.x >= 0.0 && tex.x < 1.0 \n" -" && tex.y >= 0.0 && tex.y < 1.0))\n" -" return vec4(0.0, 0.0, 0.0, set_alpha);\n" -" tex = (fract(tex) / wh.xy);\n" +" if (repeat >= RepeatFix) {\n" +" tex = rel_tex_coord(tex, wh, repeat);\n" +" if (repeat == RepeatFix + RepeatNone) {\n" +" if (!(tex.x >= 0.0 && tex.x < 1.0 && \n" +"tex.y >= 0.0 && tex.y < 1.0))\n" +" return vec4(0.0, 0.0, 0.0, 0.0);\n" +" tex = (fract(tex) / wh.xy);\n" +" }\n" " }\n" -" if (set_alpha != 1)\n" -" return texture2D(tex_image, tex);\n" -" else\n" -" return vec4(texture2D(tex_image, tex).rgb, 1.0);\n" +" return texture2D(tex_image, tex);\n" "}\n"; const char *source_solid_fetch = @@ -132,11 +131,8 @@ glamor_create_composite_fs(struct shader_key *key) "uniform vec4 source_wh;" "vec4 get_source()\n" "{\n" -" if (source_repeat_mode < RepeatFix)\n" -" return texture2D(source_sampler, source_texture);\n" -" else \n" -" return rel_sampler(source_sampler, source_texture,\n" -" source_wh, source_repeat_mode, 0);\n" +" return rel_sampler(source_sampler, source_texture,\n" +" source_wh, source_repeat_mode);\n" "}\n"; const char *source_pixmap_fetch = "varying vec2 source_texture;\n" @@ -144,11 +140,9 @@ glamor_create_composite_fs(struct shader_key *key) "uniform vec4 source_wh;\n" "vec4 get_source()\n" "{\n" -" if (source_repeat_mode < RepeatFix) \n" -" return vec4(texture2D(source_sampler, source_texture).rgb, 1);\n" -" else \n" -" return rel_sampler(source_sampler, source_texture,\n" -" source_wh, source_repeat_mode, 1);\n" +" return vec4(rel_sampler(source_sampler, source_texture,\n" +" source_wh, source_repeat_mode).rgb,\n" +" 1.0);\n" "}\n"; const char *mask_none = "vec4 get_mask()\n" @@ -167,11 +161,8 @@ glamor_create_composite_fs(struct shader_key *key) "uniform vec4 mask_wh;\n" "vec4 get_mask()\n" "{\n" -" if (mask_repeat_mode < RepeatFix) \n" -" return texture2D(mask_sampler, mask_texture);\n" -" else \n" -" return rel_sampler(mask_sampler, mask_texture,\n" -" mask_wh, mask_repeat_mode, 0);\n" +" return rel_sampler(mask_sampler, mask_texture,\n" +"
[PATCH xserver 14/19] glamor: Drop extra conditionals for large pixmap handling.
For a small pixmap, it's got a box from 0,0 to width/height, so we can always use that. Signed-off-by: Eric Anholt --- glamor/glamor_utils.h | 18 -- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/glamor/glamor_utils.h b/glamor/glamor_utils.h index 5128a33..f20d520 100644 --- a/glamor/glamor_utils.h +++ b/glamor/glamor_utils.h @@ -56,13 +56,8 @@ #define PIXMAP_PRIV_GET_ACTUAL_SIZE(pixmap, priv, w, h) \ do { \ - if (_X_UNLIKELY(glamor_pixmap_priv_is_large(priv))) { \ - w = priv->box.x2 - priv->box.x1;\ - h = priv->box.y2 - priv->box.y1;\ - } else {\ - w = (pixmap)->drawable.width; \ - h = (pixmap)->drawable.height; \ - } \ + w = priv->box.x2 - priv->box.x1; \ + h = priv->box.y2 - priv->box.y1; \ } while(0) #define glamor_pixmap_fbo_fix_wh_ratio(wh, pixmap, priv) \ @@ -77,13 +72,8 @@ #define pixmap_priv_get_fbo_off(_priv_, _xoff_, _yoff_)\ do {\ -if (_X_UNLIKELY(_priv_ && glamor_pixmap_priv_is_large(_priv_))) { \ - *(_xoff_) = - (_priv_)->box.x1; \ - *(_yoff_) = - (_priv_)->box.y1; \ - } else {\ - *(_xoff_) = 0; \ - *(_yoff_) = 0; \ - } \ + *(_xoff_) = - (_priv_)->box.x1; \ + *(_yoff_) = - (_priv_)->box.y1; \ } while(0) #define xFixedToFloat(_val_) ((float)xFixedToInt(_val_) \ -- 2.6.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 00/19] glamor: Cleanups all over
I've been working on vc4 X performance again, particularly looking at dragging windows with xcompmgr -c running. I'll be sending a separate patch to accelerate a8 rendering on GLES2-class renderers, but even with that things are crazy slow. I think at this point the major problem is the crazy mass of code from rel_sampler(). This series was some cleanups as I worked toward that, and finally one patch that should reduce the cost of rel_sampler on non-control-flow hardware (I see fewer texture sampler requests on vc4). No performance change on i965, but I hope that vc4 is in better shape after it. (I'm away from the hardware at the moment, but should test later today). Eric Anholt (19): ephyr: Make sure we have GLX_ARB_create_context before calling it. glamor: Label programs before linking them. glamor: Clarify when Render fallbacks happen due to an unsupported op. glamor: Drop dead *_from_x_coord_y() functions. glamor: Rename the *y_inverted helpers to not say "inverted". glamor: Drop comment about dead yInverted flag. glamor: Drop dead glamor_pict_format_is_compatible(). glamor: Set up XV sampler uniforms once at program build time. glamor: Simplify XV vertex setup. glamor: Convert XV to using glamor_program.c. glamor: Drop extra SHADER_IN type for no mask present. glamor: Reuse the glamor_program_alpha_* enums for Render. glamor: Simplify the pixmap box looping. glamor: Drop extra conditionals for large pixmap handling. glamor: Clarify some logic in RepeatFix handling. glamor: Clean up formatting of RepeatFix shader code. glamor: Clarify how the repeat values being passed around work. glamor: Cut down a bunch of conditional handling for RepeatFix. glamor: Flip around conditionals in RepeatNone fixups. glamor/glamor_composite_glyphs.c | 10 +- glamor/glamor_copy.c | 15 +-- glamor/glamor_core.c | 22 ++-- glamor/glamor_dash.c | 6 +- glamor/glamor_glyphblt.c | 12 +-- glamor/glamor_lines.c | 6 +- glamor/glamor_points.c | 7 +- glamor/glamor_priv.h | 35 +++--- glamor/glamor_rects.c | 7 +- glamor/glamor_render.c | 161 +--- glamor/glamor_segs.c | 6 +- glamor/glamor_spans.c | 23 ++-- glamor/glamor_text.c | 8 +- glamor/glamor_transfer.c | 16 +-- glamor/glamor_transform.c | 9 +- glamor/glamor_transform.h | 3 +- glamor/glamor_utils.h | 70 glamor/glamor_xv.c | 213 + hw/kdrive/ephyr/ephyr_glamor_glx.c | 34 +++--- 19 files changed, 298 insertions(+), 365 deletions(-) -- 2.6.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 04/19] glamor: Drop dead *_from_x_coord_y() functions.
They've been dead since the yInverted removal (e310387f443b6333edf02c8980daa303505382b4). Signed-off-by: Eric Anholt --- glamor/glamor_utils.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/glamor/glamor_utils.h b/glamor/glamor_utils.h index d4366c1..875c935 100644 --- a/glamor/glamor_utils.h +++ b/glamor/glamor_utils.h @@ -36,10 +36,8 @@ #include "mipict.h" #define v_from_x_coord_x(_xscale_, _x_) ( 2 * (_x_) * (_xscale_) - 1.0) -#define v_from_x_coord_y(_yscale_, _y_) (-2 * (_y_) * (_yscale_) + 1.0) #define v_from_x_coord_y_inverted(_yscale_, _y_) (2 * (_y_) * (_yscale_) - 1.0) #define t_from_x_coord_x(_xscale_, _x_) ((_x_) * (_xscale_)) -#define t_from_x_coord_y(_yscale_, _y_) (1.0 - (_y_) * (_yscale_)) #define t_from_x_coord_y_inverted(_yscale_, _y_) ((_y_) * (_yscale_)) #define pixmap_priv_get_dest_scale(pixmap, _pixmap_priv_, _pxscale_, _pyscale_) \ -- 2.6.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 16/19] glamor: Clean up formatting of RepeatFix shader code.
All sorts of weird indentation, and some cuddled conditional statements deep in the if tree. Signed-off-by: Eric Anholt --- glamor/glamor_render.c | 57 ++ 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c index 4fbf842..a2a7f4a 100644 --- a/glamor/glamor_render.c +++ b/glamor/glamor_render.c @@ -74,30 +74,33 @@ glamor_create_composite_fs(struct shader_key *key) const char *relocate_texture = "vec2 rel_tex_coord(vec2 texture, vec4 wh, int repeat) \n" "{\n" -" vec2 rel_tex; \n" -" rel_tex = texture * wh.xy; \n" +" vec2 rel_tex; \n" +" rel_tex = texture * wh.xy; \n" " if (repeat == RepeatNone)\n" " return rel_tex; \n" -" else if (repeat == RepeatNormal) \n" -" rel_tex = floor(rel_tex) + (fract(rel_tex) / wh.xy);\n" -" else if(repeat == RepeatPad) { \n" -" if (rel_tex.x >= 1.0) rel_tex.x = 1.0 - wh.z * wh.x / 2.; \n" -" else if(rel_tex.x < 0.0) rel_tex.x = 0.0; \n" -" if (rel_tex.y >= 1.0) rel_tex.y = 1.0 - wh.w * wh.y / 2.; \n" -" else if(rel_tex.y < 0.0) rel_tex.y = 0.0; \n" -" rel_tex = rel_tex / wh.xy; \n" -"} \n" -" else if(repeat == RepeatReflect) {\n" +" else if (repeat == RepeatNormal) \n" +" rel_tex = floor(rel_tex) + (fract(rel_tex) / wh.xy); \n" +" else if (repeat == RepeatPad) { \n" +" if (rel_tex.x >= 1.0) \n" +" rel_tex.x = 1.0 - wh.z * wh.x / 2.; \n" +" else if (rel_tex.x < 0.0) \n" +" rel_tex.x = 0.0; \n" +" if (rel_tex.y >= 1.0) \n" +" rel_tex.y = 1.0 - wh.w * wh.y / 2.; \n" +" else if (rel_tex.y < 0.0) \n" +" rel_tex.y = 0.0; \n" +" rel_tex = rel_tex / wh.xy; \n" +" } else if (repeat == RepeatReflect) {\n" " if ((1.0 - mod(abs(floor(rel_tex.x)), 2.0)) < 0.001)\n" -" rel_tex.x = 2.0 - (1.0 - fract(rel_tex.x))/wh.x;\n" +" rel_tex.x = 2.0 - (1.0 - fract(rel_tex.x)) / wh.x;\n" " else \n" -" rel_tex.x = fract(rel_tex.x)/wh.x;\n" +" rel_tex.x = fract(rel_tex.x) / wh.x;\n" " if ((1.0 - mod(abs(floor(rel_tex.y)), 2.0)) < 0.001)\n" -" rel_tex.y = 2.0 - (1.0 - fract(rel_tex.y))/wh.y;\n" +" rel_tex.y = 2.0 - (1.0 - fract(rel_tex.y)) / wh.y;\n" " else \n" -" rel_tex.y = fract(rel_tex.y)/wh.y;\n" -"} \n" -" return rel_tex; \n" +" rel_tex.y = fract(rel_tex.y) / wh.y;\n" +" } \n" +" return rel_tex; \n" "}\n"; /* The texture and the pixmap size is not match eaxctly, so can't sample it directly. * rel_sampler will recalculate the texture coords.*/ @@ -105,7 +108,7 @@ glamor_create_composite_fs(struct shader_key *key) " vec4 rel_sampler(sampler2D tex_image, vec2 tex, vec4 wh, int repeat, int set_alpha)\n" "{\n" " tex = rel_tex_coord(tex, wh, repeat - RepeatFix);\n" -" if (repeat == RepeatFix) {\n" +" if (repeat == RepeatFix) {\n" " if (!(tex.x >= 0.0 && tex.x < 1.0 \n" " && tex.y >= 0.0 && tex.y < 1.0))\n" " return vec4(0.0, 0.0, 0.0, set_alpha);\n" @@ -129,9 +132,9 @@ glamor_create_composite_fs(struct shader_key *key) "uniform vec4 source_wh;" "vec4 get_source()\n" "{\n" -" if (source_repeat_mode < RepeatFix)\n" +" if (source_repeat_mode < RepeatFix)\n" " return texture2D(source_sampler, source_texture);\n" -" else \n" +"
[PATCH xserver 13/19] glamor: Simplify the pixmap box looping.
We had a double loop across h and w, and passed the current x and y out to callers who then used w to multiply/add to an index. Instead, just single loop across w * h. Signed-off-by: Eric Anholt --- glamor/glamor_composite_glyphs.c | 10 ++ glamor/glamor_copy.c | 15 --- glamor/glamor_dash.c | 6 +++--- glamor/glamor_glyphblt.c | 12 ++-- glamor/glamor_lines.c| 6 +++--- glamor/glamor_points.c | 7 --- glamor/glamor_priv.h | 20 +--- glamor/glamor_rects.c| 7 --- glamor/glamor_segs.c | 6 +++--- glamor/glamor_spans.c| 23 --- glamor/glamor_text.c | 8 +--- glamor/glamor_transfer.c | 16 glamor/glamor_transform.c| 7 +++ glamor/glamor_transform.h| 3 +-- glamor/glamor_xv.c | 6 +++--- 15 files changed, 78 insertions(+), 74 deletions(-) diff --git a/glamor/glamor_composite_glyphs.c b/glamor/glamor_composite_glyphs.c index 2e4dfe2..f51ff6d 100644 --- a/glamor/glamor_composite_glyphs.c +++ b/glamor/glamor_composite_glyphs.c @@ -237,10 +237,10 @@ glamor_glyphs_flush(CARD8 op, PicturePtr src, PicturePtr dst, glamor_screen_private *glamor_priv = glamor_get_screen_private(drawable->pScreen); PixmapPtr atlas_pixmap = atlas->atlas; glamor_pixmap_private *atlas_priv = glamor_get_pixmap_private(atlas_pixmap); -glamor_pixmap_fbo *atlas_fbo = glamor_pixmap_fbo_at(atlas_priv, 0, 0); +glamor_pixmap_fbo *atlas_fbo = glamor_pixmap_fbo_at(atlas_priv, 0); PixmapPtr pixmap = glamor_get_drawable_pixmap(drawable); glamor_pixmap_private *pixmap_priv = glamor_get_pixmap_private(pixmap); -int box_x, box_y; +int box_index; int off_x, off_y; glamor_put_vbo_space(drawable->pScreen); @@ -255,11 +255,13 @@ glamor_glyphs_flush(CARD8 op, PicturePtr src, PicturePtr dst, glUniform1i(prog->atlas_uniform, 1); -glamor_pixmap_loop(pixmap_priv, box_x, box_y) { +glamor_pixmap_loop(pixmap_priv, box_index) { BoxPtr box = RegionRects(dst->pCompositeClip); int nbox = RegionNumRects(dst->pCompositeClip); -glamor_set_destination_drawable(drawable, box_x, box_y, TRUE, FALSE, prog->matrix_uniform, &off_x, &off_y); +glamor_set_destination_drawable(drawable, box_index, TRUE, FALSE, +prog->matrix_uniform, +&off_x, &off_y); /* Run over the clip list, drawing the glyphs * in each box diff --git a/glamor/glamor_copy.c b/glamor/glamor_copy.c index 1adfba0..5fed89f 100644 --- a/glamor/glamor_copy.c +++ b/glamor/glamor_copy.c @@ -307,7 +307,7 @@ glamor_copy_fbo_fbo_draw(DrawablePtr src, PixmapPtr dst_pixmap = glamor_get_drawable_pixmap(dst); glamor_pixmap_private *src_priv = glamor_get_pixmap_private(src_pixmap); glamor_pixmap_private *dst_priv = glamor_get_pixmap_private(dst_pixmap); -int src_box_x, src_box_y, dst_box_x, dst_box_y; +int src_box_index, dst_box_index; int dst_off_x, dst_off_y; int src_off_x, src_off_y; GLshort *v; @@ -368,19 +368,20 @@ glamor_copy_fbo_fbo_draw(DrawablePtr src, glEnable(GL_SCISSOR_TEST); -glamor_pixmap_loop(src_priv, src_box_x, src_box_y) { -BoxPtr src_box = glamor_pixmap_box_at(src_priv, src_box_x, src_box_y); +glamor_pixmap_loop(src_priv, src_box_index) { +BoxPtr src_box = glamor_pixmap_box_at(src_priv, src_box_index); args.dx = dx + src_off_x - src_box->x1; args.dy = dy + src_off_y - src_box->y1; -args.src = glamor_pixmap_fbo_at(src_priv, src_box_x, src_box_y); +args.src = glamor_pixmap_fbo_at(src_priv, src_box_index); if (!glamor_use_program(dst_pixmap, gc, prog, &args)) goto bail_ctx; -glamor_pixmap_loop(dst_priv, dst_box_x, dst_box_y) { -glamor_set_destination_drawable(dst, dst_box_x, dst_box_y, FALSE, FALSE, -prog->matrix_uniform, &dst_off_x, &dst_off_y); +glamor_pixmap_loop(dst_priv, dst_box_index) { +glamor_set_destination_drawable(dst, dst_box_index, FALSE, FALSE, +prog->matrix_uniform, +&dst_off_x, &dst_off_y); glScissor(dst_off_x - args.dx, dst_off_y - args.dy, diff --git a/glamor/glamor_dash.c b/glamor/glamor_dash.c index 101228e..a6a11c1 100644 --- a/glamor/glamor_dash.c +++ b/glamor/glamor_dash.c @@ -205,16 +205,16 @@ glamor_dash_loop(DrawablePtr drawable, GCPtr gc, glamor_program *prog, { PixmapPtr pixmap = glamor_get_drawable_pixmap(drawable); glamor_pixmap_private *pixmap_priv = glam
[PATCH xserver 07/19] glamor: Drop dead glamor_pict_format_is_compatible().
This hasn't been used since 2f80c7791bb0b11f261cb1e3e0d89163dcdd0342 (GLAMOR_SEPARATE_TEXTURE removal). Signed-off-by: Eric Anholt --- glamor/glamor_utils.h | 20 1 file changed, 20 deletions(-) diff --git a/glamor/glamor_utils.h b/glamor/glamor_utils.h index 3adc687..5128a33 100644 --- a/glamor/glamor_utils.h +++ b/glamor/glamor_utils.h @@ -852,26 +852,6 @@ glamor_get_rgba_from_pixel(CARD32 pixel, } inline static Bool -glamor_pict_format_is_compatible(PicturePtr picture) -{ -GLenum iformat; -PixmapPtr pixmap = glamor_get_drawable_pixmap(picture->pDrawable); - -iformat = gl_iformat_for_pixmap(pixmap); -switch (iformat) { -case GL_RGBA: -return (picture->format == PICT_a8r8g8b8 || -picture->format == PICT_x8r8g8b8); -case GL_ALPHA: -case GL_RED: -case GL_LUMINANCE: -return (picture->format == PICT_a8); -default: -return FALSE; -} -} - -inline static Bool glamor_is_large_pixmap(PixmapPtr pixmap) { glamor_pixmap_private *priv; -- 2.6.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 17/19] glamor: Clarify how the repeat values being passed around work.
Signed-off-by: Eric Anholt --- glamor/glamor_render.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c index a2a7f4a..ed425f5 100644 --- a/glamor/glamor_render.c +++ b/glamor/glamor_render.c @@ -76,11 +76,11 @@ glamor_create_composite_fs(struct shader_key *key) "{\n" " vec2 rel_tex; \n" " rel_tex = texture * wh.xy; \n" -" if (repeat == RepeatNone)\n" +" if (repeat == RepeatFix + RepeatNone)\n" " return rel_tex; \n" -" else if (repeat == RepeatNormal) \n" +" else if (repeat == RepeatFix + RepeatNormal) \n" " rel_tex = floor(rel_tex) + (fract(rel_tex) / wh.xy); \n" -" else if (repeat == RepeatPad) { \n" +" else if (repeat == RepeatFix + RepeatPad) { \n" " if (rel_tex.x >= 1.0) \n" " rel_tex.x = 1.0 - wh.z * wh.x / 2.; \n" " else if (rel_tex.x < 0.0) \n" @@ -90,7 +90,7 @@ glamor_create_composite_fs(struct shader_key *key) " else if (rel_tex.y < 0.0) \n" " rel_tex.y = 0.0; \n" " rel_tex = rel_tex / wh.xy; \n" -" } else if (repeat == RepeatReflect) {\n" +" } else if (repeat == RepeatFix + RepeatReflect) {\n" " if ((1.0 - mod(abs(floor(rel_tex.x)), 2.0)) < 0.001)\n" " rel_tex.x = 2.0 - (1.0 - fract(rel_tex.x)) / wh.x;\n" " else \n" @@ -107,8 +107,8 @@ glamor_create_composite_fs(struct shader_key *key) const char *rel_sampler = " vec4 rel_sampler(sampler2D tex_image, vec2 tex, vec4 wh, int repeat, int set_alpha)\n" "{\n" -" tex = rel_tex_coord(tex, wh, repeat - RepeatFix);\n" -" if (repeat == RepeatFix) {\n" +" tex = rel_tex_coord(tex, wh, repeat);\n" +" if (repeat == RepeatFix + RepeatNone) {\n" " if (!(tex.x >= 0.0 && tex.x < 1.0 \n" " && tex.y >= 0.0 && tex.y < 1.0))\n" " return vec4(0.0, 0.0, 0.0, set_alpha);\n" -- 2.6.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 06/19] glamor: Drop comment about dead yInverted flag.
Wait long enough, and you don't need to think about it at all. Signed-off-by: Eric Anholt --- glamor/glamor_transform.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/glamor/glamor_transform.c b/glamor/glamor_transform.c index ad06943..564a52d 100644 --- a/glamor/glamor_transform.c +++ b/glamor/glamor_transform.c @@ -77,8 +77,6 @@ glamor_set_destination_drawable(DrawablePtr drawable, * gl_x = (render_x + drawable->x + off_x) * 2 / width - 1 * * gl_x = (render_x) * 2 / width + (drawable->x + off_x) * 2 / width - 1 - * - * I'll think about yInverted later, when I have some way to test */ if (do_drawable_translate) { -- 2.6.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 05/19] glamor: Rename the *y_inverted helpers to not say "inverted".
Signed-off-by: Eric Anholt --- glamor/glamor_utils.h | 30 ++ 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/glamor/glamor_utils.h b/glamor/glamor_utils.h index 875c935..3adc687 100644 --- a/glamor/glamor_utils.h +++ b/glamor/glamor_utils.h @@ -36,9 +36,9 @@ #include "mipict.h" #define v_from_x_coord_x(_xscale_, _x_) ( 2 * (_x_) * (_xscale_) - 1.0) -#define v_from_x_coord_y_inverted(_yscale_, _y_) (2 * (_y_) * (_yscale_) - 1.0) +#define v_from_x_coord_y(_yscale_, _y_) (2 * (_y_) * (_yscale_) - 1.0) #define t_from_x_coord_x(_xscale_, _x_) ((_x_) * (_xscale_)) -#define t_from_x_coord_y_inverted(_yscale_, _y_) ((_y_) * (_yscale_)) +#define t_from_x_coord_y(_yscale_, _y_) ((_y_) * (_yscale_)) #define pixmap_priv_get_dest_scale(pixmap, _pixmap_priv_, _pxscale_, _pyscale_) \ do { \ @@ -311,7 +311,7 @@ texcoord) \ do { \ (texcoord)[0] = t_from_x_coord_x(xscale, _tx_); \ -(texcoord)[1] = t_from_x_coord_y_inverted(yscale, _ty_);\ +(texcoord)[1] = t_from_x_coord_y(yscale, _ty_); \ DEBUGF("normalized point tx %f ty %f \n", (texcoord)[0], \ (texcoord)[1]); \ } while(0) @@ -330,7 +330,7 @@ tx += fbo_x_off; \ ty += fbo_y_off; \ (texcoord)[0] = t_from_x_coord_x(xscale, tx); \ -(texcoord)[1] = t_from_x_coord_y_inverted(yscale, ty); \ +(texcoord)[1] = t_from_x_coord_y(yscale, ty); \ DEBUGF("normalized tx %f ty %f \n", (texcoord)[0], (texcoord)[1]); \ } while(0) @@ -482,8 +482,8 @@ (vertices)[1 * stride] = _t2_ = t_from_x_coord_x(xscale, tx2); \ (vertices)[2 * stride] = _t2_; \ (vertices)[3 * stride] = _t0_; \ -(vertices)[1] = _t1_ = t_from_x_coord_y_inverted(yscale, ty1); \ -(vertices)[2 * stride + 1] = _t5_ = t_from_x_coord_y_inverted(yscale, ty2); \ +(vertices)[1] = _t1_ = t_from_x_coord_y(yscale, ty1); \ +(vertices)[2 * stride + 1] = _t5_ = t_from_x_coord_y(yscale, ty2); \ (vertices)[1 * stride + 1] = _t1_; \ (vertices)[3 * stride + 1] = _t5_; \ } while(0) @@ -562,8 +562,8 @@ (vertices)[2] = t_from_x_coord_x(xscale, x2); \ (vertices)[6] = (vertices)[2]; \ (vertices)[4] = (vertices)[0]; \ -(vertices)[1] = t_from_x_coord_y_inverted(yscale, y1); \ -(vertices)[7] = t_from_x_coord_y_inverted(yscale, y2); \ +(vertices)[1] = t_from_x_coord_y(yscale, y1); \ +(vertices)[7] = t_from_x_coord_y(yscale, y2); \ (vertices)[3] = (vertices)[1]; \ (vertices)[5] = (vertices)[7]; \ } while(0) @@ -596,7 +596,7 @@ vertices) \ do { \ (vertices)[0] = v_from_x_coord_x(xscale, x);\ -(vertices)[1] = v_from_x_coord_y_inverted(yscale, y); \ +(vertices)[1] = v_from_x_coord_y(yscale, y);\ } while(0) #define glamor_set_normalize_tri_vcoords(xscale, yscale, vtx, \ @@ -639,11 +639,9 @@ x2 + fbo_x_off);\ (vertices)[2 * stride] = _t2_; \ (vertices)[3 * stride] = _t0_; \ -(vertices)[1] = _t1_ = v_from_x_coord_y_inverted(yscale, \ - y1 + fbo_y_off); \ +(vertices)[1] = _t1_ = v_from_x_coord_y(yscale, y1 + fbo_y_off);\ (vertices)[2 * stride + 1] = _t5_ = \ -v_from_x_coord_y_inverted(yscale, \ - y2 + fbo_y_off); \ +v_from_x_coord_y(yscale, y2 + fbo_y_off); \ (vertices)[1 * stride + 1] = _t1_; \ (vertices)[3 * stride + 1] = _t5_; \ } while(0) @@ -675,8 +673,8 @@ (vertices)[2] = v_from_x_coord_x(xscale, x2); \
[PATCH xserver 08/19] glamor: Set up XV sampler uniforms once at program build time.
No sense doing it on every draw. Signed-off-by: Eric Anholt --- glamor/glamor_xv.c | 20 +++- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/glamor/glamor_xv.c b/glamor/glamor_xv.c index 6e1a588..5d31fee 100644 --- a/glamor/glamor_xv.c +++ b/glamor/glamor_xv.c @@ -114,7 +114,7 @@ static void glamor_init_xv_shader(ScreenPtr screen) { glamor_screen_private *glamor_priv; -GLint fs_prog, vs_prog; +GLint fs_prog, vs_prog, sampler_loc; glamor_priv = glamor_get_screen_private(screen); glamor_make_current(glamor_priv); @@ -130,6 +130,15 @@ glamor_init_xv_shader(ScreenPtr screen) glBindAttribLocation(glamor_priv->xv_prog, GLAMOR_VERTEX_SOURCE, "v_texcoord0"); glamor_link_glsl_prog(screen, glamor_priv->xv_prog, "xv"); + +glUseProgram(glamor_priv->xv_prog); +sampler_loc = glGetUniformLocation(glamor_priv->xv_prog, "y_sampler"); +glUniform1i(sampler_loc, 0); +sampler_loc = glGetUniformLocation(glamor_priv->xv_prog, "u_sampler"); +glUniform1i(sampler_loc, 1); +sampler_loc = glGetUniformLocation(glamor_priv->xv_prog, "v_sampler"); +glUniform1i(sampler_loc, 2); + } #define ClipValue(v,min,max) ((v) < (min) ? (min) : (v) > (max) ? (max) : (v)) @@ -258,7 +267,7 @@ glamor_xv_render(glamor_port_private *port_priv) float uco[3], vco[3], off[3]; float bright, cont, gamma; int ref = port_priv->transform_index; -GLint uloc, sampler_loc; +GLint uloc; GLfloat *v; char *vbo_offset; @@ -329,13 +338,6 @@ glamor_xv_render(glamor_port_private *port_priv) glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE); glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE); -sampler_loc = glGetUniformLocation(glamor_priv->xv_prog, "y_sampler"); -glUniform1i(sampler_loc, 0); -sampler_loc = glGetUniformLocation(glamor_priv->xv_prog, "u_sampler"); -glUniform1i(sampler_loc, 1); -sampler_loc = glGetUniformLocation(glamor_priv->xv_prog, "v_sampler"); -glUniform1i(sampler_loc, 2); - glEnableVertexAttribArray(GLAMOR_VERTEX_POS); glEnableVertexAttribArray(GLAMOR_VERTEX_SOURCE); -- 2.6.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 15/19] glamor: Clarify some logic in RepeatFix handling.
wh ratios are != 1.0 only when large, so with that we can simplify down how we end up with RepeatFix being used. Signed-off-by: Eric Anholt --- glamor/glamor_render.c | 21 +++-- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c index ec757b3..4fbf842 100644 --- a/glamor/glamor_render.c +++ b/glamor/glamor_render.c @@ -565,22 +565,15 @@ glamor_set_composite_texture(glamor_screen_private *glamor_priv, int unit, * GLES2 doesn't support RepeatNone. We need to fix it anyway. * **/ -if (repeat_type != RepeatNone) -repeat_type += RepeatFix; -else if (glamor_priv->gl_flavor == GLAMOR_GL_ES2 - || glamor_pixmap_priv_is_large(pixmap_priv)) { -if (picture->transform) -repeat_type += RepeatFix; -} -if (repeat_type >= RepeatFix) { +if (glamor_pixmap_priv_is_large(pixmap_priv) || +(glamor_priv->gl_flavor == GLAMOR_GL_ES2 && repeat_type == RepeatNone && + picture->transform)) { glamor_pixmap_fbo_fix_wh_ratio(wh, pixmap, pixmap_priv); -if ((wh[0] != 1.0 || wh[1] != 1.0) -|| (glamor_priv->gl_flavor == GLAMOR_GL_ES2 -&& repeat_type == RepeatFix)) -glUniform4fv(wh_location, 1, wh); -else -repeat_type -= RepeatFix; +glUniform4fv(wh_location, 1, wh); + +repeat_type += RepeatFix; } + glUniform1i(repeat_location, repeat_type); } -- 2.6.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 03/19] glamor: Clarify when Render fallbacks happen due to an unsupported op.
Signed-off-by: Eric Anholt --- glamor/glamor_render.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c index 5712cf8..51718d1 100644 --- a/glamor/glamor_render.c +++ b/glamor/glamor_render.c @@ -1577,8 +1577,10 @@ glamor_composite(CARD8 op, if (!glamor_pixmap_has_fbo(dest_pixmap)) goto fail; -if (op >= ARRAY_SIZE(composite_op_info)) +if (op >= ARRAY_SIZE(composite_op_info)) { +glamor_fallback("Unsupported composite op %x\n", op); goto fail; +} if (mask && mask->componentAlpha && !glamor_priv->has_dual_blend) { if (op == PictOpAtop -- 2.6.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 01/19] ephyr: Make sure we have GLX_ARB_create_context before calling it.
This should fix aborts()s from epoxy on old software stacks. Signed-off-by: Eric Anholt --- hw/kdrive/ephyr/ephyr_glamor_glx.c | 34 -- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/hw/kdrive/ephyr/ephyr_glamor_glx.c b/hw/kdrive/ephyr/ephyr_glamor_glx.c index 0981144..636150d 100644 --- a/hw/kdrive/ephyr/ephyr_glamor_glx.c +++ b/hw/kdrive/ephyr/ephyr_glamor_glx.c @@ -330,20 +330,26 @@ ephyr_glamor_glx_screen_init(xcb_window_t win) "GLX_EXT_create_context_es2_profile\n"); } } else { -static const int context_attribs[] = { -GLX_CONTEXT_PROFILE_MASK_ARB, -GLX_CONTEXT_CORE_PROFILE_BIT_ARB, -GLX_CONTEXT_MAJOR_VERSION_ARB, -GLAMOR_GL_CORE_VER_MAJOR, -GLX_CONTEXT_MINOR_VERSION_ARB, -GLAMOR_GL_CORE_VER_MINOR, -0, -}; -oldErrorHandler = XSetErrorHandler(ephyr_glx_error_handler); -ctx = glXCreateContextAttribsARB(dpy, fb_config, NULL, True, - context_attribs); -XSync(dpy, False); -XSetErrorHandler(oldErrorHandler); +if (epoxy_has_glx_extension(dpy, DefaultScreen(dpy), +"GLX_ARB_create_context")) { +static const int context_attribs[] = { +GLX_CONTEXT_PROFILE_MASK_ARB, +GLX_CONTEXT_CORE_PROFILE_BIT_ARB, +GLX_CONTEXT_MAJOR_VERSION_ARB, +GLAMOR_GL_CORE_VER_MAJOR, +GLX_CONTEXT_MINOR_VERSION_ARB, +GLAMOR_GL_CORE_VER_MINOR, +0, +}; +oldErrorHandler = XSetErrorHandler(ephyr_glx_error_handler); +ctx = glXCreateContextAttribsARB(dpy, fb_config, NULL, True, + context_attribs); +XSync(dpy, False); +XSetErrorHandler(oldErrorHandler); +} else { +ctx = NULL; +} + if (!ctx) ctx = glXCreateContext(dpy, visual_info, NULL, True); } -- 2.6.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 19/19] glamor: Flip around conditionals in RepeatNone fixups.
Signed-off-by: Eric Anholt --- glamor/glamor_render.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c index da45920..73ac831 100644 --- a/glamor/glamor_render.c +++ b/glamor/glamor_render.c @@ -110,8 +110,8 @@ glamor_create_composite_fs(struct shader_key *key) " if (repeat >= RepeatFix) {\n" " tex = rel_tex_coord(tex, wh, repeat);\n" " if (repeat == RepeatFix + RepeatNone) {\n" -" if (!(tex.x >= 0.0 && tex.x < 1.0 && \n" -"tex.y >= 0.0 && tex.y < 1.0))\n" +" if (tex.x < 0.0 || tex.x >= 1.0 || \n" +" tex.y < 0.0 || tex.y >= 1.0)\n" " return vec4(0.0, 0.0, 0.0, 0.0);\n" " tex = (fract(tex) / wh.xy);\n" " }\n" -- 2.6.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 12/19] glamor: Reuse the glamor_program_alpha_* enums for Render.
This is a step toward using glamor_program.c for Render acceleration. Signed-off-by: Eric Anholt --- glamor/glamor_priv.h | 12 ++-- glamor/glamor_render.c | 28 ++-- 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h index b70533a..60b0a66 100644 --- a/glamor/glamor_priv.h +++ b/glamor/glamor_priv.h @@ -107,14 +107,6 @@ enum shader_mask { SHADER_MASK_COUNT, }; -enum shader_in { -SHADER_IN_NORMAL, -SHADER_IN_CA_SOURCE, -SHADER_IN_CA_ALPHA, -SHADER_IN_CA_DUAL_BLEND, -SHADER_IN_COUNT, -}; - enum shader_dest_swizzle { SHADER_DEST_SWIZZLE_DEFAULT, SHADER_DEST_SWIZZLE_ALPHA_TO_RED, @@ -124,7 +116,7 @@ enum shader_dest_swizzle { struct shader_key { enum shader_source source; enum shader_mask mask; -enum shader_in in; +glamor_program_alpha in; enum shader_dest_swizzle dest_swizzle; }; @@ -291,7 +283,7 @@ typedef struct glamor_screen_private { int render_nr_quads; glamor_composite_shader composite_shader[SHADER_SOURCE_COUNT] [SHADER_MASK_COUNT] -[SHADER_IN_COUNT] +[glamor_program_alpha_count] [SHADER_DEST_SWIZZLE_COUNT]; /* shaders to restore a texture to another texture. */ diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c index d1b7a15..ec757b3 100644 --- a/glamor/glamor_render.c +++ b/glamor/glamor_render.c @@ -278,16 +278,16 @@ glamor_create_composite_fs(struct shader_key *key) header = header_norm; switch (key->in) { -case SHADER_IN_NORMAL: +case glamor_program_alpha_normal: in = in_normal; break; -case SHADER_IN_CA_SOURCE: +case glamor_program_alpha_ca_first: in = in_ca_source; break; -case SHADER_IN_CA_ALPHA: +case glamor_program_alpha_ca_second: in = in_ca_alpha; break; -case SHADER_IN_CA_DUAL_BLEND: +case glamor_program_alpha_dual_blend: in = in_ca_dual_blend; header = header_ca_dual_blend; break; @@ -368,7 +368,7 @@ glamor_create_composite_shader(ScreenPtr screen, struct shader_key *key, glBindAttribLocation(prog, GLAMOR_VERTEX_SOURCE, "v_texcoord0"); glBindAttribLocation(prog, GLAMOR_VERTEX_MASK, "v_texcoord1"); -if (key->in == SHADER_IN_CA_DUAL_BLEND) { +if (key->in == glamor_program_alpha_dual_blend) { glBindFragDataLocationIndexed(prog, 0, 0, "color0"); glBindFragDataLocationIndexed(prog, 0, 1, "color1"); } @@ -674,7 +674,7 @@ static const int pict_format_combine_tab[][3] = { static Bool combine_pict_format(PictFormatShort * des, const PictFormatShort src, -const PictFormatShort mask, enum shader_in in_ca) +const PictFormatShort mask, glamor_program_alpha in_ca) { PictFormatShort new_vis; int src_type, mask_type, src_bpp; @@ -691,19 +691,19 @@ combine_pict_format(PictFormatShort * des, const PictFormatShort src, new_vis = PICT_FORMAT_VIS(src) | PICT_FORMAT_VIS(mask); switch (in_ca) { -case SHADER_IN_NORMAL: +case glamor_program_alpha_normal: src_type = PICT_FORMAT_TYPE(src); mask_type = PICT_TYPE_A; break; -case SHADER_IN_CA_SOURCE: +case glamor_program_alpha_ca_first: src_type = PICT_FORMAT_TYPE(src); mask_type = PICT_FORMAT_TYPE(mask); break; -case SHADER_IN_CA_ALPHA: +case glamor_program_alpha_ca_second: src_type = PICT_TYPE_A; mask_type = PICT_FORMAT_TYPE(mask); break; -case SHADER_IN_CA_DUAL_BLEND: +case glamor_program_alpha_dual_blend: src_type = PICT_FORMAT_TYPE(src); mask_type = PICT_FORMAT_TYPE(mask); break; @@ -867,19 +867,19 @@ glamor_composite_choose_shader(CARD8 op, } if (!mask->componentAlpha) { -key.in = SHADER_IN_NORMAL; +key.in = glamor_program_alpha_normal; } else { if (op == PictOpClear) key.mask = SHADER_MASK_NONE; else if (glamor_priv->has_dual_blend) -key.in = SHADER_IN_CA_DUAL_BLEND; +key.in = glamor_program_alpha_dual_blend; else if (op == PictOpSrc || op == PictOpAdd || op == PictOpIn || op == PictOpOut || op == PictOpOverReverse) -key.in = SHADER_IN_CA_SOURCE; +key.in = glamor_program_alpha_ca_second; else if (op == PictOpOutReverse || op == PictOpInReverse) { -key.in = SHADER_IN_CA_ALPHA; +key.in = glamor_program_alpha_ca_first; } else { glamor_fallback("Unsupported component alpha op: %d\n", op); -- 2.6.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http:/
[PATCH xserver 10/19] glamor: Convert XV to using glamor_program.c.
One less custom path! By following the common glamor_program.c use pattern, we get the ability to handle large pixmaps as the destination. It's also one less place where glamor_utils.h coordinate transformation happens. Signed-off-by: Eric Anholt --- glamor/glamor_priv.h | 2 +- glamor/glamor_xv.c | 147 --- 2 files changed, 71 insertions(+), 78 deletions(-) diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h index f1eed5b..d78db7f 100644 --- a/glamor/glamor_priv.h +++ b/glamor/glamor_priv.h @@ -318,7 +318,7 @@ typedef struct glamor_screen_private { Bool logged_any_fbo_allocation_failure; /* xv */ -GLint xv_prog; +glamor_program xv_prog; struct glamor_context ctx; } glamor_screen_private; diff --git a/glamor/glamor_xv.c b/glamor/glamor_xv.c index 2593d47..e8c849d 100644 --- a/glamor/glamor_xv.c +++ b/glamor/glamor_xv.c @@ -37,6 +37,7 @@ #endif #include "glamor_priv.h" +#include "glamor_transform.h" #include "glamor_transfer.h" #include @@ -58,36 +59,36 @@ typedef struct tagREF_TRANSFORM { #define RTFContrast(a) (1.0 + ((a)*1.0)/1000.0) #define RTFHue(a) (((a)*3.1416)/1000.0) -static const char *xv_vs = "attribute vec4 v_position;\n" -"attribute vec4 v_texcoord0;\n" -"varying vec2 tcs;\n" -"void main()\n" -"{\n" -" gl_Position = v_position;\n" -"tcs = v_texcoord0.xy;\n" -"}\n"; - -static const char *xv_ps = GLAMOR_DEFAULT_PRECISION -"uniform sampler2D y_sampler;\n" -"uniform sampler2D u_sampler;\n" -"uniform sampler2D v_sampler;\n" -"uniform vec4 offsetyco;\n" -"uniform vec4 ucogamma;\n" -"uniform vec4 vco;\n" -"varying vec2 tcs;\n" -"float sample;\n" -"vec4 temp1;\n" -"void main()\n" -"{\n" -"sample = texture2D(y_sampler, tcs).w;\n" -"temp1.xyz = offsetyco.www * vec3(sample) + offsetyco.xyz;\n" -"sample = texture2D(u_sampler, tcs).w;\n" -"temp1.xyz = ucogamma.xyz * vec3(sample) + temp1.xyz;\n" -"sample = texture2D(v_sampler, tcs).w;\n" -"temp1.xyz = clamp(vco.xyz * vec3(sample) + temp1.xyz, 0.0, 1.0);\n" -"temp1.w = 1.0;\n" -"gl_FragColor = temp1;\n" -"}\n"; +static const glamor_facet glamor_facet_xv_planar = { +.name = "xv_planar", + +.source_name = "v_texcoord0", +.vs_vars = ("attribute vec2 position;\n" +"attribute vec2 v_texcoord0;\n" +"varying vec2 tcs;\n"), +.vs_exec = (GLAMOR_POS(gl_Position, position) +"tcs = v_texcoord0;\n"), + +.fs_vars = ("uniform sampler2D y_sampler;\n" +"uniform sampler2D u_sampler;\n" +"uniform sampler2D v_sampler;\n" +"uniform vec4 offsetyco;\n" +"uniform vec4 ucogamma;\n" +"uniform vec4 vco;\n" +"varying vec2 tcs;\n"), +.fs_exec = ( +"float sample;\n" +"vec4 temp1;\n" +"sample = texture2D(y_sampler, tcs).w;\n" +"temp1.xyz = offsetyco.www * vec3(sample) + offsetyco.xyz;\n" +"sample = texture2D(u_sampler, tcs).w;\n" +"temp1.xyz = ucogamma.xyz * vec3(sample) + temp1.xyz;\n" +"sample = texture2D(v_sampler, tcs).w;\n" +"temp1.xyz = clamp(vco.xyz * vec3(sample) + temp1.xyz, 0.0, 1.0);\n" +"temp1.w = 1.0;\n" +"gl_FragColor = temp1;\n" +), +}; #define MAKE_ATOM(a) MakeAtom(a, sizeof(a) - 1, TRUE) @@ -113,30 +114,19 @@ int glamor_xv_num_images = ARRAY_SIZE(glamor_xv_images); static void glamor_init_xv_shader(ScreenPtr screen) { -glamor_screen_private *glamor_priv; -GLint fs_prog, vs_prog, sampler_loc; - -glamor_priv = glamor_get_screen_private(screen); -glamor_make_current(glamor_priv); -glamor_priv->xv_prog = glCreateProgram(); - -vs_prog = glamor_compile_glsl_prog(GL_VERTEX_SHADER, xv_vs); -fs_prog = glamor_compile_glsl_prog(GL_FRAGMENT_SHADER, xv_ps); -glAttachShader(glamor_priv->xv_prog, vs_prog); -glAttachShader(glamor_priv->xv_prog, fs_prog); +glamor_screen_private *glamor_priv = glamor_get_screen_private(screen); +GLint sampler_loc; -glBindAttribLocation(glamor_priv->xv_prog, - GLAMOR_VERTEX_POS, &qu
[PATCH xserver 11/19] glamor: Drop extra SHADER_IN type for no mask present.
We can just hand in a constant mask and the driver will optimize away the multiplication for us. Signed-off-by: Eric Anholt --- glamor/glamor_priv.h | 1 - glamor/glamor_render.c | 17 ++--- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h index d78db7f..b70533a 100644 --- a/glamor/glamor_priv.h +++ b/glamor/glamor_priv.h @@ -108,7 +108,6 @@ enum shader_mask { }; enum shader_in { -SHADER_IN_SOURCE_ONLY, SHADER_IN_NORMAL, SHADER_IN_CA_SOURCE, SHADER_IN_CA_ALPHA, diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c index 51718d1..d1b7a15 100644 --- a/glamor/glamor_render.c +++ b/glamor/glamor_render.c @@ -147,6 +147,11 @@ glamor_create_composite_fs(struct shader_key *key) " return rel_sampler(source_sampler, source_texture,\n" " source_wh, source_repeat_mode, 1);\n" "}\n"; +const char *mask_none = +"vec4 get_mask()\n" +"{\n" +" return vec4(0.0, 0.0, 0.0, 1.0);\n" +"}\n"; const char *mask_solid_fetch = "uniform vec4 mask;\n" "vec4 get_mask()\n" @@ -190,11 +195,6 @@ glamor_create_composite_fs(struct shader_key *key) " return vec4(color.a, undef, undef, undef);" "}"; -const char *in_source_only = -"void main()\n" -"{\n" -" gl_FragColor = dest_swizzle(get_source());\n" -"}\n"; const char *in_normal = "void main()\n" "{\n" @@ -246,6 +246,7 @@ glamor_create_composite_fs(struct shader_key *key) switch (key->mask) { case SHADER_MASK_NONE: +mask_fetch = mask_none; break; case SHADER_MASK_SOLID: mask_fetch = mask_solid_fetch; @@ -277,9 +278,6 @@ glamor_create_composite_fs(struct shader_key *key) header = header_norm; switch (key->in) { -case SHADER_IN_SOURCE_ONLY: -in = in_source_only; -break; case SHADER_IN_NORMAL: in = in_normal; break; @@ -693,8 +691,6 @@ combine_pict_format(PictFormatShort * des, const PictFormatShort src, new_vis = PICT_FORMAT_VIS(src) | PICT_FORMAT_VIS(mask); switch (in_ca) { -case SHADER_IN_SOURCE_ONLY: -return TRUE; case SHADER_IN_NORMAL: src_type = PICT_FORMAT_TYPE(src); mask_type = PICT_TYPE_A; @@ -893,7 +889,6 @@ glamor_composite_choose_shader(CARD8 op, } else { key.mask = SHADER_MASK_NONE; -key.in = SHADER_IN_SOURCE_ONLY; } if (dest_pixmap->drawable.bitsPerPixel <= 8 && -- 2.6.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 00/19] glamor: Cleanups all over
Dave Airlie writes: > On 28 January 2016 at 18:21, Dave Airlie wrote: >> On 28 January 2016 at 11:56, Eric Anholt wrote: >>> I've been working on vc4 X performance again, particularly looking at >>> dragging windows with xcompmgr -c running. I'll be sending a separate >>> patch to accelerate a8 rendering on GLES2-class renderers, but even >>> with that things are crazy slow. I think at this point the major >>> problem is the crazy mass of code from rel_sampler(). >>> >>> This series was some cleanups as I worked toward that, and finally one >>> patch that should reduce the cost of rel_sampler on non-control-flow >>> hardware (I see fewer texture sampler requests on vc4). No >>> performance change on i965, but I hope that vc4 is in better shape >>> after it. (I'm away from the hardware at the moment, but should test >>> later today). >> >> 1-3 look good to me, >> >> Reviewed-by: Dave Airlie > > Okay I've dug through the rest, assuming they don't break anything all > look good. > > Reviewed-by: Dave Airlie > > good to know if they have any effect on VC4. Finally got my giant edifice of scripts working: 8.09199% +/- 0.416675% (n=5) on comppixwin100 on vc4. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 14/19] glamor: Drop extra conditionals for large pixmap handling.
Eric Anholt writes: > For a small pixmap, it's got a box from 0,0 to width/height, so we can > always use that. This seemed so sensible, but it turns out that for MEMORY pixmaps we'll attach an FBO to it in the glamor_picture.c code, without setting its box (which is still zeroed). This may be lunacy, but it means this patch is busted for now. I'm going to revisit this after landing the rest of the series. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH rendercheck 3/5] Use ELF sections to make test setup easier.
Managing the group logic was really error-prone (forget to edit success_mask when copy and pasting? Forget to printf a description of the group?). Most new tests being written can be described as a single call that does a couple subtests. This doesn't convert all of the tests. Some of the remaining ones use "win" for presenting results (which we may want to just put in a global?), and the rest use the pre-created pictures, which would need some much bigger refactoring if we want to move their test logic into their test files. Signed-off-by: Eric Anholt --- main.c | 47 ++- rendercheck.h| 51 --- t_gtk_argb_xbgr.c| 17 - t_libreoffice_xrgb.c | 18 -- tests.c | 37 ++--- 5 files changed, 120 insertions(+), 50 deletions(-) diff --git a/main.c b/main.c index b5d67cc..4cec99b 100644 --- a/main.c +++ b/main.c @@ -121,27 +121,38 @@ struct { {TEST_REPEAT, "repeat"}, {TEST_TRIANGLES, "triangles"}, {TEST_BUG7366, "bug7366"}, -{TEST_GTK_ARGB_XBGR, "gtk_argb_xbgr"}, -{TEST_LIBREOFFICE_XRGB, "libreoffice_xrgb"}, {0, NULL} }; +static void +print_test_separator(int i) +{ +if (i % 5 == 0) { +if(i != 0) +putc('\n', stderr); +putc('\t', stderr); +} else { +fprintf(stderr, ", "); +} +} + void print_tests(FILE *file, int tests) { int i, j; for (i=0, j=0; available_tests[i].name; i++) { if (!(available_tests[i].flag & tests)) continue; -if (j % 5 == 0) { -if(j != 0) -putc('\n', stderr); -putc('\t', stderr); -} else { -fprintf(stderr, ", "); -} + print_test_separator(j++); fprintf(stderr, "%s", available_tests[i].name); j++; } +for_each_test(test) { + if (!(test->bit & tests)) + continue; + print_test_separator(j++); +fprintf(stderr, "%s", test->arg_name); +j++; +} if (j) fprintf(file, "\n"); } @@ -169,7 +180,7 @@ int main(int argc, char **argv) XSetWindowAttributes as; picture_info window; char *display = NULL; - char *test, *format, *opname, *nextname; + char *test_name, *format, *opname, *nextname; static struct option longopts[] = { { "display",required_argument, NULL, 'd' }, @@ -239,15 +250,25 @@ int main(int argc, char **argv) /* disable all tests */ enabled_tests = 0; - while ((test = strsep(&nextname, ",")) != NULL) { + while ((test_name = strsep(&nextname, ",")) != NULL) { int i; + bool found = false; for(i=0; available_tests[i].name; i++) { - if(strcmp(test, available_tests[i].name) == 0) { + if(strcmp(test_name, available_tests[i].name) == 0) { enabled_tests |= available_tests[i].flag; + found = true; + break; + } + } + for_each_test(test) { + if (strcmp(test_name, + test->arg_name) == 0) { + enabled_tests |= test->bit; + found = true; break; } } - if(available_tests[i].name == NULL) + if (!found) usage(argv[0]); } diff --git a/rendercheck.h b/rendercheck.h index 2195cb4..f0fa7b7 100644 --- a/rendercheck.h +++ b/rendercheck.h @@ -64,6 +64,19 @@ struct op_info { bool disabled; }; +struct rendercheck_test_result { + int tests; + int passed; +}; + +static inline void +record_result(struct rendercheck_test_result *result, bool success) +{ + result->tests++; + if (success) + result->passed++; +} + #define TEST_FILL 0x0001 #define TEST_DSTCOORDS 0x0002 #define TEST_SRCCOORDS 0x0004 @@ -77,8 +90,27 @@ struct op_info { #de
[PATCH rendercheck 5/5] autogen: Set a default prefix for patches if unset.
Signed-off-by: Eric Anholt --- autogen.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/autogen.sh b/autogen.sh index 1b15e18..c262d6b 100755 --- a/autogen.sh +++ b/autogen.sh @@ -11,3 +11,6 @@ autoreconf -v --install || exit 1 cd $ORIGDIR || exit $? $srcdir/configure --enable-maintainer-mode "$@" + +git config --local --get format.subjectPrefix || +git config --local format.subjectPrefix "PATCH rendercheck" -- 2.7.0 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH rendercheck 2/5] Save the list of active formats in a global for use by tests.
I'd like to move driving of tests out of tests.c and into t_*.c, and part of that will be allowing tests to use the formats list. While I'm at it, save the name of the format in the array so it doesn't need to be recomputed. Signed-off-by: Eric Anholt --- rendercheck.h | 8 tests.c | 51 ++- 2 files changed, 34 insertions(+), 25 deletions(-) diff --git a/rendercheck.h b/rendercheck.h index 55ffcff..2195cb4 100644 --- a/rendercheck.h +++ b/rendercheck.h @@ -80,6 +80,14 @@ struct op_info { #define TEST_GTK_ARGB_XBGR 0x2000 #define TEST_LIBREOFFICE_XRGB 0x4000 +struct render_format { + XRenderPictFormat *format; + char *name; +}; + +extern struct render_format *formats; +extern int nformats; + extern int pixmap_move_iter; extern int win_width, win_height; extern struct op_info ops[]; diff --git a/tests.c b/tests.c index 62fa34b..456e778 100644 --- a/tests.c +++ b/tests.c @@ -27,8 +27,8 @@ #include "rendercheck.h" -static XRenderPictFormat **format_list; -static int nformats; +struct render_format *formats; +int nformats; static int argb32index; /* Note: changing the order of these colors may disrupt tests that depend on @@ -240,36 +240,35 @@ create_formats_list(Display *dpy) { int i; int nformats_allocated = 5; -XRenderPictFormat templ; +XRenderPictFormat templ, *format; memset(&templ, 0, sizeof(templ)); templ.type = PictTypeDirect; -format_list = malloc(sizeof(XRenderPictFormat *) * nformats_allocated); -if (format_list == NULL) +formats = malloc(sizeof(*formats) * nformats_allocated); +if (formats == NULL) errx(1, "malloc error"); nformats = 0; argb32index = -1; for (i = 0; ; i++) { - char *name; int alphabits, redbits; if (nformats + 1 == nformats_allocated) { nformats_allocated *= 2; - format_list = realloc(format_list, sizeof(XRenderPictFormat *) * - nformats_allocated); - if (format_list == NULL) + formats = realloc(formats, sizeof(*formats) * nformats_allocated); + if (formats == NULL) errx(1, "realloc error"); } - format_list[nformats] = XRenderFindFormat(dpy, PictFormatType, &templ, - i); - if (format_list[nformats] == NULL) - break; + format = XRenderFindFormat(dpy, PictFormatType, &templ, i); + if (!format) + break; - alphabits = bit_count(format_list[nformats]->direct.alphaMask); - redbits = bit_count(format_list[nformats]->direct.redMask); + formats[nformats].format = format; + + alphabits = bit_count(format->direct.alphaMask); + redbits = bit_count(format->direct.redMask); /* Our testing code isn't all that hot, so don't bother trying at * the low depths yet. @@ -280,31 +279,33 @@ create_formats_list(Display *dpy) continue; } - describe_format(&name, NULL, format_list[nformats]); + describe_format(&formats[nformats].name, NULL, format); if (format_whitelist_len != 0) { bool ok = false; int j; for (j = 0; j < format_whitelist_len; j++) { - if (strcmp(format_whitelist[j], name) == 0) { + if (strcmp(format_whitelist[j], formats[nformats].name) == 0) { ok = true; break; } } if (!ok) { - printf("Ignoring server-supported format: %s\n", name); + printf("Ignoring server-supported format: %s\n", + formats[nformats].name); + free(formats[nformats].name); + formats[nformats].name = NULL; continue; } } - if (format_list[nformats] == XRenderFindStandardFormat(dpy, - PictStandardARGB32)) + if (format == XRenderFindStandardFormat(dpy, PictStandardARGB32)) { argb32index = nformats; } - printf("Found server-supported format: %s\n", name); + printf("Found server-supported format: %s\n", formats[nformats].name); nformats++; } @@ -336,7 +337,7 @@ do_tests(Display *dpy, picture_info *win) errx(1, "malloc error"); for (i = 0; i < num_dests; i++) { - dests[i].format = format_list[i]; + dests[i].format = formats[i].format; dests[i].d = XCreatePixmap(dpy, DefaultRootWindow(dpy), win_width, win_height, dests[i].format->depth); dests[i].pict = XRenderCreatePicture(dpy, dests[i].d, @@ -355,7 +356,7 @@ do_tests(Display *dpy, picture_info *win) color4d *c = &colors[i
[PATCH rendercheck 4/5] shmblend: New test for XRenderComposite() from a pixmap in SHM.
There's a giant pile of code in glamor for uploading SHM pixmaps to temporary GL memory for accelerating a Composite operation, and most of its code is about how you convert formats. Add a test that runs through all the formats, to give us some coverage. Signed-off-by: Eric Anholt --- Makefile.am | 1 + configure.ac | 2 +- rendercheck.h | 3 + t_shmblend.c | 245 ++ tests.c | 2 - 5 files changed, 250 insertions(+), 3 deletions(-) create mode 100644 t_shmblend.c diff --git a/Makefile.am b/Makefile.am index 852d174..f77cb4f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -16,6 +16,7 @@ rendercheck_SOURCES = \ t_gtk_argb_xbgr.c \ t_libreoffice_xrgb.c \ t_repeat.c \ + t_shmblend.c \ t_srccoords.c \ t_tsrccoords.c \ t_tsrccoords2.c \ diff --git a/configure.ac b/configure.ac index 655a1b6..8ad0b3b 100644 --- a/configure.ac +++ b/configure.ac @@ -23,7 +23,7 @@ XORG_TESTSET_CFLAG(CWARNFLAGS, [-Wno-shadow]) AC_CHECK_HEADERS([err.h]) # Checks for pkg-config packages -PKG_CHECK_MODULES(RC, [xrender x11 xproto >= 7.0.17]) +PKG_CHECK_MODULES(RC, [xrender xext x11 xproto >= 7.0.17]) AC_CONFIG_FILES([Makefile man/Makefile]) diff --git a/rendercheck.h b/rendercheck.h index f0fa7b7..1c392e8 100644 --- a/rendercheck.h +++ b/rendercheck.h @@ -24,6 +24,7 @@ #include #include #include +#include #if HAVE_ERR_H # include @@ -44,6 +45,7 @@ static inline void errx(int eval, const char *fmt, ...) { #define min(a, b) (a < b ? a : b) #define max(a, b) (a > b ? a : b) +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) typedef struct _color4d { @@ -92,6 +94,7 @@ record_result(struct rendercheck_test_result *result, bool success) #define TEST_BUG7366 0x1000 #define TEST_gtk_argb_xbgr 0x2000 #define TEST_libreoffice_xrgb 0x4000 +#define TEST_shmblend 0x8000 struct rendercheck_test { int bit; diff --git a/t_shmblend.c b/t_shmblend.c new file mode 100644 index 000..752e17a --- /dev/null +++ b/t_shmblend.c @@ -0,0 +1,245 @@ +/* + * Copyright © 2016 Broadcom + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + */ + +#include +#include +#include +#include "rendercheck.h" +#include + +bool had_x_error; +static int (*orig_x_error_handler)(Display *, XErrorEvent *); + +static int +shmerrorhandler(Display *d, XErrorEvent *e) +{ + had_x_error = true; + if (e->error_code == BadAccess) { + fprintf(stderr,"failed to attach shared memory\n"); + return 0; + } else { + return (*orig_x_error_handler)(d,e); + } +} + +static XShmSegmentInfo * +get_x_shm_info(Display *dpy, size_t size) +{ + XShmSegmentInfo *shm_info = calloc(1, sizeof(*shm_info)); + + if (!XShmQueryExtension(dpy)) + return NULL; + + shm_info->shmid = shmget(IPC_PRIVATE, size, IPC_CREAT|0777); + if (shm_info->shmid < 0) { + free(shm_info); + return NULL; + } + + shm_info->shmaddr = shmat(shm_info->shmid, NULL, 0); + if (shm_info->shmaddr == (void *)-1) { + free(shm_info); + return NULL; + } + + shm_info->readOnly = false; + + XSync(dpy, true); + had_x_error = false; + orig_x_error_handler = XSetErrorHandler(shmerrorhandler); + XShmAttach(dpy, shm_info); + XSync(dpy, true); + XSetErrorHandler(orig_x_error_handler); + + return shm_info; +} + +static void +fill_shm_with_formatted_color(Display *dpy, XShmSegmentInfo *shm_info, + XRenderPictFormat *format, + int w, int h, color4d *color) +{ + XImage *image; + XRenderDirectForma
[PATCH rendercheck 1/5] Start using stdbool.h instead of Xlib or custom bools.
I have a hard time typing anything else at this point. Signed-off-by: Eric Anholt --- main.c | 21 ++ ops.c| 2 +- rendercheck.h| 58 +++- t_blend.c| 8 +++ t_bug7366.c | 32 +-- t_composite.c| 12 +- t_dstcoords.c| 6 ++--- t_fill.c | 6 ++--- t_gradient.c | 32 +-- t_gtk_argb_xbgr.c| 8 +++ t_libreoffice_xrgb.c | 10 - t_repeat.c | 22 +-- t_srccoords.c| 14 ++-- t_triangles.c| 24 ++-- t_tsrccoords.c | 12 +- t_tsrccoords2.c | 14 ++-- tests.c | 62 ++-- 17 files changed, 171 insertions(+), 172 deletions(-) diff --git a/main.c b/main.c index a7035b9..b5d67cc 100644 --- a/main.c +++ b/main.c @@ -27,7 +27,7 @@ #include #include -Bool is_verbose = FALSE, minimalrendering = FALSE; +bool is_verbose = false, minimalrendering = false; int enabled_tests = ~0;/* Enable all tests by default */ int format_whitelist_len = 0; @@ -163,7 +163,8 @@ int main(int argc, char **argv) Display *dpy; XEvent ev; int i, o, maj, min; - static Bool is_sync = FALSE, print_version = FALSE; + static int is_sync = false, print_version = false; + static int longopt_minimalrendering = 0; XWindowAttributes a; XSetWindowAttributes as; picture_info window; @@ -177,10 +178,10 @@ int main(int argc, char **argv) { "tests", required_argument, NULL, 't' }, { "ops",required_argument, NULL, 'o' }, { "verbose",no_argument,NULL, 'v' }, - { "sync", no_argument,&is_sync, TRUE}, - { "minimalrendering", no_argument, &minimalrendering, - TRUE}, - { "version",no_argument,&print_version, TRUE }, + { "sync", no_argument,&is_sync, true}, + { "minimalrendering", no_argument, + &longopt_minimalrendering, true}, + { "version",no_argument,&print_version, true }, { NULL, 0, NULL, 0 } }; @@ -194,7 +195,7 @@ int main(int argc, char **argv) break; case 'o': for (i = 0; i < num_ops; i++) - ops[i].disabled = TRUE; + ops[i].disabled = true; nextname = optarg; while ((opname = strsep(&nextname, ",")) != NULL) { @@ -202,7 +203,7 @@ int main(int argc, char **argv) if (strcasecmp(ops[i].name, opname) != 0) continue; - ops[i].disabled = FALSE; + ops[i].disabled = false; break; } if (i == num_ops) @@ -252,7 +253,7 @@ int main(int argc, char **argv) break; case 'v': - is_verbose = TRUE; + is_verbose = true; break; case 0: break; @@ -262,6 +263,8 @@ int main(int argc, char **argv) } } + minimalrendering = longopt_minimalrendering; + /* Print the version string. Bail out if --version was requested and * continue otherwise. */ diff --git a/ops.c b/ops.c index 0e03550..b7803da 100644 --- a/ops.c +++ b/ops.c @@ -210,7 +210,7 @@ do_composite(int op, const color4d *mask, const color4d *dst, color4d *result, -Bool componentAlpha) +bool componentAlpha) { color4d srcval, srcalpha; diff --git a/rendercheck.h b/rendercheck.h index 67efdbf..55ffcff 100644 --- a/rendercheck.h +++ b/rendercheck.h @@ -22,6 +22,7 @@ #include #include +#include #include #if HAVE_ERR_H @@ -44,11 +45,6 @@ static inline void errx(int eval, const char *fmt, ...) { #define min(a, b) (a < b ? a : b) #define max(a, b) (a > b ? a : b) -#ifndef TRUE -#define TRUE 1 -#define FALSE 0 -#endif - typedef struct _color4d { double r, g, b, a; @@ -65,7 +61,7 @@ typedef struct _picture_info { struct op_info { int op; const ch
[PATCH xserver 08/12] glamor: Generalize the a1-to-a8 conversion path.
Pixman is quite qualified to allocate our temporary memory, and all we need to do is decide what formats to convert from and to. Signed-off-by: Eric Anholt --- glamor/glamor_picture.c | 65 + 1 file changed, 28 insertions(+), 37 deletions(-) diff --git a/glamor/glamor_picture.c b/glamor/glamor_picture.c index 63d4365..e0f5828 100644 --- a/glamor/glamor_picture.c +++ b/glamor/glamor_picture.c @@ -204,21 +204,26 @@ glamor_get_tex_format_type_from_pictformat(ScreenPtr pScreen, return TRUE; } -static void * -glamor_convert_a1_a8(void *src_bits, void *dst_bits, int w, int h, int stride) +/** + * Takes a set of source bits with a given format and returns an + * in-memory pixman image of those bits in a destination format. + */ +static pixman_image_t * +glamor_get_converted_image(PictFormatShort dst_format, + PictFormatShort src_format, + void *src_bits, + int src_stride, + int w, int h) { -PictFormatShort dst_format = PICT_a8, src_format = PICT_a1; pixman_image_t *dst_image; pixman_image_t *src_image; -int src_stride = PixmapBytePad(w, 1); -dst_image = pixman_image_create_bits(dst_format, w, h, dst_bits, stride); +dst_image = pixman_image_create_bits(dst_format, w, h, NULL, 0); if (dst_image == NULL) { return NULL; } -src_image = pixman_image_create_bits(src_format, - w, h, src_bits, src_stride); +src_image = pixman_image_create_bits(src_format, w, h, src_bits, src_stride); if (src_image == NULL) { pixman_image_unref(dst_image); @@ -229,8 +234,7 @@ glamor_convert_a1_a8(void *src_bits, void *dst_bits, int w, int h, int stride) 0, 0, 0, 0, 0, 0, w, h); pixman_image_unref(src_image); -pixman_image_unref(dst_image); -return dst_bits; +return dst_image; } /** @@ -302,34 +306,21 @@ _glamor_upload_bits_to_pixmap_texture(PixmapPtr pixmap, GLenum format, glamor_get_screen_private(pixmap->drawable.pScreen); float dst_xscale, dst_yscale; GLuint tex = 0; -int need_free_bits = 0; +pixman_image_t *converted_image = NULL; if (bits == NULL) goto ready_to_upload; if (revert == REVERT_UPLOADING_A1) { -/* XXX if we are restoring the pixmap, then we may not need to allocate - * new buffer */ -void *converted_bits; - -if (pixmap->drawable.depth == 1) -stride = (((w * 8 + 7) / 8) + 3) & ~3; - -converted_bits = xallocarray(h, stride); - -if (converted_bits == NULL) -return FALSE; -bits = glamor_convert_a1_a8(bits, converted_bits, w, h, stride); -if (bits == NULL) { -free(converted_bits); -ErrorF("Failed to convert pixmap no_alpha %d," - "revert mode %d, swap mode %d\n", no_alpha, revert, swap_rb); +converted_image = glamor_get_converted_image(PICT_a8, + PICT_a1, + bits, + PixmapBytePad(w, 1), + w, h); +if (!converted_image) return FALSE; -} -no_alpha = 0; -revert = REVERT_NONE; -swap_rb = SWAP_NONE_UPLOADING; -need_free_bits = TRUE; + +bits = pixman_image_get_data(converted_image); } ready_to_upload: @@ -355,8 +346,8 @@ _glamor_upload_bits_to_pixmap_texture(PixmapPtr pixmap, GLenum format, x + fbo_x_off, y + fbo_y_off, w, h, bits, pbo)) { -if (need_free_bits) -free(bits); +if (converted_image) +pixman_image_unref(bits); return FALSE; } } else { @@ -382,8 +373,8 @@ _glamor_upload_bits_to_pixmap_texture(PixmapPtr pixmap, GLenum format, if (!__glamor_upload_pixmap_to_texture(pixmap, &tex, format, type, 0, 0, w, h, bits, pbo)) { -if (need_free_bits) -free(bits); +if (converted_image) +pixman_image_unref(bits); return FALSE; } @@ -416,8 +407,8 @@ _glamor_upload_bits_to_pixmap_texture(PixmapPtr pixmap, GLenum format, glBindFramebuffer(GL_FRAMEBUFFER, 0); } -if (need_free_bits) -free(bits); +if (converted_image) +pixman_image_unref(bits); return TRUE; } -- 2.7.0 ___ xorg-devel@lists.x.org: X.Org development Archives
[PATCH xserver 00/12] glamor_picture.c total rewrite
I started incrementally cleaning up glamor_picture.c after being surprised by its behavior during the last series, and ended up with a total rewrite. There will be some small losses in acceleration paths for GLES2, but since GLES2 hasn't worked in several releases (and a bunch of glamor_picture.c on GLES2 started out broken), I don't think that's a big deal. We could get back to acceleration for those paths using the converted_format output, if someone cared. I tested this with a full xts-render run, the new rendercheck/shmblend test I wrote, and I also forced the upload paths to use the GLES2 cases for a rendercheck/shmblend to make sure I had their swizzles right (I didn't). Eric Anholt (12): glamor: Simplify temporary picture uploading call stack. glamor: Make sure that GLAMOR_MEMORY pixmaps don't retain an FBO. glamor: Drop dead fbo handling from GLAMORY_MEMORY pict uploads. glamor: Propagate that is_upload is always true. glamor: Merge the two GL-type-from-pictformat paths. glamor: Drop the GLES2 REVERT_UPLOADING_2_10_10_10 paths. glamor: Drop the REVERT_UPLOADING_1_5_5_5 path. glamor: Generalize the a1-to-a8 conversion path. glamor: Drop unused PBO code in temporary picture uploading. glamor: Drop dead large-pixmap handling code in temp picture uploads. glamor: Replace "finish access" shader with texture swizzling. ephyr: Fix redisplay with glamor on GLES. glamor/glamor.c|9 +- glamor/glamor_core.c | 168 -- glamor/glamor_fbo.c| 12 + glamor/glamor_picture.c| 1003 glamor/glamor_priv.h | 27 +- glamor/glamor_render.c | 38 +- glamor/glamor_utils.h | 11 +- hw/kdrive/ephyr/ephyr_glamor_glx.c |4 +- 8 files changed, 267 insertions(+), 1005 deletions(-) -- 2.7.0 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 10/12] glamor: Drop dead large-pixmap handling code in temp picture uploads.
The glamor_pixmap_ensure_fbo() in glamor_pixmap_upload_prepare() will always fail on a large pixmap, so we can just be explicit about bailing out here and then dump the rest of this garbage. Signed-off-by: Eric Anholt --- glamor/glamor_picture.c | 127 ++-- glamor/glamor_priv.h| 1 - 2 files changed, 16 insertions(+), 112 deletions(-) diff --git a/glamor/glamor_picture.c b/glamor/glamor_picture.c index e0458a6..42fee1b 100644 --- a/glamor/glamor_picture.c +++ b/glamor/glamor_picture.c @@ -315,11 +315,7 @@ _glamor_upload_bits_to_pixmap_texture(PixmapPtr pixmap, GLenum format, /* Try fast path firstly, upload the pixmap to the texture attached * to the fbo directly. */ if (no_alpha == 0 -&& revert == REVERT_NONE && swap_rb == SWAP_NONE_UPLOADING -#ifdef WALKAROUND_LARGE_TEXTURE_MAP -&& glamor_pixmap_priv_is_small(pixmap_priv) -#endif -) { +&& revert == REVERT_NONE && swap_rb == SWAP_NONE_UPLOADING) { int fbo_x_off, fbo_y_off; assert(pixmap_priv->fbo->tex); @@ -429,26 +425,6 @@ glamor_pixmap_upload_prepare(PixmapPtr pixmap, GLenum format, int no_alpha, return 0; } -/* - * upload sub region to a large region. - * */ -static void -glamor_put_bits(char *dst_bits, int dst_stride, char *src_bits, -int src_stride, int bpp, int x, int y, int w, int h) -{ -int j; -int byte_per_pixel; - -byte_per_pixel = bpp / 8; -src_bits += y * src_stride + (x * byte_per_pixel); - -for (j = y; j < y + h; j++) { -memcpy(dst_bits, src_bits, w * byte_per_pixel); -src_bits += src_stride; -dst_bits += dst_stride; -} -} - /** * Uploads a picture based on a GLAMOR_MEMORY pixmap to a texture in a * temporary FBO. @@ -464,11 +440,19 @@ glamor_upload_picture_to_texture(PicturePtr picture) GLenum format, type; int no_alpha, revert, swap_rb; glamor_pixmap_private *pixmap_priv = glamor_get_pixmap_private(pixmap); -Bool force_clip; assert(glamor_pixmap_is_memory(pixmap)); assert(!pixmap_priv->fbo); +/* No handling of large pixmap pictures here (would need to make + * an FBO array and split the uploads across it). + */ +if (!glamor_check_fbo_size(glamor_priv, + pixmap->drawable.width, + pixmap->drawable.height)) { +return FALSE; +} + if (!glamor_get_tex_format_type_from_pictformat(screen, picture->format, &format, @@ -481,89 +465,10 @@ glamor_upload_picture_to_texture(PicturePtr picture) if (glamor_pixmap_upload_prepare(pixmap, format, no_alpha, revert, swap_rb)) return FALSE; -force_clip = glamor_priv->gl_flavor != GLAMOR_GL_DESKTOP; - -if (glamor_pixmap_priv_is_large(pixmap_priv) || force_clip) { -RegionRec region; -BoxRec box; -int n_region; -glamor_pixmap_clipped_regions *clipped_regions; -void *sub_bits; -int i, j; - -sub_bits = xallocarray(pixmap->drawable.height, stride); -if (sub_bits == NULL) -return FALSE; -box.x1 = 0; -box.y1 = 0; -box.x2 = pixmap->drawable.width; -box.y2 = pixmap->drawable.height; -RegionInitBoxes(®ion, &box, 1); -if (!force_clip) -clipped_regions = -glamor_compute_clipped_regions(pixmap, ®ion, &n_region, - 0, 0, 0); -else -clipped_regions = -glamor_compute_clipped_regions_ext(pixmap, ®ion, - &n_region, - pixmap_priv->block_w, - pixmap_priv->block_h, - 0, - 0); -DEBUGF("prepare upload %dx%d to a large pixmap %p\n", w, h, pixmap); -for (i = 0; i < n_region; i++) { -BoxPtr boxes; -int nbox; -int temp_stride; -void *temp_bits; - -glamor_set_pixmap_fbo_current(pixmap_priv, clipped_regions[i].block_idx); - -boxes = RegionRects(clipped_regions[i].region); -nbox = RegionNumRects(clipped_regions[i].region); -DEBUGF("split to %d boxes\n", nbox); -for (j = 0; j < nbox; j++) { -temp_stride = PixmapBytePad(boxes[j].x2 - boxes[j].x1, -pixmap->drawable.depth); - -if (boxes[j].x1 == 0 && temp_stride == stride) { -temp_bits = (char *) bits + boxes[j].y1 * stride;
[PATCH xserver 09/12] glamor: Drop unused PBO code in temporary picture uploading.
Signed-off-by: Eric Anholt --- glamor/glamor_picture.c | 30 +++--- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/glamor/glamor_picture.c b/glamor/glamor_picture.c index e0f5828..e0458a6 100644 --- a/glamor/glamor_picture.c +++ b/glamor/glamor_picture.c @@ -246,7 +246,7 @@ __glamor_upload_pixmap_to_texture(PixmapPtr pixmap, unsigned int *tex, GLenum format, GLenum type, int x, int y, int w, int h, - void *bits, int pbo) + void *bits) { glamor_screen_private *glamor_priv = glamor_get_screen_private(pixmap->drawable.pScreen); @@ -269,11 +269,6 @@ __glamor_upload_pixmap_to_texture(PixmapPtr pixmap, unsigned int *tex, glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST); glPixelStorei(GL_UNPACK_ALIGNMENT, 4); -assert(pbo || bits != 0); -if (bits == NULL) { -glBindBuffer(GL_PIXEL_UNPACK_BUFFER, pbo); -glUnmapBuffer(GL_PIXEL_UNPACK_BUFFER); -} glamor_priv->suppress_gl_out_of_memory_logging = true; if (non_sub) glTexImage2D(GL_TEXTURE_2D, 0, iformat, w, h, 0, format, type, bits); @@ -288,9 +283,6 @@ __glamor_upload_pixmap_to_texture(PixmapPtr pixmap, unsigned int *tex, return FALSE; } -if (bits == NULL) -glBindBuffer(GL_PIXEL_UNPACK_BUFFER, 0); - return TRUE; } @@ -298,7 +290,7 @@ static Bool _glamor_upload_bits_to_pixmap_texture(PixmapPtr pixmap, GLenum format, GLenum type, int no_alpha, int revert, int swap_rb, int x, int y, int w, int h, - int stride, void *bits, int pbo) + int stride, void *bits) { ScreenPtr screen = pixmap->drawable.pScreen; glamor_pixmap_private *pixmap_priv = glamor_get_pixmap_private(pixmap); @@ -308,9 +300,6 @@ _glamor_upload_bits_to_pixmap_texture(PixmapPtr pixmap, GLenum format, GLuint tex = 0; pixman_image_t *converted_image = NULL; -if (bits == NULL) -goto ready_to_upload; - if (revert == REVERT_UPLOADING_A1) { converted_image = glamor_get_converted_image(PICT_a8, PICT_a1, @@ -323,8 +312,6 @@ _glamor_upload_bits_to_pixmap_texture(PixmapPtr pixmap, GLenum format, bits = pixman_image_get_data(converted_image); } - ready_to_upload: - /* Try fast path firstly, upload the pixmap to the texture attached * to the fbo directly. */ if (no_alpha == 0 @@ -345,7 +332,7 @@ _glamor_upload_bits_to_pixmap_texture(PixmapPtr pixmap, GLenum format, format, type, x + fbo_x_off, y + fbo_y_off, w, h, - bits, pbo)) { + bits)) { if (converted_image) pixman_image_unref(bits); return FALSE; @@ -371,8 +358,7 @@ _glamor_upload_bits_to_pixmap_texture(PixmapPtr pixmap, GLenum format, glamor_make_current(glamor_priv); if (!__glamor_upload_pixmap_to_texture(pixmap, &tex, - format, type, 0, 0, w, h, bits, - pbo)) { + format, type, 0, 0, w, h, bits)) { if (converted_image) pixman_image_unref(bits); return FALSE; @@ -556,11 +542,10 @@ glamor_upload_picture_to_texture(PicturePtr picture) boxes[j].x1, boxes[j].y1, boxes[j].x2 - boxes[j].x1, boxes[j].y2 - boxes[j].y1, temp_stride); -if (_glamor_upload_bits_to_pixmap_texture +if (!_glamor_upload_bits_to_pixmap_texture (pixmap, format, type, no_alpha, revert, swap_rb, boxes[j].x1, boxes[j].y1, boxes[j].x2 - boxes[j].x1, - boxes[j].y2 - boxes[j].y1, temp_stride, temp_bits, - 0) == FALSE) { + boxes[j].y2 - boxes[j].y1, temp_stride, temp_bits)) { RegionUninit(®ion); free(sub_bits); assert(0); @@ -580,6 +565,5 @@ glamor_upload_picture_to_texture(PicturePtr picture) 0, 0, pixmap->drawable.width, pixmap->drawable.height, - stride, bits, -
[PATCH xserver 07/12] glamor: Drop the REVERT_UPLOADING_1_5_5_5 path.
There was only a pretty special case that could have even worked -- you've got a GLES2 renderer, you've got a SHM pixmap, it's 1555 (not the usual 565 for 16-bit), and you're little endian (BE was broken, since GL's 5_5_5_1 picks the 1 bit from the lowest bit of the short, and on BE we weren't doing the conversion path that swaps around the channels). This is just not worth the complexity. Signed-off-by: Eric Anholt --- glamor/glamor_picture.c | 123 ++-- glamor/glamor_utils.h | 1 - 2 files changed, 5 insertions(+), 119 deletions(-) diff --git a/glamor/glamor_picture.c b/glamor/glamor_picture.c index 5c6a1a5..63d4365 100644 --- a/glamor/glamor_picture.c +++ b/glamor/glamor_picture.c @@ -151,11 +151,7 @@ glamor_get_tex_format_type_from_pictformat(ScreenPtr pScreen, if (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP) { *tex_type = GL_UNSIGNED_SHORT_1_5_5_5_REV; } else { -*tex_type = GL_UNSIGNED_SHORT_5_5_5_1; -if (is_little_endian) -*revert = REVERT_UPLOADING_1_5_5_5; -else -*revert = REVERT_NONE; +return FALSE; } break; @@ -166,13 +162,7 @@ glamor_get_tex_format_type_from_pictformat(ScreenPtr pScreen, *tex_format = GL_BGRA; *tex_type = GL_UNSIGNED_SHORT_1_5_5_5_REV; } else { -*tex_format = GL_RGBA; -*tex_type = GL_UNSIGNED_SHORT_5_5_5_1; -if (is_little_endian) -*revert = REVERT_UPLOADING_1_5_5_5; -else -*revert = REVERT_NONE; -*swap_rb = SWAP_UPLOADING; +return FALSE; } break; @@ -215,8 +205,7 @@ glamor_get_tex_format_type_from_pictformat(ScreenPtr pScreen, } static void * -_glamor_color_convert_a1_a8(void *src_bits, void *dst_bits, int w, int h, -int stride) +glamor_convert_a1_a8(void *src_bits, void *dst_bits, int w, int h, int stride) { PictFormatShort dst_format = PICT_a8, src_format = PICT_a1; pixman_image_t *dst_image; @@ -244,107 +233,6 @@ _glamor_color_convert_a1_a8(void *src_bits, void *dst_bits, int w, int h, return dst_bits; } -#define ADJUST_BITS(d, src_bits, dst_bits) (((dst_bits) == (src_bits)) ? (d) : \ - (((dst_bits) > (src_bits)) ?\ - (((d) << ((dst_bits) - (src_bits))) \ - + (( 1 << ((dst_bits) - (src_bits))) >> 1)) \ - : ((d) >> ((src_bits) - (dst_bits) - -#define GLAMOR_DO_CONVERT(src, dst, no_alpha, swap,\ - a_shift_src, a_bits_src, \ - b_shift_src, b_bits_src, \ - g_shift_src, g_bits_src, \ - r_shift_src, r_bits_src, \ - a_shift, a_bits, \ - b_shift, b_bits, \ - g_shift, g_bits, \ - r_shift, r_bits) \ - do {\ - typeof(src) a,b,g,r;\ - typeof(src) a_mask_src, b_mask_src, g_mask_src, r_mask_src;\ - a_mask_src = (((1 << (a_bits_src)) - 1) << a_shift_src);\ - b_mask_src = (((1 << (b_bits_src)) - 1) << b_shift_src);\ - g_mask_src = (((1 << (g_bits_src)) - 1) << g_shift_src);\ - r_mask_src = (((1 << (r_bits_src)) - 1) << r_shift_src);\ - if (no_alpha) \ - a = (a_mask_src) >> (a_shift_src); \ - else\ - a = ((src) & (a_mask_src)) >> (a_shift_src);\ - b = ((src) & (b_mask_src)) >> (b_shift_src);\ - g = ((src) & (g_mask_src)) >> (g_shift_src);\ - r = ((src) & (r_mask_src)) >> (r_shift_src);\ - a = ADJUST_BITS(a, a_bits_src, a_bits); \ - b = ADJUST_BITS(b, b_bits_src, b_bits); \ - g = ADJUST_BITS(g, g_bits_src, g_bits); \ - r = ADJUST_BITS(r, r_bits_src, r_bits); \ - if (swap == 0)
[PATCH xserver 12/12] ephyr: Fix redisplay with glamor on GLES.
glamor_transfer.c is still totally broken, though. Signed-off-by: Eric Anholt --- hw/kdrive/ephyr/ephyr_glamor_glx.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/kdrive/ephyr/ephyr_glamor_glx.c b/hw/kdrive/ephyr/ephyr_glamor_glx.c index 636150d..2f21914 100644 --- a/hw/kdrive/ephyr/ephyr_glamor_glx.c +++ b/hw/kdrive/ephyr/ephyr_glamor_glx.c @@ -225,8 +225,10 @@ ephyr_glamor_damage_redisplay(struct ephyr_glamor *glamor, if (glamor->vao) { glGetIntegerv(GL_VERTEX_ARRAY_BINDING, &old_vao); glBindVertexArray(glamor->vao); -} else +} else { +glBindBuffer(GL_ARRAY_BUFFER, glamor->vbo); ephyr_glamor_set_vertices(glamor); +} glBindFramebuffer(GL_FRAMEBUFFER, 0); glUseProgram(glamor->texture_shader); -- 2.7.0 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 01/12] glamor: Simplify temporary picture uploading call stack.
glamor_upload_sub_pixmap_to_texture() only had the one caller, so we can merge it in, fix its silly return value, and propagate a bunch of constants. Signed-off-by: Eric Anholt --- glamor/glamor_picture.c | 68 ++--- glamor/glamor_priv.h| 18 + glamor/glamor_render.c | 27 3 files changed, 37 insertions(+), 76 deletions(-) diff --git a/glamor/glamor_picture.c b/glamor/glamor_picture.c index b069ce5..e91461c 100644 --- a/glamor/glamor_picture.c +++ b/glamor/glamor_picture.c @@ -797,11 +797,15 @@ glamor_put_bits(char *dst_bits, int dst_stride, char *src_bits, } } -static Bool -glamor_upload_sub_pixmap_to_texture(PixmapPtr pixmap, int x, int y, int w, -int h, int stride, void *bits, int pbo, -PictFormatShort pict_format) +/* Upload picture to texture. We may need to flip the y axis or + * wire alpha to 1. So we may conditional create fbo for the picture. + * */ +Bool +glamor_upload_picture_to_texture(PicturePtr picture) { +PixmapPtr pixmap = glamor_get_drawable_pixmap(picture->pDrawable); +void *bits = pixmap->devPrivate.ptr; +int stride = pixmap->devKind; ScreenPtr screen = pixmap->drawable.pScreen; glamor_screen_private *glamor_priv = glamor_get_screen_private(screen); GLenum format, type; @@ -810,7 +814,7 @@ glamor_upload_sub_pixmap_to_texture(PixmapPtr pixmap, int x, int y, int w, Bool force_clip; if (glamor_get_tex_format_type_from_pixmap(pixmap, - pict_format, + picture->format, &format, &type, &no_alpha, @@ -822,8 +826,7 @@ glamor_upload_sub_pixmap_to_texture(PixmapPtr pixmap, int x, int y, int w, return FALSE; pixmap_priv = glamor_get_pixmap_private(pixmap); -force_clip = glamor_priv->gl_flavor != GLAMOR_GL_DESKTOP -&& !glamor_check_fbo_size(glamor_priv, w, h); +force_clip = glamor_priv->gl_flavor != GLAMOR_GL_DESKTOP; if (glamor_pixmap_priv_is_large(pixmap_priv) || force_clip) { RegionRec region; @@ -833,13 +836,13 @@ glamor_upload_sub_pixmap_to_texture(PixmapPtr pixmap, int x, int y, int w, void *sub_bits; int i, j; -sub_bits = xallocarray(h, stride); +sub_bits = xallocarray(pixmap->drawable.height, stride); if (sub_bits == NULL) return FALSE; -box.x1 = x; -box.y1 = y; -box.x2 = x + w; -box.y2 = y + h; +box.x1 = 0; +box.y1 = 0; +box.x2 = pixmap->drawable.width; +box.y2 = pixmap->drawable.height; RegionInitBoxes(®ion, &box, 1); if (!force_clip) clipped_regions = @@ -860,8 +863,6 @@ glamor_upload_sub_pixmap_to_texture(PixmapPtr pixmap, int x, int y, int w, int temp_stride; void *temp_bits; -assert(pbo == 0); - glamor_set_pixmap_fbo_current(pixmap_priv, clipped_regions[i].block_idx); boxes = RegionRects(clipped_regions[i].region); @@ -871,26 +872,26 @@ glamor_upload_sub_pixmap_to_texture(PixmapPtr pixmap, int x, int y, int w, temp_stride = PixmapBytePad(boxes[j].x2 - boxes[j].x1, pixmap->drawable.depth); -if (boxes[j].x1 == x && temp_stride == stride) { -temp_bits = (char *) bits + (boxes[j].y1 - y) * stride; +if (boxes[j].x1 == 0 && temp_stride == stride) { +temp_bits = (char *) bits + boxes[j].y1 * stride; } else { temp_bits = sub_bits; glamor_put_bits(temp_bits, temp_stride, bits, stride, pixmap->drawable.bitsPerPixel, -boxes[j].x1 - x, boxes[j].y1 - y, +boxes[j].x1, boxes[j].y1, boxes[j].x2 - boxes[j].x1, boxes[j].y2 - boxes[j].y1); } DEBUGF("upload x %d y %d w %d h %d temp stride %d \n", - boxes[j].x1 - x, boxes[j].y1 - y, + boxes[j].x1, boxes[j].y1, boxes[j].x2 - boxes[j].x1, boxes[j].y2 - boxes[j].y1, temp_stride); if (_glamor_upload_bits_to_pixmap_texture (pixmap, format, type, no_alpha, revert, swap_rb, boxes[j].x1, boxes[j].y1, boxes[j].x2 - boxes[j].x1, boxes[j].y2 - boxes[j].y1, temp_strid
[PATCH xserver 11/12] glamor: Replace "finish access" shader with texture swizzling.
For pictures without alpha, and for most other formats for GLES2, we would make a temporary FBO, make another temporary texture, upload our GLAMORY_MEMORY pixmap to the texture, then run the "finish access" shader across it to swizzle its values around into the temporary FBO (which we would use for a single Render operation and then throw away). We can simplify everything by using GL_ARB_texture_swizzle (or its GLES3 counterpart). It's just not worth the complexity to try to improve the performance of this already low-performance path (SHM pixmaps + Render) on GLES2. Signed-off-by: Eric Anholt --- glamor/glamor.c | 9 +- glamor/glamor_core.c| 158 glamor/glamor_fbo.c | 11 ++ glamor/glamor_picture.c | 381 ++-- glamor/glamor_priv.h| 8 +- 5 files changed, 160 insertions(+), 407 deletions(-) diff --git a/glamor/glamor.c b/glamor/glamor.c index e9c1d9e..efe5953 100644 --- a/glamor/glamor.c +++ b/glamor/glamor.c @@ -605,9 +605,15 @@ glamor_init(ScreenPtr screen, unsigned int flags) glamor_priv->max_fbo_size = MAX_FBO_SIZE; #endif +glamor_priv->has_texture_swizzle = +(epoxy_has_gl_extension("GL_ARB_texture_swizzle") || + (glamor_priv->gl_flavor != GLAMOR_GL_DESKTOP && gl_version >= 30)); + glamor_priv->one_channel_format = GL_ALPHA; -if (epoxy_has_gl_extension("GL_ARB_texture_rg") && epoxy_has_gl_extension("GL_ARB_texture_swizzle")) +if (epoxy_has_gl_extension("GL_ARB_texture_rg") && +glamor_priv->has_texture_swizzle) { glamor_priv->one_channel_format = GL_RED; +} glamor_set_debug_level(&glamor_debug_level); @@ -668,7 +674,6 @@ glamor_init(ScreenPtr screen, unsigned int flags) glamor_init_vbo(screen); glamor_init_pixmap_fbo(screen); -glamor_init_finish_access_shaders(screen); #ifdef GLAMOR_GRADIENT_SHADER glamor_init_gradient_shader(screen); diff --git a/glamor/glamor_core.c b/glamor/glamor_core.c index a8768f4..7b2b396 100644 --- a/glamor/glamor_core.c +++ b/glamor/glamor_core.c @@ -113,164 +113,6 @@ glamor_link_glsl_prog(ScreenPtr screen, GLint prog, const char *format, ...) } } -/* - * When downloading a unsupported color format to CPU memory, -we need to shuffle the color elements and then use a supported -color format to read it back to CPU memory. - -For an example, the picture's format is PICT_b8g8r8a8, -Then the expecting color layout is as below (little endian): -0 1 2 3 : address -a r g b - -Now the in GLES2 the supported color format is GL_RGBA, type is -GL_UNSIGNED_TYPE, then we need to shuffle the fragment -color as : - frag_color = sample(texture).argb; -before we use glReadPixel to get it back. - -For the uploading process, the shuffle is a revert shuffle. -We still use GL_RGBA, GL_UNSIGNED_BYTE to upload the color -to a texture, then let's see -0 1 2 3 : address -a r g b : correct colors -R G B A : GL_RGBA with GL_UNSIGNED_BYTE - -Now we need to shuffle again, the mapping rule is -r = G, g = B, b = A, a = R. Then the uploading shuffle is as -below: - frag_color = sample(texture).gbar; -*/ - -void -glamor_init_finish_access_shaders(ScreenPtr screen) -{ -glamor_screen_private *glamor_priv; -const char *vs_source = -"attribute vec4 v_position;\n" -"attribute vec4 v_texcoord0;\n" -"varying vec2 source_texture;\n" -"void main()\n" -"{\n" -" gl_Position = v_position;\n" -" source_texture = v_texcoord0.xy;\n" -"}\n"; - -const char *common_source = -GLAMOR_DEFAULT_PRECISION -"varying vec2 source_texture;\n" -"uniform sampler2D sampler;\n" -"uniform int revert;\n" -"uniform int swap_rb;\n" -"#define REVERT_NONE 0\n" -"#define REVERT_NORMAL 1\n" -"#define SWAP_UPLOADING2\n" -"#define SWAP_NONE_UPLOADING 3\n"; - -const char *fs_source = -"void main()\n" -"{\n" -" vec4 color = texture2D(sampler, source_texture);\n" -" if (revert == REVERT_NONE) \n" -"{ \n" -" if ((swap_rb != SWAP_NONE_UPLOADING)) \n" -" gl_FragColor = color.bgra;\n" -" else \n" -" gl_FragColor = color.rgba;\n" -"} \n" -" els
[PATCH xserver 02/12] glamor: Make sure that GLAMOR_MEMORY pixmaps don't retain an FBO.
glamor_composite_choose_shader() may upload our scratch pixmaps to get a Render operation completed. We don't want to hang onto GL memory for our scratch pixmaps, since we'll just have to reallocate them at a new w/h next time around, and the contents will be updated as well. Signed-off-by: Eric Anholt --- glamor/glamor_fbo.c | 1 + glamor/glamor_picture.c | 13 - glamor/glamor_render.c | 11 +-- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/glamor/glamor_fbo.c b/glamor/glamor_fbo.c index 5bfffe5..0bfc1dd 100644 --- a/glamor/glamor_fbo.c +++ b/glamor/glamor_fbo.c @@ -510,6 +510,7 @@ glamor_pixmap_destroy_fbo(PixmapPtr pixmap) for (i = 0; i < priv->block_wcnt * priv->block_hcnt; i++) glamor_destroy_fbo(glamor_priv, priv->fbo_array[i]); free(priv->fbo_array); +priv->fbo_array = NULL; } else { fbo = glamor_pixmap_detach_fbo(priv); diff --git a/glamor/glamor_picture.c b/glamor/glamor_picture.c index e91461c..9bb2c74 100644 --- a/glamor/glamor_picture.c +++ b/glamor/glamor_picture.c @@ -797,9 +797,10 @@ glamor_put_bits(char *dst_bits, int dst_stride, char *src_bits, } } -/* Upload picture to texture. We may need to flip the y axis or - * wire alpha to 1. So we may conditional create fbo for the picture. - * */ +/** + * Uploads a picture based on a GLAMOR_MEMORY pixmap to a texture in a + * temporary FBO. + */ Bool glamor_upload_picture_to_texture(PicturePtr picture) { @@ -810,9 +811,12 @@ glamor_upload_picture_to_texture(PicturePtr picture) glamor_screen_private *glamor_priv = glamor_get_screen_private(screen); GLenum format, type; int no_alpha, revert, swap_rb; -glamor_pixmap_private *pixmap_priv; +glamor_pixmap_private *pixmap_priv = glamor_get_pixmap_private(pixmap); Bool force_clip; +assert(glamor_pixmap_is_memory(pixmap)); +assert(!pixmap_priv->fbo); + if (glamor_get_tex_format_type_from_pixmap(pixmap, picture->format, &format, @@ -825,7 +829,6 @@ glamor_upload_picture_to_texture(PicturePtr picture) if (glamor_pixmap_upload_prepare(pixmap, format, no_alpha, revert, swap_rb)) return FALSE; -pixmap_priv = glamor_get_pixmap_private(pixmap); force_clip = glamor_priv->gl_flavor != GLAMOR_GL_DESKTOP; if (glamor_pixmap_priv_is_large(pixmap_priv) || force_clip) { diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c index d47e7d7..65f7059 100644 --- a/glamor/glamor_render.c +++ b/glamor/glamor_render.c @@ -1125,7 +1125,7 @@ glamor_composite_with_shader(CARD8 op, &key, &shader, &op_info, &saved_source_format, ca_state)) { glamor_fallback("glamor_composite_choose_shader failed\n"); -return ret; +goto fail; } if (ca_state == CA_TWO_PASS) { if (!glamor_composite_choose_shader(PictOpAdd, source, mask, dest, @@ -1135,7 +1135,7 @@ glamor_composite_with_shader(CARD8 op, &key_ca, &shader_ca, &op_info_ca, &saved_source_format, ca_state)) { glamor_fallback("glamor_composite_choose_shader failed\n"); -return ret; +goto fail; } } @@ -1267,6 +1267,13 @@ glamor_composite_with_shader(CARD8 op, source->format = saved_source_format; ret = TRUE; + +fail: +if (mask_pixmap && glamor_pixmap_is_memory(mask_pixmap)) +glamor_pixmap_destroy_fbo(mask_pixmap); +if (source_pixmap && glamor_pixmap_is_memory(source_pixmap)) +glamor_pixmap_destroy_fbo(source_pixmap); + return ret; } -- 2.7.0 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver 05/12] glamor: Merge the two GL-type-from-pictformat paths.
It clarifies what the difference is between the two paths, and would potentially encourage us to handle GLES extensions that expose additional types. Signed-off-by: Eric Anholt --- glamor/glamor_picture.c | 322 ++-- 1 file changed, 121 insertions(+), 201 deletions(-) diff --git a/glamor/glamor_picture.c b/glamor/glamor_picture.c index 34cf4a3..e11280f 100644 --- a/glamor/glamor_picture.c +++ b/glamor/glamor_picture.c @@ -40,275 +40,195 @@ * * Return 0 if find a matched texture type. Otherwise return -1. **/ -static int -glamor_get_tex_format_type_from_pictformat_gl(ScreenPtr pScreen, - PictFormatShort format, - GLenum *tex_format, - GLenum *tex_type, - int *no_alpha, - int *revert, - int *swap_rb) +static Bool +glamor_get_tex_format_type_from_pictformat(ScreenPtr pScreen, + PictFormatShort format, + GLenum *tex_format, + GLenum *tex_type, + int *no_alpha, + int *revert, + int *swap_rb) { glamor_screen_private *glamor_priv = glamor_get_screen_private(pScreen); +Bool is_little_endian = IMAGE_BYTE_ORDER == LSBFirst; + *no_alpha = 0; *revert = REVERT_NONE; *swap_rb = SWAP_NONE_UPLOADING; + switch (format) { case PICT_a1: *tex_format = glamor_priv->one_channel_format; *tex_type = GL_UNSIGNED_BYTE; *revert = REVERT_UPLOADING_A1; break; -case PICT_b8g8r8x8: -*no_alpha = 1; -case PICT_b8g8r8a8: -*tex_format = GL_BGRA; -*tex_type = GL_UNSIGNED_INT_8_8_8_8; -break; - -case PICT_x8r8g8b8: -*no_alpha = 1; -case PICT_a8r8g8b8: -*tex_format = GL_BGRA; -*tex_type = GL_UNSIGNED_INT_8_8_8_8_REV; -break; -case PICT_x8b8g8r8: -*no_alpha = 1; -case PICT_a8b8g8r8: -*tex_format = GL_RGBA; -*tex_type = GL_UNSIGNED_INT_8_8_8_8_REV; -break; -case PICT_x2r10g10b10: -*no_alpha = 1; -case PICT_a2r10g10b10: -*tex_format = GL_BGRA; -*tex_type = GL_UNSIGNED_INT_2_10_10_10_REV; -break; -case PICT_x2b10g10r10: -*no_alpha = 1; -case PICT_a2b10g10r10: -*tex_format = GL_RGBA; -*tex_type = GL_UNSIGNED_INT_2_10_10_10_REV; -break; - -case PICT_r5g6b5: -*tex_format = GL_RGB; -*tex_type = GL_UNSIGNED_SHORT_5_6_5; -break; -case PICT_b5g6r5: -*tex_format = GL_RGB; -*tex_type = GL_UNSIGNED_SHORT_5_6_5_REV; -break; -case PICT_x1b5g5r5: -*no_alpha = 1; -case PICT_a1b5g5r5: -*tex_format = GL_RGBA; -*tex_type = GL_UNSIGNED_SHORT_1_5_5_5_REV; -break; - -case PICT_x1r5g5b5: -*no_alpha = 1; -case PICT_a1r5g5b5: -*tex_format = GL_BGRA; -*tex_type = GL_UNSIGNED_SHORT_1_5_5_5_REV; -break; -case PICT_a8: -*tex_format = glamor_priv->one_channel_format; -*tex_type = GL_UNSIGNED_BYTE; -break; -case PICT_x4r4g4b4: -*no_alpha = 1; -case PICT_a4r4g4b4: -*tex_format = GL_BGRA; -*tex_type = GL_UNSIGNED_SHORT_4_4_4_4_REV; -break; - -case PICT_x4b4g4r4: -*no_alpha = 1; -case PICT_a4b4g4r4: -*tex_format = GL_RGBA; -*tex_type = GL_UNSIGNED_SHORT_4_4_4_4_REV; -break; - -default: -return -1; -} -return 0; -} - -#define IS_LITTLE_ENDIAN (IMAGE_BYTE_ORDER == LSBFirst) - -static int -glamor_get_tex_format_type_from_pictformat_gles2(ScreenPtr pScreen, - PictFormatShort format, - GLenum *tex_format, - GLenum *tex_type, - int *no_alpha, - int *revert, - int *swap_rb) -{ -glamor_screen_private *glamor_priv = glamor_get_screen_private(pScreen); -int need_swap_rb = 0; -*no_alpha = 0; -*revert = IS_LITTLE_ENDIAN ? REVERT_NONE : REVERT_NORMAL; - -switch (format) { case PICT_b8g8r8x8: *no_alpha = 1; case PICT_b8g8r8a8: -*tex_format = GL_RGBA; -*tex_type = GL_UNSIGNED_BYTE; -need_swap_rb = 1; -*revert = IS_LITTLE_ENDIAN ? REVERT_NORMAL : REVERT_NONE; +if (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP) { +*t
[PATCH xserver 04/12] glamor: Propagate that is_upload is always true.
Signed-off-by: Eric Anholt --- glamor/glamor_core.c| 18 ++ glamor/glamor_picture.c | 142 ++-- glamor/glamor_utils.h | 6 -- 3 files changed, 46 insertions(+), 120 deletions(-) diff --git a/glamor/glamor_core.c b/glamor/glamor_core.c index b9948b5..a8768f4 100644 --- a/glamor/glamor_core.c +++ b/glamor/glamor_core.c @@ -164,8 +164,6 @@ glamor_init_finish_access_shaders(ScreenPtr screen) "uniform int swap_rb;\n" "#define REVERT_NONE 0\n" "#define REVERT_NORMAL 1\n" -"#define SWAP_NONE_DOWNLOADING 0\n" -"#define SWAP_DOWNLOADING 1\n" "#define SWAP_UPLOADING2\n" "#define SWAP_NONE_UPLOADING 3\n"; @@ -175,18 +173,14 @@ glamor_init_finish_access_shaders(ScreenPtr screen) " vec4 color = texture2D(sampler, source_texture);\n" " if (revert == REVERT_NONE) \n" "{ \n" -" if ((swap_rb != SWAP_NONE_DOWNLOADING) && (swap_rb != SWAP_NONE_UPLOADING)) \n" +" if ((swap_rb != SWAP_NONE_UPLOADING)) \n" " gl_FragColor = color.bgra;\n" " else \n" " gl_FragColor = color.rgba;\n" "} \n" " else \n" "{ \n" -" if (swap_rb == SWAP_DOWNLOADING) \n" -" gl_FragColor = color.argb;\n" -" else if (swap_rb == SWAP_NONE_DOWNLOADING)\n" -" gl_FragColor = color.abgr;\n" -" else if (swap_rb == SWAP_UPLOADING)\n" +" if (swap_rb == SWAP_UPLOADING)\n" " gl_FragColor = color.gbar;\n" " else if (swap_rb == SWAP_NONE_UPLOADING)\n" " gl_FragColor = color.abgr;\n" @@ -199,18 +193,14 @@ glamor_init_finish_access_shaders(ScreenPtr screen) " vec4 color = texture2D(sampler, source_texture);\n" " if (revert == REVERT_NONE) \n" "{ \n" -" if ((swap_rb != SWAP_NONE_DOWNLOADING) && (swap_rb != SWAP_NONE_UPLOADING)) \n" +" if (swap_rb != SWAP_NONE_UPLOADING) \n" " gl_FragColor = vec4(color.bgr, 1);\n" " else \n" " gl_FragColor = vec4(color.rgb, 1);\n" "} \n" " else \n" "{ \n" -" if (swap_rb == SWAP_DOWNLOADING) \n" -" gl_FragColor = vec4(1, color.rgb);\n" -" else if (swap_rb == SWAP_NONE_DOWNLOADING)\n" -" gl_FragColor = vec4(1, color.bgr);\n" -" else if (swap_rb == SWAP_UPLOADING)\n" +" if (swap_rb == SWAP_UPLOADING)\n" " gl_FragColor = vec4(color.gba, 1);\n" " else if (swap_rb == SWAP_NONE_UPLOADING)\n" " gl_FragColor = vec4(color.abg, 1);\n" diff --git a/glamor/glamor_picture.c b/glamor/glamor_picture.c index a032ed0..34cf4a3 100644 --- a/glamor/glamor_picture.c +++ b/glamor/glamor_picture.c @@ -47,17 +47,17 @@ glamor_get_tex_format_type_from_pictformat_gl(ScreenPtr pScreen, GLenum *tex_type, int *no_alpha, int *revert, - int *swap_rb, int is_upload) + int *swap_rb) { glamor_screen_private *glamor_priv = glamor_get_screen_private(pScreen); *no_alpha = 0; *revert = REVERT_NONE; -*swap_rb = is_upload ? SWAP_NONE_UPLOADING : SWAP_NONE_DOWNLOADING; +*swap_rb = SWAP_NONE_UPLOADING; switch (format) { case PICT_a1: *tex_format = glamor_priv->one_channel_format; *tex_type = GL_UNSIGNED_BYTE; -*revert = is_upload ? REVERT_UPLOADING_A1 : REVERT_DOWNLOADING_A1; +*revert = REVERT_UPLOADING_A1; break; case PICT_b8g8r8x8: *no_alpha = 1; @@ -145,7 +145,7 @@ glamor_get_tex_format_type_from_pictformat_gles2(ScreenPtr pScreen, GLenum *tex_type, int *no_alpha, int *revert, - int *swap_rb, int is_upload) +
[PATCH xserver 06/12] glamor: Drop the GLES2 REVERT_UPLOADING_2_10_10_10 paths.
These just smash your 2_10_10_10 data into , despite what the comments said. That's not valid rendering, so just ditch this path and fall back to software. One might also note in the code being removed here that the REVERT_UPLOADING_10_10_10_2 path wasn't even connected. Signed-off-by: Eric Anholt --- glamor/glamor_picture.c | 57 ++--- glamor/glamor_utils.h | 2 -- 2 files changed, 2 insertions(+), 57 deletions(-) diff --git a/glamor/glamor_picture.c b/glamor/glamor_picture.c index e11280f..5c6a1a5 100644 --- a/glamor/glamor_picture.c +++ b/glamor/glamor_picture.c @@ -111,18 +111,7 @@ glamor_get_tex_format_type_from_pictformat(ScreenPtr pScreen, *tex_format = GL_BGRA; *tex_type = GL_UNSIGNED_INT_2_10_10_10_REV; } else { -/* glReadPixmap doesn't support - * GL_UNSIGNED_INT_10_10_10_2. We have to use - * GL_UNSIGNED_BYTE and do the conversion in a shader - * later. - */ -*tex_format = GL_RGBA; -*tex_type = GL_UNSIGNED_BYTE; -if (!is_little_endian) -*revert = REVERT_UPLOADING_10_10_10_2; -else -*revert = REVERT_UPLOADING_2_10_10_10; -*swap_rb = SWAP_UPLOADING; +return FALSE; } break; @@ -133,13 +122,7 @@ glamor_get_tex_format_type_from_pictformat(ScreenPtr pScreen, *tex_format = GL_RGBA; *tex_type = GL_UNSIGNED_INT_2_10_10_10_REV; } else { -*tex_format = GL_RGBA; -*tex_type = GL_UNSIGNED_BYTE; -if (!is_little_endian) -*revert = REVERT_UPLOADING_10_10_10_2; -else -*revert = REVERT_UPLOADING_2_10_10_10; -break; +return FALSE; } break; @@ -300,37 +283,6 @@ _glamor_color_convert_a1_a8(void *src_bits, void *dst_bits, int w, int h, (*dst) = ((a) << (a_shift)) | ((r) << (b_shift)) | ((g) << (g_shift)) | ((b) << (r_shift)); \ } while (0) -static void * -_glamor_color_revert_x2b10g10r10(void *src_bits, void *dst_bits, int w, int h, - int stride, int no_alpha, - int swap_rb) -{ -int x, y; -unsigned int *words, *saved_words, *source_words; -int swap = swap_rb != SWAP_NONE_UPLOADING; - -source_words = src_bits; -words = dst_bits; -saved_words = words; - -for (y = 0; y < h; y++) { -DEBUGF("Line %d : ", y); -for (x = 0; x < w; x++) { -unsigned int pixel = source_words[x]; - -GLAMOR_DO_CONVERT(pixel, &words[x], no_alpha, swap, - 30, 2, 20, 10, 10, 10, 0, 10, - 24, 8, 16, 8, 8, 8, 0, 8); -DEBUGF("%x:%x ", pixel, words[x]); -} -DEBUGF("\n"); -words += stride / sizeof(*words); -source_words += stride / sizeof(*words); -} -DEBUGF("\n"); -return saved_words; - -} static void * _glamor_color_revert_x1b5g5r5(void *src_bits, void *dst_bits, int w, int h, @@ -371,7 +323,6 @@ _glamor_color_revert_x1b5g5r5(void *src_bits, void *dst_bits, int w, int h, * If it is set, then we need to wire the alpha value to 1. * @revert: REVERT_UPLOADING_A1 : convert an A1 buffer to an Alpha8 buffer - REVERT_UPLOADING_2_10_10_10 : convert X2B10G10R10 to R10G10B10X2 REVERT_UPLOADING_1_5_5_5: convert X1R5G5B5 to B5G5R5X1 @swap_rb: if we have the swap_rb set, then we need to swap the R and B's position. * @@ -384,10 +335,6 @@ glamor_color_convert_to_bits(void *src_bits, void *dst_bits, int w, int h, if (revert == REVERT_UPLOADING_A1) { return _glamor_color_convert_a1_a8(src_bits, dst_bits, w, h, stride); } -else if (revert == REVERT_UPLOADING_2_10_10_10) { -return _glamor_color_revert_x2b10g10r10(src_bits, dst_bits, w, h, -stride, no_alpha, swap_rb); -} else if (revert == REVERT_UPLOADING_1_5_5_5) { return _glamor_color_revert_x1b5g5r5(src_bits, dst_bits, w, h, stride, no_alpha, swap_rb); diff --git a/glamor/glamor_utils.h b/glamor/glamor_utils.h index e23de86..fc33995 100644 --- a/glamor/glamor_utils.h +++ b/glamor/glamor_utils.h @@ -768,9 +768,7 @@ format_for_pixmap(PixmapPtr pixmap) #define REVERT_NONE0 #define REVERT_NORMAL 1 #define REVERT_UPLOADING_A13 -#define REVERT_UPLOADING_2_10_10_105 #define REVERT_UPLOADING_1_5_5_5 8 -#define REVERT_UPLOADING_10_10_10_210 #define SWAP_UPLOADING 2 #define SWAP_NONE_UPLOADING3 -- 2.7.0 _
[PATCH xserver 03/12] glamor: Drop dead fbo handling from GLAMORY_MEMORY pict uploads.
The previous commit asserts that we don't have one. Signed-off-by: Eric Anholt --- glamor/glamor_picture.c | 25 ++--- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/glamor/glamor_picture.c b/glamor/glamor_picture.c index 9bb2c74..a032ed0 100644 --- a/glamor/glamor_picture.c +++ b/glamor/glamor_picture.c @@ -734,27 +734,10 @@ glamor_pixmap_upload_prepare(PixmapPtr pixmap, GLenum format, int no_alpha, int revert, int swap_rb) { int flag = 0; -glamor_pixmap_private *pixmap_priv; -glamor_screen_private *glamor_priv; -glamor_pixmap_fbo *fbo; +glamor_screen_private *glamor_priv = +glamor_get_screen_private(pixmap->drawable.pScreen); GLenum iformat; -pixmap_priv = glamor_get_pixmap_private(pixmap); -glamor_priv = glamor_get_screen_private(pixmap->drawable.pScreen); - -if (pixmap_priv->gl_fbo != GLAMOR_FBO_UNATTACHED) -return 0; - -if (pixmap_priv->fbo -&& (pixmap_priv->fbo->width < pixmap->drawable.width -|| pixmap_priv->fbo->height < pixmap->drawable.height)) { -fbo = glamor_pixmap_detach_fbo(pixmap_priv); -glamor_destroy_fbo(glamor_priv, fbo); -} - -if (pixmap_priv->fbo && pixmap_priv->fbo->fb) -return 0; - if (!(no_alpha || (revert == REVERT_NORMAL) || (swap_rb != SWAP_NONE_UPLOADING))) { /* We don't need a fbo, a simple texture uploading should work. */ @@ -762,10 +745,6 @@ glamor_pixmap_upload_prepare(PixmapPtr pixmap, GLenum format, int no_alpha, flag = GLAMOR_CREATE_FBO_NO_FBO; } -if ((flag == GLAMOR_CREATE_FBO_NO_FBO - && pixmap_priv->fbo && pixmap_priv->fbo->tex)) -return 0; - if (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP) iformat = gl_iformat_for_pixmap(pixmap); else -- 2.7.0 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH rendercheck 1/5] Start using stdbool.h instead of Xlib or custom bools.
Alex Deucher writes: > On Mon, Feb 1, 2016 at 4:48 PM, Eric Anholt wrote: >> I have a hard time typing anything else at this point. >> >> Signed-off-by: Eric Anholt >> --- >> main.c | 21 ++ >> ops.c| 2 +- >> rendercheck.h| 58 +++- >> t_blend.c| 8 +++ >> t_bug7366.c | 32 +-- >> t_composite.c| 12 +- >> t_dstcoords.c| 6 ++--- >> t_fill.c | 6 ++--- >> t_gradient.c | 32 +-- >> t_gtk_argb_xbgr.c| 8 +++ >> t_libreoffice_xrgb.c | 10 - >> t_repeat.c | 22 +-- >> t_srccoords.c| 14 ++-- >> t_triangles.c| 24 ++-- >> t_tsrccoords.c | 12 +- >> t_tsrccoords2.c | 14 ++-- >> tests.c | 62 >> ++-- >> 17 files changed, 171 insertions(+), 172 deletions(-) >> >> diff --git a/main.c b/main.c >> index a7035b9..b5d67cc 100644 >> --- a/main.c >> +++ b/main.c >> @@ -27,7 +27,7 @@ >> #include >> #include >> >> -Bool is_verbose = FALSE, minimalrendering = FALSE; >> +bool is_verbose = false, minimalrendering = false; >> int enabled_tests = ~0;/* Enable all tests by default */ >> >> int format_whitelist_len = 0; >> @@ -163,7 +163,8 @@ int main(int argc, char **argv) >> Display *dpy; >> XEvent ev; >> int i, o, maj, min; >> - static Bool is_sync = FALSE, print_version = FALSE; >> + static int is_sync = false, print_version = false; > > Did you mean bool here rather than int? > > Alex No, because it's an argument to getopt_long. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: Glamor bug in XVideo / XvPutImage when src_y != 0
Hans de Goede writes: > Hi Eric, > > While working on getting tvtime to work on cards using > the modesetting driver + glamor, I have hit what I believe > is a bug in glamor's XVideo implementation. I've tried > with the current xserver master and it seems the bug is > still present there. > > The problem is that src_y seems to get added to dest_y > resulting in the bottom of the window where the tv > is displaying via xv showing garbage when tvtime is > using XvPutImage / XvShmPutImag to put only part of > the tv image on the screen (to get rid of overscan or > show a 16:9 show properly while it is transmitted as 4:3) > > Besides the bottom showing garbage the top of the > actual desired content is missing, so src_x does seem > to get correctly applied to the source image, as said > it seems as if it is also getting added to dest_x > (which is 0) shifting the desired image up in the window, > and (correctly) clipping the top of the image of. > > Using tvtime requires an old analog tvcard + patches > which I've in my personal git to get it work with > planar yuv XVideo (until now it only supported > packed yuv). > > I do not know of another way to reproduce this, but if > you can spot the problem and send me a patch on top of > current xserver master I will happily test it. Maybe s/nlines/height/ in glamor_xv_put_image's boxes, and remove the shifts by "top"? I think we're uploading the rectangle from (0,srcy + width,srch), but then we're texturing from the (src_x,srcy + src_w,src_h) box within that uploaded content. We could try to correct the other way and upload from (srcx,srcy + srcw,srch) and then texture from (0,0+srcw,srch), but then you get bad things when srch flips between even/odd iirc. Handing us a chunk of data over the wire and then asking to only use a subset of that is kind of silly, and it's probably not something we should be optimizing for. Since we can XV into pixmaps these days, maybe it's time to try to write some tests. I'm not sure where we'd want to put them, though. I've never tried to write an xts test, and it sounds terrifying. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 1/4] glx: Macroize building the attribute list in DoGetDrawableAttributes
Adam Jackson writes: > No functional change, just a little easier to read and harder to get > wrong. > > Signed-off-by: Adam Jackson > --- > glx/glxcmds.c | 38 +++--- > 1 file changed, 15 insertions(+), 23 deletions(-) > > diff --git a/glx/glxcmds.c b/glx/glxcmds.c > index 6eb3541..65d0739 100644 > --- a/glx/glxcmds.c > +++ b/glx/glxcmds.c > @@ -1945,31 +1945,23 @@ DoGetDrawableAttributes(__GLXclientState * cl, XID > drawId) > if (pGlxDraw) > pDraw = pGlxDraw->pDraw; > > -attributes[2*num] = GLX_Y_INVERTED_EXT; > -attributes[2*num+1] = GL_FALSE; > -num++; > -attributes[2*num] = GLX_WIDTH; > -attributes[2*num+1] = pDraw->width; > -num++; > -attributes[2*num] = GLX_HEIGHT; > -attributes[2*num+1] = pDraw->height; > -num++; > +#define ATTRIB(a, v) { \ > +attributes[2*num] = (a); \ > +attributes[2*num+1] = (v); \ > +num++; \ > +} I think some compilers/static checkers will whine unless this is a do { ... } while (0) macro, becauuse of the "stray" semicolons we end up with otherwise. With that changed, Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 4/4] glx: Implement GLX_EXT_fbconfig_packed_float
Adam Jackson writes: > The tokens for this are already defined by GLX_ARB_fbconfig_float, which > we already support, so just add the extension to the list and let the > driver provide those configs if it wants. The last 3 are: Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] [RFC] glamor: implement write-only prepares
Dave Airlie writes: > From: Dave Airlie > > For some putimages we know we won't ever care about the data we readback, > we are going to trash it with the putimage contents. So implement > GLAMOR_ACCESS_WO mode. > > This will avoid doing the readbacks. Use it for putimages that are copy > with a solid planemask. > > inspired by Ilia and xlock -mode wator which does loads of XYBitmap > putimages. > > Signed-off-by: Dave Airlie > --- > glamor/glamor_image.c | 6 +- > glamor/glamor_prepare.c | 9 + > glamor/glamor_priv.h| 1 + > 3 files changed, 11 insertions(+), 5 deletions(-) > > diff --git a/glamor/glamor_image.c b/glamor/glamor_image.c > index 3158749..87fdcf6 100644 > --- a/glamor/glamor_image.c > +++ b/glamor/glamor_image.c > @@ -88,7 +88,11 @@ static void > glamor_put_image_bail(DrawablePtr drawable, GCPtr gc, int depth, int x, int > y, >int w, int h, int leftPad, int format, char *bits) > { > -if (glamor_prepare_access_box(drawable, GLAMOR_ACCESS_RW, x, y, w, h)) > +int access = GLAMOR_ACCESS_RW; > + > +if (gc->alu == GXcopy && glamor_pm_is_solid(gc->depth, gc->planemask)) > +access = GLAMOR_ACCESS_WO; > +if (glamor_prepare_access_box(drawable, access, x, y, w, h)) > fbPutImage(drawable, gc, depth, x, y, w, h, leftPad, format, bits); > glamor_finish_access(drawable); > } As I noted on IRC, the putimage only affects the composite clip, while you're prepare/finishing a bounding box, so this would trash the contents outside of the composite clip. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] glamor: Make context current in glamor_pixmap_fbo_cache_put
Michel Dänzer writes: > From: Michel Dänzer > > Without this, we may be manipulating the context of another screen. > > In a system with two GPUs using glamor, this fixes lots of > > (EE) glamor256: GL error: GL_INVALID_OPERATION in glBindTexture(non-gen name) > > spew since 0b4c0c75 ('glamor: Replace "finish access" shader with texture > swizzling'). > > Signed-off-by: Michel Dänzer > --- > > BTW, is fbo->tex guaranteed to be non-0 here? Yeah, you shouldn't be able to have a glamor_pixmap_fbo without ->tex. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] glamor: add glamor_finish API
Dave Airlie writes: > From: Dave Airlie > > Some drivers are calling glFinish, they really should be doing this. > > This also is needed for some reverse prime scenarios. > > Signed-off-by: Dave Airlie Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] modesetting: port clean start code from amdgpu.
Dave Airlie writes: > From: Dave Airlie > > Both radeon and amdgpu don't set the mode until the first blockhandler, > this means everything should be rendered on the screen correctly by then. > > This ports this code, it also removes the tail call of EnterVT from > ScreenInit, it really isn't necessary and causes us to set a dirty > mode with -modesetting always anyways. This seems like a really nice feature to have, and the implementation is simple enough. Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] modesetting: add support for background none.
Dave Airlie writes: > From: Dave Airlie > > This adds support using glamor for background None. > > loosely based off the amdgpu code. relies on the > glamor_finish code. > > Signed-off-by: Dave Airlie > --- > hw/xfree86/drivers/modesetting/driver.c | 21 + > hw/xfree86/drivers/modesetting/driver.h | 2 +- > hw/xfree86/drivers/modesetting/drmmode_display.c | 102 > +-- > hw/xfree86/drivers/modesetting/drmmode_display.h | 4 +- > 4 files changed, 103 insertions(+), 26 deletions(-) > > diff --git a/hw/xfree86/drivers/modesetting/driver.c > b/hw/xfree86/drivers/modesetting/driver.c > index 8f60eae..3895aca 100644 > --- a/hw/xfree86/drivers/modesetting/driver.c > +++ b/hw/xfree86/drivers/modesetting/driver.c > @@ -1099,6 +1099,22 @@ SetMaster(ScrnInfoPtr pScrn) > } > I would have expected the copy to be in CreateScreenResources, but I'm probably just missing something about the fine init sequence. It's also a bit unfortunate that we have to glamor_finish immediately after the copy so that we don't modeset to a not-yet-finished pixmap, since we're delaying the modeset until later anyway. I think that's a limitation of our non-pageflip modesetting API, though. Still, if this is how other drivers have been doing this, it's fine with me. It would be nice to give this the comment from another driver: /* When the root window is created, initialize the screen contents from * console if -background none was specified on the command line */ which explains a bit why this weird thing is stuffed in CreateWindow. > static Bool > +CreateWindow_oneshot(WindowPtr pWin) > +{ > +ScreenPtr pScreen = pWin->drawable.pScreen; > +ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen); > +modesettingPtr ms = modesettingPTR(pScrn); > +Bool ret; > + > +pScreen->CreateWindow = ms->CreateWindow; > +ret = pScreen->CreateWindow(pWin); > + > +if (ret) > +drmmode_copy_fb(pScrn, &ms->drmmode); > +return ret; > +} > + With that, Acked-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH rendercheck 1/4] triangles: Fix tests for conjoint and disjoint ops
Adam Jackson writes: > The expected destination color needs to vary by op in the same way as > for the base ops. Takes us from 1170 tests passed of 1710 total to > 1710/1710 on Xvfb. These 4 are: Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 13/13] glx: Implement GLX_EXT_libglvnd
Adam Jackson writes: > For the dri2 backend, we depend on xfree86 already, so we can walk the > options for the screen looking for a vendor string from xorg.conf. For > the swrast backend we don't have that luxury, so just say mesa. This > extension isn't really meaningful on Windows or OSX yet (since libglvnd > isn't really functional there yet), so on those platforms we don't say > anything and return BadValue for the token from QueryServerString. > > Signed-off-by: Adam Jackson > diff --git a/glx/glxdri2.c b/glx/glxdri2.c > index 15253d1..a3ea273 100644 > --- a/glx/glxdri2.c > +++ b/glx/glxdri2.c > @@ -934,12 +934,23 @@ initializeExtensions(__GLXscreen * screen) > /* white lie */ > extern glx_func_ptr glXGetProcAddressARB(const char *); > > +enum { > +GLXOPT_VENDOR_LIBRARY, > +}; > + > +static const OptionInfoRec GLXOptions[] = { > +{ GLXOPT_VENDOR_LIBRARY, "GlxVendorLibrary", OPTV_STRING, {0}, FALSE }, > +{ -1, NULL, OPTV_NONE, {0}, FALSE }, > +}; > + > static __GLXscreen * > __glXDRIscreenProbe(ScreenPtr pScreen) > { > const char *driverName, *deviceName; > __GLXDRIscreen *screen; > ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen); > +const char *glvnd = NULL; > +OptionInfoPtr options; > > screen = calloc(1, sizeof *screen); > if (screen == NULL) > @@ -985,6 +996,19 @@ __glXDRIscreenProbe(ScreenPtr pScreen) > GLX_PIXMAP_BIT | > GLX_PBUFFER_BIT); > > +options = malloc(sizeof(GLXOptions)); > +if (options) { > +memcpy(options, GLXOptions, sizeof(GLXOptions)); > +xf86ProcessOptions(pScrn->scrnIndex, pScrn->options, options); > +glvnd = xf86GetOptValString(options, GLXOPT_VENDOR_LIBRARY); > +if (glvnd) > +screen->base.glvnd = strdup(glvnd); > +free(options); I think you want s/strdup/xnfstrdup/g in this patch. Other than (optionally) that, this series looks really good to me. Patch 12/13 made me particularly happy, so this is all: Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] dix: Remove pointless client-state callbacks
Adam Jackson writes: > Private storage is pre-zeroed by the private system itself. Interestingly, this also drops some zeroing of the struct as the client is closing down, as far as I can tell. I don't think that zeroing was ever intended, so: Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 0/7] --disable-iglx
Adam Jackson writes: > We already disable indirect GLX by default at runtime, this makes it > something you can disable at compile time. And if you do that, libglx no > longer needs to link against libGL, which means Xvnc and friends don't > have their swrast support broken just because you installed some other > vendor's libGL. > > I think there's still a few more things that could be conditionalized > away beyond this. There's no need to track render mode or feedback or > selection buffers for direct contexts, for example. And the alternate > top-level dispatch table kind of points out how useless it is to build > the Single_dispatch_info tree. But if I go too far along those lines I > start needing to modify the generated code, so, one thing at a time. > > This series still builds iglx support by default. I could easily be > persuaded to flip that switch though. Opinions? Patches 1-4 are: Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 5/7] glx: Don't flush an arbitrary GL context from DrawableGone
Adam Jackson writes: > The current context at this point could be anything; we've done nothing > to bind the context whose drawable is about to go away. There's not > really much point in doing the flush anyway since any rendering is going > to be thrown away, so just do nothing. > > Signed-off-by: Adam Jackson This reverts 5c606c0a89e74fa223a99864be11cc3be60a159b. The commit seems quite wrong, as you've pointed out, though. I suspect that what that commit should have done instead was catch a GLXBadCurrentWindow from DoMakeCurrent's __glXForceCurrent() and just skip the glFlush() when it happens. We might want to slip in a better fix before this revert, and make the revert explicit. I'm OK with just reverting without the fix, though. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 6/7] glx: Push internal glF{lus, inis}h calls into the backend
Adam Jackson writes: > These calls are appropriate for indirect contexts; when we're building > without indirect support we don't want to reference libGL at all. It seems to me like a lot the code blocks calling these flushes ought to be only in the iglx build anyway. How would a glx/-implemented SwapBuffers() or CopySubBufferMesa() do the right thing on a direct context? signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] glamor: Disable logic ops when doing compositing
Keith Packard writes: > If the logic op gets left enabled, it overrides the blending > operation, causing incorrect contents on the display. > > Signed-off-by: Keith Packard Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] glamor: Disable logic ops when doing compositing
Keith Packard writes: > Eric Anholt writes: > >> Keith Packard writes: >> >>> If the logic op gets left enabled, it overrides the blending >>> operation, causing incorrect contents on the display. >>> >>> Signed-off-by: Keith Packard >> >> Reviewed-by: Eric Anholt > > And it's actually wrong (sigh). Need to do this in all paths through > this function (even the PictOpSrc case), and we only want to do it on > non-ES2 flavors. Here's an updated version. > > From 1ac2ef66369f36ef132f643969ad176db39babe7 Mon Sep 17 00:00:00 2001 > From: Keith Packard > Date: Fri, 13 May 2016 04:25:43 -0700 > Subject: [PATCH xserver] glamor: Disable logic ops when doing compositing [v2] > > 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 Oh, yeah. Even better. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] glamor: Limit outstanding drawing queue with glClientWaitSync
Keith Packard writes: > Michel Dänzer writes: > >> Is this about x11perf by any chance? I was seeing long lag with that as >> well, which turned out to be because we weren't correctly synchronizing >> with the hardware for XGetImage. > > x11perf only does XGetImage once per run of the test; without some > intra-test queue management, other applications are unusable while the > test is running. This particular case isn't terribly interesting, but as > we improve glamor to need fewer and fewer synchronizations between CPU > and GPU, this should become relevant for interactions between regular > applications. > >> In the amdgpu driver we now have a GPU scheduler managing the userspace >> command submissions. > > That does seem like a fine place to manage this; right now, the intel > driver appears to only do this in SwapBuffers. vc4 just waits until you don't have too many (5, arbitrarily) execs outstanding before submitting another one. I think this is a problem of the Intel driver, not glamor. We ran into it with Intel on cairo-gl, too, and I should have just fixed it in the driver back then. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: Custom KMS driver + X
Jose Abreu writes: > Hi Adam, > > Thanks for your answer! > > > On 06-07-2016 15:57, Adam Jackson wrote: >> On Tue, 2016-07-05 at 15:28 +0100, Jose Abreu wrote: >> >>> - First: My driver only supports 24 bpp (I mean real 24 >>> bits, not 24 packed in 32 bits). Is there a way to specify to X >>> (or specify in the driver itself) to use 24bpp **only** in the >>> new driver that I created? >> Kind of. The arguments to xf86SetDepthBpp (which you're probably >> calling in your PreInit hook) specify how the root window is to be >> treated. If you can only do 24bpp, you'll want to pass Support24bppFb >> (and nothing else) as the last argument. By default (ie, without exa or >> glamor or other acceleration) other pixmaps will be created in host >> memory as 32bpp, and the software renderer will convert between formats >> as needed. If you want to add support for pixmaps in video memory >> you'll need to enforce the 24bpp restriction in your accel support >> code. > > Ok, something is missing because I am not calling > xf86SetDetpthBpp at all. You mean I need to have a X driver (or > module, I don't know the nomenclature) in order to use my KMS > driver? Can't I just use the 'modesetting' driver? Sorry if I am > making some dumb mistake but my understanding of the inner > workings of X is barely null. > > Right now I am able to force X to use 24bpp in the KMS driver. I > just return -EINVAL in the dumb_create() callback of DRM when the > bpp is not 24 and then X tries again with the right bpp, but when > I do this the X stops working and in the log it appears the stack > trace that I sent in the first email. Yeah, I would think that updating the modesetting driver would be the way to go. You could probably use drmModeGetPlaneResources() / drmModeGetPlane() to figure out what formats are supported on the primary plane and adjust xf86SetDepthBpp() according to what it returns. That said, you're going to be so much better off if you can possibly do 32bpp instead. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH RFC xserver] dix: Work around non-premultiplied ARGB cursor data
Michel Dänzer writes: > From: Michel Dänzer > > Some games incorrectly use non-premultiplied ARGB cursor data, presumably > because that's what Windows uses. On some hardware (and with SWcursor), > this breaks areas of the cursor which are supposed to be transparent > (and presumably also translucent areas, but that's less noticeable). > > This change checks for pixels with alpha == 0 and any non-alpha component > != 0. If any such pixel is found, the data is assumed to be > non-premultiplied and fixed up by multiplying the RGB components with the > alpha component. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92309 > Signed-off-by: Michel Dänzer > --- > > I'm on the fence about whether this is a good or bad idea. This seems like a good idea to me: Make our software do something sensible when given data that makes no sense any other way. "But we should just make the app developers fix their stuff!" never works. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 0/4] glamor: Bugfixes in memory management
Max Staudt writes: > Low-spec'd platforms such as the Raspberry Pi tend to run out of VRAM > since GLAMOR only frees its BO cache once it grows beyond 1 GB. > > These patches reduce the cache and also flush it more often (based on > BO age and GL_OUT_OF_MEMORY). > > > Is there a reason why GLAMOR's cache was set at 1 GB in size? I still want to just delete the FBO cache entirely. We shouldn't hog all that system memory for a 1.7% performance improvement on cairo-perf. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 0/4] glamor: Bugfixes in memory management
Adam Jackson writes: > On Mon, 2016-07-18 at 14:36 +0200, Max Staudt wrote: >> On 07/15/2016 09:00 PM, Eric Anholt wrote: >> > I still want to just delete the FBO cache entirely. We shouldn't hog >> > all that system memory for a 1.7% performance improvement on cairo-perf. >> >> >> Oh, I was looking into analyzing the performance difference. If it's >> just that, then I'm for deleting it, too. >> >> >> Shall I send a patch to delete GLAMOR's BO cache instead? > > Yes please, that would be lovely. https://lists.freedesktop.org/archives/xorg-devel/2015-November/047776.html signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] modesetting: Delete dead drmmode_bo_for_pixmap function.
Kenneth Graunke writes: > Embarassingly, it looks like I introduced this dead function in > commit 13c7d53df8dac45ea2a685826cd45a39bcb51657 a year ago. > Nothing ever used it, not even then. Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] Pass ClientPtr to FlushCallback
Michel Dänzer writes: > From: Michel Dänzer > > This change has two effects: > > 1. Only calls FlushCallbacks when we're actually flushing data to a >client. The unnecessary FlushCallback calls could cause significant >performance degradation with compositing, which is significantly >reduced even without any driver changes. > > 2. By passing the ClientPtr to FlushCallbacks, drivers can completely >eliminate unnecessary flushing of GPU commands by keeping track of >whether we're flushing any XDamageNotify events to the client for >which the corresponding rendering commands haven't been flushed to >the GPU yet. > > Signed-off-by: Michel Dänzer > --- > > See https://lists.freedesktop.org/archives/amd-gfx/2016-August/000977.html > for an example of how to take advantage of this change to eliminate > unnecessary GPU flushes. Note: Mesa's DRI2 is (supposed to be) doing XSync() during glXWaitX() to ensure that the server has processed the client's X requests and flushed its batchbuffers, so that the kernel serializes the batchbuffer from X before the next rendering by Mesa. I think your xf86-video-ati patches will break that. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] Pass ClientPtr to FlushCallback
Keith Packard writes: > Michel Dänzer writes: > >> From: Michel Dänzer >> >> This change has two effects: >> >> 1. Only calls FlushCallbacks when we're actually flushing data to a >>client. The unnecessary FlushCallback calls could cause significant >>performance degradation with compositing, which is significantly >>reduced even without any driver changes. > > Seems like a completely reasonable plan. As you can see, the original > goal was to have the callback called only once when flushing output to > multiple clients though. That appears to only be used by the record > extension, so perhaps we just don't care. > >> 2. By passing the ClientPtr to FlushCallbacks, drivers can completely >>eliminate unnecessary flushing of GPU commands by keeping track of >>whether we're flushing any XDamageNotify events to the client for >>which the corresponding rendering commands haven't been flushed to >>the GPU yet. > > Is this something we should be doing in either glamor or DIX itself? It > looks like the ATI driver has a number that is incremented every time > commands are sent to the GPU and that clients need to be flushed > whenever they haven't been flushed since the last time that number was > changed? > > I don't even know how this works in the generic modesetting driver at > this point; what makes sure that glFlush is called before data are sent > to the client? We glFlush in the BlockHandler in glamor. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] Pass ClientPtr to FlushCallback
Keith Packard writes: > Eric Anholt writes: > >> We glFlush in the BlockHandler in glamor. > > Sure, what ensures that is called before damage events are delivered? You had assured me in the past that BlockHandler gets called before things are written to clients. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libpciaccess] Support for 32-bit domains
Michel Dänzer writes: > On 10/08/16 06:39 AM, Keith Busch wrote: >> A pci "domain" is a purely software construct, and need not be limited >> to the 16-bit ACPI defined segment. The Linux kernel currently supports >> 32-bit domains, so this patch matches up with those capabilities to make >> it usable on systems exporting such domains. >> >> Reported-by: Pawel Baldysiak >> Signed-off-by: Keith Busch >> --- >> include/pciaccess.h | 2 +- >> src/linux_sysfs.c | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/include/pciaccess.h b/include/pciaccess.h >> index 1d7aa4b..93ed76f 100644 >> --- a/include/pciaccess.h >> +++ b/include/pciaccess.h >> @@ -321,7 +321,7 @@ struct pci_device { >> * the domain will always be zero. >> */ >> /*@{*/ >> -uint16_tdomain; >> +uint32_tdomain; > > This change breaks ABI, so as is it would require bumping the library > SONAME (which is painful and thus generally discouraged from a > downstream POV). Given that libpciaccess allocates the struct, and the struct isn't embedded in any other public structs, I think you could just tack a "uint16_t domain_high;" at the end of the struct and fill that with the high bits. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libpciaccess] Support for 32-bit domains
Keith Busch writes: > On Thu, Aug 11, 2016 at 12:03:30PM -0700, Eric Anholt wrote: >> Given that libpciaccess allocates the struct, and the struct isn't >> embedded in any other public structs, I think you could just tack a >> "uint16_t domain_high;" at the end of the struct and fill that with the >> high bits. > > Hmm, then we have to change every usage of domain to combine the two, > and every usage thereafter must do the same. > > How about tacking 'uint32_t domain' to the end for the full domain, > rename the existing 'uint16_t domain' to 'domain_low', and then just > copy the lower bits from the full domain into there? That should satisfy > legacy usage, and everywhere else will automatically use the full value. > > Does something like this look more reasonable? That looks even better to me! Let's give people a few days to comment, but I like this solution. Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] Build glamor when Xorg or Xephyr are built.
Keith Packard writes: > Requires gbm when building Xorg so that xf86-video-modesetting will > work. > > Signed-off-by: Keith Packard Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCHv2 libpciaccess] Ignore 32-bit domains
Keith Busch writes: > A pci "domain" need not be limited to the 16-bits. The Linux kernel > currently supports 32-bit domains which cause startx to segfault. Updating > libpciaccess to support 32-bit domains breaks the library's ABI, and > domains requiring 32-bits are not necessary for startx anyway, so this > patch ignores them. > > Reported-by: Pawel Baldysiak > Signed-off-by: Keith Busch Reviewed and pushed. Thanks! signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 1/3] sync: Don't do return client->noClientException
Adam Jackson writes: > On Tue, 2016-06-28 at 15:54 -0400, Adam Jackson wrote: >> Hasn't been necessary since: >> >> commit 92ed75ac59e2d3af149cddb962efd05fc8487750 >> Author: Jamey Sharp >> Date: Mon May 10 20:22:05 2010 -0700 >> >> Eliminate boilerplate around client->noClientException. >> >> Signed-off-by: Adam Jackson > > Ping, this series is still applicable. Series is: Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: Michel Dänzer is invited to help maintain master X server branch
Keith Packard writes: > Michel Dänzer writes: > >> Many glamor changes have been pushed by Adam or Keith lately. Is that >> intentional, i.e. is it okay for others to push glamor changes which are >> ready, or should we always get in touch with Eric first? > > Would saying "it depends" help at all? When there's a major chunk of > code getting ready to be merged, then coordinating through a single > person can avoid merge conflicts. In quiet times, then working > separately will reduce communication overhead. > > So, I think the key is to know when things are busy and when things are > quiet, and to have a sense of how much conflict any particular patch is > likely to cause with other changes in process. It's probably not a great > idea to merge a whitespace patch touching every file in the repo just > before someone merges a pile of changes to a single file. > > When in doubt, feel free to just ask around. If several of us agree that > merging a patch seems fine, then at least it's not entirely your fault? I'm generally in favor of people pushing reviewed patches to glamor. The only thing I'll get grumpy about is new code without coverage in XTS or rendercheck. X rendering is hard to get right, so make sure that there's a test covering it. However, the manual operation of our testsuites is also awful (which subsets am I supposed to actually run? How do I know if I succeeded, when there are expected failures?). Having spent a while recently working in a project with actual CI, I'll try not to be too irritated about others not doing testing on X when I haven't actually made X's rendering testing usable. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver v3] glamor: Handle bitplane in glamor_copy_fbo_cpu
Michel Dänzer writes: > On 18/08/16 11:09 PM, Alex Deucher wrote: >> On Thu, Aug 18, 2016 at 5:42 AM, Michel Dänzer wrote: >>> From: Michel Dänzer >>> >>> This can significantly speed up at least some CopyPlane cases, e.g. >>> indirectly for stippled fills. >>> >>> v2: >>> * Make temporary pixmap the same size as the destination pixmap >>> (instead of the destination drawable size), and fix coordinate >>> parameters passed to fbCopyXtoX and glamor_upload_boxes. Fixes >>> incorrect rendering rendering with x11perf -copyplane* and crashes >>> with the xscreensaver phosphor hack. >>> v3: >>> * Make the change a bit more compact and hopefully more readable by >>> re-using the existing src_* locals in the bitplane case as well. >>> >>> Reported-by: Keith Raghubar >>> Signed-off-by: Michel Dänzer >> >> Reviewed-by: Alex Deucher > > Thanks, Alex. > > > Eric, what do you think about this patch? It can speed up clients > hitting this path by more than an order of magnitude, but it's not > exercised by XTS, and I currently don't have the bandwidth to change that. Yeah, this is the example I was thinking of when I was saying "if we're going to be writing code to accelerate things, we'd better have tests for it", exemplified by the v2 changes. However, I won't be getting around to writing the tests either, so we're in a bit of a bind. I think, ultimately, getting glamor's performance up to expected matters the most, even if people suffer rendering failures along the way because we're bad at testing. So, Acked-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xts 2/3] editorconfig: Set up some default indentation settings for the project.
Emacs was indenting badly in the previous change without it. --- .editorconfig | 12 1 file changed, 12 insertions(+) create mode 100644 .editorconfig diff --git a/.editorconfig b/.editorconfig new file mode 100644 index ..5fa2d6311abd --- /dev/null +++ b/.editorconfig @@ -0,0 +1,12 @@ +# To use this config on you editor, follow the instructions at: +# http://editorconfig.org + +root = true + +[*] +charset = utf-8 +insert_final_newline = true + +[*.{c,h,cpp,hpp,cc,hh}] +indent_style = tab +indent_size = 8 -- 2.9.3 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xts 3/3] autogen: Set the default patch prefix.
--- autogen.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/autogen.sh b/autogen.sh index fc34bd55c443..ee30ef4aa858 100755 --- a/autogen.sh +++ b/autogen.sh @@ -9,6 +9,9 @@ cd $srcdir autoreconf -v --install || exit 1 cd $ORIGDIR || exit $? +git config --local --get format.subjectPrefix || +git config --local format.subjectPrefix "PATCH xts" + if test -z "$NOCONFIGURE"; then $srcdir/configure "$@" fi -- 2.9.3 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xts 1/3] Default to using the tetexec.cfg from the build directory.
One less environment variable the user needs to set to prevent spurious failures. --- src/tet3/apilib/Makefile.am | 3 ++- src/tet3/apilib/dconfig.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/tet3/apilib/Makefile.am b/src/tet3/apilib/Makefile.am index 70391ed910df..e55afb3e6617 100644 --- a/src/tet3/apilib/Makefile.am +++ b/src/tet3/apilib/Makefile.am @@ -1,4 +1,5 @@ -AM_CPPFLAGS = -I$(srcdir)/../inc -I$(top_srcdir)/include +AM_CPPFLAGS = -I$(srcdir)/../inc -I$(top_srcdir)/include \ + -DTET_DEFAULT_CONFIG=\"$(abs_top_builddir)/tetexec.cfg\" # libapi_s is the shared portion of the library xtslibdir = $(libexecdir)/xts5 diff --git a/src/tet3/apilib/dconfig.c b/src/tet3/apilib/dconfig.c index 129557909dd1..acad188e 100644 --- a/src/tet3/apilib/dconfig.c +++ b/src/tet3/apilib/dconfig.c @@ -144,7 +144,7 @@ TET_IMPORT void tet_config() /* determine the config file name from the environment */ file = getenv("TET_CONFIG"); if (file == NULL || *file == '\0') - return; + file = TET_DEFAULT_CONFIG; /* open the file */ if ((fp = fopen(file, "r")) == (FILE *) 0) { -- 2.9.3 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver] test: Run some XTS5 integration tests against Xvfb if possible.
By default the tests will be skipped. However, if you set XTEST_DIR to the repo of a built X Test Suite and PIGLIT_DIR to a piglit repo (no build necessary), make check will run piglit's xts-render tests against Xvfb. We could run more of XTS5, but I haven't spent the time identifying what additional subset would be worth running, since much of it is only really testing the client libraries. We want to make sure that we keep the runtime down, and this subset of the test suite took 92 seconds according to piglit. --- Note that this requires the xts5 and piglit series that I've just sent out. Next step is to test glamor in a similar way. test/Makefile.am | 25 -- test/scripts/xinit-piglit-session.sh | 47 + test/scripts/xvfb-piglit.sh | 66 3 files changed, 135 insertions(+), 3 deletions(-) create mode 100755 test/scripts/xinit-piglit-session.sh create mode 100755 test/scripts/xvfb-piglit.sh diff --git a/test/Makefile.am b/test/Makefile.am index f5e54680b005..5a35e2bb198f 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -12,8 +12,23 @@ endif endif check_LTLIBRARIES = libxservertest.la -TESTS=$(noinst_PROGRAMS) -TESTS_ENVIRONMENT = $(XORG_MALLOC_DEBUG_ENV) +if XVFB +XVFB_TESTS = scripts/xvfb-piglit.sh +endif + +SCRIPT_TESTS = \ + $(XVFB_TESTS) \ + $(NULL) + +TESTS = \ + $(noinst_PROGRAMS) \ + $(SCRIPT_TESTS) \ + $(NULL) + +TESTS_ENVIRONMENT = \ + XSERVER_DIR=$(abs_top_builddir) \ + $(XORG_MALLOC_DEBUG_ENV) \ + $(NULL) AM_CFLAGS = $(DIX_CFLAGS) @XORG_CFLAGS@ AM_CPPFLAGS = $(XORG_INCS) @@ -139,5 +154,9 @@ endif libxservertest_la_DEPENDENCIES = $(libxservertest_la_LIBADD) endif -EXTRA_DIST = ddxstubs.c +EXTRA_DIST = \ + $(SCRIPT_TESTS) \ + scripts/xinit-piglit-session.sh \ + ddxstubs.c \ + $(NULL) diff --git a/test/scripts/xinit-piglit-session.sh b/test/scripts/xinit-piglit-session.sh new file mode 100755 index ..0f0c7943d34e --- /dev/null +++ b/test/scripts/xinit-piglit-session.sh @@ -0,0 +1,47 @@ +#!/bin/sh + +# .xinitrc replacement to run piglit and exit. +# +# Note that piglit will run many processes against the server, so +# running the server with -noreset is recommended to improve runtime. + +set -e + +if test "x$PIGLIT_DIR" = "x"; then +echo "PIGLIT_DIR must be set to the directory of the piglit repository." +exit 1 +fi + +if test "x$PIGLIT_RESULTS_DIR" = "x"; then +echo "PIGLIT_RESULTS_DIR must be defined" +exit 1 +fi + +if test "x$XTEST_DIR" = "x"; then +echo "XTEST_DIR must be set to the root of the built xtest tree." +exit 1 +fi + +cd $PIGLIT_DIR + +# Write the piglit.conf we'll use for our testing. Don't use the +# default piglit.conf name because that may overwrite a local +# piglit.conf. +PIGLITCONF=piglit-xserver-test.conf +cat < $PIGLITCONF +[xts] +path=$XTEST_DIR +EOF + +# Skip some tests that are failing at the time of importing the script. +#"REPORT: min_bounds, rbearing was 0, expecting 2" +PIGLIT_ARGS="$PIGLIT_ARGS -x xlistfontswithinfo@3" +PIGLIT_ARGS="$PIGLIT_ARGS -x xlistfontswithinfo@4" +PIGLIT_ARGS="$PIGLIT_ARGS -x xloadqueryfont@1" +PIGLIT_ARGS="$PIGLIT_ARGS -x xqueryfont@1" +PIGLIT_ARGS="$PIGLIT_ARGS -x xqueryfont@2" +#"REPORT: Incorrect pixel on inside of area at point (0, 0): 0x0 != 0x1" +PIGLIT_ARGS="$PIGLIT_ARGS -x xgetimage@7" +PIGLIT_ARGS="$PIGLIT_ARGS -x xgetsubimage@7" + +exec ./piglit-run.py xts-render -f $PIGLITCONF $PIGLIT_ARGS $PIGLIT_RESULTS_DIR diff --git a/test/scripts/xvfb-piglit.sh b/test/scripts/xvfb-piglit.sh new file mode 100755 index ..2a4e9405268d --- /dev/null +++ b/test/scripts/xvfb-piglit.sh @@ -0,0 +1,66 @@ +#!/bin/sh + +set -e + +if test "x$XTEST_DIR" = "x"; then +echo "XTEST_DIR must be set to the directory of the xtest repository." +# Exit as a "skip" so make check works even without piglit. +exit 77 +fi + +if test "x$PIGLIT_DIR" = "x"; then +echo "PIGLIT_DIR must be set to the directory of the piglit repository." +# Exit as a "skip" so make check works even without piglit. +exit 77 +fi + +if test "x$XSERVER_DIR" = "x"; then +echo "XSERVER_DIR must be set to the directory of the xserver repository." +# Exit as a real failure because it should always be set. +exit 1 +fi + +export PIGLIT_RESULTS_DIR=$PIGLIT_DIR/results/xvfb + +startx \ +$XSERVER_DIR/test/scripts/xinit-piglit-session.sh \ +-- \ +$XSERVER_DIR/hw/vfb/Xvfb \ +-noreset \ +-screen scrn 1280x1024x24 + +# Write out piglit-summaries. +SHORT_SUMMARY=$PIGLIT_RESULTS_DIR/summary +LONG_SUMMARY=$PIGLIT_RESULTS_DIR/long-summary +$PIGLIT_DIR/piglit-summary.py -s $PIGLIT_RESULTS_DIR > $SHORT_SUMMARY +$PIGLIT_DIR/piglit-summary.py $PIGLIT_RESULTS_DIR > $LONG_SUMMARY + +# Write the short summary to make check's log file. +cat $SHORT_SUMMARY + +# Parse the piglit summary to decide on our e
Re: [PATCH v2 util-modular] build.sh: add libinput and xf86-input-libinput
Peter Hutterer writes: > Signed-off-by: Peter Hutterer > --- > Well, that'll teach me to send untested patches. Sorry. Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] glx: Fix computation of GLX_X_RENDERABLE fbconfig attribute
Adam Jackson writes: > From the GLX spec: > > "GLX_X_RENDERABLE is a boolean indicating whether X can be used to > render into a drawable created with the GLXFBConfig. This attribute > is True if the GLXFBConfig supports GLX windows and/or pixmaps." > > Every backend was setting this to true unconditionally, and then the > core ignored that value and sent true unconditionally on its own. This > is broken for ARB_fbconfig_float and EXT_fbconfig_packed_float, which > only apply to pbuffers, which are not renderable from non-GLX APIs. > > Instead compute GLX_X_RENDERABLE from the supported drawable types. The > dri backends were getting _that_ wrong too, so fix that as well. > > This is not a functional change, as there are no mesa drivers that claim > to support __DRI_ATTRIB_{UNSIGNED_,}FLOAT_BIT yet. Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] glamor: Fix crash when master gpu is using glamor and another gpu is hotplugged
Hans de Goede writes: > When a GPU gets hotplugged while X is already running, glamor_egl_init() > gets called and changes the current egl context at a point where glamor > does not expect this. > > This causes glamor to e.g. crash in the next glamor_create_pixmap() call > (caled through the master's screen->CreatePixmap), note this is not the > only troublesome entry point I've seen other backtraces when using a > composting window manager. > > Adding glamor_make_current calls to all entry points is quite expensive, > so this commit consists of a miminal fix for this problem by restoring the > original egl context when leaving glamor_egl_init() with an error, based > on only usb gpu's getting hotplugged and they do not support glamor. I think the problem is just mismatching our lastGLContext with the actual makecurrent state. Couldn't we just use glamor_make_current() instead of our own eglMakeCurrent() call in this function? signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] glamor: Fix crash when master gpu is using glamor and another gpu is hotplugged
Hans de Goede writes: > Hi, > > On 06-09-16 22:22, Eric Anholt wrote: >> Hans de Goede writes: >> >>> When a GPU gets hotplugged while X is already running, glamor_egl_init() >>> gets called and changes the current egl context at a point where glamor >>> does not expect this. >>> >>> This causes glamor to e.g. crash in the next glamor_create_pixmap() call >>> (caled through the master's screen->CreatePixmap), note this is not the >>> only troublesome entry point I've seen other backtraces when using a >>> composting window manager. >>> >>> Adding glamor_make_current calls to all entry points is quite expensive, >>> so this commit consists of a miminal fix for this problem by restoring the >>> original egl context when leaving glamor_egl_init() with an error, based >>> on only usb gpu's getting hotplugged and they do not support glamor. >> >> I think the problem is just mismatching our lastGLContext with the >> actual makecurrent state. Couldn't we just use glamor_make_current() >> instead of our own eglMakeCurrent() call in this function? > > We cannot use glamor_make_current() in the problem scenario I'm > trying to fix, because we do not need to set the egl context > from the current screen, we need to not mess up the egl context > which was current *before* glamor_egl_init() gets called for > a different screen then the one which owns the current egl context. > > What happens is: > > 1.current egl context is the master GPUs / Screens egl context > 2. USB GPU gets hotplugged > 3. modesetting driver's PreInit() gets called on the > USB-GPU's Screen > 4. PreInit() calls glamor_egl_init() which creates a new > egl context for the USB-GPU Screen, makes that current, > then fails and sets the current egl context to NONE > 6. master Screen->CreatePixmap gets called, this is > glamor_make_pixmap, this fails (in an assert -> boom) > because the current egl context is NONE There's this callchain: glamor_make_pixmap() -> glamor_create_fbo() -> glamor_create_tex() -> glamor_make_current() And every entrypoint should be doing that before making any other GL calls (if there are any missing, they must be fixed). The problem is that glamor_egl_init() is making a GL context current without updating lastGLcontext, so glamor_make_current() short-circuits. Calling glamor_make_current() from glamor_egl_init() was my proposal for keeping lastGLcontext updated. If we can't use that for some reason, we should just NULL out lastGLcontext at our custom MakeCurrent time so that it's never out of sync with reality. signature.asc Description: PGP signature ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel