[Mesa-dev] [PATCH v4 5/5] panfrost: Add support for KHR_partial_update()

[Boris Brezillon]

On Wed, 26 Jun 2019 12:19:28 -0700
Alyssa Rosenzweig <alyssa.rosenzweig at 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 :-(.