Adam Jackson <a...@redhat.com> 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(&pChild->borderClip, dx, dy);
>                          RegionTranslate(&pChild->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(&childUnion, &pChild->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(&childUnion, &pChild->borderSize);

Same here.

>              }
>          }
>          RegionValidate(&childUnion, &overlap);
>  
>          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

Attachment: 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

Reply via email to