[PATCH weston 11/11] simple-damage: Add --use-buffer-damage flag

[Pekka Paalanen]

On Fri, 27 Nov 2015 14:55:13 -0600
Derek Foreman <derekf at osg.samsung.com> wrote:

> On 27/11/15 07:47 AM, Pekka Paalanen wrote:
> > On Wed, 18 Nov 2015 16:32:34 -0600
> > Derek Foreman <derekf at osg.samsung.com> wrote:
> >   
> >> Add a new flag for testing damage in buffer co-ordinates
> >>
> >> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
> >> ---
> >>  clients/simple-damage.c | 62 +++++++++++++++++++++++++++++++++++++------------
> >>  1 file changed, 47 insertions(+), 15 deletions(-)
> >>  
> > 
> > Hi Derek,
> > 
> > even without your series, running simple-damage --use-viewport
> > --scale=2 will leave trails. It's not a damage issue, because
> > compositor repaints do not clear it. It seems to be a rendering issue
> > in the app. Using viewport with any scale > 1 will trigger that.  
> 
> Yes, I was seeing this too.  Doesn't happen with the pixman renderer.
> 
> Figuring it out is on my TODO list somewhere, right next to sorting out
> the damage issues when zoomed and when changing transforms between
> commits. :)
> 
> If I mess up the gl renderer to do a full texture upload with ever
> damage commit the problem goes away.

Ha! You're right, I forgot the compositor has a copy of the surface
contents and uses damage to update that. D'oh.

> Checking the damage co-ordinates in gl_renderer_flush_damage(),
> sometimes thin areas are getting shrunk to 0 (when they look like they
> should be getting expanded by the transform)
> 
> I guess the thing damage regions come from the two (erase, replace)
> damage regions being bashed together and overlapping when the ball is
> moving slowly...
> 
> weston_surface_to_buffer_rect() looks like it could be part of the
> problem, since it looks like it does the viewport stuff first, then
> truncates to integer, then transforms the resulting rect.  Could be
> losing thing rectangles in the middle?

You are on to something there. On quick look, I'd blame the rounding of
x2,y2 being in the wrong direction. But then users need to ensure not
crossing over image boundaries.

> Appears to not happen with my transforms branch that replaced all sorts
> of this stuff with matrix mults with no conversion to integer in
> between.  Likely because it doesn't have to round off stuff to ints
> between steps?

Quite likely, and maybe the final rounding is also done towards
enlarging the damage region there?

> I think that's as far as I'm going to go in debugging this one.

If simply fixing the rounding in weston_surface_to_buffer_rect() fixes
it, maybe it could be landed fast?

> > Anyway, I didn't see this patch changing any behaviour that I tried.

> > Otherwise this patch looks fine, but I have hard time understanding how
> > everything works here. Especially the rotations made my brain fall off.
> > That's why just a  
> 
> We're only really rotating a circle, and it's orientation independent,
> right? ;)

Yeeeah... except when you are rounding to integer pixels and if the
rounding direction makes the circle not centered and...

Actually I was more trying to understand the code that was already in
redraw(), that you clarified to do things like off_x = bwidth - tx; and
then it does paint_box(buffer->shm_data, bpitch, off_x, off_y, bwidth,
bborder, 0xffffffff);, so would that not paint up to off_x + bwidth =
bwidth - tx + bwidth which will be greater than bwidth, i.e. drawing
beyond the image border? But when I looked at the rendering, I didn't
see any evidence of that, so there is something I don't understand.
After all, tx > bwidth cannot be, right? Ooh, but bwidth is only half
of the actual bwidth... that's probably the thing I missed. :-P

Yeah, hard time following all that.


Thanks,
pq

> 
> > Acked-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > 
> > Whether to use damage_buffer by default when available, I don't think
> > it matters. It depends on your toolkit which one is more
> > straightforward to use, otherwise they are equally good.  
> 
> Ok, I'll leave it alone out of sheer laziness then.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20151130/07e8944f/attachment.sig>