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

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

!(flags & (Desktop | Alpha)

That can be expressed as a temporary rather than in a compound if statement

const bool ignoreWindow = (flags & (Desktop | Alpha)

119 + /*
120 + * Alpha windows (status == false) can cover/cancel fullscreen
121 + * unredirected windows. But they can't be unredirected themselves.
122 + * I wonder if this is sensible as some games may use fullscreen
123 + * RGBA (alpha) windows that are fully opaque and need unredirect.
124 + */

This comment is confusing. We're checking whether or not the window has an rgba visual (eg, w->alpha () not whether or not it failed to paint (as status == false) would indicate to me.

Also, the default visual for most windows would be an RGB24 visual, so I don't think the concern is too much. (And we wouldn't be able to detect it anyways, as an RGBA visual just means a client is at liberty to have alpha values in its backing pixmap, which we must blend).

I'm also a bit confused as to why it matters that the window has an alpha channel as to whether or not a fullscreen window underneath it is unredirected. Shouldn't the fullscreen window underneath always be unredirected if there is some other window that is occluding it?

review: Needs Information

« Back to merge proposal