[v4,5/5] panfrost: Add support for KHR_partial_update()

Submitted by Boris Brezillon on June 25, 2019, 4:37 p.m.

Details

Message ID 20190625163749.15721-6-boris.brezillon@collabora.com
State New
Headers show
Series "EGL_KHR_partial_update support" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Boris Brezillon June 25, 2019, 4:37 p.m.
Implement ->set_damage_region() region to support partial updates.

This is a dummy implementation in that it does not try to merge
damage rects. It also does not deal with distinct regions and instead
pick the largest quad as the only damage rect and generate up to 4
reload rects out of it (the left/right/top/bottom regions surrounding
the biggest damage rect).

We also do not try to reduce the number of draws by passing all quad
vertices to the blit request (would require extending u_blitter)

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 src/gallium/drivers/panfrost/pan_blit.c     | 14 ++---
 src/gallium/drivers/panfrost/pan_context.c  | 49 ++++++++++++++++-
 src/gallium/drivers/panfrost/pan_job.c      | 11 ++++
 src/gallium/drivers/panfrost/pan_job.h      |  5 ++
 src/gallium/drivers/panfrost/pan_resource.c | 58 +++++++++++++++++++++
 src/gallium/drivers/panfrost/pan_resource.h | 12 ++++-
 src/gallium/drivers/panfrost/pan_screen.c   |  1 +
 7 files changed, 141 insertions(+), 9 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/gallium/drivers/panfrost/pan_blit.c b/src/gallium/drivers/panfrost/pan_blit.c
index 5859f92f9d1b..3a45277ee287 100644
--- a/src/gallium/drivers/panfrost/pan_blit.c
+++ b/src/gallium/drivers/panfrost/pan_blit.c
@@ -105,18 +105,18 @@  panfrost_blit(struct pipe_context *pipe,
  */
 
 void
-panfrost_blit_wallpaper(struct panfrost_context *ctx)
+panfrost_blit_wallpaper(struct panfrost_context *ctx, struct pipe_box *rect)
 {
         struct pipe_blit_info binfo = { };
 
         panfrost_blitter_save(ctx);
 
-	binfo.src.resource = binfo.dst.resource = ctx->pipe_framebuffer.cbufs[0]->texture;
-	binfo.src.level = binfo.dst.level = 0;
-	binfo.src.box.x = binfo.dst.box.x = 0;
-	binfo.src.box.y = binfo.dst.box.y = 0;
-	binfo.src.box.width = binfo.dst.box.width = ctx->pipe_framebuffer.width;
-	binfo.src.box.height = binfo.dst.box.height = ctx->pipe_framebuffer.height;
+        binfo.src.resource = binfo.dst.resource = ctx->pipe_framebuffer.cbufs[0]->texture;
+        binfo.src.level = binfo.dst.level = 0;
+        binfo.src.box.x = binfo.dst.box.x = rect->x;
+        binfo.src.box.y = binfo.dst.box.y = rect->y;
+        binfo.src.box.width = binfo.dst.box.width = rect->width;
+        binfo.src.box.height = binfo.dst.box.height = rect->height;
 
 	/* This avoids an assert due to missing nir_texop_txb support */
 	//binfo.src.box.depth = binfo.dst.box.depth = 1;
diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c
index de6dd38c5566..c1075c6693e8 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -1528,7 +1528,54 @@  panfrost_draw_wallpaper(struct pipe_context *pipe)
         struct panfrost_job *batch = panfrost_get_job_for_fbo(ctx);
 
         ctx->wallpaper_batch = batch;
-        panfrost_blit_wallpaper(ctx);
+
+        /* Clamp the rendering area to the damage extent. The
+         * KHR_partial_update() spec states that trying to render outside of
+         * the damage region is "undefined behavior", so we should be safe.
+         */
+        panfrost_job_intersection_scissor(batch, rsrc->damage.extent.minx,
+                                          rsrc->damage.extent.miny,
+                                          rsrc->damage.extent.maxx,
+                                          rsrc->damage.extent.maxy);
+
+        struct pipe_scissor_state damage;
+        struct pipe_box rects[4];
+
+        /* Clamp the damage box to the rendering area. */
+        damage.minx = MAX2(batch->minx, rsrc->damage.biggest_rect.x);
+        damage.miny = MAX2(batch->miny, rsrc->damage.biggest_rect.y);
+        damage.maxx = MIN2(batch->maxx,
+                           rsrc->damage.biggest_rect.x +
+                           rsrc->damage.biggest_rect.width);
+        damage.maxy = MIN2(batch->maxy,
+                           rsrc->damage.biggest_rect.y +
+                           rsrc->damage.biggest_rect.height);
+
+        /* One damage rectangle means we can end up with at most 4 reload
+         * regions:
+         * 1: left region, only exists if damage.x > 0
+         * 2: right region, only exists if damage.x + damage.width < fb->width
+         * 3: top region, only exists if damage.y > 0. The intersection with
+         *    the left and right regions are dropped
+         * 4: bottom region, only exists if damage.y + damage.height < fb->height.
+         *    The intersection with the left and right regions are dropped
+         */
+        u_box_2d(batch->minx, batch->miny, damage.minx - batch->minx,
+                 batch->maxy - batch->miny, &rects[0]);
+        u_box_2d(damage.maxx, batch->miny, batch->maxx - damage.maxx,
+                 batch->maxy - batch->miny, &rects[1]);
+        u_box_2d(damage.minx, batch->miny, damage.maxx - damage.minx,
+                 damage.miny - batch->miny, &rects[2]);
+        u_box_2d(damage.minx, damage.maxy, damage.maxx - damage.minx,
+                 batch->maxy - damage.maxy, &rects[3]);
+
+        for (unsigned i = 0; i < 4; i++) {
+                if (!rects[i].width || !rects[i].height)
+                        continue;
+
+                /* Blit the wallpaper in */
+                panfrost_blit_wallpaper(ctx, &rects[i]);
+        }
         ctx->wallpaper_batch = NULL;
 }
 
diff --git a/src/gallium/drivers/panfrost/pan_job.c b/src/gallium/drivers/panfrost/pan_job.c
index 22f0f492b5d8..5fa81dbb896d 100644
--- a/src/gallium/drivers/panfrost/pan_job.c
+++ b/src/gallium/drivers/panfrost/pan_job.c
@@ -295,6 +295,17 @@  panfrost_job_union_scissor(struct panfrost_job *job,
         job->maxy = MAX2(job->maxy, maxy);
 }
 
+void
+panfrost_job_intersection_scissor(struct panfrost_job *job,
+                                  unsigned minx, unsigned miny,
+                                  unsigned maxx, unsigned maxy)
+{
+        job->minx = MAX2(job->minx, minx);
+        job->miny = MAX2(job->miny, miny);
+        job->maxx = MIN2(job->maxx, maxx);
+        job->maxy = MIN2(job->maxy, maxy);
+}
+
 void
 panfrost_job_init(struct panfrost_context *ctx)
 {
diff --git a/src/gallium/drivers/panfrost/pan_job.h b/src/gallium/drivers/panfrost/pan_job.h
index b4c9db9828e2..f98572387ed0 100644
--- a/src/gallium/drivers/panfrost/pan_job.h
+++ b/src/gallium/drivers/panfrost/pan_job.h
@@ -152,6 +152,11 @@  panfrost_job_union_scissor(struct panfrost_job *job,
                 unsigned minx, unsigned miny,
                 unsigned maxx, unsigned maxy);
 
+void
+panfrost_job_intersection_scissor(struct panfrost_job *job,
+                                  unsigned minx, unsigned miny,
+                                  unsigned maxx, unsigned maxy);
+
 /* Scoreboarding */
 
 void
diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c
index 1a691a5be343..95f8177a9f51 100644
--- a/src/gallium/drivers/panfrost/pan_resource.c
+++ b/src/gallium/drivers/panfrost/pan_resource.c
@@ -312,6 +312,62 @@  panfrost_create_bo(struct panfrost_screen *screen, const struct pipe_resource *t
         return bo;
 }
 
+static void
+panfrost_resource_reset_damage(struct panfrost_resource *pres)
+{
+        /* We set the damage extent to the full resource size but keep the
+         * damage box empty so that the FB content is reloaded by default.
+         */
+        memset(&pres->damage, 0, sizeof(pres->damage));
+        pres->damage.extent.maxx = pres->base.width0;
+        pres->damage.extent.maxy = pres->base.height0;
+}
+
+void
+panfrost_resource_set_damage_region(struct pipe_screen *screen,
+                                    struct pipe_resource *res,
+                                    unsigned int nrects, int *rects)
+{
+        struct panfrost_resource *pres = pan_resource(res);
+        struct pipe_box *damage_rect = &pres->damage.biggest_rect;
+        struct pipe_scissor_state *damage_extent = &pres->damage.extent;
+        unsigned int i;
+
+	if (!nrects) {
+		panfrost_resource_reset_damage(pres);
+		return;
+	}
+
+        memset(&pres->damage, 0, sizeof(pres->damage));
+        damage_extent->minx = 0xffff;
+        damage_extent->miny = 0xffff;
+        for (i = 0; i < nrects; i++) {
+                struct pipe_scissor_state ss;
+                int *rect = &rects[i * 4];
+                int y = res->height0 - (rect[1] + rect[3]);
+                struct pipe_box box;
+
+                u_box_2d(rect[0], y, rect[2], rect[3], &box);
+                if (damage_rect->width * damage_rect->height < box.width * box.height)
+                       *damage_rect = box;
+
+                /* Looks like aligning on a tile is not enough, but aligning on
+                 * twice the tile size works.
+                 */
+                ss.minx = rect[0] & ~((MALI_TILE_LENGTH * 2) - 1);
+                ss.miny = y & ~((MALI_TILE_LENGTH * 2) - 1);
+                ss.maxx = MIN2(ALIGN(rect[0] + rect[2], MALI_TILE_LENGTH * 2),
+                               res->width0);
+                ss.maxy = MIN2(ALIGN(y + rect[3], MALI_TILE_LENGTH * 2),
+                               res->height0);
+
+                damage_extent->minx = MIN2(damage_extent->minx, ss.minx);
+                damage_extent->miny = MIN2(damage_extent->miny, ss.miny);
+                damage_extent->maxx = MAX2(damage_extent->maxx, ss.maxx);
+                damage_extent->maxy = MAX2(damage_extent->maxy, ss.maxy);
+        }
+}
+
 static struct pipe_resource *
 panfrost_resource_create(struct pipe_screen *screen,
                          const struct pipe_resource *template)
@@ -368,6 +424,8 @@  panfrost_resource_create(struct pipe_screen *screen,
                 so->bo = panfrost_create_bo(pscreen, template);
         }
 
+        panfrost_resource_reset_damage(so);
+
         return (struct pipe_resource *)so;
 }
 
diff --git a/src/gallium/drivers/panfrost/pan_resource.h b/src/gallium/drivers/panfrost/pan_resource.h
index 632250fa2aa9..095c078e6f18 100644
--- a/src/gallium/drivers/panfrost/pan_resource.h
+++ b/src/gallium/drivers/panfrost/pan_resource.h
@@ -98,6 +98,10 @@  panfrost_bo_unreference(struct pipe_screen *screen, struct panfrost_bo *bo);
 
 struct panfrost_resource {
         struct pipe_resource base;
+        struct {
+                struct pipe_box biggest_rect;
+                struct pipe_scissor_state extent;
+        } damage;
 
         struct panfrost_bo *bo;
         struct renderonly_scanout *scanout;
@@ -144,6 +148,12 @@  panfrost_blit(struct pipe_context *pipe,
               const struct pipe_blit_info *info);
 
 void
-panfrost_blit_wallpaper(struct panfrost_context *ctx);
+panfrost_blit_wallpaper(struct panfrost_context *ctx,
+                        struct pipe_box *damage);
+
+void
+panfrost_resource_set_damage_region(struct pipe_screen *screen,
+                                    struct pipe_resource *res,
+                                    unsigned int nrects, int *rects);
 
 #endif /* PAN_RESOURCE_H */
diff --git a/src/gallium/drivers/panfrost/pan_screen.c b/src/gallium/drivers/panfrost/pan_screen.c
index 15e68fbe2c10..3f27378088d6 100644
--- a/src/gallium/drivers/panfrost/pan_screen.c
+++ b/src/gallium/drivers/panfrost/pan_screen.c
@@ -599,6 +599,7 @@  panfrost_create_screen(int fd, struct renderonly *ro)
         screen->base.get_compiler_options = panfrost_screen_get_compiler_options;
         screen->base.fence_reference = panfrost_fence_reference;
         screen->base.fence_finish = panfrost_fence_finish;
+        screen->base.set_damage_region = panfrost_resource_set_damage_region;
 
 	screen->last_fragment_flushed = true;
         screen->last_job = NULL;

Comments


Hi,
To be honest, I haven't been able to look too closely at this one. I
wasn't able to easily reason about the twists and turns, so had to run
away to reviews elsewhere. But as long as we reload every single
region passed in - be it individually or just lazily pulling in the
extents, it's correct.

There's no need to intersect the partial_update region with the
scissor, since rendering outside of the partial_update area is
explicitly undefined.

On Wed, 26 Jun 2019 at 18:51, Alyssa Rosenzweig
<alyssa.rosenzweig@collabora.com> wrote:
> > +        binfo.src.resource = binfo.dst.resource = ctx->pipe_framebuffer.cbufs[0]->texture;
> > +        binfo.src.level = binfo.dst.level = 0;
> > +        binfo.src.box.x = binfo.dst.box.x = rect->x;
> > +        binfo.src.box.y = binfo.dst.box.y = rect->y;
> > +        binfo.src.box.width = binfo.dst.box.width = rect->width;
> > +        binfo.src.box.height = binfo.dst.box.height = rect->height;
> >
> >       /* This avoids an assert due to missing nir_texop_txb support */
> >       //binfo.src.box.depth = binfo.dst.box.depth = 1;
>
> This will need to be rebased in a slightly messy way, since
> panfrost_blit_wallpaper was edited pretty heavily in the mipmap series
> that just landed. Sorry for the conflicts, although conceptually this
> looks good.
>
> Have you considered if this interacts with mipmapping, by the way? I
> suppose surfaces that get partial updates *by definition* are not
> mipmapped, so that's an easy "who cares?" :)

Yeah, partial_update only applies to winsys surfaces.

> > +        u_box_2d(batch->minx, batch->miny, damage.minx - batch->minx,
> > +                 batch->maxy - batch->miny, &rects[0]);
> > +        u_box_2d(damage.maxx, batch->miny, batch->maxx - damage.maxx,
> > +                 batch->maxy - batch->miny, &rects[1]);
> > +        u_box_2d(damage.minx, batch->miny, damage.maxx - damage.minx,
> > +                 damage.miny - batch->miny, &rects[2]);
> > +        u_box_2d(damage.minx, damage.maxy, damage.maxx - damage.minx,
> > +                 batch->maxy - damage.maxy, &rects[3]);
> > +
> > +        for (unsigned i = 0; i < 4; i++) {
> > +                if (!rects[i].width || !rects[i].height)
> > +                        continue;
>
> This 'if' statement seems a little magic. Does u_box_2d clamp
> width/height positive automatically? Is it possible to get negative
> width/height? If the answer is "yes; no" respectively, which seems to be
> how the code works, maybe add a quick comment explaining that.

Negative width or height isn't explicitly allowed in the
partial_update spec; by that omission I would say it's not defined
behaviour.

Cheers,
Daniel
On Wed, 26 Jun 2019 10:51:23 -0700
Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com> wrote:

> Overall, I'm quite happy with how this turns out; I was fearful it would
> be a lot more complicated, though there's always time for that ;)

Same here. I thought it would be more complicated than that, but it
turned out to be pretty simple (mainly because I didn't go into too much
optimization to discard as much of the wallpapering area as could
theoretically be).

> 
> Some specific comments follow: (mostly minor):
> -----------------------------------------------
> 
> > +panfrost_blit_wallpaper(struct panfrost_context *ctx, struct pipe_box *rect)
> >  {
> >          struct pipe_blit_info binfo = { };
> >  
> >          panfrost_blitter_save(ctx);
> >  
> > -	binfo.src.resource = binfo.dst.resource = ctx->pipe_framebuffer.cbufs[0]->texture;
> > -	binfo.src.level = binfo.dst.level = 0;
> > -	binfo.src.box.x = binfo.dst.box.x = 0;
> > -	binfo.src.box.y = binfo.dst.box.y = 0;
> > -	binfo.src.box.width = binfo.dst.box.width = ctx->pipe_framebuffer.width;
> > -	binfo.src.box.height = binfo.dst.box.height = ctx->pipe_framebuffer.height;
> > +        binfo.src.resource = binfo.dst.resource = ctx->pipe_framebuffer.cbufs[0]->texture;
> > +        binfo.src.level = binfo.dst.level = 0;
> > +        binfo.src.box.x = binfo.dst.box.x = rect->x;
> > +        binfo.src.box.y = binfo.dst.box.y = rect->y;
> > +        binfo.src.box.width = binfo.dst.box.width = rect->width;
> > +        binfo.src.box.height = binfo.dst.box.height = rect->height;
> >  
> >  	/* This avoids an assert due to missing nir_texop_txb support */
> >  	//binfo.src.box.depth = binfo.dst.box.depth = 1;  
> 
> This will need to be rebased in a slightly messy way, since
> panfrost_blit_wallpaper was edited pretty heavily in the mipmap series
> that just landed. Sorry for the conflicts, although conceptually this
> looks good.

No problem, I'll take care of that (not the first time I rebase the
patch series BTW).

> 
> Have you considered if this interacts with mipmapping, by the way? I
> suppose surfaces that get partial updates *by definition* are not
> mipmapped, so that's an easy "who cares?" :)

Daniel already replied to that one.

> 
> > +        u_box_2d(batch->minx, batch->miny, damage.minx - batch->minx,
> > +                 batch->maxy - batch->miny, &rects[0]);
> > +        u_box_2d(damage.maxx, batch->miny, batch->maxx - damage.maxx,
> > +                 batch->maxy - batch->miny, &rects[1]);
> > +        u_box_2d(damage.minx, batch->miny, damage.maxx - damage.minx,
> > +                 damage.miny - batch->miny, &rects[2]);
> > +        u_box_2d(damage.minx, damage.maxy, damage.maxx - damage.minx,
> > +                 batch->maxy - damage.maxy, &rects[3]);
> > +
> > +        for (unsigned i = 0; i < 4; i++) {
> > +                if (!rects[i].width || !rects[i].height)
> > +                        continue;  
> 
> This 'if' statement seems a little magic. Does u_box_2d clamp
> width/height positive automatically? Is it possible to get negative
> width/height? If the answer is "yes; no" respectively, which seems to be
> how the code works, maybe add a quick comment explaining that.

I'll add a comment to explain that.

> 
> > +        /* We set the damage extent to the full resource size but keep the
> > +         * damage box empty so that the FB content is reloaded by default.
> > +         */  
> 
> ....English, please? Francais, s'il te plait? I'm not too familiar with
> winsys or the extension -- what's the difference between damage extent
> and damage box?

Yeah, reading the comment again I realize it's not clear at all. The
damage extent is the quad covering all damage rects (even if they don't
intersect or only partially intersect). The damage box is actually the
biggest damage rect (rect1 in the following example):

              _______________________
              |            |         |
              |__________  |  rect 2 |
              |         |  |_________|
              | rect 1  |______      |
              |         |rect3 |     |
              |_________|______|_____|

                    damage extent

> 
> > +                /* Looks like aligning on a tile is not enough, but aligning on
> > +                 * twice the tile size works.
> > +                 */
> > +                ss.minx = rect[0] & ~((MALI_TILE_LENGTH * 2) - 1);
> > +                ss.miny = y & ~((MALI_TILE_LENGTH * 2) - 1);
> > +                ss.maxx = MIN2(ALIGN(rect[0] + rect[2], MALI_TILE_LENGTH * 2),
> > +                               res->width0);
> > +                ss.maxy = MIN2(ALIGN(y + rect[3], MALI_TILE_LENGTH * 2),
> > +                               res->height0);  
> 
> If aligning to 32x32 but not 16x16 works, that's probably masking over a
> bug somewhere else in the code.

I wish I could come with a better explanation, but I couldn't find
anything explaining why this alignment is requirement or spot any
obvious bugs in the code :-/.

> The tiles used in the fragment (the
> union/intersection_scissor) are 16x16,

Oops, I fear intersection of non-32x32-aligned regions is not safe, it's
just that I didn't test this case :-). Note that union would not be
a problem here, because the intersection is applied last (just before
drawing the wallpaper). That's only true if we assume the intersection
func aligns things on 32x32 pixels of course (which is not the case
right now).

> and the wallpapering blits are
> pixel-accurate.

That part is true, and I actually rely on it (only reload parts of the
tiles that are not dirty).

> What's really going on here?

If only I knew...

> 
> > +panfrost_blit_wallpaper(struct panfrost_context *ctx,
> > +                        struct pipe_box *damage);  
> 
> Bikeshedding, but is it appropriate to name the field `damage` rather
> than just generically `box`? Conceptually, blit_wallpaper just cares
> about where to blit, it doesn't care *why* you're blitting there
> (separation of concerns); it's not really that function's business that
> we're blitting because of damage (and not some other reason we might
> need to blit. Hypothetical example: we're doing a blit-based transfer
> from a compressed format and we need to blit partial tiles on the edges.
> That's not really 'damage').

Will rename this argument.


On Wed, 26 Jun 2019 12:19:28 -0700
Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com> wrote:

> > > > +        /* We set the damage extent to the full resource size but keep the
> > > > +         * damage box empty so that the FB content is reloaded by default.
> > > > +         */    
> > > 
> > > ....English, please? Francais, s'il te plait? I'm not too familiar with
> > > winsys or the extension -- what's the difference between damage extent
> > > and damage box?  
> > 
> > Yeah, reading the comment again I realize it's not clear at all. The
> > damage extent is the quad covering all damage rects (even if they don't
> > intersect or only partially intersect). The damage box is actually the
> > biggest damage rect (rect1 in the following example):
> > 
> >               _______________________
> >               |            |         |
> >               |__________  |  rect 2 |
> >               |         |  |_________|
> >               | rect 1  |______      |
> >               |         |rect3 |     |
> >               |_________|______|_____|
> > 
> >                     damage extent  
> 
> Hmm, ok.
> 
> > > If aligning to 32x32 but not 16x16 works, that's probably masking over a
> > > bug somewhere else in the code.  
> > 
> > I wish I could come with a better explanation, but I couldn't find
> > anything explaining why this alignment is requirement or spot any
> > obvious bugs in the code :-/.  
> 
> Hmmmm. what was the symptom?

The symptom is, black areas around the damage rect when the rendering
area (the area you define in mali_payload_fragment) is not
32x32-aligned. If you want to test it, remove the "* 2" in the code and
run weston+desktop-shell (the partial_update() logic has been merged
earlier today, so you just have to build master) and start a terminal.

> Also, maybe try with the commit I pushed to
> the `tile-aligned` branch on gitlab.fd.o/alyssa/mesa

Will try it. Thanks.

> 
> > Oops, I fear intersection of non-32x32-aligned regions is not safe, it's
> > just that I didn't test this case :-). Note that union would not be
> > a problem here, because the intersection is applied last (just before
> > drawing the wallpaper). That's only true if we assume the intersection
> > func aligns things on 32x32 pixels of course (which is not the case
> > right now).  
> 
> I'm not sure I follow why the intersection of, say, 16x16 aligned
> regions (that are not also 32x32 aligned) shouldn't work?

I'm not saying it shouldn't work, just saying it doesn't work in
practice :P.

> 16x16 *is* the
> tile size for all intents and purposes here; 32x32 only comes up in
> hierarchical tiling which is irrelevant to this code (and in fact, we
> can disable hierarchical tiling entirely if I need to make my point even
> clearer ;P Just poking fun)

As I said, I wish I had a better understanding of the issue, but that's
not the case :-(.
> The symptom is, black areas around the damage rect when the rendering
> area (the area you define in mali_payload_fragment) is not
> 32x32-aligned. If you want to test it, remove the "* 2" in the code and
> run weston+desktop-shell (the partial_update() logic has been merged
> earlier today, so you just have to build master) and start a terminal.

So, if the wallpaper is drawing to a 32x32 aligned area but the
payload_fragment bounds are only 16x16 aligned, that doesn't do it?
On Wed, 26 Jun 2019 14:03:45 -0700
Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com> wrote:

> > The symptom is, black areas around the damage rect when the rendering
> > area (the area you define in mali_payload_fragment) is not
> > 32x32-aligned. If you want to test it, remove the "* 2" in the code and
> > run weston+desktop-shell (the partial_update() logic has been merged
> > earlier today, so you just have to build master) and start a terminal.  
> 
> So, if the wallpaper is drawing to a 32x32 aligned area but the
> payload_fragment bounds are only 16x16 aligned, that doesn't do it?

Aligning only the wallpaper drawing to 32x32 doesn't work.