Re: [PATCH xserver 2/7] mi: miValidateTree based on paintable not viewable
Adam Jackson writes:
Reading through this, I don't see what this patch ends up changing --
TreatAsTransparent will always return True for unmapped Always windows,
so every place you're checking paintable && !TreatAsTransparent, you
could equivalently be checking viewable && !TreatAsTrasparent without
having changed TreatAsTransparent.
What needs changing is that miComputeClips needs to be called for
paintable windows, not just viewable windows; everything else seems like
it doesn't need changing at all?
> +}
> +else if (pParent->paintable)
> newVis = VisibilityFullyObscured;
> -break;
> +else {
> +newVis = VisibilityNotViewable;
> }
This isn't right -- unmapped windows should always be NotViewable, as
seen through the protocol.
> pParent->visibility = newVis;
> -if (oldVis != newVis &&
> +if (pParent->realized &&
> +oldVis != newVis &&
> ((pParent->
>eventMask | wOtherEventMasks(pParent)) & VisibilityChangeMask))
> SendVisibilityNotify(pParent);
> @@ -293,14 +303,13 @@ miComputeClips(WindowPtr pParent,
> (oldVis == VisibilityUnobscured))) {
> pChild = pParent;
> while (1) {
> -if (pChild->viewable) {
> +if (pChild->paintable) {
> if (pChild->visibility != VisibilityFullyObscured) {
> RegionTranslate(>borderClip, dx, dy);
> RegionTranslate(>clipList, dx, dy);
And this is going to end up wrong -- I think we need to ignore
visibility for purposes of computing clip values here, and just check
for empty clip lists? In any case, the use of visibility to optimize
clip computations is going to serve us poorly, especially if we ever
want to fix it with composite windows.
> for (; pChild; pChild = pChild->nextSib) {
> -if (pChild->viewable && !TreatAsTransparent(pChild))
> +if (pChild->paintable && !TreatAsTransparent(pChild))
> RegionAppend(, >borderSize);
This should be checking viewable, in which case you don't need to change
TreatAsTransparent
> for (pChild = pParent->lastChild; pChild; pChild =
> pChild->prevSib) {
> -if (pChild->viewable && !TreatAsTransparent(pChild))
> +if (pChild->paintable && !TreatAsTransparent(pChild))
> RegionAppend(, >borderSize);
Same here.
> }
> }
> RegionValidate(, );
>
> for (pChild = pParent->firstChild; pChild; pChild = pChild->nextSib)
> {
> -if (pChild->viewable) {
> +if (pChild->paintable) {
> /*
> - * If the child is viewable, we want to remove its extents
> + * If the child is paintable, we want to remove its extents
> * from the current universe, but we only re-clip it if
> * it's been marked.
I'm not sure you need to use paintable here -- unmapped paintable
windows will never want to be removed from their parent anyways, so this
is equivalent to viewable.
> */
> @@ -564,7 +573,7 @@ miValidateTree(WindowPtr pParent, /* Parent to
> validate */
> ScreenPtr pScreen;
> WindowPtr pWin;
> Bool overlap;
> -int viewvals;
> +int paintables = 0;
I think viewvals is correct here; the only windows affecting their
parent clip list are those which are viewable; unmapped paintable
windows have no impact on their parents.
--
-keith
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 xserver 2/7] mi: miValidateTree based on paintable not viewable
Signed-off-by: Adam Jackson
---
mi/mivaltree.c | 108 ++---
1 file changed, 58 insertions(+), 50 deletions(-)
diff --git a/mi/mivaltree.c b/mi/mivaltree.c
index ea6889fdc0..f47cfa4571 100644
--- a/mi/mivaltree.c
+++ b/mi/mivaltree.c
@@ -164,14 +164,16 @@ miShapedWindowIn(RegionPtr universe, RegionPtr bounding,
/*
* Manual redirected windows are treated as transparent; they do not obscure
- * siblings or parent windows
+ * siblings or parent windows. Likewise windows that are paintable but not
+ * mapped.
*/
-#ifdef COMPOSITE
-#define TreatAsTransparent(w) ((w)->redirectDraw == RedirectDrawManual)
-#else
-#define TreatAsTransparent(w) FALSE
-#endif
+static Bool
+TreatAsTransparent(WindowPtr w)
+{
+return (w->redirectDraw == RedirectDrawManual) ||
+(w->paintable && !w->mapped);
+}
#define HasParentRelativeBorder(w) (!(w)->borderIsPixel && \
HasBorder(w) && \
@@ -181,7 +183,7 @@ miShapedWindowIn(RegionPtr universe, RegionPtr bounding,
*---
* miComputeClips --
* Recompute the clipList, borderClip, exposed and borderExposed
- * regions for pParent and its children. Only viewable windows are
+ * regions for pParent and its children. Only paintable windows are
* taken into account.
*
* Results:
@@ -240,37 +242,45 @@ miComputeClips(WindowPtr pParent,
}
#endif
-oldVis = pParent->visibility;
-switch (RegionContainsRect(universe, )) {
-case rgnIN:
-newVis = VisibilityUnobscured;
-break;
-case rgnPART:
-newVis = VisibilityPartiallyObscured;
-{
-RegionPtr pBounding;
+oldVis = newVis = pParent->visibility;
+if (pParent->realized) {
+switch (RegionContainsRect(universe, )) {
+case rgnIN:
+newVis = VisibilityUnobscured;
+break;
+case rgnPART:
+newVis = VisibilityPartiallyObscured;
+{
+RegionPtr pBounding;
-if ((pBounding = wBoundingShape(pParent))) {
-switch (miShapedWindowIn(universe, pBounding,
- ,
- pParent->drawable.x,
- pParent->drawable.y)) {
-case rgnIN:
-newVis = VisibilityUnobscured;
-break;
-case rgnOUT:
-newVis = VisibilityFullyObscured;
-break;
+if ((pBounding = wBoundingShape(pParent))) {
+switch (miShapedWindowIn(universe, pBounding,
+ ,
+ pParent->drawable.x,
+ pParent->drawable.y)) {
+case rgnIN:
+newVis = VisibilityUnobscured;
+break;
+case rgnOUT:
+newVis = VisibilityFullyObscured;
+break;
+}
}
}
+break;
+default:
+newVis = VisibilityFullyObscured;
+break;
}
-break;
-default:
+}
+else if (pParent->paintable)
newVis = VisibilityFullyObscured;
-break;
+else {
+newVis = VisibilityNotViewable;
}
pParent->visibility = newVis;
-if (oldVis != newVis &&
+if (pParent->realized &&
+oldVis != newVis &&
((pParent->
eventMask | wOtherEventMasks(pParent)) & VisibilityChangeMask))
SendVisibilityNotify(pParent);
@@ -293,14 +303,13 @@ miComputeClips(WindowPtr pParent,
(oldVis == VisibilityUnobscured))) {
pChild = pParent;
while (1) {
-if (pChild->viewable) {
+if (pChild->paintable) {
if (pChild->visibility != VisibilityFullyObscured) {
RegionTranslate(>borderClip, dx, dy);
RegionTranslate(>clipList, dx, dy);
pChild->drawable.serialNumber = NEXT_SERIAL_NUMBER;
if (pScreen->ClipNotify)
(*pScreen->ClipNotify) (pChild, dx, dy);
-
}
if (pChild->valdata) {
RegionNull(>valdata->after.borderExposed);
@@ -399,22 +408,22 @@ miComputeClips(WindowPtr pParent,
((pChild->drawable.y == pParent->lastChild->drawable.y) &&
(pChild->drawable.x < pParent->lastChild->drawable.x))) {
for (; pChild; pChild = pChild->nextSib) {
-if (pChild->viewable && !TreatAsTransparent(pChild))
+if (pChild->paintable && !TreatAsTransparent(pChild))
