From kwin  Mon Feb 11 16:04:03 2008
From: Rivo Laks <rivolaks () hot ! ee>
Date: Mon, 11 Feb 2008 16:04:03 +0000
To: kwin
Subject: Re: RFC: Multiple clipping
Message-Id: <200802111804.03657.rivolaks () hot ! ee>
X-MARC-Message: https://marc.info/?l=kwin&m=120274593330175

Ühel kenal päeval (laupäev 26 jaanuar 2008) kirjutas Lubos Lunak:
>  Hello,
>
>  I'd like to commit the attached patch, but I'd like to get comments on it
> first. The patch basically adds class PaintClipper that can be used to
> specify the area where it's allowed to paint, and these add up (or, rather,
> intersect). It fixes the bug with DesktopGrid where an overlapped window
> can be sometimes seen even on desktops where it's not (a desktop with
> smaller number). Since neither glScissor nor XFixesSetPictureClipRegion()
> can be stacked, there could be a problem with possible multiple clipping.
> Actually, I could simply use it here, because the rest is painted
> transformed and as such not clipped, but maybe there can be such cases.
>
>  The patch is a bit bigger because I had to move few things around, but the
> important parts are the PaintClipper class, the parts in
> magnifier.cpp/desktopgrid.cpp and in renderGLGeometry(), those should be
> enough to get the idea. Shortly, every clipping is specified using
> PaintClipper, it's used when painting anything and there no longer is a
> flag for 'clip', as there's always clipping, with e.g. transformed screen
> the clipping is done by infiniteRegion().
>
>  The thing I'm wondering is whether it's worth it. I have no idea how big
> performance impact has the increased use of glScissor(). I'm also not sure
> there actually realistically can happen cases where multiple clipping would
> be needed - basically, currently we can't do very complicated stacked
> transformations, because they'd probably break something (including
> PaintClipper presumably).

Hi

What I like about the patch is that it makes the code more readable by hiding 
the clipping stuff in the PaintClipper class.

What I'm a bit concerned about it that we should now always render geometry 
multiple times - corresponding to the number of rectangles in the clip 
region. I'm not sure how many rectangles that region usually consists of, do 
you have any info on that?
Actually, if nothing push()es additional regions then the number of rectangles 
in clip region (= number of times geometry need to be rendered) should be 
same as ATM, right? So in that sense it shouldn't actually have any 
performance impact as long as the effects aren't using it.
We should probably note in apidox that it's not a good idea to form complex 
regions as it might slow down painting but otherwise it can go in IMHO.

In the patch I don't see where the PaintClipper::push() is used when we're 
doing a partial update of the screen. Shouldn't something in Scene push the 
correponding region before the painting begins?

Rivo
_______________________________________________
Kwin mailing list
Kwin@kde.org
https://mail.kde.org/mailman/listinfo/kwin