Code review comment for lp:~vanvugt/compiz/fix-1089811

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Hmm, this irks me somewhat. It will have some interesting side effects, like if the input shape region changes while the window is minimized. It also represents some behavioural coupling that I'd like to avoid.

The one thing I can think of is that I believe the nBounding < 1 and nInput < 1 checks were only put in place in the case of shape support being missing (hence, region and inputRegion would be empty). As such, those checks should really be removed and the assignments should only happen if we actually queried the shape:

So:

bool queriedShape = false;

if (screen->XShape ())
{
    ... Query Shape

    queriedShape = true;
}

r.x = -geom.border ();
...

if (!queriedShape)
{
    boundingShaperects = &r;
    nBounding = 1;
    inputShapeRects = &r;
    nInput = 1;
}

priv->region += ....

if (!queriedShape)
{
    if (!boundingShapeRects)
        XFree ...
}

review: Needs Information

« Back to merge proposal