Merge lp://staging/~smspillaz/compiz-core/fix_894688 into lp://staging/compiz-core/0.9.5

Make the decor plugin work with paint offsets.

Moved getting the shift distance due to window gravity into a utility function in core and made it so that whenever a plugin tries to change the frame extents of a window, this will always update the saved co-ordinates of maximized windows so that they restore to the correct position if their frame extents change while they are maximized (eg, the unity case)

ln 130 - is the change from 'geometry' to 'serverGeometry' intentional?

ln 214 - Why not write this as:

cScreen->damageRegion(oldGeometry + newGeometry);

ln 980 - this obviously only works as long as both structures have exactly
 the same contents and are laid out in exactly the same manner. I assume you have checked that this is the case?

As far as I can tell, this is looking good, with the few points mentioned above.

> ln 130 - is the change from 'geometry' to 'serverGeometry' intentional?
>

Yes, the serverGeometry is what is used to paint the window while we wait on the server to give us the new geometry for the window (which is always guarunteed to be the geometry last sent (eg serverGeometry) as we are the window manager).

> ln 214 - Why not write this as:
>
> cScreen->damageRegion(oldGeometry + newGeometry);

fixed

>
> ln 980 - this obviously only works as long as both structures have exactly
> the same contents and are laid out in exactly the same manner. I assume you
> have checked that this is the case?

Yes, they are both CompWindowExtents. Perhaps I should make it so that you can't derive from that class though, I might ask Tim about that.

>
>
> As far as I can tell, this is looking good, with the few points mentioned
> above.

>> ln 214 - Why not write this as:
>>
>> cScreen->damageRegion(oldGeometry + newGeometry);

> fixed

Actually, there's no operator+ defined in CompRect which returns a region, and I'm not really sure if I want to introduce a cyclic dependency like that. So I think the explicit conversion is better off for now.

OK, I'm happy.

> (memcmp (&priv->serverInput, i, sizeof (CompWindowExtents)) ||
> memcmp (&priv->border, b, sizeof (CompWindowExtents)))

Please write an operator == and not use memcmp. Even for simple structs.
Then implement operator != in terms of operator ==.

The main reason (other than stylistic and religious) for not using memcmp on structs is because structs are often not perfectly aligned and so contain padding bytes. The padding bytes are uninitialized, unpredictable and inaccessible unless you also guarantee each struct has first been memset(&s,0,sizeof(s)).

In general, memcmp is highly optimized for the architecture and is in fact faster than a bespoke operator. However you can't use memcmp with structs unless you guarantee they're always fully initialized with memset to the full sizeof(yourstruct).

On Thu, 15 Dec 2011, Daniel van Vugt wrote:

> The main reason (other than stylistic and religious) for not using memcmp on structs is because structs are often not perfectly aligned and so contain padding bytes. The padding bytes are uninitialized, unpredictable and inaccessible unless you also guarantee each struct has first been memset(&s,0,sizeof(s)).
>
> In general, memcmp is highly optimized for the architecture and is in fact faster than a bespoke operator. However you can't use memcmp with structs unless you guarantee they're always fully initialized with memset to the full sizeof(yourstruct).

Thanks Daniel - I didn't know that, so I'll keep it in mind (and probably
just implement operator==)

> --
> https://code.launchpad.net/~smspillaz/compiz-core/fix_894688/+merge/84058
> You are the owner of lp:~smspillaz/compiz-core/fix_894688.
>

I made the same mistake myself...
http://sourceforge.net/p/dapar/code/ci/d92e7bf1ec46ad6f984efa714483f2e19a40a3ab

Looks good to me.