Merge lp:~smspillaz/compiz-core/compiz-core.work_923683 into lp:compiz-core

Proposed by Sam Spilsbury on 2012-04-05
Status: Superseded
Proposed branch: lp:~smspillaz/compiz-core/compiz-core.work_923683
Merge into: lp:compiz-core
Prerequisite: lp:~smspillaz/compiz-core/compiz-core.fix_969101
Diff against target: 1942 lines (+384/-681)
17 files modified
include/core/window.h (+2/-2)
plugins/composite/src/window.cpp (+16/-16)
plugins/decor/src/decor.cpp (+13/-15)
plugins/move/src/move.cpp (+10/-12)
plugins/opengl/src/window.cpp (+2/-2)
plugins/place/src/constrain-to-workarea/src/constrain-to-workarea.cpp (+4/-6)
plugins/place/src/place.cpp (+19/-24)
plugins/resize/src/resize.cpp (+5/-5)
plugins/wobbly/src/wobbly.cpp (+8/-8)
src/actions.cpp (+2/-2)
src/event.cpp (+1/-1)
src/privatewindow.h (+7/-8)
src/screen.cpp (+4/-2)
src/window.cpp (+244/-540)
src/window/geometry/include/core/windowgeometry.h (+6/-0)
src/window/geometry/tests/window-geometry/src/test-window-geometry.cpp (+10/-0)
src/windowgeometry.cpp (+31/-38)
To merge this branch: bzr merge lp:~smspillaz/compiz-core/compiz-core.work_923683
Reviewer Review Type Date Requested Status
Daniel van Vugt 2012-04-05 Needs Fixing on 2012-04-11
Alan Griffiths 2012-04-05 Approve on 2012-04-10
Sam Spilsbury Pending
Review via email: mp+101010@code.launchpad.net

This proposal supersedes a proposal from 2012-04-04.

This proposal has been superseded by a proposal from 2012-04-11.

Description of the change

Resubmitted now that lp:~smspillaz/compiz-core/compiz-core.fix_969108.2 and lp:~smspillaz/compiz-core/compiz-core.fix_969101 are active

Note: you'll get funny behaviour around window edges unless you use lp:~smspillaz/compiz-snap-plugin/compiz-snap-plugin.work_923683 . The snap plugin was using the interfaces incorrectly.

Always use the asynchronous codepaths in core. This commit changes the following:

 * CompWindow::move is now just a wrapper around CompWindow::configureXWindow
 * CompWindow::configureXWindow will always call through to moveNotify
 * moveNotify is only ever called pre-request to the server in the case of
   managed windows (SubstructureRedirectMask guaruntees they will end up in
   the right place) and post ConfigureNotify for override redirect windows
 * resizeNotify is always called post ConfigureNotify regardless of whether
   or not the window is managed or not
 * composite and opengl now use the geometry last sent to the server in order
   to compute display matrices and window positions as well as damage regions
 * decor now also does the above
 * const correctness in include/core/window.h
 * removed priv->width and priv->height , which was just redundant data used for
   window shading and honestly, is a ticking time bomb for future maintainers.
 * CompWindow::syncPosition is now deprecated and a no-op
 * Removed the majority of PrivateWindow::updateFrameWindow - this function had a lot
   of redundant code which PrivateWindow::reconfigureXWindow had anyways - the big work
   there was to send synthetic ConfigureNotify events to ourselves to notify about windows
   moving within frame windows that don't move themselves. Its safer to use reconfigureXWindow
   in this case and probably more sane too. We should look into removing the other update* functions.

Caveats:

 0) There are no tests yet
 1) Window movement performance will regress with this branch. This is *probably* due to the fact that for every slight movement, we send a full ConfigureWindow request to the X Server and it has to post back to us a full ConfigureNotify, for pretty much every pixel the window moves. In the case that we're not doing compositing thats fine, but in every other case its *super* chatty and largely unnecessary. This can probably be optimized a lot, but that will require work to gather up requests that we want to send to the X server - combine them (eg, merging the stack of requests based on the change mask and using the latest values where appropriate). Thats a lot of work and probably not appropriate in this branch
 2) The diff is a little big
 3) No tests
 4) Window shading is more than likely going to break again, mostly because I haven't tested it yet, but also because shading does some strange things and manipulated geometry values in strange ways
 5) No tests

Testing plan?

I would imagine here that the PendingConfigureRequests object can probably be split out into something testable, as can the logic to strip flags from frame window geometry and client window geometry updates.

To post a comment you must log in.
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal
Download full text (10.3 KiB)

The diff is big and scary, but its not too complicated. I'll explain each separate part

1 === modified file 'include/core/window.h'
2 --- include/core/window.h 2012-01-20 15:20:44 +0000
3 +++ include/core/window.h 2012-01-31 17:12:59 +0000
4 @@ -398,9 +398,9 @@
5
6 bool hasUnmapReference ();
7
8 - bool resize (XWindowAttributes);
9 + bool resize (const XWindowAttributes &);
10
11 - bool resize (Geometry);
12 + bool resize (const Geometry &);
13
14 bool resize (int x, int y, int width, int height,
15 int border = 0);

const correctness

31 int x1, x2, y1, y2;
32
33 - CompWindow::Geometry geom = priv->window->geometry ();
34 - CompWindowExtents output = priv->window->output ();
35 + const CompWindow::Geometry &geom = priv->window->serverGeometry ();
36 + const CompWindowExtents &output = priv->window->output ();

more const correctness

35 + const CompWindow::Geometry &geom = priv->window->serverGeometry ();
36 + const CompWindowExtents &output = priv->window->output ();

Lots of this - in paint code we want to use serverFoo because it was the last thing sent to the server. And this is always guarunteed to come back to us.

166 - unsigned int width = window->size ().width ();
167 - unsigned int height = window->size ().height ();
168 + unsigned int width = window->geometry ().width ();
169 + unsigned int height = window->geometry ().height ();

CompWindow::size was just returning CompSize (priv->width, priv->height) which was just redundant data which should be removed. Replaced it with geometry () for clarity

299 + void move (int dx, int dy, bool sync);
300 + bool resize (int dx, int dy, int dwidth, int dheight, int dborder);
301 + bool resize (const CompWindow::Geometry &g);
302 + bool resize (const XWindowAttributes &attrib);
303 +

Added PrivateWindow::move and PrivateWindow::resize - to be called whenever a new position / size comes back from the server. (cut'n'paste code from CompWindow::move and CompWindow::resize

335 -
336 - gettimeofday (&lastConfigureRequest, NULL);
337 - /* Flush any changes made to serverFrameGeometry or serverGeometry to the server
338 - * since there is a race condition where geometries will go out-of-sync with
339 - * window movement */
340 -
341 - window->syncPosition ();
342 -
343 - if (serverInput.left || serverInput.right || serverInput.top || serverInput.bottom)
344 - {
345 - int bw = serverGeometry.border () * 2;
346 -
347 - xwc.x = serverGeometry.x () - serverInput.left;
348 - xwc.y = serverGeometry.y () - serverInput.top;
349 - xwc.width = serverGeometry.width () + serverInput.left + serverInput.right + bw;
350 - if (shaded)
351 - xwc.height = serverInput.top + serverInput.bottom + bw;
352 - else
353 - xwc.height = serverGeometry.height () + serverInput.top + serverInput.bottom + bw;
354 -
355 - if (shaded)
356 - height = serverInput.top + serverInput.bottom;
357 -
358 - if (serverFrameGeometry.x () == xwc.x)
359 - valueMask &= ~(CWX);
360 - else
361 - serverFrameGeometry.setX (xwc.x);
362 -
*snip*
616 - XMoveResizeWindow (screen->dpy (), id, 0, 0,
617 - serverGeometry.width (), serverGeometry.height ());
618 - window->sendConfigureNotify ();
619 - window->windowNotify (CompWindowNotifyFrameUpdate...

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

824 -
825 - if (!pendingConfigures.pending ())
826 - {
827 - /* Tell plugins its ok to start doing stupid things again but
828 - * obviously FIXME */
829 - CompOption::Vector options;
830 - CompOption::Value v;
831 -
832 - options.push_back (CompOption ("window", CompOption::TypeInt));
833 - v.set ((int) id);
834 - options.back ().set (v);
835 - options.push_back (CompOption ("active", CompOption::TypeInt));
836 - v.set ((int) 0);
837 - options.back ().set (v);
838 -
839 - /* Notify other plugins that it is unsafe to change geometry or serverGeometry
840 - * FIXME: That API should not be accessible to plugins, this is a hack to avoid
841 - * breaking ABI */
842 -
843 - screen->handleCompizEvent ("core", "lock_position", options);
844 - }

DIE

924 -bool
925 -PrivateWindow::checkClear ()
926 -{
927 - if (pendingConfigures.pending ())
928 - {
929 - /* FIXME: This is a hack to avoid performance regressions
930 - * and must be removed in 0.9.6 */
931 - compLogMessage ("core", CompLogLevelWarn, "failed to receive ConfigureNotify event on 0x%x\n",
932 - id);
933 - pendingConfigures.dump ();
934 - pendingConfigures.clear ();
935 - }
936 -
937 - return false;
938 -}
939 -
940 void
941 compiz::X11::PendingEventQueue::add (PendingEvent::Ptr p)
942 {
943 @@ -2454,21 +2148,6 @@
944 mValueMask (valueMask),
945 mXwc (*xwc)
946 {
947 - CompOption::Vector options;
948 - CompOption::Value v;
949 -
950 - options.push_back (CompOption ("window", CompOption::TypeInt));
951 - v.set ((int) w);
952 - options.back ().set (v);
953 - options.push_back (CompOption ("active", CompOption::TypeInt));
954 - v.set ((int) 1);
955 - options.back ().set (v);
956 -
957 - /* Notify other plugins that it is unsafe to change geometry or serverGeometry
958 - * FIXME: That API should not be accessible to plugins, this is a hack to avoid
959 - * breaking ABI */
960 -
961 - screen->handleCompizEvent ("core", "lock_position", options);
962 }

DIE

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

GOOD IN THEORY:

* Most of the description sounds like steps in the right direction.

* Simplified: Replacing 559 lines of code with 247.

UNSURE ABOUT THE THEORY:

* "composite and opengl now use the geometry last sent to the server"
This should give maximum performance, sure, but what if (for some reason) the server rejects the configure request as being somehow invalid? Would you ever encounter that, like moving a window to an invalid location?
For stability and correctness (which should be the primary goal right now) don't we want "composite and opengl now use the geometry last received from the server"?

* Caveat #1 is not really a caveat. It's the right (intended) design. But if you want to see a (buggy) example of how to solve the chattiness look at the timer code here:
https://code.launchpad.net/~vanvugt/compiz-core/fix-764330-trunk/+merge/86497
This was slightly buggy though as it broke keyboatrd-initiated moves (incorrect pointer warp). Simple to fix, probably.

* Still no XFlush's to ensure XConfigureWindow happens immediately. Or are there adequate XSyncs already?

BAD IN THEORY:

* The diff is too big for a mere mortal (who hasn't worked on all the code before) to fully load into their mental interpreter and evaluate its theoretical correctness. Please read that sentence again; it's important.

GOOD IN PRACTICE:

* Window movement is smooth and predictable, but not really better than in oneiric.

BAD IN PRACTICE:

* The bug introduced is worse than the bug being fixed; Window contents offset from the frame/decorations whenever a window is created or maximized.

* All that code, but we still haven't reduced the lag below oneiric-levels. Still not as good as Gnome Shell.

CONCLUSION:

The theory is mostly good but there is at least one serious bug that needs fixing. We probably shouldn't risk 0.9.7.0 any more with large changes like this. We should aim to get this code into a later release when it's more stable, not 0.9.7.0.

review: Needs Fixing
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal
Download full text (3.2 KiB)

> GOOD IN THEORY:
>
> * Most of the description sounds like steps in the right direction.
>
> * Simplified: Replacing 559 lines of code with 247.
>
>
> UNSURE ABOUT THE THEORY:
>
> * "composite and opengl now use the geometry last sent to the server"
> This should give maximum performance, sure, but what if (for some reason) the
> server rejects the configure request as being somehow invalid? Would you ever
> encounter that, like moving a window to an invalid location?
> For stability and correctness (which should be the primary goal right now)
> don't we want "composite and opengl now use the geometry last received from
> the server"?

See http://tronche.com/gui/x/xlib/window/XConfigureWindow.html

Since we have SubstructureRedirectMask, it is guarunteed that any ConfigureWindow request we make on a child of the root window is guarunteed to end up in that position (hence the event tracker).

The only error cases I can think of is where you ask for a window to be of a size < 0 or > MAXSHORT. Ideally we should check for that case.

>
> * Caveat #1 is not really a caveat. It's the right (intended) design. But if
> you want to see a (buggy) example of how to solve the chattiness look at the
> timer code here:
> https://code.launchpad.net/~vanvugt/compiz-core/fix-764330-trunk/+merge/86497
> This was slightly buggy though as it broke keyboatrd-initiated moves
> (incorrect pointer warp). Simple to fix, probably.
>

There are some optimizations that can be done here - for example, if a window is grabbed, you can withold sending positions to the server until it is ungrabbed, or there is some other reason why you would want to do that. When that happens you can send it all at once.

> * Still no XFlush's to ensure XConfigureWindow happens immediately. Or are
> there adequate XSyncs already?

The GLib mainloop work introduced an XFlush as soon as we have finished dispatching the X event source.

>
>
> BAD IN THEORY:
>
> * The diff is too big for a mere mortal (who hasn't worked on all the code
> before) to fully load into their mental interpreter and evaluate its
> theoretical correctness. Please read that sentence again; it's important.

Indeed. The good thing is that its mostly all deletion and consolidation.

>
>
> GOOD IN PRACTICE:
>
> * Window movement is smooth and predictable, but not really better than in
> oneiric.
>
>
> BAD IN PRACTICE:
>
> * The bug introduced is worse than the bug being fixed; Window contents offset
> from the frame/decorations whenever a window is created or maximized.

Alright. I haven't seen this yet, but I have an idea of what might cause it. Will have a look into it when I get some time.

>
> * All that code, but we still haven't reduced the lag below oneiric-levels.
> Still not as good as Gnome Shell.

This will be fixed once the chattyness goes away. (as above). Watch your X.org CPU usage as you move windows - it skyrockets because at the moment we flood it with requests.

>
>
> CONCLUSION:
>
> The theory is mostly good but there is at least one serious bug that needs
> fixing. We probably shouldn't risk 0.9.7.0 any more with large changes like
> this. We should aim to get this code into a later r...

Read more...

Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

OK, I'm still wading through the changes. But I feel like a rant...

1307 CompSize
1308 CompWindow::size () const
1309 {
1310 - return CompSize (priv->width + priv->geometry.border () * 2,
1311 - priv->height + priv->geometry.border () * 2);
1312 + return CompSize (priv->geometry.width () + priv->geometry.border () * 2,
1313 + priv->geometry.height () + priv->geometry.border () * 2);
1314 }

As a change this doesn't look too bad - but it ignores a horrid design!

1. Chained operations like "priv->geometry.width ()" imply too muck knowledge of the details of "priv".
   That is "priv->width ()" would reflect less coupling.

2. You're tracing the path "priv->geometry..." many times, which suggests that the logic belongs in "geometry".
   "return priv->geometry.size ()" or "return CompSize(priv->geometry)" would reflect better coherence.

So, assuming (because borders may be optional?) that there's an unambiguous mapping from CompWindow::Geometry:

a. Add a constructor to CompSize: "explicit CompSize(CompWindow::Geometry const& g) : mWidth(g.width () + g.border () * 2) ..."
b. Add an inline method "CompSize PrivateWindow::size () const { return CompSize(priv->geometry); }"
c. Rewrite the above as "CompSize CompWindow::size () const { "return priv->size ()"; }"

Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

264 - /* been shaded */
265 + /* been shaded
266 if (w->priv->height == 0)
267 {
268 if (w->id () == priv->activeWindow)
269 w->moveInputFocusTo ();
270 - }
271 + }*/

Comments are not for storing old versions of the code - that's what we use bzr for. ;)

review: Needs Fixing
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

There's a lot of "<blah>.width () + <blah>.border () * 2" and "<blah>.height () + <blah>.border () * 2" around. Surely CompWindow::Geometry could have a "widthWithBorders" method - or maybe a free function?

I'm tempted by

template<Typename T>
inline int heightWithBorders(T const& blah) { return blah.height () + blah.border () * 2; }

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

> OK, I'm still wading through the changes. But I feel like a rant...
>
> 1307 CompSize
> 1308 CompWindow::size () const
> 1309 {
> 1310 - return CompSize (priv->width + priv->geometry.border () * 2,
> 1311 - priv->height + priv->geometry.border () * 2);
> 1312 + return CompSize (priv->geometry.width () + priv->geometry.border ()
> * 2,
> 1313 + priv->geometry.height () + priv->geometry.border () * 2);
> 1314 }
>
> As a change this doesn't look too bad - but it ignores a horrid design!
>
> 1. Chained operations like "priv->geometry.width ()" imply too muck knowledge
> of the details of "priv".
> That is "priv->width ()" would reflect less coupling.

Probably, although priv is just a private member with implementation details, I don't really see this as a large design problem.

>
> 2. You're tracing the path "priv->geometry..." many times, which suggests that
> the logic belongs in "geometry".
> "return priv->geometry.size ()" or "return CompSize(priv->geometry)" would
> reflect better coherence.
>
> So, assuming (because borders may be optional?) that there's an unambiguous
> mapping from CompWindow::Geometry:
>
> a. Add a constructor to CompSize: "explicit CompSize(CompWindow::Geometry
> const& g) : mWidth(g.width () + g.border () * 2) ..."
> b. Add an inline method "CompSize PrivateWindow::size () const { return
> CompSize(priv->geometry); }"
> c. Rewrite the above as "CompSize CompWindow::size () const { "return
> priv->size ()"; }"

+1 for all three

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

> 264 - /* been shaded */
> 265 + /* been shaded
> 266 if (w->priv->height == 0)
> 267 {
> 268 if (w->id () == priv->activeWindow)
> 269 w->moveInputFocusTo ();
> 270 - }
> 271 + }*/
>
> Comments are not for storing old versions of the code - that's what we use bzr
> for. ;)

Indeed, this is a small portion of the code and I'm not yet sure what to do with it. When I get around to revisiting this section (when I actually get time to look at this again, wanted to get it up for review early) I'll see what can be done.

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

> There's a lot of "<blah>.width () + <blah>.border () * 2" and "<blah>.height
> () + <blah>.border () * 2" around. Surely CompWindow::Geometry could have a
> "widthWithBorders" method - or maybe a free function?
>
> I'm tempted by
>
> template<Typename T>
> inline int heightWithBorders(T const& blah) { return blah.height () +
> blah.border () * 2; }

I'm not really sure ?

The complexity here comes from a legacy part of the X Server coming into play - a window could have fixed dimentions, but also specify a "border" which would be exclusive of the fixed dimentions, so you'd have to take this into account for all positioning operations (Xterm comes to mind here).

I definitely agree that geom.width () + geom.border () * 2 feels fragile and indeed, that has tripped me up many times before. Maybe a default parameter with a bitflag makes sense here, eg IncludeBorder , IncludeBorderFirst , IncludeBorderSecond (as it is on both sides)

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

I'll note that the above isn't *really* all that relevant to this merge though, but good things to keep in mind in any case. I'd rather not see the diff get any better except for adding tests.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Please resubmit for target branch lp:compiz-core (0.9.7)

review: Resubmit
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

A lot of the above is about making the code better (which the proposal doesn't attempt). However, in a previous version I pointed out that:

> 264 - /* been shaded */
> 265 + /* been shaded
> 266 if (w->priv->height == 0)
> 267 {
> 268 if (w->id () == priv->activeWindow)
> 269 w->moveInputFocusTo ();
> 270 - }
> 271 + }*/
>
> Comments are not for storing old versions of the code - that's what we use bzr for. ;)

I still think that needs fixing.

review: Needs Fixing
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

Alright, I've updated this so that there's no distorted windows on decoration size change. Was (ironically) a race condition.

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

For the sake of not making this diff any bigger, I'm not going to introduce unit tests here.

As for testing this I'd say the following is appropriate:

 * Add testcase for PendingConfigureEvent
 * Add testcase for rectsToRegion (serverGeometry should really be DI'd here)

Notes for the future:

 * As alan has said - the whole priv->geometry ().width () + priv->geometry ().border () * 2 is /really/ awkward and error prone, but we need it to support windows like xterm that still use this (stupid, legacy) attribute on their windows. I would say that the role of compiz::window::Geometry should thus be expanded somewhat
   - it should also encapsulate priv->region and really, geometry::x and friends should be made with reference to that. Ideally we'll store two regions, one with borders and one without. That sucks, since its a little memory hungry, but at least the cost is only really born whenever those regions need to be updated (::translate on a region with one rectangle is basically free, slightly more complicated on regions with multiple rects (eg, shaped window). The default should be to return the size /without/ borders, and have a default-parameter enum to get the size with borders.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Unfortunately still fails basic testing. The same bug remains;

* The bug introduced is worse than the bug being fixed; Window contents offset from the frame/decorations whenever a window is created or maximized.

review: Needs Fixing
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Also... I would have thought/hoped that a simplified/stable solution would eliminate "serverGeometry" completely.

        /**
        * Geometry retrieved from the
         * last ConfigureNotify event received
         */
        Geometry & geometry () const;

        /**
         * Geometry last sent to the server
         */
        Geometry & serverGeometry () const;

I understand why we have, and why we might want, serverGeometry. However it is an optimization which only makes sense to attempt if the code is stable and bug-free to begin with.

While serious bugs remain, I think the first goal should be to simplify the logic down to just using "geometry" and remove or stub "serverGeometry".

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

> Also... I would have thought/hoped that a simplified/stable solution would
> eliminate "serverGeometry" completely.
>
> /**
> * Geometry retrieved from the
> * last ConfigureNotify event received
> */
> Geometry & geometry () const;
>
> /**
> * Geometry last sent to the server
> */
> Geometry & serverGeometry () const;
>
> I understand why we have, and why we might want, serverGeometry. However it is
> an optimization which only makes sense to attempt if the code is stable and
> bug-free to begin with.
>
> While serious bugs remain, I think the first goal should be to simplify the
> logic down to just using "geometry" and remove or stub "serverGeometry".

There are some usescases for when we need to know what the last /received/ geometry from the server is. That's why the whole geometry/serverGeometry thing exists.

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

Also, I'm not seeing this offset problem - could you give me some steps to reproduce it ?

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

> > Also... I would have thought/hoped that a simplified/stable solution would
> > eliminate "serverGeometry" completely.
> >
> > /**
> > * Geometry retrieved from the
> > * last ConfigureNotify event received
> > */
> > Geometry & geometry () const;
> >
> > /**
> > * Geometry last sent to the server
> > */
> > Geometry & serverGeometry () const;
> >
> > I understand why we have, and why we might want, serverGeometry. However it
> is
> > an optimization which only makes sense to attempt if the code is stable and
> > bug-free to begin with.
> >
> > While serious bugs remain, I think the first goal should be to simplify the
> > logic down to just using "geometry" and remove or stub "serverGeometry".
>
>
> There are some usescases for when we need to know what the last /received/
> geometry from the server is. That's why the whole geometry/serverGeometry
> thing exists.

Hmm, we could make it so that serverGeometry is returned for managed windows and geometry is returned for override redirect windows on the public API. That of course means that we have to break the public API and update all the plugins.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

The practical problem:

Just start compiz --replace composite opengl move resize decor
and gtk-window-decorator --replace
Then every window will have its contents horizontally shifted around 15 pixels from the correct location relative to its decorations. The shift occurs whenever a window is created, maximized or restored. The shift goes away (corrects itself) when you start moving the window. On a positive note, the lag *appears* totally fixed and dragging windows is performing as well as Gnome Shell now.

The theoretical problem:

You can ignore my comments about geometry/serverGeometry for now. They are insignificant compared to the above bug. And besides, the use of both geometry and serverGeometry now appears to be yielding the desired reduction in lag.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Screenshot of the bug introduced by this branch:
https://launchpadlibrarian.net/92172452/shift.png

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

Alright, I'll have another look into it ? (Not seeing it here :( )

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

And it gets worse:
* The usual lag has come back. I have no idea how or why it was fixed when I ran it the first time.
* When dragging windows around, the area of desktop recently exposed flashes white.

So there is no improvement in performance and 2 serious regressions introduced :(

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

    //XSynchronize (dpy, TRUE);

:) Haven't merged your other branch

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Yikes. That would be another regression to be missing:
src/screen.cpp: XSynchronize (dpy, synchronousX ? True : False);

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Perhaps remember to pull from trunk more often.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

where trunk == lp:compiz-core

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Why doesn't the removal of XSynchronize show up in the below diff? It's obvious if I merge the branch myself. Maybe LaunchPad needs a kick to update the below diff?

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

I think the horizontal shift bug is coming from upstream lp:compiz-core, and is exposed moreso by this branch. I can reproduce the same kind of horizontal shift jitter using just lp:compiz-core and resizing a window rapidly.

Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

> The default should be to return the size /without/ borders,
> and have a default-parameter enum to get the size with borders.

There are very few designs where default parameters are better than multiple functions. This isn't one.

auto sizeWithoutBorders = geometry.size();
auto sizeWithBorders = geometry.sizeWithBorders(); // not geometry.size(::compiz::geometry::WithBorders);

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

OK, the horizontal shifting bug does not seem to be caused by this branch. Just made worse by this branch.

Sam, please review that nasty issue first: bug 928173.

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

This branch will incidentally help with a lot of the resizing issues, so I think it should be merged first.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

I would prefer not to approve this branch so long as the offset/shift bug is as bad as it is. If absolutely necessary, we could release without a fix for bug 928173 because it is hidden by the default Ubuntu config. But introducing this branch makes the bug unacceptably worse, as the screenshot showed.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

To clarify, "unacceptably worse" means that the bug will no longer be hidden in Ubuntu and will occur on every new window that opens. At least on my machine (and presumably others).

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

.... I'm still not even seeing this issue.

I can look into resizing tonight if you really think that this is a blocker.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

I think I may have confused the situation by suggesting the regression introduced by this branch is bug 928173. The symptoms are actually slightly different. It's only a theory that it's the same bug, because the size of the horizontal offset looks similar.

The bug I see with this branch would be worthy of a new bug report (a critical regression) that I don't want to log. And we won't have to log that bug so long as it's fixed before this branch is merged.

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

On Wed, 8 Feb 2012, Daniel van Vugt wrote:

> I think I may have confused the situation by suggesting the regression introduced by this branch is bug 928173. The symptoms are actually slightly different. It's only a theory that it's the same bug, because the size of the horizontal offset looks similar.
>
> The bug I see with this branch would be worthy of a new bug report (a critical regression) that I don't want to log. And we won't have to log that bug so long as it's fixed before this branch is merged.
> --
> https://code.launchpad.net/~smspillaz/compiz-core/compiz-core.work_923683/+merge/91654
> You are the owner of lp:~smspillaz/compiz-core/compiz-core.work_923683.
>

Alright.

I am looking into the decor plugin for a way to resolve this.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Despite the success of the fix for #928173: lp:~smspillaz/compiz-core/compiz-core.decor_928173
the offset bug introduced by this branch persists.

This confirms that the horizontal offset/shift problems introduced (or exposed) by this branch are certainly not the same as bug 928173.

review: Needs Fixing
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

I still can't even observe this problem that you're referring to :(

Can you specify, exactly what windows this occurs on and /exact/ steps to reproduce it ?

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Steps to reproduce the issue:

1. Start with an exact clone of lp:compiz-core
2. Merge in lp:~smspillaz/compiz-core/compiz-core.work_923683
3. Build and install it.
4. Run compiz --replace composite opengl move resize decor
5. Run gtk-window-decorator --replace

Now every window will be corrupted. Not just the existing ones, but any new ones you open too. The corruption persists during window resizing, but vanishes as soon as you move a window.

Here is a new screenshot:
https://launchpadlibrarian.net/92390683/shifted2.png

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

Is the window itself shifted (eg are the input regions correctly lined up) or is it just the image that is shifted ?

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

The window itself is shifted. The input regions correctly line up with the image still.

However, it looks like the damage events are not shifted. This causes some redraw problems.

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

Can you post the xwininfo -all of the window, xwininfo -all -id of the parent and xwininfo -all -parent of the parent of that ?

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Attachments sent via email. This is interesting though;

$ diff correct.xwininfo shifted.xwininfo
11c11
< 0x2c03ca7 (has no name): () 1x1+-1+-1 +45+69
---
> 0x2c03ca7 (has no name): () 1x1+-1+-1 +34+69
13c13
< Absolute upper-left X: 46
---
> Absolute upper-left X: 35
31,32c31,32
< Corners: +46+70 -1232+70 -1232-720 +46-720
< -geometry 80x24+35+41
---
> Corners: +35+70 -1243+70 -1243-720 +35-720
> -geometry 80x24+34+41

It looks like geometry/serverGeometry might be out of sync.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

And why do I have so many 1x1 pixel windows?! I swear occasionally I see them on screen too.

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

On Sun, 12 Feb 2012, Daniel van Vugt wrote:

> And why do I have so many 1x1 pixel windows?!

Applications use them as a means of doing IPC. Gtk+ is a serial offender
here.

> I swear occasionally I see them on screen too.

Under what circumstances?

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

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

On Sun, 12 Feb 2012, Daniel van Vugt wrote:

The frame window is in the wrong position. This means that serverGeometry
isn't the last thing sent to the server. serverGeometry is still writable
in this branch, so it could be $evilplugin doing the wrong thing here.

I don't want to spend any more time on this until wednesday. So I'll pick
it up then.

(compiz run with --debug will have some logs which can confirm this btw)

Incidentally, you don't see any of those "unmatched ConfigureNotify"
warnings do you? They're all gone here but they indicate the first sign of
trouble.

> Attachments sent via email. This is interesting though;
>
> $ diff correct.xwininfo shifted.xwininfo
> 11c11
> < 0x2c03ca7 (has no name): () 1x1+-1+-1 +45+69
> ---
>> 0x2c03ca7 (has no name): () 1x1+-1+-1 +34+69
> 13c13
> < Absolute upper-left X: 46
> ---
>> Absolute upper-left X: 35
> 31,32c31,32
> < Corners: +46+70 -1232+70 -1232-720 +46-720
> < -geometry 80x24+35+41
> ---
>> Corners: +35+70 -1243+70 -1243-720 +35-720
>> -geometry 80x24+34+41
>
> It looks like geometry/serverGeometry might be out of sync.
> --
> https://code.launchpad.net/~smspillaz/compiz-core/compiz-core.work_923683/+merge/91654
> You are the owner of lp:~smspillaz/compiz-core/compiz-core.work_923683.
>

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

It's random and very rare. But sometimes I see 1-pixel windows (with shadow). Can't ever seem to get xwininfo for them. Sometimes I also get very large white anonymous windows blocking my view. But it's all very hard to reproduce. And off-topic.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Correct. I did stop getting "Warn: failed to receive ConfigureNotify event on ..." when using this branch.

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

Well, you should stop getting "failed to receive" since I've removed the timeout which checks for unmatched events :) (A necessary hack coming up to the oneiric release).

I'm more interested in warnings that say "unmatched ConfigureNotify"

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

Could you have a look again to see if the issue is still occurring for you ? I've just re-synced with trunk, so it may be fixed.

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

OK, I found the issue. Its actually a bug that's been in the code for quite some time which would cause the client to not move within the frame when the frame was updated until the client position itself was updated.

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

There's still one more issue here though to do with override redirect windows not getting their paint regions updated, so that needs to be looked into as well. And tests.

review: Needs Fixing
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

^ Those fix the offset issue confirmed here (was finally able to reproduce it with qtcreator)

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

1. Confirmed the shifted window regression is now fixed. Seems to work without any obvious bugs now.

2. I'm not entirely sure about replacing geometry with serverGeometry. I thought it would be a good idea to do the opposite. There would be a slight performance penalty, but using just "geometry" would guarantee that compiz always agrees with the actual server geometry, instead of guessing/assuming that serverGeometry is accepted by the server (which does not always seem to be true, hence bug 886192).

3. There are two instances of double semicolons ";;" in this proposal.

4. I suspect this proposal will conflict just slightly with the proposal for bug 940139. But it should be minor.

5. NEW REGRESSION:
375 - XSynchronize (dpy, synchronousX ? True : False);
376 +// priv->connection = XGetXCBConnection (priv->dpy);

6. Why always two spaces before "* 2" ?

review: Needs Fixing
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

> 2. I'm not entirely sure about replacing geometry with serverGeometry. I
> thought it would be a good idea to do the opposite. There would be a slight
> performance penalty, but using just "geometry" would guarantee that compiz
> always agrees with the actual server geometry, instead of guessing/assuming
> that serverGeometry is accepted by the server (which does not always seem to
> be true, hence bug 886192).
>

serverGeometry will always be "accepted" by the server as long as the window is managed and the requested window geometry will not generate a BadValue error (eg, 0 < 1 < MAXINT) (eg, it is not override redirect and it is a child of a window for which we have a SubstructureRedirectMask.

Bug 886192 is not an example of this. In fact, the behaviour exhibited by bug 886192 is primarily /because/ we use the geometry last received from the server rather than geometry last sent, and the latency of which explains the reason why the window movement lags the cursor, because the server is "catching up".

The only case where you can't guaruntee that a similar update for geometry is going to be delivered by the server for serverGeometry as stored on XConfigureWindow is either in the case that A) Another client has SubstructureRedirectMask on the root window or a parent window owned by compiz and in that case compiz shouldn't even touch that window at all or B) override redirect windows, and as you can see in the code, we /always/ use geometry for override redirect windows in placement sensitive operations. Incidentally, override redirect windows are why getting window stacking right is such a nightmare, but thats a topic for another day.

> 3. There are two instances of double semicolons ";;" in this proposal.

Fix
>
> 4. I suspect this proposal will conflict just slightly with the proposal for
> bug 940139. But it should be minor.

Yes, fixable

>
> 5. NEW REGRESSION:
> 375 - XSynchronize (dpy, synchronousX ? True : False);
> 376 +// priv->connection = XGetXCBConnection (priv->dpy);

Please elaborate further on why this is a regression.

>
> 6. Why always two spaces before "* 2" ?

Most likely copy-and-paste errors

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

#5 is a regression because it will make "--sync" be ignored. Line 375 is important and should not be deleted.

review: Needs Fixing
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

On Mon, 27 Feb 2012, Daniel van Vugt wrote:

> Review: Needs Fixing
>
> #5 is a regression because it will make "--sync" be ignored. Line 375 is important and should not be deleted.

Thank you. Fixing.

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

Mikkel Kamstrup Erlandsen (kamstrup) wrote : Posted in a previous version of this proposal

> > #5 is a regression because it will make "--sync" be ignored. Line 375 is
> important and should not be deleted.
>
> Thank you. Fixing.

Be sure to add a comment there explaining why that call needs to be there - this review demonstrates that it should have been there in the first place :-)

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Comments are good. Though the string "synchronousX" is unique so its use should be quite clear already without a comment.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Looks like a mistake:

710 - if (x2 > priv->width)
711 - x2 = priv->width;
712 - if (y2 > priv->height)
713 - y2 = priv->height;
714 + if (x2 > priv->serverGeometry.width ())
715 + x2 = priv->serverGeometry.height (); <====== width?
716 + if (y2 > priv->serverGeometry.width ()) <====== height?
717 + y2 = priv->serverGeometry.height ();

review: Needs Fixing
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Also, there seems to be a HUGE performance regression when moving windows with this new branch. Try running under callgrind. With lp:compiz-core it's nice and fast. However with this branch it is painfully slow moving windows.

About 75% of the time is spent in:
  775,164,942 /home/dan/bzr/compiz-core/sam/plugins/move/src/move.cpp:moveHandleMotionEvent(CompScreen*, int, int) [/home/dan/sam/lib/compiz/libmove.so]
  772,734,309 /home/dan/bzr/compiz-core/sam/src/window.cpp:CompWindow::configureXWindow(unsigned int, XWindowChanges*) [/home/dan/sam/lib/libcompiz_core.so.0.9.7.0]
  771,062,144 /home/dan/bzr/compiz-core/sam/src/window.cpp:CompWindow::move(int, int, bool) [/home/dan/sam/lib/libcompiz_core.so.0.9.7.0]
  636,443,803 /home/dan/bzr/compiz-core/sam/src/window.cpp:CompWindow::moveNotify(int, int, bool) [/home/dan/sam/lib/libcompiz_core.so.0.9.7.0]
  636,266,731 /home/dan/bzr/compiz-core/sam/plugins/decor/src/decor.cpp:DecorWindow::moveNotify(int, int, bool) [/home/dan/sam/lib/compiz/libdecor.so]

review: Needs Fixing
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

The main paths causing this massive performance regression are:

70% of the time:
CompWindow::move
DecorWindow::moveNotify
DecorWindow::computeShadowRegion

18% of the time:
CompWindow::move
CompWindow::configureXWindow
PrivateWindow::reconfigureXWindow

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

Hmm, I would imagine this is the throttling of motion events that we mentioned earlier. I think in lp:compiz-core we would have just spent all that time in CompWindow::move although I'll conceed that configureXWindow is a bit more of an ... involved function (although I haven't noticed any *noticable* performance problems).

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

I also recommend "kcachegrind" to display callgrind output in a pretty graphical way.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Try using valgrind/callgrind to simulate a slow machine. You will see a monumental difference in performance when dragging windows. Even my i7 (running callgrind) can't keep up with dragging windows using this branch. But it is almost perfectly fluid without this branch.

Maybe it is the fact that all motion events are getting though now. But regardless, people with slow-to-regular machines will be noticeably impacted. We need a fix before accepting this.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

From what I can tell, the problem is that moveNotify is called much more often with this branch. However moveNotify is far too expensive to call so often, mostly due to DecorWindow::moveNotify.

Does DecorWindow::moveNotify really need to computeShadowRegion on EVERY window whenever a single window moves?

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

One small caveat is that I haven't merged lp:compiz-core into this one today, which includes the region fixes.

> Does DecorWindow::moveNotify really need to computeShadowRegion on EVERY window whenever a single window moves?

Yes it does. There are some optimizations we can do there to make it only compute regions on certain windows. I've done them before, so its not too hard. Not for this branch.

> From what I can tell, the problem is that moveNotify is called much more often with this branch. However moveNotify is far too expensive to call so often, mostly due to DecorWindow::moveNotify.

So I think there are three optimizations which can be applied here, all of which are quite involved, so I'll need to put them in separate branches.

Firstly, we need to throttle motion events, so they aren't dispatched spuriously. That's easy enough to do.

The second which is a bit more complicated is to also throttle dispatch of serverGeometry updates / moveNotify / XConfigureWindow as well. There are some plugins which need immediate move notify updates but probably not immediate move notify updates 1000 times a second.

Another optimization is to only call XConfigureWindow when we actually need to update the server side position and not necessarily all the time. This does mean that serverGeometry will be "ahead" of whats being sent to the server, but that's OK since we can just flush the changes on demand (and besides, its meant to be ahead, which is why its there).

Incidentally, I'm not able to get such high readings in callgrind for window movement, but I guess this is a case-by-case thing.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Window movement accounts for all my CPU because that's all I am testing since I noticed the problem;
  Start compiz
  Move windows around lots
  Stop compiz

With this branch I see movement account for 60-80% of compiz' time and is incredibly slow and stuttering. Without this branch, it only uses maximum 20% of compiz' time and is visually much smoother.

Daniel van Vugt (vanvugt) : Posted in a previous version of this proposal
review: Needs Fixing
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

I merged lp:compiz-core from today. Could you give it another try and let me know how it goes ?

Also some thoughts on those optimizations would be nice :)

Currently I'm testing using the entire unity stack, but I can reduce it down to just compiz and a basic set of plugins.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

I was already testing this branch merged with the latest lp:compiz-core (including yesterday's optimizations) all along. Which makes the regression from this branch more worrying.

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

tOn Tue, 28 Feb 2012, Daniel van Vugt wrote:

> I was already testing this branch merged with the latest lp:compiz-core (including yesterday's optimizations) all along. Which makes the regression from this branch more worrying.
> --
> https://code.launchpad.net/~smspillaz/compiz-core/compiz-core.work_923683/+merge/94923
> You are the owner of lp:~smspillaz/compiz-core/compiz-core.work_923683.
>

OK, well, I know there are some code paths which can be slow, so lets look
at optimizing. Do you have any thoughts about the ideas I raised ?

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

I don't have the encyclopaedic knowledge of CompWindow or the time to research your suggestions right now. So no comment.

But I'm happy to test/profile any further additions to this proposal... tomorrow.

(Daniel logs off)

review: Needs Fixing
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

I don't see anything obviously wrong, but would like to hear the results of Daniel's profiling.

review: Abstain
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Tested revision 2994. Moving windows is still so slow it's unusable (under valgrind). We should fix this because it's a good indication that the same would occur in the wild on a slow machine.

compiz is spending 70% of it's CPU in or below moveHandleMotionEvent. The two main CPU hogs are:

(73% of the 70%)
moveHandleMotionEvent
CompWindow::move
DecorWindow::moveNotify
DecorWindow::computeShadowRegion

(16% of the 70%)
moveHandleMotionEvent
CompWindow::move
CompWindow::configureXWindow
PrivateWindow::reconfigureXWindow

review: Needs Fixing
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

And no, combining this branch with lp:~smspillaz/compiz-core/compiz-core.optimization-inlining
does not solve or change the performance problem.

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

Indeed, that's not meant to completely fix it, but it does fix some hotspots and other inefficiencies I observed :)

Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

I'm concerned that the code still looks "fragile" (to use Sam's word). For example:

50 + y1 = geom.height () + geom.border ();
...
59 + y2 = geom.height () - geom.border ();

It isn't explicit why border is added in one case and subtracted in the other. I suspect that there's an abstraction missing (or at least suitably named functions that derive the appropriate result from geom). Something like "internalHeight = internalHeight(geom)" is much easier to follow.

I've not tested the latest version enough to be confident in it. Will aim to do so on Thursday.

Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

> I'm concerned that the code still looks "fragile" (to use Sam's word). For
> example:
>
> 50 + y1 = geom.height () + geom.border ();
> ...
> 59 + y2 = geom.height () - geom.border ();
>
> It isn't explicit why border is added in one case and subtracted in the other.
> I suspect that there's an abstraction missing (or at least suitably named
> functions that derive the appropriate result from geom). Something like
> "internalHeight = internalHeight(geom)" is much easier to follow.
>
> I've not tested the latest version enough to be confident in it. Will aim to
> do so on Thursday.

Ack, I'll add something like that to compiz::window::Geometry

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Tested this branch by itself and merged with lp:~smspillaz/compiz-core/compiz-core.fix_969108.2

By itself, this branch still makes window movement unacceptably slow. With the other branch designed to make movement more efficient, windows now often don't move at all when dragged.

Unfortunately performance is getting worse, not better.

review: Needs Fixing
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

Its probably a small typo during refactoring. Please don't panic

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

I got this branch performing somewhat better by combining it with:
https://code.launchpad.net/~vanvugt/compiz-core/lastMotionTime/+merge/100742

However I would say still noticeably slower than lp:compiz-core. :(

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Confirmed the performance problems with this branch do seem to be fixed by:
https://code.launchpad.net/~smspillaz/compiz-core/compiz-core.fix_969101/+merge/100270

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Please resubmit with prerequisite:
lp:~smspillaz/compiz-core/compiz-core.fix_969101

review: Resubmit
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

This branch does appear to fix bug 862430.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

I was so close to approving and merging this today. But in final testing I noticed it triggers window border/shadow artefacts when resizing. They are easiest to see in Normal resize mode, but still happen much less noticeably in Rectangle resize mode too.

Just put a Nautilus window in front of a large white window and resize the Nautilus window rapidly from its top-right corner. Ugly artefacts will be left on the window behind.

I actually suspect the resize plugin being to blame for this, because I have encountered similar bugs with it before. And read that the GLES branch seems to have too.

However right now, it's being triggered by this branch. So I can't approve it just yet.

Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

I can't reproduce the same resize artefacts with my experimental branch, only with this one.

Perhaps we didn't notice the artefacts before because they are the result of the latest compiz-core commit r3086 combining with this branch...

review: Needs Fixing
Sam Spilsbury (smspillaz) wrote :

Resubmitted again.

LP #923520 is also fixed in this branch. I'm not sure if the artefacts are fixed "for good" but I can't reproduce them anymore since fixing that one.

(I cannot remove that fix from this branch, since its actually dependent on the fact that we can update window regions before sending geometry to the server, which wouldn't be possible in core at the moment)

Sam Spilsbury (smspillaz) wrote :

> (I cannot remove that fix from this branch, since its actually dependent on
> the fact that we can update window regions before sending geometry to the
> server, which wouldn't be possible in core at the moment)

Clarifying ambiguous words: I can't separate out that fix.

Sam Spilsbury (smspillaz) wrote :

Typo : bug 932520

2999. By Sam Spilsbury on 2012-04-05

Merge lp:compiz-core

3000. By Sam Spilsbury on 2012-04-07

Fix typoo

Alan Griffiths (alan-griffiths) wrote :

The intent has always looked sensible. The latest code looks the tidiest. And I've been running it all morning without noticing anything untoward.

review: Approve
Alan Griffiths (alan-griffiths) wrote :

Probably a reason not to merge, but running with "wobbly windows", and dragging a window across others there are repaint issues on the "behind" windows that don't resolve until the moved window is released.

Alan Griffiths (alan-griffiths) wrote :

> Probably a reason not to merge, but running with "wobbly windows", and
> dragging a window across others there are repaint issues on the "behind"
> windows that don't resolve until the moved window is released.

That should be "Probably /not/ a reason not to merge"

Sam Spilsbury (smspillaz) wrote :

> Probably a reason not to merge, but running with "wobbly windows", and
> dragging a window across others there are repaint issues on the "behind"
> windows that don't resolve until the moved window is released.

I'll resolve this anyhow since it could impact other plugins.

Daniel van Vugt (vanvugt) wrote :

Please remove the ABI changes (include/core/window.h at least). I agree they're nice changes, but we can't change the ABI.

review: Needs Fixing
Daniel van Vugt (vanvugt) wrote :

Also, when I maximize and restore a window, it moves left by about 1 pixel each time. So I don't think we can declare this branch as having fixed bug 892012.

Daniel van Vugt (vanvugt) wrote :

And confirmed the regression with wobbly windows this branch introduces. Although not officially enabled by default, enough people do use wobbly windows for us to want to avoid introducing such a regression.

review: Needs Fixing
Daniel van Vugt (vanvugt) wrote :

Confirmed the flicker bug 862430 is fixed. So that's some good news.

3001. By Sam Spilsbury on 2012-04-11

Merge trunk

3002. By Sam Spilsbury on 2012-04-11

Remove abi breaks

3003. By Sam Spilsbury on 2012-04-11

Don't break abi

3004. By Sam Spilsbury on 2012-04-11

Use geometry last sent to server to calculate damage rects

3005. By Sam Spilsbury on 2012-04-11

Use server side rects for calculating clip regions

3006. By Sam Spilsbury on 2012-04-12

Use serverGeometry

3007. By Sam Spilsbury on 2012-04-12

Damage the screen when windows are resized

3008. By Sam Spilsbury on 2012-04-12

Style issue

3009. By Sam Spilsbury on 2012-04-14

Merged trunk and fixed a number of problems with resize synchronization

3010. By Sam Spilsbury on 2012-04-14

Send resize notification immediately for windows that aren't override redirect

3011. By Sam Spilsbury on 2012-04-14

Fix jumping around on resize because we were rebinding way too much at the wrong time.

Only rebind on paints.

3012. By Sam Spilsbury on 2012-04-14

Don't not update decorations when we're grabbed for resizing

3013. By Sam Spilsbury on 2012-04-14

Null check

3014. By Sam Spilsbury on 2012-04-15

resize: unconditionally finish the resize operation when releasing the button

3015. By Sam Spilsbury on 2012-04-16

Merge trunk

3016. By Sam Spilsbury on 2012-04-16

Don't send a moveNotify and resizeNotify if the window was moved and resized

3017. By Sam Spilsbury on 2012-04-16

Cleanup wobbly model update code

3018. By Sam Spilsbury on 2012-04-17

Ensure that override redirect windows get their regions set correctly.
Also they can't have input extents.

3019. By Sam Spilsbury on 2012-04-18

Force all override redirect windows to use geometry last received from
server. Force all managed windows to use geometry last sent.

The difference between geometry and server* is now moot and can be
removed in a future release

3020. By Sam Spilsbury on 2012-04-18

Clean some cruft

3021. By Sam Spilsbury on 2012-04-18

Update decoration matrix of shaded windows

3022. By Sam Spilsbury on 2012-04-18

Send a resizeNotify to indicate to plugins when map operations have completed

3023. By Sam Spilsbury on 2012-04-18

Merge lp:compiz-core

3024. By Sam Spilsbury on 2012-04-18

Don't notify plugins of unreparent in the right place

3025. By Sam Spilsbury on 2012-04-18

Merged lp:~vanvugt/compiz-core/compiz-core/fix-981703-properly

3026. By Sam Spilsbury on 2012-04-18

updateState is bitwise, so never clobber it

3027. By Sam Spilsbury on 2012-04-18

Base decision to send notifications to plugins on absolute position
changes

3028. By Sam Spilsbury on 2012-04-19

Don't set height = 0 if the window still has a pixmap

3029. By Sam Spilsbury on 2012-04-19

Remove some unnecessary cahgnes in the wobbly plugin

3030. By Sam Spilsbury on 2012-04-19

Merge lp:compiz-core

3031. By Sam Spilsbury on 2012-04-19

Now that geometry and serverGeometry both return the same thing, the changes
in client code don't make any sense to keep

3032. By Sam Spilsbury on 2012-04-20

Fix logic errors

3033. By Sam Spilsbury on 2012-04-20

!= 0 not necessary

3034. By Sam Spilsbury on 2012-04-23

Merge lp:compiz-core

Unmerged revisions

3034. By Sam Spilsbury on 2012-04-23

Merge lp:compiz-core

3033. By Sam Spilsbury on 2012-04-20

!= 0 not necessary

3032. By Sam Spilsbury on 2012-04-20

Fix logic errors

3031. By Sam Spilsbury on 2012-04-19

Now that geometry and serverGeometry both return the same thing, the changes
in client code don't make any sense to keep

3030. By Sam Spilsbury on 2012-04-19

Merge lp:compiz-core

3029. By Sam Spilsbury on 2012-04-19

Remove some unnecessary cahgnes in the wobbly plugin

3028. By Sam Spilsbury on 2012-04-19

Don't set height = 0 if the window still has a pixmap

3027. By Sam Spilsbury on 2012-04-18

Base decision to send notifications to plugins on absolute position
changes

3026. By Sam Spilsbury on 2012-04-18

updateState is bitwise, so never clobber it

3025. By Sam Spilsbury on 2012-04-18

Merged lp:~vanvugt/compiz-core/compiz-core/fix-981703-properly

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/core/window.h'
2--- include/core/window.h 2012-03-22 17:00:51 +0000
3+++ include/core/window.h 2012-04-07 03:14:19 +0000
4@@ -410,9 +410,9 @@
5
6 bool hasUnmapReference ();
7
8- bool resize (XWindowAttributes);
9+ bool resize (const XWindowAttributes &);
10
11- bool resize (Geometry);
12+ bool resize (const Geometry &);
13
14 bool resize (int x, int y, int width, int height,
15 int border = 0);
16
17=== modified file 'plugins/composite/src/window.cpp'
18--- plugins/composite/src/window.cpp 2012-03-22 17:00:51 +0000
19+++ plugins/composite/src/window.cpp 2012-04-07 03:14:19 +0000
20@@ -250,7 +250,7 @@
21
22 if (x2 > x1 && y2 > y1)
23 {
24- CompWindow::Geometry geom = priv->window->geometry ();
25+ const CompWindow::Geometry &geom = priv->window->serverGeometry ();
26
27 x1 += geom.x () + geom.border ();
28 y1 += geom.y () + geom.border ();
29@@ -272,20 +272,20 @@
30 {
31 int x1, x2, y1, y2;
32
33- CompWindow::Geometry geom = priv->window->geometry ();
34- CompWindowExtents output = priv->window->output ();
35+ const CompWindow::Geometry &geom = priv->window->serverGeometry ();
36+ const CompWindowExtents &output = priv->window->output ();
37
38 /* top */
39 x1 = -output.left - geom.border ();
40 y1 = -output.top - geom.border ();
41- x2 = priv->window->size ().width () + output.right - geom.border ();
42+ x2 = geom.width () + geom.border () + output.right;
43 y2 = -geom.border ();
44
45 if (x1 < x2 && y1 < y2)
46 addDamageRect (CompRect (x1, y1, x2 - x1, y2 - y1));
47
48 /* bottom */
49- y1 = priv->window->size ().height () - geom.border ();
50+ y1 = geom.height () + geom.border ();
51 y2 = y1 + output.bottom - geom.border ();
52
53 if (x1 < x2 && y1 < y2)
54@@ -295,13 +295,13 @@
55 x1 = -output.left - geom.border ();
56 y1 = -geom.border ();
57 x2 = -geom.border ();
58- y2 = priv->window->size ().height () - geom.border ();
59+ y2 = geom.height () - geom.border ();
60
61 if (x1 < x2 && y1 < y2)
62 addDamageRect (CompRect (x1, y1, x2 - x1, y2 - y1));
63
64 /* right */
65- x1 = priv->window->size ().width () - geom.border ();
66+ x1 = geom.width () - geom.border ();
67 x2 = x1 + output.right - geom.border ();
68
69 if (x1 < x2 && y1 < y2)
70@@ -322,7 +322,7 @@
71 x = rect.x ();
72 y = rect.y ();
73
74- CompWindow::Geometry geom = priv->window->geometry ();
75+ const CompWindow::Geometry &geom = priv->window->serverGeometry ();
76 x += geom.x () + geom.border ();
77 y += geom.y () + geom.border ();
78
79@@ -341,16 +341,16 @@
80 if (priv->window->shaded () || force ||
81 (priv->window->isViewable ()))
82 {
83- int border = priv->window->geometry ().border ();
84+ int border = priv->window->serverGeometry ().border ();
85
86 int x1 = -MAX (priv->window->output ().left,
87 priv->window->input ().left) - border;
88 int y1 = -MAX (priv->window->output ().top,
89 priv->window->input ().top) - border;
90- int x2 = priv->window->size ().width () +
91+ int x2 = priv->window->serverGeometry ().width () +
92 MAX (priv->window->output ().right,
93 priv->window->input ().right) ;
94- int y2 = priv->window->size ().height () +
95+ int y2 = priv->window->serverGeometry ().height () +
96 MAX (priv->window->output ().bottom,
97 priv->window->input ().bottom) ;
98 CompRect r (x1, y1, x2 - x1, y2 - y1);
99@@ -410,7 +410,7 @@
100
101 if (!w->damageRect (initial, CompRect (x, y, width, height)))
102 {
103- CompWindow::Geometry geom = w->priv->window->geometry ();
104+ CompWindow::Geometry geom = w->priv->window->serverGeometry ();
105
106 x += geom.x () + geom.border ();
107 y += geom.y () + geom.border ();
108@@ -622,14 +622,14 @@
109 {
110 int x, y, x1, x2, y1, y2;
111
112- x = window->geometry ().x ();
113- y = window->geometry ().y ();
114+ x = window->serverGeometry ().x ();
115+ y = window->serverGeometry ().y ();
116
117 x1 = x - window->output ().left - dx;
118 y1 = y - window->output ().top - dy;
119- x2 = x + window->size ().width () +
120+ x2 = x + window->serverGeometry ().width () +
121 window->output ().right - dx;
122- y2 = y + window->size ().height () +
123+ y2 = y + window->serverGeometry ().height () +
124 window->output ().bottom - dy;
125
126 cScreen->damageRegion (CompRegion (CompRect (x1, y1, x2 - x1, y2 - y1)));
127
128=== modified file 'plugins/decor/src/decor.cpp'
129--- plugins/decor/src/decor.cpp 2012-03-30 14:29:18 +0000
130+++ plugins/decor/src/decor.cpp 2012-04-07 03:14:19 +0000
131@@ -1040,8 +1040,8 @@
132 for (i = 0; i < wd->nQuad; i++)
133 {
134 int x, y;
135- unsigned int width = window->size ().width ();
136- unsigned int height = window->size ().height ();
137+ unsigned int width = window->geometry ().width ();
138+ unsigned int height = window->geometry ().height ();
139
140 if (window->shaded ())
141 height = 0;
142@@ -1050,8 +1050,8 @@
143 &x1, &y1, &x2, &y2, &sx, &sy);
144
145 /* Translate by x and y points of this window */
146- x = window->geometry ().x ();
147- y = window->geometry ().y ();
148+ x = window->serverGeometry ().x ();
149+ y = window->serverGeometry ().y ();
150
151 wd->quad[i].box.x1 = x1 + x;
152 wd->quad[i].box.y1 = y1 + y;
153@@ -1079,8 +1079,8 @@
154 bool
155 DecorWindow::checkSize (const Decoration::Ptr &decoration)
156 {
157- return (decoration->minWidth <= (int) window->size ().width () &&
158- decoration->minHeight <= (int) window->size ().height ());
159+ return (decoration->minWidth <= (int) window->geometry ().width () &&
160+ decoration->minHeight <= (int) window->geometry ().height ());
161 }
162
163 /*
164@@ -1682,7 +1682,6 @@
165 XRectangle rects[4];
166 int x, y, width, height;
167 CompWindow::Geometry server = window->serverGeometry ();
168- int bw = server.border () * 2;
169 CompWindowExtents input;
170 CompWindowExtents border;
171 Window parent;
172@@ -1709,8 +1708,8 @@
173
174 x = window->border ().left - border.left;
175 y = window->border ().top - border.top;
176- width = server.width () + input.left + input.right + bw;
177- height = server.height ()+ input.top + input.bottom + bw;
178+ width = server.widthIncBorders () + input.left + input.right;
179+ height = server.heightIncBorders ()+ input.top + input.bottom ;
180
181 /* Non switcher windows are rooted relative to the frame window of the client
182 * and switchers need to be offset by the window geometry of the client */
183@@ -1849,7 +1848,6 @@
184 XRectangle rects[4];
185 int x, y, width, height;
186 CompWindow::Geometry server = window->serverGeometry ();
187- int bw = server.border () * 2;
188 CompWindowExtents input;
189
190 /* Determine frame extents */
191@@ -1860,8 +1858,8 @@
192
193 x = window->input ().left - input.left;
194 y = window->input ().top - input.top;
195- width = server.width () + input.left + input.right + bw;
196- height = server.height ()+ input.top + input.bottom + bw;
197+ width = server.widthIncBorders () + input.left + input.right;
198+ height = server.heightIncBorders ()+ input.top + input.bottom;
199
200 if (window->shaded ())
201 height = input.top + input.bottom;
202@@ -2092,8 +2090,8 @@
203 {
204 int x, y;
205
206- x = window->geometry (). x ();
207- y = window->geometry (). y ();
208+ x = window->serverGeometry (). x ();
209+ y = window->serverGeometry (). y ();
210
211 region += frameRegion.translated (x - wd->decor->input.left,
212 y - wd->decor->input.top);
213@@ -2114,7 +2112,7 @@
214 void
215 DecorWindow::updateWindowRegions ()
216 {
217- const CompRect &input (window->inputRect ());
218+ const CompRect &input (window->serverInputRect ());
219
220 if (regions.size () != gWindow->textures ().size ())
221 regions.resize (gWindow->textures ().size ());
222
223=== modified file 'plugins/move/src/move.cpp'
224--- plugins/move/src/move.cpp 2012-02-16 05:31:28 +0000
225+++ plugins/move/src/move.cpp 2012-04-07 03:14:19 +0000
226@@ -138,8 +138,8 @@
227 {
228 int xRoot, yRoot;
229
230- xRoot = w->geometry ().x () + (w->size ().width () / 2);
231- yRoot = w->geometry ().y () + (w->size ().height () / 2);
232+ xRoot = w->serverGeometry ().x () + (w->size ().width () / 2);
233+ yRoot = w->serverGeometry ().y () + (w->size ().height () / 2);
234
235 s->warpPointer (xRoot - pointerX, yRoot - pointerY);
236 }
237@@ -169,8 +169,8 @@
238 if (ms->w)
239 {
240 if (state & CompAction::StateCancel)
241- ms->w->move (ms->savedX - ms->w->geometry ().x (),
242- ms->savedY - ms->w->geometry ().y (), false);
243+ ms->w->move (ms->savedX - ms->w->serverGeometry ().x (),
244+ ms->savedY - ms->w->serverGeometry ().y (), false);
245
246 ms->w->syncPosition ();
247
248@@ -314,12 +314,10 @@
249
250 w = ms->w;
251
252- wX = w->geometry ().x ();
253- wY = w->geometry ().y ();
254- wWidth = w->geometry ().width () +
255- w->geometry ().border () * 2;
256- wHeight = w->geometry ().height () +
257- w->geometry ().border () * 2;
258+ wX = w->serverGeometry ().x ();
259+ wY = w->serverGeometry ().y ();
260+ wWidth = w->serverGeometry ().widthIncBorders ();
261+ wHeight = w->serverGeometry ().heightIncBorders ();
262
263 ms->x += xRoot - lastPointerX;
264 ms->y += yRoot - lastPointerY;
265@@ -484,8 +482,8 @@
266
267 if (dx || dy)
268 {
269- w->move (wX + dx - w->geometry ().x (),
270- wY + dy - w->geometry ().y (), false);
271+ w->move (wX + dx - w->serverGeometry ().x (),
272+ wY + dy - w->serverGeometry ().y (), false);
273
274 if (!ms->optionGetLazyPositioning ())
275 w->syncPosition ();
276
277=== modified file 'plugins/opengl/src/window.cpp'
278--- plugins/opengl/src/window.cpp 2012-03-22 17:00:51 +0000
279+++ plugins/opengl/src/window.cpp 2012-04-07 03:14:19 +0000
280@@ -76,7 +76,7 @@
281 void
282 PrivateGLWindow::setWindowMatrix ()
283 {
284- CompRect input (window->inputRect ());
285+ CompRect input (window->serverInputRect ());
286
287 if (textures.size () != matrices.size ())
288 matrices.resize (textures.size ());
289@@ -344,7 +344,7 @@
290 void
291 PrivateGLWindow::updateWindowRegions ()
292 {
293- CompRect input (window->inputRect ());
294+ CompRect input (window->serverInputRect ());
295
296 if (regions.size () != textures.size ())
297 regions.resize (textures.size ());
298
299=== modified file 'plugins/place/src/constrain-to-workarea/src/constrain-to-workarea.cpp'
300--- plugins/place/src/constrain-to-workarea/src/constrain-to-workarea.cpp 2012-01-20 06:13:07 +0000
301+++ plugins/place/src/constrain-to-workarea/src/constrain-to-workarea.cpp 2012-04-07 03:14:19 +0000
302@@ -61,13 +61,11 @@
303 }
304
305 left = x - border.left;
306- right = left + g.width () + (border.left +
307- border.right +
308- 2 * g.border ());
309+ right = left + g.widthIncBorders () + (border.left +
310+ border.right);
311 top = y - border.top;
312- bottom = top + g.height () + (border.top +
313- border.bottom +
314- 2 * g.border ());
315+ bottom = top + g.heightIncBorders () + (border.top +
316+ border.bottom);
317
318 if ((right - left) > workArea.width ())
319 {
320
321=== modified file 'plugins/place/src/place.cpp'
322--- plugins/place/src/place.cpp 2012-02-02 17:01:15 +0000
323+++ plugins/place/src/place.cpp 2012-04-07 03:14:19 +0000
324@@ -343,37 +343,36 @@
325 CompWindow::Geometry geom;
326 int output;
327
328+ geom.set (xwc->x, xwc->y, xwc->width, xwc->height,
329+ window->serverGeometry ().border ());
330+
331 if (clampToViewport)
332 {
333 /* left, right, top, bottom target coordinates, clamed to viewport
334 * sizes as we don't need to validate movements to other viewports;
335 * we are only interested in inner-viewport movements */
336
337- x = xwc->x % screen->width ();
338- if ((x + xwc->width) < 0)
339+ x = geom.x () % screen->width ();
340+ if ((geom.x2 ()) < 0)
341 x += screen->width ();
342
343- y = xwc->y % screen->height ();
344- if ((y + xwc->height) < 0)
345+ y = geom.y () % screen->height ();
346+ if ((geom.y2 ()) < 0)
347 y += screen->height ();
348 }
349 else
350 {
351- x = xwc->x;
352- y = xwc->y;
353+ x = geom.x ();
354+ y = geom.y ();
355 }
356
357 left = x - window->border ().left;
358- right = left + xwc->width + (window->border ().left +
359- window->border ().right +
360- 2 * window->serverGeometry ().border ());
361+ right = left + geom.widthIncBorders () + (window->border ().left +
362+ window->border ().right);
363 top = y - window->border ().top;
364- bottom = top + xwc->height + (window->border ().top +
365- window->border ().bottom +
366- 2 * window->serverGeometry ().border ());
367+ bottom = top + geom.heightIncBorders () + (window->border ().top +
368+ window->border ().bottom);
369
370- geom.set (xwc->x, xwc->y, xwc->width, xwc->height,
371- window->serverGeometry ().border ());
372 output = screen->outputDeviceForGeometry (geom);
373 workArea = screen->getWorkareaForOutput (output);
374
375@@ -746,10 +745,8 @@
376 {
377 if (PlaceScreen::get (screen)->getPointerPosition (pos))
378 {
379- unsigned int dx = (window->serverGeometry ().width () / 2) -
380- window->serverGeometry ().border ();
381- unsigned int dy = (window->serverGeometry ().height () / 2) -
382- window->serverGeometry ().border ();
383+ unsigned int dx = (window->serverGeometry ().widthIncBorders () / 2);
384+ unsigned int dy = (window->serverGeometry ().heightIncBorders () / 2);
385 pos -= CompPoint (dx, dy);
386 }
387 else
388@@ -1200,14 +1197,12 @@
389
390 extents.left = pos.x () - window->border ().left;
391 extents.top = pos.y () - window->border ().top;
392- extents.right = extents.left + window->serverWidth () +
393+ extents.right = extents.left + window->serverGeometry ().heightIncBorders () +
394 (window->border ().left +
395- window->border ().right +
396- 2 * window->serverGeometry ().border ());
397- extents.bottom = extents.top + window->serverHeight () +
398+ window->border ().right);
399+ extents.bottom = extents.top + window->serverGeometry ().widthIncBorders () +
400 (window->border ().top +
401- window->border ().bottom +
402- 2 * window->serverGeometry ().border ());
403+ window->border ().bottom);
404
405 delta = workArea.right () - extents.right;
406 if (delta < 0)
407
408=== modified file 'plugins/resize/src/resize.cpp'
409--- plugins/resize/src/resize.cpp 2012-02-16 05:31:28 +0000
410+++ plugins/resize/src/resize.cpp 2012-04-07 03:14:19 +0000
411@@ -56,7 +56,7 @@
412 void
413 ResizeWindow::getStretchScale (BoxPtr pBox, float *xScale, float *yScale)
414 {
415- CompRect rect (window->borderRect ());
416+ CompRect rect (window->serverBorderRect ());
417
418 *xScale = (rect.width ()) ? (pBox->x2 - pBox->x1) /
419 (float) rect.width () : 1.0f;
420@@ -1467,10 +1467,10 @@
421 getPaintRectangle (&box);
422 damageRectangle (&box);
423
424- box.x1 = w->outputRect ().x ();
425- box.y1 = w->outputRect ().y ();
426- box.x2 = box.x1 + w->outputRect ().width ();
427- box.y2 = box.y1 + w->outputRect ().height ();
428+ box.x1 = w->serverOutputRect ().x ();
429+ box.y1 = w->serverOutputRect ().y ();
430+ box.x2 = box.x1 + w->serverOutputRect ().width ();
431+ box.y2 = box.y1 + w->serverOutputRect ().height ();
432
433 damageRectangle (&box);
434 }
435
436=== modified file 'plugins/wobbly/src/wobbly.cpp'
437--- plugins/wobbly/src/wobbly.cpp 2011-03-14 16:12:45 +0000
438+++ plugins/wobbly/src/wobbly.cpp 2012-04-07 03:14:19 +0000
439@@ -1251,7 +1251,7 @@
440 if (!model)
441 {
442 unsigned int edgeMask = 0;
443- CompRect outRect (window->outputRect ());
444+ CompRect outRect (window->serverOutputRect ());
445
446 if (window->type () & CompWindowTypeNormalMask)
447 edgeMask = WestEdgeMask | EastEdgeMask | NorthEdgeMask |
448@@ -1578,7 +1578,7 @@
449 }
450 }
451
452- CompRect outRect (window->outputRect ());
453+ CompRect outRect (window->serverOutputRect ());
454 wx = outRect.x ();
455 wy = outRect.y ();
456 width = outRect.width ();
457@@ -1772,7 +1772,7 @@
458
459 if (ww->isWobblyWin () && ww->ensureModel ())
460 {
461- CompRect outRect (w->outputRect ());
462+ CompRect outRect (w->serverOutputRect ());
463
464 ww->model->setMiddleAnchor (outRect.x (), outRect.y (),
465 outRect.width (), outRect.height ());
466@@ -1901,7 +1901,7 @@
467 switch (focusEffect) {
468 case WobblyOptions::FocusEffectShiver:
469 {
470- CompRect outRect (w->outputRect ());
471+ CompRect outRect (w->serverOutputRect ());
472
473 ww->model->adjustObjectsForShiver (outRect.x (),
474 outRect.y (),
475@@ -1977,7 +1977,7 @@
476 wScreen->optionGetMapWindowMatch ().evaluate (window) &&
477 ensureModel ())
478 {
479- CompRect outRect (window->outputRect ());
480+ CompRect outRect (window->serverOutputRect ());
481
482 model->initObjects (outRect.x (), outRect.y (),
483 outRect.width (), outRect.height ());
484@@ -2006,7 +2006,7 @@
485 int dwidth,
486 int dheight)
487 {
488- CompRect outRect (window->outputRect ());
489+ CompRect outRect (window->serverOutputRect ());
490
491 if (wScreen->optionGetMaximizeEffect () &&
492 isWobblyWin () &&
493@@ -2144,7 +2144,7 @@
494
495 if (wScreen->optionGetMaximizeEffect ())
496 {
497- CompRect outRect (window->outputRect ());
498+ CompRect outRect (window->serverOutputRect ());
499
500 if (window->state () & MAXIMIZE_STATE)
501 {
502@@ -2252,7 +2252,7 @@
503 if (wScreen->optionGetMaximizeEffect () &&
504 (state & MAXIMIZE_STATE))
505 {
506- CompRect outRect (window->outputRect ());
507+ CompRect outRect (window->serverOutputRect ());
508
509 model->addEdgeAnchors (outRect.x (), outRect.y (),
510 outRect.width (), outRect.height ());
511
512=== modified file 'src/actions.cpp'
513--- src/actions.cpp 2012-01-31 14:52:20 +0000
514+++ src/actions.cpp 2012-04-07 03:14:19 +0000
515@@ -201,9 +201,9 @@
516 time = CompOption::getIntOptionNamed (options, "time", CurrentTime);
517 button = CompOption::getIntOptionNamed (options, "button", 0);
518 x = CompOption::getIntOptionNamed (options, "x",
519- w->geometry ().x ());
520+ w->serverGeometry ().x ());
521 y = CompOption::getIntOptionNamed (options, "y",
522- w->geometry ().y ());
523+ w->serverGeometry ().y ());
524
525 screen->toolkitAction (Atoms::toolkitActionWindowMenu,
526 time, w->id (), button, x, y);
527
528=== modified file 'src/event.cpp'
529--- src/event.cpp 2012-04-04 03:11:55 +0000
530+++ src/event.cpp 2012-04-07 03:14:19 +0000
531@@ -1251,7 +1251,7 @@
532 }
533
534 /* been shaded */
535- if (w->priv->height == 0)
536+ if (w->shaded ())
537 {
538 if (w->id () == priv->activeWindow)
539 w->moveInputFocusTo ();
540
541=== modified file 'src/privatewindow.h'
542--- src/privatewindow.h 2012-03-02 18:02:07 +0000
543+++ src/privatewindow.h 2012-04-07 03:14:19 +0000
544@@ -34,7 +34,6 @@
545 #include <core/timer.h>
546 #include "privatescreen.h"
547
548-
549 typedef CompWindowExtents CompFullscreenMonitorSet;
550
551 class PrivateWindow {
552@@ -167,6 +166,11 @@
553
554 bool handleSyncAlarm ();
555
556+ void move (int dx, int dy, bool sync);
557+ bool resize (int dx, int dy, int dwidth, int dheight, int dborder);
558+ bool resize (const CompWindow::Geometry &g);
559+ bool resize (const XWindowAttributes &attrib);
560+
561 void configure (XConfigureEvent *ce);
562
563 void configureFrame (XConfigureEvent *ce);
564@@ -241,13 +245,8 @@
565 XSizeHints sizeHints;
566 XWMHints *hints;
567
568- struct timeval lastGeometryUpdate;
569- struct timeval lastConfigureRequest;
570-
571 bool inputHint;
572 bool alpha;
573- int width;
574- int height;
575 CompRegion region;
576 CompRegion inputRegion;
577 CompRegion frameRegion;
578@@ -290,8 +289,6 @@
579 typedef std::pair <XWindowChanges, unsigned int> XWCValueMask;
580
581 compiz::X11::PendingEventQueue pendingConfigures;
582- CompTimer mClearCheckTimeout;
583- bool pendingPositionUpdates;
584
585 char *startupId;
586 char *resName;
587@@ -327,6 +324,8 @@
588
589 bool closeRequests;
590 Time lastCloseRequestTime;
591+
592+ bool nextMoveImmediate;
593 };
594
595 #endif
596
597=== modified file 'src/screen.cpp'
598--- src/screen.cpp 2012-03-30 06:26:33 +0000
599+++ src/screen.cpp 2012-04-07 03:14:19 +0000
600@@ -4060,8 +4060,8 @@
601 CompRect rect (gm);
602 int offset;
603
604- rect.setWidth (rect.width () + (gm.border () * 2));
605- rect.setHeight (rect.height () + (gm.border () * 2));
606+ rect.setWidth (gm.widthIncBorders ());
607+ rect.setHeight (gm.heightIncBorders ());
608
609 offset = rect.centerX () < 0 ? -1 : 0;
610 viewport.setX (priv->vp.x () + ((rect.centerX () / width ()) + offset) %
611@@ -4657,6 +4657,8 @@
612 return false;
613 }
614
615+ /* Use synchronous behaviour when running with --sync, useful
616+ * for getting stacktraces when X Errors occurr */
617 XSynchronize (dpy, synchronousX ? True : False);
618
619 snprintf (displayString, 255, "DISPLAY=%s",
620
621=== modified file 'src/window.cpp'
622--- src/window.cpp 2012-04-04 03:11:55 +0000
623+++ src/window.cpp 2012-04-07 03:14:19 +0000
624@@ -77,8 +77,8 @@
625 PrivateWindow::isInvisible() const
626 {
627 return attrib.map_state != IsViewable ||
628- attrib.x + width + output.right <= 0 ||
629- attrib.y + height + output.bottom <= 0 ||
630+ attrib.x + geometry.width () + output.right <= 0 ||
631+ attrib.y + geometry.height () + output.bottom <= 0 ||
632 attrib.x - output.left >= (int) screen->width () ||
633 attrib.y - output.top >= (int) screen->height ();
634 }
635@@ -810,292 +810,14 @@
636 if (!serverFrame)
637 return;
638
639-
640- gettimeofday (&lastConfigureRequest, NULL);
641- /* Flush any changes made to serverFrameGeometry or serverGeometry to the server
642- * since there is a race condition where geometries will go out-of-sync with
643- * window movement */
644-
645- window->syncPosition ();
646-
647- if (serverInput.left || serverInput.right || serverInput.top || serverInput.bottom)
648- {
649- int bw = serverGeometry.border () * 2;
650-
651- xwc.x = serverGeometry.x () - serverInput.left;
652- xwc.y = serverGeometry.y () - serverInput.top;
653- xwc.width = serverGeometry.width () + serverInput.left + serverInput.right + bw;
654- if (shaded)
655- xwc.height = serverInput.top + serverInput.bottom + bw;
656- else
657- xwc.height = serverGeometry.height () + serverInput.top + serverInput.bottom + bw;
658-
659- if (shaded)
660- height = serverInput.top + serverInput.bottom;
661-
662- if (serverFrameGeometry.x () == xwc.x)
663- valueMask &= ~(CWX);
664- else
665- serverFrameGeometry.setX (xwc.x);
666-
667- if (serverFrameGeometry.y () == xwc.y)
668- valueMask &= ~(CWY);
669- else
670- serverFrameGeometry.setY (xwc.y);
671-
672- if (serverFrameGeometry.width () == xwc.width)
673- valueMask &= ~(CWWidth);
674- else
675- serverFrameGeometry.setWidth (xwc.width);
676-
677- if (serverFrameGeometry.height () == xwc.height)
678- valueMask &= ~(CWHeight);
679- else
680- serverFrameGeometry.setHeight (xwc.height);
681-
682- /* Geometry is the same, so we're not going to get a ConfigureNotify
683- * event when the window is configured, which means that other plugins
684- * won't know that the client, frame and wrapper windows got shifted
685- * around (and might result in display corruption, eg in OpenGL */
686- if (valueMask == 0)
687- {
688- XConfigureEvent xev;
689- XWindowAttributes attrib;
690- unsigned int nchildren = 0;
691- Window rootRet = 0, parentRet = 0;
692- Window *children = NULL;
693-
694- xev.type = ConfigureNotify;
695- xev.event = screen->root ();
696- xev.window = priv->serverFrame;
697-
698- XGrabServer (screen->dpy ());
699-
700- if (XGetWindowAttributes (screen->dpy (), priv->serverFrame, &attrib))
701- {
702- xev.x = attrib.x;
703- xev.y = attrib.y;
704- xev.width = attrib.width;
705- xev.height = attrib.height;
706- xev.border_width = attrib.border_width;
707- xev.above = None;
708-
709- /* We need to ensure that the stacking order is
710- * based on the current server stacking order so
711- * find the sibling to this window's frame in the
712- * server side stack and stack above that */
713- XQueryTree (screen->dpy (), screen->root (), &rootRet, &parentRet, &children, &nchildren);
714-
715- if (nchildren)
716- {
717- for (unsigned int i = 0; i < nchildren; i++)
718- {
719- if (i + 1 == nchildren ||
720- children[i + 1] == ROOTPARENT (window))
721- {
722- xev.above = children[i];
723- break;
724- }
725- }
726- }
727-
728- if (children)
729- XFree (children);
730-
731- if (!xev.above)
732- xev.above = (window->serverPrev) ? ROOTPARENT (window->serverPrev) : None;
733-
734- xev.override_redirect = priv->attrib.override_redirect;
735-
736- }
737-
738- compiz::X11::PendingEvent::Ptr pc =
739- boost::shared_static_cast<compiz::X11::PendingEvent> (compiz::X11::PendingConfigureEvent::Ptr (
740- new compiz::X11::PendingConfigureEvent (
741- screen->dpy (), serverFrame, valueMask, &xwc)));
742-
743- pendingConfigures.add (pc);
744- if (priv->mClearCheckTimeout.active ())
745- priv->mClearCheckTimeout.stop ();
746- priv->mClearCheckTimeout.start (boost::bind (&PrivateWindow::checkClear, priv),
747- 2000, 2500);
748-
749- XSendEvent (screen->dpy (), screen->root (), false,
750- SubstructureNotifyMask, (XEvent *) &xev);
751-
752- XUngrabServer (screen->dpy ());
753- XSync (screen->dpy (), false);
754- }
755- else
756- {
757- compiz::X11::PendingEvent::Ptr pc =
758- boost::shared_static_cast<compiz::X11::PendingEvent> (compiz::X11::PendingConfigureEvent::Ptr (
759- new compiz::X11::PendingConfigureEvent (
760- screen->dpy (), serverFrame, valueMask, &xwc)));
761-
762- pendingConfigures.add (pc);
763- if (priv->mClearCheckTimeout.active ())
764- priv->mClearCheckTimeout.stop ();
765- priv->mClearCheckTimeout.start (boost::bind (&PrivateWindow::checkClear, priv),
766- 2000, 2500);
767- XConfigureWindow (screen->dpy (), serverFrame, valueMask, &xwc);
768- }
769-
770- if (shaded)
771- {
772- XUnmapWindow (screen->dpy (), wrapper);
773- }
774- else
775- {
776- XMapWindow (screen->dpy (), wrapper);
777- XMoveResizeWindow (screen->dpy (), wrapper, serverInput.left, serverInput.top,
778- serverGeometry.width (), serverGeometry.height ());
779- }
780- XMoveResizeWindow (screen->dpy (), id, 0, 0,
781- serverGeometry.width (), serverGeometry.height ());
782- window->sendConfigureNotify ();
783- window->windowNotify (CompWindowNotifyFrameUpdate);
784- }
785- else
786- {
787- int bw = serverGeometry.border () * 2;
788-
789- xwc.x = serverGeometry.x ();
790- xwc.y = serverGeometry.y ();
791- xwc.width = serverGeometry.width () + bw;
792-
793- /* FIXME: It doesn't make much sense to allow undecorated windows to be
794- * shaded */
795- if (shaded)
796- xwc.height = bw;
797- else
798- xwc.height = serverGeometry.height () + bw;
799-
800- if (serverFrameGeometry.x () == xwc.x)
801- valueMask &= ~(CWX);
802- else
803- serverFrameGeometry.setX (xwc.x);
804-
805- if (serverFrameGeometry.y () == xwc.y)
806- valueMask &= ~(CWY);
807- else
808- serverFrameGeometry.setY (xwc.y);
809-
810- if (serverFrameGeometry.width () == xwc.width)
811- valueMask &= ~(CWWidth);
812- else
813- serverFrameGeometry.setWidth (xwc.width);
814-
815- if (serverFrameGeometry.height () == xwc.height)
816- valueMask &= ~(CWHeight);
817- else
818- serverFrameGeometry.setHeight (xwc.height);
819-
820- /* Geometry is the same, so we're not going to get a ConfigureNotify
821- * event when the window is configured, which means that other plugins
822- * won't know that the client, frame and wrapper windows got shifted
823- * around (and might result in display corruption, eg in OpenGL */
824- if (valueMask == 0)
825- {
826- XConfigureEvent xev;
827- XWindowAttributes attrib;
828- unsigned int nchildren = 0;
829- Window rootRet = 0, parentRet = 0;
830- Window *children = NULL;
831-
832- xev.type = ConfigureNotify;
833- xev.event = screen->root ();
834- xev.window = priv->serverFrame;
835-
836- XGrabServer (screen->dpy ());
837-
838- if (XGetWindowAttributes (screen->dpy (), priv->serverFrame, &attrib))
839- {
840- xev.x = attrib.x;
841- xev.y = attrib.y;
842- xev.width = attrib.width;
843- xev.height = attrib.height;
844- xev.border_width = attrib.border_width;
845- xev.above = None;
846-
847- /* We need to ensure that the stacking order is
848- * based on the current server stacking order so
849- * find the sibling to this window's frame in the
850- * server side stack and stack above that */
851- XQueryTree (screen->dpy (), screen->root (), &rootRet, &parentRet, &children, &nchildren);
852-
853- if (nchildren)
854- {
855- for (unsigned int i = 0; i < nchildren; i++)
856- {
857- if (i + 1 == nchildren ||
858- children[i + 1] == ROOTPARENT (window))
859- {
860- xev.above = children[i];
861- break;
862- }
863- }
864- }
865-
866- if (children)
867- XFree (children);
868-
869- if (!xev.above)
870- xev.above = (window->serverPrev) ? ROOTPARENT (window->serverPrev) : None;
871-
872- xev.override_redirect = priv->attrib.override_redirect;
873-
874- }
875-
876- compiz::X11::PendingEvent::Ptr pc =
877- boost::shared_static_cast<compiz::X11::PendingEvent> (compiz::X11::PendingConfigureEvent::Ptr (
878- new compiz::X11::PendingConfigureEvent (
879- screen->dpy (), serverFrame, valueMask, &xwc)));
880-
881- pendingConfigures.add (pc);
882- if (priv->mClearCheckTimeout.active ())
883- priv->mClearCheckTimeout.stop ();
884- priv->mClearCheckTimeout.start (boost::bind (&PrivateWindow::checkClear, priv),
885- 2000, 2500);
886-
887- XSendEvent (screen->dpy (), screen->root (), false,
888- SubstructureNotifyMask, (XEvent *) &xev);
889-
890- XUngrabServer (screen->dpy ());
891- XSync (screen->dpy (), false);
892- }
893- else
894- {
895- compiz::X11::PendingEvent::Ptr pc =
896- boost::shared_static_cast<compiz::X11::PendingEvent> (compiz::X11::PendingConfigureEvent::Ptr (
897- new compiz::X11::PendingConfigureEvent (
898- screen->dpy (), serverFrame, valueMask, &xwc)));
899-
900- pendingConfigures.add (pc);
901- if (priv->mClearCheckTimeout.active ())
902- priv->mClearCheckTimeout.stop ();
903- priv->mClearCheckTimeout.start (boost::bind (&PrivateWindow::checkClear, priv),
904- 2000, 2500);
905-
906- XConfigureWindow (screen->dpy (), serverFrame, valueMask, &xwc);
907- }
908-
909- if (shaded)
910- {
911- XUnmapWindow (screen->dpy (), wrapper);
912- }
913- else
914- {
915- XMapWindow (screen->dpy (), wrapper);
916- XMoveResizeWindow (screen->dpy (), wrapper, 0, 0,
917- serverGeometry.width (), serverGeometry.height ());
918- }
919-
920- XMoveResizeWindow (screen->dpy (), id, 0, 0,
921- serverGeometry.width (), serverGeometry.height ());
922- window->sendConfigureNotify ();
923- window->windowNotify (CompWindowNotifyFrameUpdate);
924- }
925+ xwc.x = serverGeometry.x ();
926+ xwc.y = serverGeometry.y ();
927+ xwc.width = serverGeometry.width ();
928+ xwc.height = serverGeometry.height ();
929+ xwc.border_width = serverGeometry.border ();
930+
931+ window->configureXWindow (valueMask, &xwc);
932+ window->windowNotify (CompWindowNotifyFrameUpdate);
933 window->recalcActions ();
934 }
935
936@@ -1138,8 +860,8 @@
937
938 for (unsigned int i = 0; i < n; i++)
939 {
940- x1 = rects[i].x + priv->geometry.border ();
941- y1 = rects[i].y + priv->geometry.border ();
942+ x1 = rects[i].x + priv->serverGeometry.border ();
943+ y1 = rects[i].y + priv->serverGeometry.border ();
944 x2 = x1 + rects[i].width;
945 y2 = y1 + rects[i].height;
946
947@@ -1147,17 +869,17 @@
948 x1 = 0;
949 if (y1 < 0)
950 y1 = 0;
951- if (x2 > priv->width)
952- x2 = priv->width;
953- if (y2 > priv->height)
954- y2 = priv->height;
955+ if (x2 > priv->serverGeometry.width ())
956+ x2 = priv->serverGeometry.width ();
957+ if (y2 > priv->serverGeometry.height ())
958+ y2 = priv->serverGeometry.height ();
959
960 if (y1 < y2 && x1 < x2)
961 {
962- x1 += priv->geometry.x ();
963- y1 += priv->geometry.y ();
964- x2 += priv->geometry.x ();
965- y2 += priv->geometry.y ();
966+ x1 += priv->serverGeometry.x ();
967+ y1 += priv->serverGeometry.y ();
968+ x2 += priv->serverGeometry.x ();
969+ y2 += priv->serverGeometry.y ();
970
971 ret += CompRect (x1, y1, x2 - x1, y2 - y1);
972 }
973@@ -1178,24 +900,26 @@
974 XRectangle *inputShapeRects = NULL;
975 int nBounding = 0, nInput = 0;
976
977- priv->region = CompRegion ();
978- priv->inputRegion = CompRegion ();
979+ priv->region -= infiniteRegion;
980+ priv->inputRegion -= infiniteRegion;
981
982 if (screen->XShape ())
983 {
984 int order;
985
986+ /* We should update the server here */
987+ XSync (screen->dpy (), false);
988+
989 boundingShapeRects = XShapeGetRectangles (screen->dpy (), priv->id,
990 ShapeBounding, &nBounding, &order);
991 inputShapeRects = XShapeGetRectangles (screen->dpy (), priv->id,
992 ShapeInput, &nInput, &order);
993-
994 }
995
996- r.x = -priv->geometry.border ();
997- r.y = -priv->geometry.border ();
998- r.width = priv->width + priv->geometry.border ();
999- r.height = priv->height + priv->geometry.border ();
1000+ r.x = -priv->serverGeometry.border ();
1001+ r.y = -priv->serverGeometry.border ();
1002+ r.width = priv->serverGeometry.widthIncBorders ();
1003+ r.height = priv->serverGeometry.heightIncBorders ();
1004
1005 if (nBounding < 1)
1006 {
1007@@ -1729,7 +1453,7 @@
1008 priv->attrib.map_state = IsUnmapped;
1009 priv->invisible = true;
1010
1011- if (priv->shaded && priv->height)
1012+ if (priv->shaded)
1013 {
1014 priv->updateFrameWindow ();
1015 }
1016@@ -1797,7 +1521,7 @@
1017 }
1018
1019 bool
1020-CompWindow::resize (XWindowAttributes attr)
1021+CompWindow::resize (const XWindowAttributes &attr)
1022 {
1023 return resize (Geometry (attr.x, attr.y, attr.width, attr.height,
1024 attr.border_width));
1025@@ -1814,7 +1538,7 @@
1026 }
1027
1028 bool
1029-CompWindow::resize (CompWindow::Geometry gm)
1030+PrivateWindow::resize (const CompWindow::Geometry &gm)
1031 {
1032 /* Input extents are now the last thing sent
1033 * from the server. This might not work in some
1034@@ -1831,12 +1555,8 @@
1035 priv->geometry.height () != gm.height () ||
1036 priv->geometry.border () != gm.border ())
1037 {
1038- int pw, ph;
1039 int dx, dy, dwidth, dheight;
1040
1041- pw = gm.width () + gm.border () * 2;
1042- ph = gm.height () + gm.border () * 2;
1043-
1044 dx = gm.x () - priv->geometry.x ();
1045 dy = gm.y () - priv->geometry.y ();
1046 dwidth = gm.width () - priv->geometry.width ();
1047@@ -1846,37 +1566,58 @@
1048 gm.width (), gm.height (),
1049 gm.border ());
1050
1051- priv->width = pw;
1052- priv->height = ph;
1053+ if (priv->attrib.override_redirect)
1054+ {
1055+ priv->serverGeometry = priv->geometry;
1056+ priv->serverFrameGeometry = priv->frameGeometry;
1057+ }
1058
1059- if (priv->mapNum)
1060+ if (priv->mapNum && attrib.override_redirect)
1061 priv->updateRegion ();
1062
1063- resizeNotify (dx, dy, dwidth, dheight);
1064+ window->resizeNotify (dx, dy, dwidth, dheight);
1065
1066 priv->invisible = priv->isInvisible ();
1067 }
1068 else if (priv->geometry.x () != gm.x () || priv->geometry.y () != gm.y ())
1069 {
1070- int dx, dy;
1071-
1072- dx = gm.x () - priv->geometry.x ();
1073- dy = gm.y () - priv->geometry.y ();
1074-
1075- priv->geometry.setX (gm.x ());
1076- priv->geometry.setY (gm.y ());
1077-
1078- priv->region.translate (dx, dy);
1079- priv->inputRegion.translate (dx, dy);
1080- if (!priv->frameRegion.isEmpty ())
1081- priv->frameRegion.translate (dx, dy);
1082-
1083- priv->invisible = priv->isInvisible ();
1084-
1085- moveNotify (dx, dy, true);
1086+ move (gm.x () - priv->geometry.x (),
1087+ gm.y () - priv->geometry.y (), true);
1088 }
1089
1090- updateFrameRegion ();
1091+ return true;
1092+}
1093+
1094+bool
1095+PrivateWindow::resize (const XWindowAttributes &attr)
1096+{
1097+ return resize (CompWindow::Geometry (attr.x, attr.y, attr.width, attr.height,
1098+ attr.border_width));
1099+}
1100+
1101+bool
1102+PrivateWindow::resize (int x,
1103+ int y,
1104+ int width,
1105+ int height,
1106+ int border)
1107+{
1108+ return resize (CompWindow::Geometry (x, y, width, height, border));
1109+}
1110+
1111+bool
1112+CompWindow::resize (const CompWindow::Geometry &gm)
1113+{
1114+ XWindowChanges xwc = XWINDOWCHANGES_INIT;
1115+ unsigned int valueMask = CWX | CWY | CWWidth | CWHeight | CWBorderWidth;
1116+
1117+ xwc.x = gm.x ();
1118+ xwc.y = gm.y ();
1119+ xwc.width = gm.width ();
1120+ xwc.height = gm.height ();
1121+ xwc.border_width = gm.border ();
1122+
1123+ configureXWindow (valueMask, &xwc);
1124
1125 return true;
1126 }
1127@@ -2044,13 +1785,7 @@
1128 ce->border_width);
1129 else
1130 {
1131- if (ce->override_redirect)
1132- {
1133- priv->serverGeometry.set (ce->x, ce->y, ce->width, ce->height,
1134- ce->border_width);
1135- }
1136-
1137- window->resize (ce->x, ce->y, ce->width, ce->height, ce->border_width);
1138+ resize (ce->x, ce->y, ce->width, ce->height, ce->border_width);
1139 }
1140
1141 if (ce->event == screen->root ())
1142@@ -2120,9 +1855,9 @@
1143 * windows since we didn't resize them
1144 * on configureXWindow */
1145 if (priv->shaded)
1146- height = priv->serverGeometry.height () - priv->serverGeometry.border () * 2 - priv->serverInput.top - priv->serverInput.bottom;
1147+ height = priv->serverGeometry.heightIncBorders () - priv->serverInput.top - priv->serverInput.bottom;
1148 else
1149- height = ce->height - priv->serverGeometry.border () * 2 - priv->serverInput.top - priv->serverInput.bottom;
1150+ height = ce->height + priv->serverGeometry.border () * 2 - priv->serverInput.top - priv->serverInput.bottom;
1151
1152 /* set the frame geometry */
1153 priv->frameGeometry.set (ce->x, ce->y, ce->width, ce->height, ce->border_width);
1154@@ -2131,7 +1866,7 @@
1155 if (priv->syncWait)
1156 priv->syncGeometry.set (x, y, width, height, ce->border_width);
1157 else
1158- window->resize (x, y, width, height, ce->border_width);
1159+ resize (x, y, width, height, ce->border_width);
1160
1161 if (priv->restack (ce->above))
1162 priv->updatePassiveButtonGrabs ();
1163@@ -2140,27 +1875,6 @@
1164
1165 if (above)
1166 above->priv->updatePassiveButtonGrabs ();
1167-
1168- if (!pendingConfigures.pending ())
1169- {
1170- /* Tell plugins its ok to start doing stupid things again but
1171- * obviously FIXME */
1172- CompOption::Vector options;
1173- CompOption::Value v;
1174-
1175- options.push_back (CompOption ("window", CompOption::TypeInt));
1176- v.set ((int) id);
1177- options.back ().set (v);
1178- options.push_back (CompOption ("active", CompOption::TypeInt));
1179- v.set ((int) 0);
1180- options.back ().set (v);
1181-
1182- /* Notify other plugins that it is unsafe to change geometry or serverGeometry
1183- * FIXME: That API should not be accessible to plugins, this is a hack to avoid
1184- * breaking ABI */
1185-
1186- screen->handleCompizEvent ("core", "lock_position", options);
1187- }
1188 }
1189
1190 void
1191@@ -2183,23 +1897,34 @@
1192 {
1193 if (dx || dy)
1194 {
1195- gettimeofday (&priv->lastGeometryUpdate, NULL);
1196-
1197- /* Don't allow window movement to overwrite working geometries
1198- * last received from the server if we know there are pending
1199- * ConfigureNotify events on this window. That's a clunky workaround
1200- * and a FIXME in any case, however, until we can break the API
1201- * and remove CompWindow::move, this will need to be the case */
1202-
1203- if (!priv->pendingConfigures.pending ())
1204+ XWindowChanges xwc = XWINDOWCHANGES_INIT;
1205+ unsigned int valueMask = CWX | CWY;
1206+
1207+ xwc.x = priv->serverGeometry.x () + dx;
1208+ xwc.y = priv->serverGeometry.y () + dy;
1209+
1210+ priv->nextMoveImmediate = immediate;
1211+
1212+ configureXWindow (valueMask, &xwc);
1213+ }
1214+}
1215+
1216+void
1217+PrivateWindow::move (int dx,
1218+ int dy,
1219+ bool immediate)
1220+{
1221+ if (dx || dy)
1222+ {
1223+ priv->geometry.setX (priv->geometry.x () + dx);
1224+ priv->geometry.setY (priv->geometry.y () + dy);
1225+ priv->frameGeometry.setX (priv->frameGeometry.x () + dx);
1226+ priv->frameGeometry.setY (priv->frameGeometry.y () + dy);
1227+
1228+ if (priv->attrib.override_redirect)
1229 {
1230- priv->geometry.setX (priv->geometry.x () + dx);
1231- priv->geometry.setY (priv->geometry.y () + dy);
1232- priv->frameGeometry.setX (priv->frameGeometry.x () + dx);
1233- priv->frameGeometry.setY (priv->frameGeometry.y () + dy);
1234-
1235- priv->pendingPositionUpdates = true;
1236-
1237+ priv->serverGeometry = priv->geometry;
1238+ priv->serverFrameGeometry = priv->frameGeometry;
1239 priv->region.translate (dx, dy);
1240 priv->inputRegion.translate (dx, dy);
1241 if (!priv->frameRegion.isEmpty ())
1242@@ -2207,19 +1932,7 @@
1243
1244 priv->invisible = priv->isInvisible ();
1245
1246- moveNotify (dx, dy, immediate);
1247- }
1248- else
1249- {
1250- XWindowChanges xwc = XWINDOWCHANGES_INIT;
1251- unsigned int valueMask = CWX | CWY;
1252- compLogMessage ("core", CompLogLevelDebug, "pending configure notifies on 0x%x, "\
1253- "moving window asyncrhonously!", (unsigned int) priv->serverId);
1254-
1255- xwc.x = priv->serverGeometry.x () + dx;
1256- xwc.y = priv->serverGeometry.y () + dy;
1257-
1258- configureXWindow (valueMask, &xwc);
1259+ window->moveNotify (dx, dy, true);
1260 }
1261 }
1262 }
1263@@ -2230,22 +1943,6 @@
1264 return !mEvents.empty ();
1265 }
1266
1267-bool
1268-PrivateWindow::checkClear ()
1269-{
1270- if (pendingConfigures.pending ())
1271- {
1272- /* FIXME: This is a hack to avoid performance regressions
1273- * and must be removed in 0.9.6 */
1274- compLogMessage ("core", CompLogLevelWarn, "failed to receive ConfigureNotify event on 0x%x\n",
1275- id);
1276- pendingConfigures.dump ();
1277- pendingConfigures.clear ();
1278- }
1279-
1280- return false;
1281-}
1282-
1283 void
1284 compiz::X11::PendingEventQueue::add (PendingEvent::Ptr p)
1285 {
1286@@ -2467,21 +2164,6 @@
1287 mValueMask (valueMask),
1288 mXwc (*xwc)
1289 {
1290- CompOption::Vector options;
1291- CompOption::Value v;
1292-
1293- options.push_back (CompOption ("window", CompOption::TypeInt));
1294- v.set ((int) w);
1295- options.back ().set (v);
1296- options.push_back (CompOption ("active", CompOption::TypeInt));
1297- v.set ((int) 1);
1298- options.back ().set (v);
1299-
1300- /* Notify other plugins that it is unsafe to change geometry or serverGeometry
1301- * FIXME: That API should not be accessible to plugins, this is a hack to avoid
1302- * breaking ABI */
1303-
1304- screen->handleCompizEvent ("core", "lock_position", options);
1305 }
1306
1307 compiz::X11::PendingConfigureEvent::~PendingConfigureEvent ()
1308@@ -2491,57 +2173,6 @@
1309 void
1310 CompWindow::syncPosition ()
1311 {
1312- gettimeofday (&priv->lastConfigureRequest, NULL);
1313-
1314- unsigned int valueMask = CWX | CWY;
1315- XWindowChanges xwc = XWINDOWCHANGES_INIT;
1316-
1317- if (priv->pendingPositionUpdates && !priv->pendingConfigures.pending ())
1318- {
1319- if (priv->serverFrameGeometry.x () == priv->frameGeometry.x ())
1320- valueMask &= ~(CWX);
1321- if (priv->serverFrameGeometry.y () == priv->frameGeometry.y ())
1322- valueMask &= ~(CWY);
1323-
1324- /* Because CompWindow::move can update the geometry last
1325- * received from the server, we must indicate that no values
1326- * changed, because when the ConfigureNotify comes around
1327- * the values are going to be the same. That's obviously
1328- * broken behaviour and worthy of a FIXME, but requires
1329- * larger changes to the window movement system. */
1330- if (valueMask)
1331- {
1332- priv->serverGeometry.setX (priv->geometry.x ());
1333- priv->serverGeometry.setY (priv->geometry.y ());
1334- priv->serverFrameGeometry.setX (priv->frameGeometry.x ());
1335- priv->serverFrameGeometry.setY (priv->frameGeometry.y ());
1336-
1337- xwc.x = priv->serverFrameGeometry.x ();
1338- xwc.y = priv->serverFrameGeometry.y ();
1339-
1340- compiz::X11::PendingEvent::Ptr pc =
1341- boost::shared_static_cast<compiz::X11::PendingEvent> (compiz::X11::PendingConfigureEvent::Ptr (
1342- new compiz::X11::PendingConfigureEvent (
1343- screen->dpy (), priv->serverFrame, 0, &xwc)));
1344-
1345- priv->pendingConfigures.add (pc);
1346-
1347- /* Got 3 seconds to get its stuff together */
1348- if (priv->mClearCheckTimeout.active ())
1349- priv->mClearCheckTimeout.stop ();
1350- priv->mClearCheckTimeout.start (boost::bind (&PrivateWindow::checkClear, priv),
1351- 2000, 2500);
1352- XConfigureWindow (screen->dpy (), ROOTPARENT (this), valueMask, &xwc);
1353-
1354- if (priv->serverFrame)
1355- {
1356- XMoveWindow (screen->dpy (), priv->wrapper,
1357- priv->serverInput.left, priv->serverInput.top);
1358- sendConfigureNotify ();
1359- }
1360- }
1361- priv->pendingPositionUpdates = false;
1362- }
1363 }
1364
1365 bool
1366@@ -3340,16 +2971,28 @@
1367 PrivateWindow::reconfigureXWindow (unsigned int valueMask,
1368 XWindowChanges *xwc)
1369 {
1370+ int dx = valueMask & CWX ? xwc->x - serverGeometry.x () : 0;
1371+ int dy = valueMask & CWY ? xwc->y - serverGeometry.y () : 0;
1372+
1373 unsigned int frameValueMask = 0;
1374
1375- /* Immediately sync window position
1376- * if plugins were updating w->geometry () directly
1377- * in order to avoid a race condition */
1378-
1379- window->syncPosition ();
1380+ if (id == screen->root ())
1381+ {
1382+ compLogMessage ("core", CompLogLevelWarn, "attempted to reconfigure root window");
1383+ return;
1384+ }
1385
1386 /* Remove redundant bits */
1387
1388+ xwc->x = valueMask & CWX ? xwc->x : serverGeometry.x ();
1389+ xwc->y = valueMask & CWY ? xwc->y : serverGeometry.y ();
1390+ xwc->width = valueMask & CWWidth ? xwc->width : serverGeometry.width ();
1391+ xwc->height = valueMask & CWHeight ? xwc->height : serverGeometry.height ();
1392+ xwc->border_width = valueMask & CWBorderWidth ? xwc->border_width : serverGeometry.border ();
1393+
1394+ /* FIXME: This is a total fallacy for the reparenting case
1395+ * at least since the client doesn't actually move here, it only
1396+ * moves within the frame */
1397 if (valueMask & CWX && serverGeometry.x () == xwc->x)
1398 valueMask &= ~(CWX);
1399
1400@@ -3416,18 +3059,15 @@
1401 compLogMessage ("core", CompLogLevelWarn, "restack_mode not Above");
1402 }
1403
1404- frameValueMask = valueMask;
1405+ frameValueMask = CWX | CWY | CWWidth | CWHeight | (valueMask & (CWStackMode | CWSibling));
1406
1407- if (frameValueMask & CWX &&
1408- serverFrameGeometry.x () == xwc->x - serverGeometry.border () - serverInput.left)
1409+ if (serverFrameGeometry.x () == xwc->x - serverGeometry.border () - serverInput.left)
1410 frameValueMask &= ~(CWX);
1411
1412- if (frameValueMask & CWY &&
1413- serverFrameGeometry.y () == xwc->y - serverGeometry.border () - serverInput.top)
1414+ if (serverFrameGeometry.y () == xwc->y - serverGeometry.border () - serverInput.top)
1415 frameValueMask &= ~(CWY);
1416
1417- if (frameValueMask & CWWidth &&
1418- serverFrameGeometry.width () == xwc->width + serverGeometry.border () * 2
1419+ if (serverFrameGeometry.width () == xwc->width + serverGeometry.border () * 2
1420 + serverInput.left + serverInput.right)
1421 frameValueMask &= ~(CWWidth);
1422
1423@@ -3437,19 +3077,76 @@
1424
1425 if (shaded)
1426 {
1427- if (frameValueMask & CWHeight &&
1428- serverFrameGeometry.height () == serverGeometry.border () * 2
1429+ if (serverFrameGeometry.height () == serverGeometry.border () * 2
1430 + serverInput.top + serverInput.bottom)
1431 frameValueMask &= ~(CWHeight);
1432 }
1433 else
1434 {
1435- if (frameValueMask & CWHeight &&
1436- serverFrameGeometry.height () == xwc->height + serverGeometry.border () * 2
1437+ if (serverFrameGeometry.height () == xwc->height + serverGeometry.border () * 2
1438 + serverInput.top + serverInput.bottom)
1439 frameValueMask &= ~(CWHeight);
1440 }
1441
1442+ /* Don't allow anything that might generate a BadValue */
1443+ if (valueMask & CWWidth && !xwc->width)
1444+ {
1445+ compLogMessage ("core", CompLogLevelWarn, "Attempted to set < 1 width on a window");
1446+ xwc->width = 1;
1447+ }
1448+
1449+ if (valueMask & CWHeight && !xwc->height)
1450+ {
1451+ compLogMessage ("core", CompLogLevelWarn, "Attempted to set < 1 height on a window");
1452+ xwc->height = 1;
1453+ }
1454+
1455+ if (valueMask & CWStackMode &&
1456+ ((xwc->stack_mode != TopIf) && (xwc->stack_mode != BottomIf) && (xwc->stack_mode != Opposite) &&
1457+ (xwc->stack_mode != Above) && (xwc->stack_mode != Below)))
1458+ {
1459+ compLogMessage ("core", CompLogLevelWarn, "Invalid stack mode %i", xwc->stack_mode);
1460+ valueMask &= ~(CWStackMode | CWSibling);
1461+ }
1462+
1463+ /* Don't allow anything that might cause a BadMatch error */
1464+
1465+ if (valueMask & CWSibling && !(valueMask & CWStackMode))
1466+ {
1467+ compLogMessage ("core", CompLogLevelWarn, "Didn't specify a CWStackMode for CWSibling");
1468+ valueMask &= ~CWSibling;
1469+ }
1470+
1471+ if (valueMask & CWSibling && xwc->sibling == (serverFrame ? serverFrame : id))
1472+ {
1473+ compLogMessage ("core", CompLogLevelWarn, "Can't restack a window relative to itself");
1474+ valueMask &= ~CWSibling;
1475+ }
1476+
1477+ if (valueMask & CWBorderWidth && attrib.c_class == InputOnly)
1478+ {
1479+ compLogMessage ("core", CompLogLevelWarn, "Cannot set border_width of an input_only window");
1480+ valueMask &= ~CWBorderWidth;
1481+ }
1482+
1483+ if (valueMask & CWSibling)
1484+ {
1485+ CompWindow *sibling = screen->findTopLevelWindow (xwc->sibling);
1486+
1487+ if (!sibling)
1488+ {
1489+ compLogMessage ("core", CompLogLevelWarn, "Attempted to restack relative to 0x%x which is "\
1490+ "not a child of the root window or a window compiz owns", static_cast <unsigned int> (xwc->sibling));
1491+ valueMask &= ~(CWSibling | CWStackMode);
1492+ }
1493+ else if (sibling->frame () && xwc->sibling != sibling->frame ())
1494+ {
1495+ compLogMessage ("core", CompLogLevelWarn, "Attempted to restack relative to 0x%x which is "\
1496+ "not a child of the root window", static_cast <unsigned int> (xwc->sibling));
1497+ valueMask &= ~(CWSibling | CWStackMode);
1498+ }
1499+ }
1500+
1501 /* Can't set the border width of frame windows */
1502 frameValueMask &= ~(CWBorderWidth);
1503
1504@@ -3476,11 +3173,8 @@
1505 + serverInput.top + serverInput.bottom);
1506 }
1507
1508-
1509 if (serverFrame)
1510 {
1511- gettimeofday (&lastConfigureRequest, NULL);
1512-
1513 if (frameValueMask)
1514 {
1515 XWindowChanges wc = *xwc;
1516@@ -3496,13 +3190,10 @@
1517 screen->dpy (), priv->serverFrame, frameValueMask, &wc)));
1518
1519 pendingConfigures.add (pc);
1520- if (priv->mClearCheckTimeout.active ())
1521- priv->mClearCheckTimeout.stop ();
1522- priv->mClearCheckTimeout.start (boost::bind (&PrivateWindow::checkClear, priv),
1523- 2000, 2500);
1524
1525 XConfigureWindow (screen->dpy (), serverFrame, frameValueMask, &wc);
1526 }
1527+
1528 valueMask &= ~(CWSibling | CWStackMode);
1529
1530 /* If the frame has changed position (eg, serverInput.top
1531@@ -3528,6 +3219,27 @@
1532
1533 if (valueMask)
1534 XConfigureWindow (screen->dpy (), id, valueMask, xwc);
1535+
1536+ if (!attrib.override_redirect)
1537+ {
1538+ if (valueMask & (CWWidth | CWHeight))
1539+ {
1540+ updateRegion ();
1541+ }
1542+ else if (valueMask & (CWX | CWY))
1543+ {
1544+ region.translate (dx, dy);
1545+ inputRegion.translate (dx, dy);
1546+ if (!frameRegion.isEmpty ())
1547+ frameRegion.translate (dx, dy);
1548+ }
1549+
1550+ if (dx || dy)
1551+ {
1552+ window->moveNotify (dx, dy, priv->nextMoveImmediate);
1553+ priv->nextMoveImmediate = true;
1554+ }
1555+ }
1556 }
1557
1558 bool
1559@@ -4305,10 +4017,6 @@
1560 screen->dpy (), serverFrame, valueMask, &lxwc)));
1561
1562 pendingConfigures.add (pc);
1563- if (priv->mClearCheckTimeout.active ())
1564- priv->mClearCheckTimeout.stop ();
1565- priv->mClearCheckTimeout.start (boost::bind (&PrivateWindow::checkClear, priv),
1566- 2000, 2500);
1567 }
1568
1569 /* Below with no sibling puts the window at the bottom
1570@@ -4612,8 +4320,8 @@
1571 PrivateWindow::ensureWindowVisibility ()
1572 {
1573 int x1, y1, x2, y2;
1574- int width = serverGeometry.width () + serverGeometry.border () * 2;
1575- int height = serverGeometry.height () + serverGeometry.border () * 2;
1576+ int width = serverGeometry.widthIncBorders ();
1577+ int height = serverGeometry.heightIncBorders ();
1578 int dx = 0;
1579 int dy = 0;
1580
1581@@ -5473,10 +5181,10 @@
1582 }
1583 else
1584 {
1585- m = priv->geometry.x () + offX;
1586- if (m - priv->input.left < (int) s->width () - vWidth)
1587+ m = priv->serverGeometry.x () + offX;
1588+ if (m - priv->serverInput.left < (int) s->width () - vWidth)
1589 rv.setX (offX + vWidth);
1590- else if (m + priv->width + priv->input.right > vWidth)
1591+ else if (m + priv->serverGeometry.width () + priv->serverInput.right > vWidth)
1592 rv.setX (offX - vWidth);
1593 else
1594 rv.setX (offX);
1595@@ -5488,10 +5196,10 @@
1596 }
1597 else
1598 {
1599- m = priv->geometry.y () + offY;
1600- if (m - priv->input.top < (int) s->height () - vHeight)
1601+ m = priv->serverGeometry.y () + offY;
1602+ if (m - priv->serverInput.top < (int) s->height () - vHeight)
1603 rv.setY (offY + vHeight);
1604- else if (m + priv->height + priv->input.bottom > vHeight)
1605+ else if (m + priv->serverGeometry.height () + priv->serverInput.bottom > vHeight)
1606 rv.setY (offY - vHeight);
1607 else
1608 rv.setY (offY);
1609@@ -6014,8 +5722,8 @@
1610 y -= screen->vp ().y () * screen->height ();
1611 }
1612
1613- tx = x - priv->geometry.x ();
1614- ty = y - priv->geometry.y ();
1615+ tx = x - priv->serverGeometry.x ();
1616+ ty = y - priv->serverGeometry.y ();
1617
1618 if (tx || ty)
1619 {
1620@@ -6037,21 +5745,21 @@
1621
1622 if (screen->vpSize ().width ()!= 1)
1623 {
1624- m = priv->geometry.x () + tx;
1625+ m = priv->serverGeometry.x () + tx;
1626
1627 if (m - priv->output.left < (int) screen->width () - vWidth)
1628 wx = tx + vWidth;
1629- else if (m + priv->width + priv->output.right > vWidth)
1630+ else if (m + priv->serverGeometry.width () + priv->output.right > vWidth)
1631 wx = tx - vWidth;
1632 }
1633
1634 if (screen->vpSize ().height () != 1)
1635 {
1636- m = priv->geometry.y () + ty;
1637+ m = priv->serverGeometry.y () + ty;
1638
1639 if (m - priv->output.top < (int) screen->height () - vHeight)
1640 wy = ty + vHeight;
1641- else if (m + priv->height + priv->output.bottom > vHeight)
1642+ else if (m + priv->serverGeometry.height () + priv->output.bottom > vHeight)
1643 wy = ty - vHeight;
1644 }
1645
1646@@ -6206,8 +5914,8 @@
1647 svp = screen->vp ();
1648 size = *screen;
1649
1650- x = window->geometry ().x () + (svp.x () - vp.x ()) * size.width ();
1651- y = window->geometry ().y () + (svp.y () - vp.y ()) * size.height ();
1652+ x = window->serverGeometry ().x () + (svp.x () - vp.x ()) * size.width ();
1653+ y = window->serverGeometry ().y () + (svp.y () - vp.y ()) * size.height ();
1654 window->moveToViewportPosition (x, y, true);
1655
1656 if (allowWindowFocus (0, timestamp))
1657@@ -6276,9 +5984,6 @@
1658 priv->serverFrameGeometry = priv->frameGeometry = priv->syncGeometry
1659 = priv->geometry = priv->serverGeometry;
1660
1661- priv->width = priv->attrib.width + priv->attrib.border_width * 2;
1662- priv->height = priv->attrib.height + priv->attrib.border_width * 2;
1663-
1664 priv->sizeHints.flags = 0;
1665
1666 priv->recalcNormalHints ();
1667@@ -6300,8 +6005,7 @@
1668
1669 if (priv->attrib.c_class != InputOnly)
1670 {
1671- priv->region = CompRegion (priv->attrib.x, priv->attrib.y,
1672- priv->width, priv->height);
1673+ priv->region = CompRegion (priv->serverGeometry);
1674 priv->inputRegion = priv->region;
1675
1676 /* need to check for DisplayModal state on all windows */
1677@@ -6531,8 +6235,6 @@
1678 hints (NULL),
1679 inputHint (true),
1680 alpha (false),
1681- width (0),
1682- height (0),
1683 region (),
1684 wmType (0),
1685 type (CompWindowTypeUnknownMask),
1686@@ -6567,7 +6269,6 @@
1687 pendingUnmaps (0),
1688 pendingMaps (0),
1689 pendingConfigures (screen->dpy ()),
1690- pendingPositionUpdates (false),
1691
1692 startupId (0),
1693 resName (0),
1694@@ -6745,14 +6446,12 @@
1695 void
1696 CompWindow::updateFrameRegion ()
1697 {
1698- if (priv->serverFrame &&
1699- priv->serverGeometry.width () == priv->geometry.width () &&
1700- priv->serverGeometry.height () == priv->geometry.height ())
1701+ if (priv->serverFrame)
1702 {
1703 CompRect r;
1704 int x, y;
1705
1706- priv->frameRegion = CompRegion ();
1707+ priv->frameRegion -= infiniteRegion;
1708
1709 updateFrameRegion (priv->frameRegion);
1710
1711@@ -6761,16 +6460,16 @@
1712 r = priv->region.boundingRect ();
1713 priv->frameRegion -= r;
1714
1715- r.setGeometry (r.x1 () - priv->input.left,
1716- r.y1 () - priv->input.top,
1717- r.width () + priv->input.right + priv->input.left,
1718- r.height () + priv->input.bottom + priv->input.top);
1719+ r.setGeometry (r.x1 () - priv->serverInput.left,
1720+ r.y1 () - priv->serverInput.top,
1721+ r.width () + priv->serverInput.right + priv->serverInput.left,
1722+ r.height () + priv->serverInput.bottom + priv->serverInput.top);
1723
1724 priv->frameRegion &= CompRegion (r);
1725 }
1726
1727- x = priv->geometry.x () - priv->input.left;
1728- y = priv->geometry.y () - priv->input.top;
1729+ x = priv->serverGeometry.x () - priv->serverInput.left;
1730+ y = priv->serverGeometry.y () - priv->serverInput.top;
1731
1732 XShapeCombineRegion (screen->dpy (), priv->serverFrame,
1733 ShapeBounding, -x, -y,
1734@@ -6811,6 +6510,11 @@
1735
1736 priv->updateSize ();
1737 priv->updateFrameWindow ();
1738+
1739+ /* Always send a moveNotify
1740+ * whenever the frame extents update
1741+ * so that plugins can re-position appropriately */
1742+ moveNotify (0, 0, true);
1743 }
1744
1745 /* Use b for _NET_WM_FRAME_EXTENTS here because
1746@@ -7094,10 +6798,10 @@
1747 /* Wait for the reparent to finish */
1748 XSync (dpy, false);
1749
1750- xwc.x = serverGeometry.x () - serverGeometry.border ();
1751- xwc.y = serverGeometry.y () - serverGeometry.border ();
1752- xwc.width = serverGeometry.width () + serverGeometry.border () * 2;
1753- xwc.height = serverGeometry.height () + serverGeometry.border () * 2;
1754+ xwc.x = serverGeometry.xMinusBorder ();
1755+ xwc.y = serverGeometry.yMinusBorder ();
1756+ xwc.width = serverGeometry.widthIncBorders ();
1757+ xwc.height = serverGeometry.heightIncBorders ();
1758
1759 XConfigureWindow (dpy, serverFrame, CWX | CWY | CWWidth | CWHeight, &xwc);
1760
1761
1762=== modified file 'src/window/geometry/include/core/windowgeometry.h'
1763--- src/window/geometry/include/core/windowgeometry.h 2012-01-19 20:08:32 +0000
1764+++ src/window/geometry/include/core/windowgeometry.h 2012-04-07 03:14:19 +0000
1765@@ -64,6 +64,12 @@
1766 compiz::window::Geometry change (const compiz::window::Geometry &g, unsigned int mask) const;
1767 void applyChange (const compiz::window::Geometry &g, unsigned int mask);
1768
1769+ int xMinusBorder () const { return x () - mBorder; }
1770+ int yMinusBorder () const { return y () - mBorder; }
1771+
1772+ unsigned int widthIncBorders () const { return width () + mBorder * 2; }
1773+ unsigned int heightIncBorders () const { return height () + mBorder * 2; }
1774+
1775 private:
1776 int mBorder;
1777 };
1778
1779=== modified file 'src/window/geometry/tests/window-geometry/src/test-window-geometry.cpp'
1780--- src/window/geometry/tests/window-geometry/src/test-window-geometry.cpp 2012-03-30 16:30:13 +0000
1781+++ src/window/geometry/tests/window-geometry/src/test-window-geometry.cpp 2012-04-07 03:14:19 +0000
1782@@ -87,3 +87,13 @@
1783 EXPECT_EQ (rg, compiz::window::Geometry (49, 99, 199, 299, 5));
1784 EXPECT_EQ (mask, CHANGE_X | CHANGE_Y | CHANGE_WIDTH | CHANGE_HEIGHT);
1785 }
1786+
1787+TEST_F(CompWindowGeometryTestGeometry, TestBorders)
1788+{
1789+ compiz::window::Geometry g (1, 1, 1, 1, 1);
1790+
1791+ EXPECT_EQ (g.xMinusBorder (), 0);
1792+ EXPECT_EQ (g.yMinusBorder (), 0);
1793+ EXPECT_EQ (g.widthIncBorders (), 3);
1794+ EXPECT_EQ (g.heightIncBorders (), 3);
1795+}
1796
1797=== modified file 'src/windowgeometry.cpp'
1798--- src/windowgeometry.cpp 2012-01-23 05:44:19 +0000
1799+++ src/windowgeometry.cpp 2012-04-07 03:14:19 +0000
1800@@ -61,22 +61,19 @@
1801 int
1802 CompWindow::width () const
1803 {
1804- return priv->width +
1805- priv->geometry.border () * 2;
1806+ return priv->geometry.widthIncBorders ();
1807 }
1808
1809 int
1810 CompWindow::height () const
1811 {
1812- return priv->height +
1813- priv->geometry.border () * 2;;
1814+ return priv->geometry.heightIncBorders ();
1815 }
1816
1817 CompSize
1818 CompWindow::size () const
1819 {
1820- return CompSize (priv->width + priv->geometry.border () * 2,
1821- priv->height + priv->geometry.border () * 2);
1822+ return CompSize (width (), height ());
1823 }
1824
1825 int
1826@@ -102,88 +99,84 @@
1827 int
1828 CompWindow::serverWidth () const
1829 {
1830- return priv->serverGeometry.width () +
1831- 2 * priv->serverGeometry.border ();
1832+ return priv->serverGeometry.widthIncBorders ();
1833 }
1834
1835 int
1836 CompWindow::serverHeight () const
1837 {
1838- return priv->serverGeometry.height () +
1839- 2 * priv->serverGeometry.border ();
1840+ return priv->serverGeometry.heightIncBorders ();
1841 }
1842
1843 const CompSize
1844 CompWindow::serverSize () const
1845 {
1846- return CompSize (priv->serverGeometry.width () +
1847- 2 * priv->serverGeometry.border (),
1848- priv->serverGeometry.height () +
1849- 2 * priv->serverGeometry.border ());
1850+ return CompSize (priv->serverGeometry.widthIncBorders (),
1851+ priv->serverGeometry.heightIncBorders ());
1852 }
1853
1854 CompRect
1855 CompWindow::borderRect () const
1856 {
1857- return CompRect (priv->geometry.x () - priv->geometry.border () - priv->border.left,
1858- priv->geometry.y () - priv->geometry.border () - priv->border.top,
1859- priv->geometry.width () + priv->geometry.border () * 2 +
1860+ return CompRect (priv->geometry.xMinusBorder () - priv->border.left,
1861+ priv->geometry.yMinusBorder () - priv->border.top,
1862+ priv->geometry.widthIncBorders () +
1863 priv->border.left + priv->border.right,
1864- priv->geometry.height () + priv->geometry.border () * 2 +
1865+ priv->geometry.heightIncBorders () +
1866 priv->border.top + priv->border.bottom);
1867 }
1868
1869 CompRect
1870 CompWindow::serverBorderRect () const
1871 {
1872- return CompRect (priv->serverGeometry.x () - priv->geometry.border () - priv->border.left,
1873- priv->serverGeometry.y () - priv->geometry.border () - priv->border.top,
1874- priv->serverGeometry.width () + priv->geometry.border () * 2 +
1875+ return CompRect (priv->serverGeometry.xMinusBorder () - priv->border.left,
1876+ priv->serverGeometry.yMinusBorder () - priv->border.top,
1877+ priv->serverGeometry.widthIncBorders () +
1878 priv->border.left + priv->border.right,
1879- priv->serverGeometry.height () + priv->geometry.border () * 2 +
1880+ priv->serverGeometry.heightIncBorders() +
1881 priv->border.top + priv->border.bottom);
1882 }
1883
1884 CompRect
1885 CompWindow::inputRect () const
1886 {
1887- return CompRect (priv->geometry.x () - priv->geometry.border () - priv->serverInput.left,
1888- priv->geometry.y () - priv->geometry.border () - priv->serverInput.top,
1889- priv->geometry.width () + priv->geometry.border () * 2 +
1890+ return CompRect (priv->geometry.xMinusBorder () - priv->serverInput.left,
1891+ priv->geometry.yMinusBorder () - priv->serverInput.top,
1892+ priv->geometry.widthIncBorders () +
1893 priv->serverInput.left + priv->serverInput.right,
1894- priv->geometry.height () +priv->geometry.border () * 2 +
1895+ priv->geometry.heightIncBorders () +
1896 priv->serverInput.top + priv->serverInput.bottom);
1897 }
1898
1899 CompRect
1900 CompWindow::serverInputRect () const
1901 {
1902- return CompRect (priv->serverGeometry.x () - priv->serverGeometry.border () - priv->serverInput.left,
1903- priv->serverGeometry.y () - priv->serverGeometry.border () - priv->serverInput.top,
1904- priv->serverGeometry.width () + priv->serverGeometry.border () * 2 +
1905+ return CompRect (priv->serverGeometry.xMinusBorder () - priv->serverInput.left,
1906+ priv->serverGeometry.yMinusBorder () - priv->serverInput.top,
1907+ priv->serverGeometry.widthIncBorders () +
1908 priv->serverInput.left + priv->serverInput.right,
1909- priv->serverGeometry.height () + priv->serverGeometry.border () * 2 +
1910+ priv->serverGeometry.heightIncBorders () +
1911 priv->serverInput.top + priv->serverInput.bottom);
1912 }
1913
1914 CompRect
1915 CompWindow::outputRect () const
1916 {
1917- return CompRect (priv->geometry.x () - priv->serverGeometry.border ()- priv->output.left,
1918- priv->geometry.y () - priv->serverGeometry.border () - priv->output.top,
1919- priv->geometry.width () + priv->serverGeometry.border () * 2 +
1920+ return CompRect (priv->geometry.xMinusBorder ()- priv->output.left,
1921+ priv->geometry.yMinusBorder () - priv->output.top,
1922+ priv->geometry.widthIncBorders () +
1923 priv->output.left + priv->output.right,
1924- priv->geometry.height () + priv->serverGeometry.border () * 2 +
1925+ priv->geometry.heightIncBorders () +
1926 priv->output.top + priv->output.bottom);
1927 }
1928
1929 CompRect
1930 CompWindow::serverOutputRect () const
1931 {
1932- return CompRect (priv->serverGeometry.x () - priv->serverGeometry.border () - priv->output.left,
1933- priv->serverGeometry.y () - priv->serverGeometry.border () - priv->output.top,
1934- priv->serverGeometry.width () + priv->serverGeometry.border () * 2 +
1935+ return CompRect (priv->serverGeometry.xMinusBorder () - priv->output.left,
1936+ priv->serverGeometry.yMinusBorder () - priv->output.top,
1937+ priv->serverGeometry.widthIncBorders () +
1938 priv->output.left + priv->output.right,
1939- priv->serverGeometry.height () + priv->serverGeometry.border () * 2 +
1940+ priv->serverGeometry.heightIncBorders () +
1941 priv->output.top + priv->output.bottom);
1942 }

Subscribers

People subscribed via source and target branches