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

Proposed by Sam Spilsbury on 2012-02-25
Status: Superseded
Proposed branch: lp:~smspillaz/compiz-core/compiz-core.work_923683
Merge into: lp:compiz-core
Diff against target: 1374 lines (+214/-577)
10 files modified
include/core/window.h (+2/-2)
plugins/composite/src/window.cpp (+16/-16)
plugins/decor/src/decor.cpp (+15/-15)
plugins/move/src/move.cpp (+12/-12)
plugins/opengl/src/window.cpp (+2/-2)
src/event.cpp (+1/-1)
src/privatewindow.h (+8/-11)
src/screen.cpp (+1/-1)
src/window.cpp (+153/-513)
src/windowgeometry.cpp (+4/-4)
To merge this branch: bzr merge lp:~smspillaz/compiz-core/compiz-core.work_923683
Reviewer Review Type Date Requested Status
Sam Spilsbury Needs Fixing on 2012-02-25
Daniel van Vugt 2012-02-25 Pending
Alan Griffiths 2012-02-25 Pending
Review via email: mp+94643@code.launchpad.net

This proposal supersedes a proposal from 2012-02-06.

This proposal has been superseded by a proposal from 2012-02-26.

Description of the change

A lot of the work that I've wanted to do in order to properly fix this, but in a far more condensed form. (bug 923683). Proposing early so that ~vanvugt will see it :)

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 :

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 :

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 :

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
2988. By Sam Spilsbury on 2012-02-25

Also adjust the client position inside the frame whenever the frame value mask
indicates that it might have moved.

2989. By Sam Spilsbury on 2012-02-25

Added a note for a fixme

Sam Spilsbury (smspillaz) wrote :

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

2990. By Sam Spilsbury on 2012-02-25

Also send moveNotify for override redirect windows

2991. By Sam Spilsbury on 2012-02-28

Cleanup, fix removal of XSynchronize and detect error conditions

2992. By Sam Spilsbury on 2012-02-28

Fix silly typo

2993. By Sam Spilsbury on 2012-02-28

Merge lp:compiz-core

2994. By Sam Spilsbury on 2012-02-29

Use reference

2995. By Sam Spilsbury on 2012-04-03

Merge trunk

2996. By Sam Spilsbury on 2012-04-04

Removed a lot of width () / height () + border () * 2 sillyness, replace with
more sturdy abstraction

2997. By Sam Spilsbury on 2012-04-05

Fix LP #932520 - Update regions correctly when sending geometry updates

Use information last sent to server to determine window regions and shape
regions, update regions for resizing when we send the information to the server
as the shape rectangles will be correct anyways (even though it costs us a
round trip).

2998. By Sam Spilsbury on 2012-04-05

Replace instances of geometry with serverGeoemtry where appropriate

2999. By Sam Spilsbury on 2012-04-05

Merge lp:compiz-core

3000. By Sam Spilsbury on 2012-04-07

Fix typoo

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-02-16 09:55:06 +0000
3+++ include/core/window.h 2012-02-25 14:51:17 +0000
4@@ -406,9 +406,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-02-01 17:49:07 +0000
19+++ plugins/composite/src/window.cpp 2012-02-25 14:51:17 +0000
20@@ -250,7 +250,7 @@
21
22 if (x2 > x1 && y2 > y1)
23 {
24- CompWindow::Geometry geom = priv->window->geometry ();
25+ 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 = priv->window->serverGeometry ().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 = priv->window->serverGeometry ().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 = priv->window->serverGeometry ().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 = priv->window->serverGeometry ().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-02-24 08:22:28 +0000
130+++ plugins/decor/src/decor.cpp 2012-02-25 14:51:17 +0000
131@@ -91,7 +91,7 @@
132 void
133 DecorWindow::computeShadowRegion ()
134 {
135- shadowRegion = CompRegion (window->outputRect ());
136+ shadowRegion = CompRegion (window->serverOutputRect ());
137
138 if (window->type () == CompWindowTypeDropdownMenuMask ||
139 window->type () == CompWindowTypePopupMenuMask)
140@@ -142,11 +142,11 @@
141 if (window->type () == CompWindowTypeDropdownMenuMask &&
142 shadowRegion == CompRegion (window->outputRect ()))
143 {
144- CompRect area (window->outputRect ().x1 (),
145- window->outputRect ().y1 (),
146- window->outputRect ().width (),
147- window->inputRect ().y1 () -
148- window->outputRect ().y1 ());
149+ CompRect area (window->serverOutputRect ().x1 (),
150+ window->serverOutputRect ().y1 (),
151+ window->serverOutputRect ().width (),
152+ window->serverInputRect ().y1 () -
153+ window->serverOutputRect ().y1 ());
154
155 shadowRegion = shadowRegion.subtracted (area);
156 }
157@@ -1064,8 +1064,8 @@
158 for (i = 0; i < wd->nQuad; i++)
159 {
160 int x, y;
161- unsigned int width = window->size ().width ();
162- unsigned int height = window->size ().height ();
163+ unsigned int width = window->geometry ().width ();
164+ unsigned int height = window->geometry ().height ();
165
166 if (window->shaded ())
167 height = 0;
168@@ -1074,8 +1074,8 @@
169 &x1, &y1, &x2, &y2, &sx, &sy);
170
171 /* Translate by x and y points of this window */
172- x = window->geometry ().x ();
173- y = window->geometry ().y ();
174+ x = window->serverGeometry ().x ();
175+ y = window->serverGeometry ().y ();
176
177 wd->quad[i].box.x1 = x1 + x;
178 wd->quad[i].box.y1 = y1 + y;
179@@ -1103,8 +1103,8 @@
180 bool
181 DecorWindow::checkSize (const Decoration::Ptr &decoration)
182 {
183- return (decoration->minWidth <= (int) window->size ().width () &&
184- decoration->minHeight <= (int) window->size ().height ());
185+ return (decoration->minWidth <= (int) window->geometry ().width () &&
186+ decoration->minHeight <= (int) window->geometry ().height ());
187 }
188
189 /*
190@@ -2116,8 +2116,8 @@
191 {
192 int x, y;
193
194- x = window->geometry (). x ();
195- y = window->geometry (). y ();
196+ x = window->serverGeometry (). x ();
197+ y = window->serverGeometry (). y ();
198
199 region += frameRegion.translated (x - wd->decor->input.left,
200 y - wd->decor->input.top);
201@@ -2138,7 +2138,7 @@
202 void
203 DecorWindow::updateWindowRegions ()
204 {
205- const CompRect &input (window->inputRect ());
206+ const CompRect &input (window->serverInputRect ());
207
208 if (regions.size () != gWindow->textures ().size ())
209 regions.resize (gWindow->textures ().size ());
210
211=== modified file 'plugins/move/src/move.cpp'
212--- plugins/move/src/move.cpp 2012-02-16 05:31:28 +0000
213+++ plugins/move/src/move.cpp 2012-02-25 14:51:17 +0000
214@@ -138,8 +138,8 @@
215 {
216 int xRoot, yRoot;
217
218- xRoot = w->geometry ().x () + (w->size ().width () / 2);
219- yRoot = w->geometry ().y () + (w->size ().height () / 2);
220+ xRoot = w->serverGeometry ().x () + (w->size ().width () / 2);
221+ yRoot = w->serverGeometry ().y () + (w->size ().height () / 2);
222
223 s->warpPointer (xRoot - pointerX, yRoot - pointerY);
224 }
225@@ -169,8 +169,8 @@
226 if (ms->w)
227 {
228 if (state & CompAction::StateCancel)
229- ms->w->move (ms->savedX - ms->w->geometry ().x (),
230- ms->savedY - ms->w->geometry ().y (), false);
231+ ms->w->move (ms->savedX - ms->w->serverGeometry ().x (),
232+ ms->savedY - ms->w->serverGeometry ().y (), false);
233
234 ms->w->syncPosition ();
235
236@@ -314,12 +314,12 @@
237
238 w = ms->w;
239
240- wX = w->geometry ().x ();
241- wY = w->geometry ().y ();
242- wWidth = w->geometry ().width () +
243- w->geometry ().border () * 2;
244- wHeight = w->geometry ().height () +
245- w->geometry ().border () * 2;
246+ wX = w->serverGeometry ().x ();
247+ wY = w->serverGeometry ().y ();
248+ wWidth = w->serverGeometry ().width () +
249+ w->serverGeometry ().border () * 2;
250+ wHeight = w->serverGeometry ().height () +
251+ w->serverGeometry ().border () * 2;
252
253 ms->x += xRoot - lastPointerX;
254 ms->y += yRoot - lastPointerY;
255@@ -484,8 +484,8 @@
256
257 if (dx || dy)
258 {
259- w->move (wX + dx - w->geometry ().x (),
260- wY + dy - w->geometry ().y (), false);
261+ w->move (wX + dx - w->serverGeometry ().x (),
262+ wY + dy - w->serverGeometry ().y (), false);
263
264 if (!ms->optionGetLazyPositioning ())
265 w->syncPosition ();
266
267=== modified file 'plugins/opengl/src/window.cpp'
268--- plugins/opengl/src/window.cpp 2011-03-11 12:15:30 +0000
269+++ plugins/opengl/src/window.cpp 2012-02-25 14:51:17 +0000
270@@ -76,7 +76,7 @@
271 void
272 PrivateGLWindow::setWindowMatrix ()
273 {
274- CompRect input (window->inputRect ());
275+ CompRect input (window->serverInputRect ());
276
277 if (textures.size () != matrices.size ())
278 matrices.resize (textures.size ());
279@@ -344,7 +344,7 @@
280 void
281 PrivateGLWindow::updateWindowRegions ()
282 {
283- CompRect input (window->inputRect ());
284+ CompRect input (window->serverInputRect ());
285
286 if (regions.size () != textures.size ())
287 regions.resize (textures.size ());
288
289=== modified file 'src/event.cpp'
290--- src/event.cpp 2012-02-22 05:13:00 +0000
291+++ src/event.cpp 2012-02-25 14:51:17 +0000
292@@ -1249,7 +1249,7 @@
293 }
294
295 /* been shaded */
296- if (w->priv->height == 0)
297+ if (w->shaded ())
298 {
299 if (w->id () == priv->activeWindow)
300 w->moveInputFocusTo ();
301
302=== modified file 'src/privatewindow.h'
303--- src/privatewindow.h 2012-02-14 14:37:04 +0000
304+++ src/privatewindow.h 2012-02-25 14:51:17 +0000
305@@ -34,7 +34,6 @@
306 #include <core/timer.h>
307 #include "privatescreen.h"
308
309-
310 typedef CompWindowExtents CompFullscreenMonitorSet;
311
312 class PrivateWindow {
313@@ -167,6 +166,11 @@
314
315 bool handleSyncAlarm ();
316
317+ void move (int dx, int dy, bool sync);
318+ bool resize (int dx, int dy, int dwidth, int dheight, int dborder);
319+ bool resize (const CompWindow::Geometry &g);
320+ bool resize (const XWindowAttributes &attrib);
321+
322 void configure (XConfigureEvent *ce);
323
324 void configureFrame (XConfigureEvent *ce);
325@@ -207,8 +211,6 @@
326
327 void readIconHint ();
328
329- bool checkClear ();
330-
331 public:
332
333 PrivateWindow *priv;
334@@ -240,13 +242,8 @@
335 XSizeHints sizeHints;
336 XWMHints *hints;
337
338- struct timeval lastGeometryUpdate;
339- struct timeval lastConfigureRequest;
340-
341 bool inputHint;
342- bool alpha;
343- int width;
344- int height;
345+ bool alpha;;
346 CompRegion region;
347 CompRegion inputRegion;
348 CompRegion frameRegion;
349@@ -289,8 +286,6 @@
350 typedef std::pair <XWindowChanges, unsigned int> XWCValueMask;
351
352 compiz::X11::PendingEventQueue pendingConfigures;
353- CompTimer mClearCheckTimeout;
354- bool pendingPositionUpdates;
355
356 char *startupId;
357 char *resName;
358@@ -326,6 +321,8 @@
359
360 bool closeRequests;
361 Time lastCloseRequestTime;
362+
363+ bool nextMoveImmediate;
364 };
365
366 class CoreWindow
367
368=== modified file 'src/screen.cpp'
369--- src/screen.cpp 2012-02-24 11:00:05 +0000
370+++ src/screen.cpp 2012-02-25 14:51:17 +0000
371@@ -4531,7 +4531,7 @@
372 return false;
373 }
374
375- XSynchronize (dpy, synchronousX ? True : False);
376+// priv->connection = XGetXCBConnection (priv->dpy);
377
378 snprintf (displayString, 255, "DISPLAY=%s",
379 DisplayString (dpy));
380
381=== modified file 'src/window.cpp'
382--- src/window.cpp 2012-02-20 02:30:34 +0000
383+++ src/window.cpp 2012-02-25 14:51:17 +0000
384@@ -75,8 +75,8 @@
385 PrivateWindow::isInvisible() const
386 {
387 return attrib.map_state != IsViewable ||
388- attrib.x + width + output.right <= 0 ||
389- attrib.y + height + output.bottom <= 0 ||
390+ attrib.x + geometry.width () + output.right <= 0 ||
391+ attrib.y + geometry.height () + output.bottom <= 0 ||
392 attrib.x - output.left >= (int) screen->width () ||
393 attrib.y - output.top >= (int) screen->height ();
394 }
395@@ -808,292 +808,14 @@
396 if (!serverFrame)
397 return;
398
399-
400- gettimeofday (&lastConfigureRequest, NULL);
401- /* Flush any changes made to serverFrameGeometry or serverGeometry to the server
402- * since there is a race condition where geometries will go out-of-sync with
403- * window movement */
404-
405- window->syncPosition ();
406-
407- if (serverInput.left || serverInput.right || serverInput.top || serverInput.bottom)
408- {
409- int bw = serverGeometry.border () * 2;
410-
411- xwc.x = serverGeometry.x () - serverInput.left;
412- xwc.y = serverGeometry.y () - serverInput.top;
413- xwc.width = serverGeometry.width () + serverInput.left + serverInput.right + bw;
414- if (shaded)
415- xwc.height = serverInput.top + serverInput.bottom + bw;
416- else
417- xwc.height = serverGeometry.height () + serverInput.top + serverInput.bottom + bw;
418-
419- if (shaded)
420- height = serverInput.top + serverInput.bottom;
421-
422- if (serverFrameGeometry.x () == xwc.x)
423- valueMask &= ~(CWX);
424- else
425- serverFrameGeometry.setX (xwc.x);
426-
427- if (serverFrameGeometry.y () == xwc.y)
428- valueMask &= ~(CWY);
429- else
430- serverFrameGeometry.setY (xwc.y);
431-
432- if (serverFrameGeometry.width () == xwc.width)
433- valueMask &= ~(CWWidth);
434- else
435- serverFrameGeometry.setWidth (xwc.width);
436-
437- if (serverFrameGeometry.height () == xwc.height)
438- valueMask &= ~(CWHeight);
439- else
440- serverFrameGeometry.setHeight (xwc.height);
441-
442- /* Geometry is the same, so we're not going to get a ConfigureNotify
443- * event when the window is configured, which means that other plugins
444- * won't know that the client, frame and wrapper windows got shifted
445- * around (and might result in display corruption, eg in OpenGL */
446- if (valueMask == 0)
447- {
448- XConfigureEvent xev;
449- XWindowAttributes attrib;
450- unsigned int nchildren = 0;
451- Window rootRet = 0, parentRet = 0;
452- Window *children = NULL;
453-
454- xev.type = ConfigureNotify;
455- xev.event = screen->root ();
456- xev.window = priv->serverFrame;
457-
458- XGrabServer (screen->dpy ());
459-
460- if (XGetWindowAttributes (screen->dpy (), priv->serverFrame, &attrib))
461- {
462- xev.x = attrib.x;
463- xev.y = attrib.y;
464- xev.width = attrib.width;
465- xev.height = attrib.height;
466- xev.border_width = attrib.border_width;
467- xev.above = None;
468-
469- /* We need to ensure that the stacking order is
470- * based on the current server stacking order so
471- * find the sibling to this window's frame in the
472- * server side stack and stack above that */
473- XQueryTree (screen->dpy (), screen->root (), &rootRet, &parentRet, &children, &nchildren);
474-
475- if (nchildren)
476- {
477- for (unsigned int i = 0; i < nchildren; i++)
478- {
479- if (i + 1 == nchildren ||
480- children[i + 1] == ROOTPARENT (window))
481- {
482- xev.above = children[i];
483- break;
484- }
485- }
486- }
487-
488- if (children)
489- XFree (children);
490-
491- if (!xev.above)
492- xev.above = (window->serverPrev) ? ROOTPARENT (window->serverPrev) : None;
493-
494- xev.override_redirect = priv->attrib.override_redirect;
495-
496- }
497-
498- compiz::X11::PendingEvent::Ptr pc =
499- boost::shared_static_cast<compiz::X11::PendingEvent> (compiz::X11::PendingConfigureEvent::Ptr (
500- new compiz::X11::PendingConfigureEvent (
501- screen->dpy (), serverFrame, valueMask, &xwc)));
502-
503- pendingConfigures.add (pc);
504- if (priv->mClearCheckTimeout.active ())
505- priv->mClearCheckTimeout.stop ();
506- priv->mClearCheckTimeout.start (boost::bind (&PrivateWindow::checkClear, priv),
507- 2000, 2500);
508-
509- XSendEvent (screen->dpy (), screen->root (), false,
510- SubstructureNotifyMask, (XEvent *) &xev);
511-
512- XUngrabServer (screen->dpy ());
513- XSync (screen->dpy (), false);
514- }
515- else
516- {
517- compiz::X11::PendingEvent::Ptr pc =
518- boost::shared_static_cast<compiz::X11::PendingEvent> (compiz::X11::PendingConfigureEvent::Ptr (
519- new compiz::X11::PendingConfigureEvent (
520- screen->dpy (), serverFrame, valueMask, &xwc)));
521-
522- pendingConfigures.add (pc);
523- if (priv->mClearCheckTimeout.active ())
524- priv->mClearCheckTimeout.stop ();
525- priv->mClearCheckTimeout.start (boost::bind (&PrivateWindow::checkClear, priv),
526- 2000, 2500);
527- XConfigureWindow (screen->dpy (), serverFrame, valueMask, &xwc);
528- }
529-
530- if (shaded)
531- {
532- XUnmapWindow (screen->dpy (), wrapper);
533- }
534- else
535- {
536- XMapWindow (screen->dpy (), wrapper);
537- XMoveResizeWindow (screen->dpy (), wrapper, serverInput.left, serverInput.top,
538- serverGeometry.width (), serverGeometry.height ());
539- }
540- XMoveResizeWindow (screen->dpy (), id, 0, 0,
541- serverGeometry.width (), serverGeometry.height ());
542- window->sendConfigureNotify ();
543- window->windowNotify (CompWindowNotifyFrameUpdate);
544- }
545- else
546- {
547- int bw = serverGeometry.border () * 2;
548-
549- xwc.x = serverGeometry.x ();
550- xwc.y = serverGeometry.y ();
551- xwc.width = serverGeometry.width () + bw;
552-
553- /* FIXME: It doesn't make much sense to allow undecorated windows to be
554- * shaded */
555- if (shaded)
556- xwc.height = bw;
557- else
558- xwc.height = serverGeometry.height () + bw;
559-
560- if (serverFrameGeometry.x () == xwc.x)
561- valueMask &= ~(CWX);
562- else
563- serverFrameGeometry.setX (xwc.x);
564-
565- if (serverFrameGeometry.y () == xwc.y)
566- valueMask &= ~(CWY);
567- else
568- serverFrameGeometry.setY (xwc.y);
569-
570- if (serverFrameGeometry.width () == xwc.width)
571- valueMask &= ~(CWWidth);
572- else
573- serverFrameGeometry.setWidth (xwc.width);
574-
575- if (serverFrameGeometry.height () == xwc.height)
576- valueMask &= ~(CWHeight);
577- else
578- serverFrameGeometry.setHeight (xwc.height);
579-
580- /* Geometry is the same, so we're not going to get a ConfigureNotify
581- * event when the window is configured, which means that other plugins
582- * won't know that the client, frame and wrapper windows got shifted
583- * around (and might result in display corruption, eg in OpenGL */
584- if (valueMask == 0)
585- {
586- XConfigureEvent xev;
587- XWindowAttributes attrib;
588- unsigned int nchildren = 0;
589- Window rootRet = 0, parentRet = 0;
590- Window *children = NULL;
591-
592- xev.type = ConfigureNotify;
593- xev.event = screen->root ();
594- xev.window = priv->serverFrame;
595-
596- XGrabServer (screen->dpy ());
597-
598- if (XGetWindowAttributes (screen->dpy (), priv->serverFrame, &attrib))
599- {
600- xev.x = attrib.x;
601- xev.y = attrib.y;
602- xev.width = attrib.width;
603- xev.height = attrib.height;
604- xev.border_width = attrib.border_width;
605- xev.above = None;
606-
607- /* We need to ensure that the stacking order is
608- * based on the current server stacking order so
609- * find the sibling to this window's frame in the
610- * server side stack and stack above that */
611- XQueryTree (screen->dpy (), screen->root (), &rootRet, &parentRet, &children, &nchildren);
612-
613- if (nchildren)
614- {
615- for (unsigned int i = 0; i < nchildren; i++)
616- {
617- if (i + 1 == nchildren ||
618- children[i + 1] == ROOTPARENT (window))
619- {
620- xev.above = children[i];
621- break;
622- }
623- }
624- }
625-
626- if (children)
627- XFree (children);
628-
629- if (!xev.above)
630- xev.above = (window->serverPrev) ? ROOTPARENT (window->serverPrev) : None;
631-
632- xev.override_redirect = priv->attrib.override_redirect;
633-
634- }
635-
636- compiz::X11::PendingEvent::Ptr pc =
637- boost::shared_static_cast<compiz::X11::PendingEvent> (compiz::X11::PendingConfigureEvent::Ptr (
638- new compiz::X11::PendingConfigureEvent (
639- screen->dpy (), serverFrame, valueMask, &xwc)));
640-
641- pendingConfigures.add (pc);
642- if (priv->mClearCheckTimeout.active ())
643- priv->mClearCheckTimeout.stop ();
644- priv->mClearCheckTimeout.start (boost::bind (&PrivateWindow::checkClear, priv),
645- 2000, 2500);
646-
647- XSendEvent (screen->dpy (), screen->root (), false,
648- SubstructureNotifyMask, (XEvent *) &xev);
649-
650- XUngrabServer (screen->dpy ());
651- XSync (screen->dpy (), false);
652- }
653- else
654- {
655- compiz::X11::PendingEvent::Ptr pc =
656- boost::shared_static_cast<compiz::X11::PendingEvent> (compiz::X11::PendingConfigureEvent::Ptr (
657- new compiz::X11::PendingConfigureEvent (
658- screen->dpy (), serverFrame, valueMask, &xwc)));
659-
660- pendingConfigures.add (pc);
661- if (priv->mClearCheckTimeout.active ())
662- priv->mClearCheckTimeout.stop ();
663- priv->mClearCheckTimeout.start (boost::bind (&PrivateWindow::checkClear, priv),
664- 2000, 2500);
665-
666- XConfigureWindow (screen->dpy (), serverFrame, valueMask, &xwc);
667- }
668-
669- if (shaded)
670- {
671- XUnmapWindow (screen->dpy (), wrapper);
672- }
673- else
674- {
675- XMapWindow (screen->dpy (), wrapper);
676- XMoveResizeWindow (screen->dpy (), wrapper, 0, 0,
677- serverGeometry.width (), serverGeometry.height ());
678- }
679-
680- XMoveResizeWindow (screen->dpy (), id, 0, 0,
681- serverGeometry.width (), serverGeometry.height ());
682- window->sendConfigureNotify ();
683- window->windowNotify (CompWindowNotifyFrameUpdate);
684- }
685+ xwc.x = serverGeometry.x ();
686+ xwc.y = serverGeometry.y ();
687+ xwc.width = serverGeometry.width ();
688+ xwc.height = serverGeometry.height ();
689+ xwc.border_width = serverGeometry.border ();
690+
691+ window->configureXWindow (valueMask, &xwc);
692+ window->windowNotify (CompWindowNotifyFrameUpdate);
693 window->recalcActions ();
694 }
695
696@@ -1136,8 +858,8 @@
697
698 for (unsigned int i = 0; i < n; i++)
699 {
700- x1 = rects[i].x + priv->geometry.border ();
701- y1 = rects[i].y + priv->geometry.border ();
702+ x1 = rects[i].x + priv->serverGeometry.border ();
703+ y1 = rects[i].y + priv->serverGeometry.border ();
704 x2 = x1 + rects[i].width;
705 y2 = y1 + rects[i].height;
706
707@@ -1145,17 +867,17 @@
708 x1 = 0;
709 if (y1 < 0)
710 y1 = 0;
711- if (x2 > priv->width)
712- x2 = priv->width;
713- if (y2 > priv->height)
714- y2 = priv->height;
715+ if (x2 > priv->serverGeometry.width ())
716+ x2 = priv->serverGeometry.height ();
717+ if (y2 > priv->serverGeometry.width ())
718+ y2 = priv->serverGeometry.height ();
719
720 if (y1 < y2 && x1 < x2)
721 {
722- x1 += priv->geometry.x ();
723- y1 += priv->geometry.y ();
724- x2 += priv->geometry.x ();
725- y2 += priv->geometry.y ();
726+ x1 += priv->serverGeometry.x ();
727+ y1 += priv->serverGeometry.y ();
728+ x2 += priv->serverGeometry.x ();
729+ y2 += priv->serverGeometry.y ();
730
731 ret += CompRect (x1, y1, x2 - x1, y2 - y1);
732 }
733@@ -1183,17 +905,19 @@
734 {
735 int order;
736
737+ /* We should update the server here */
738+ XSync (screen->dpy (), false);
739+
740 boundingShapeRects = XShapeGetRectangles (screen->dpy (), priv->id,
741 ShapeBounding, &nBounding, &order);
742 inputShapeRects = XShapeGetRectangles (screen->dpy (), priv->id,
743 ShapeInput, &nInput, &order);
744-
745 }
746
747- r.x = -priv->geometry.border ();
748- r.y = -priv->geometry.border ();
749- r.width = priv->width + priv->geometry.border ();
750- r.height = priv->height + priv->geometry.border ();
751+ r.x = -priv->serverGeometry.border ();
752+ r.y = -priv->serverGeometry.border ();
753+ r.width = priv->serverGeometry.width () + priv->serverGeometry.border ();
754+ r.height = priv->serverGeometry.height () + priv->serverGeometry.border ();
755
756 if (nBounding < 1)
757 {
758@@ -1726,7 +1450,7 @@
759 priv->attrib.map_state = IsUnmapped;
760 priv->invisible = true;
761
762- if (priv->shaded && priv->height)
763+ if (priv->shaded)
764 {
765 priv->updateFrameWindow ();
766 }
767@@ -1794,7 +1518,7 @@
768 }
769
770 bool
771-CompWindow::resize (XWindowAttributes attr)
772+CompWindow::resize (const XWindowAttributes &attr)
773 {
774 return resize (Geometry (attr.x, attr.y, attr.width, attr.height,
775 attr.border_width));
776@@ -1811,7 +1535,7 @@
777 }
778
779 bool
780-CompWindow::resize (CompWindow::Geometry gm)
781+PrivateWindow::resize (const CompWindow::Geometry &gm)
782 {
783 /* Input extents are now the last thing sent
784 * from the server. This might not work in some
785@@ -1828,12 +1552,8 @@
786 priv->geometry.height () != gm.height () ||
787 priv->geometry.border () != gm.border ())
788 {
789- int pw, ph;
790 int dx, dy, dwidth, dheight;
791
792- pw = gm.width () + gm.border () * 2;
793- ph = gm.height () + gm.border () * 2;
794-
795 dx = gm.x () - priv->geometry.x ();
796 dy = gm.y () - priv->geometry.y ();
797 dwidth = gm.width () - priv->geometry.width ();
798@@ -1843,37 +1563,60 @@
799 gm.width (), gm.height (),
800 gm.border ());
801
802- priv->width = pw;
803- priv->height = ph;
804+ if (priv->attrib.override_redirect)
805+ {
806+ priv->serverGeometry = priv->geometry;
807+ priv->serverFrameGeometry = priv->frameGeometry;
808+ }
809
810 if (priv->mapNum)
811 priv->updateRegion ();
812
813- resizeNotify (dx, dy, dwidth, dheight);
814+ window->resizeNotify (dx, dy, dwidth, dheight);
815
816 priv->invisible = priv->isInvisible ();
817 }
818 else if (priv->geometry.x () != gm.x () || priv->geometry.y () != gm.y ())
819 {
820- int dx, dy;
821-
822- dx = gm.x () - priv->geometry.x ();
823- dy = gm.y () - priv->geometry.y ();
824-
825- priv->geometry.setX (gm.x ());
826- priv->geometry.setY (gm.y ());
827-
828- priv->region.translate (dx, dy);
829- priv->inputRegion.translate (dx, dy);
830- if (!priv->frameRegion.isEmpty ())
831- priv->frameRegion.translate (dx, dy);
832-
833- priv->invisible = priv->isInvisible ();
834-
835- moveNotify (dx, dy, true);
836+ move (gm.x () - priv->geometry.x (),
837+ gm.y () - priv->geometry.y (), true);
838 }
839
840- updateFrameRegion ();
841+ window->updateFrameRegion ();
842+
843+ return true;
844+}
845+
846+bool
847+PrivateWindow::resize (const XWindowAttributes &attr)
848+{
849+ return resize (CompWindow::Geometry (attr.x, attr.y, attr.width, attr.height,
850+ attr.border_width));
851+}
852+
853+bool
854+PrivateWindow::resize (int x,
855+ int y,
856+ int width,
857+ int height,
858+ int border)
859+{
860+ return resize (CompWindow::Geometry (x, y, width, height, border));
861+}
862+
863+bool
864+CompWindow::resize (const CompWindow::Geometry &gm)
865+{
866+ XWindowChanges xwc = XWINDOWCHANGES_INIT;
867+ unsigned int valueMask = CWX | CWY | CWWidth | CWHeight | CWBorderWidth;
868+
869+ xwc.x = gm.x ();
870+ xwc.y = gm.y ();
871+ xwc.width = gm.width ();
872+ xwc.height = gm.height ();
873+ xwc.border_width = gm.border ();
874+
875+ configureXWindow (valueMask, &xwc);
876
877 return true;
878 }
879@@ -2041,13 +1784,7 @@
880 ce->border_width);
881 else
882 {
883- if (ce->override_redirect)
884- {
885- priv->serverGeometry.set (ce->x, ce->y, ce->width, ce->height,
886- ce->border_width);
887- }
888-
889- window->resize (ce->x, ce->y, ce->width, ce->height, ce->border_width);
890+ resize (ce->x, ce->y, ce->width, ce->height, ce->border_width);
891 }
892
893 if (ce->event == screen->root ())
894@@ -2128,7 +1865,7 @@
895 if (priv->syncWait)
896 priv->syncGeometry.set (x, y, width, height, ce->border_width);
897 else
898- window->resize (x, y, width, height, ce->border_width);
899+ resize (x, y, width, height, ce->border_width);
900
901 if (priv->restack (ce->above))
902 priv->updatePassiveButtonGrabs ();
903@@ -2137,27 +1874,6 @@
904
905 if (above)
906 above->priv->updatePassiveButtonGrabs ();
907-
908- if (!pendingConfigures.pending ())
909- {
910- /* Tell plugins its ok to start doing stupid things again but
911- * obviously FIXME */
912- CompOption::Vector options;
913- CompOption::Value v;
914-
915- options.push_back (CompOption ("window", CompOption::TypeInt));
916- v.set ((int) id);
917- options.back ().set (v);
918- options.push_back (CompOption ("active", CompOption::TypeInt));
919- v.set ((int) 0);
920- options.back ().set (v);
921-
922- /* Notify other plugins that it is unsafe to change geometry or serverGeometry
923- * FIXME: That API should not be accessible to plugins, this is a hack to avoid
924- * breaking ABI */
925-
926- screen->handleCompizEvent ("core", "lock_position", options);
927- }
928 }
929
930 void
931@@ -2180,23 +1896,34 @@
932 {
933 if (dx || dy)
934 {
935- gettimeofday (&priv->lastGeometryUpdate, NULL);
936-
937- /* Don't allow window movement to overwrite working geometries
938- * last received from the server if we know there are pending
939- * ConfigureNotify events on this window. That's a clunky workaround
940- * and a FIXME in any case, however, until we can break the API
941- * and remove CompWindow::move, this will need to be the case */
942-
943- if (!priv->pendingConfigures.pending ())
944+ XWindowChanges xwc = XWINDOWCHANGES_INIT;
945+ unsigned int valueMask = CWX | CWY;
946+
947+ xwc.x = priv->serverGeometry.x () + dx;
948+ xwc.y = priv->serverGeometry.y () + dy;
949+
950+ priv->nextMoveImmediate = immediate;
951+
952+ configureXWindow (valueMask, &xwc);
953+ }
954+}
955+
956+void
957+PrivateWindow::move (int dx,
958+ int dy,
959+ bool immediate)
960+{
961+ if (dx || dy)
962+ {
963+ priv->geometry.setX (priv->geometry.x () + dx);
964+ priv->geometry.setY (priv->geometry.y () + dy);
965+ priv->frameGeometry.setX (priv->frameGeometry.x () + dx);
966+ priv->frameGeometry.setY (priv->frameGeometry.y () + dy);
967+
968+ if (priv->attrib.override_redirect)
969 {
970- priv->geometry.setX (priv->geometry.x () + dx);
971- priv->geometry.setY (priv->geometry.y () + dy);
972- priv->frameGeometry.setX (priv->frameGeometry.x () + dx);
973- priv->frameGeometry.setY (priv->frameGeometry.y () + dy);
974-
975- priv->pendingPositionUpdates = true;
976-
977+ priv->serverGeometry = priv->geometry;
978+ priv->serverFrameGeometry = priv->frameGeometry;
979 priv->region.translate (dx, dy);
980 priv->inputRegion.translate (dx, dy);
981 if (!priv->frameRegion.isEmpty ())
982@@ -2204,19 +1931,7 @@
983
984 priv->invisible = priv->isInvisible ();
985
986- moveNotify (dx, dy, immediate);
987- }
988- else
989- {
990- XWindowChanges xwc = XWINDOWCHANGES_INIT;
991- unsigned int valueMask = CWX | CWY;
992- compLogMessage ("core", CompLogLevelDebug, "pending configure notifies on 0x%x, "\
993- "moving window asyncrhonously!", (unsigned int) priv->serverId);
994-
995- xwc.x = priv->serverGeometry.x () + dx;
996- xwc.y = priv->serverGeometry.y () + dy;
997-
998- configureXWindow (valueMask, &xwc);
999+ window->moveNotify (dx, dy, true);
1000 }
1001 }
1002 }
1003@@ -2227,22 +1942,6 @@
1004 return !mEvents.empty ();
1005 }
1006
1007-bool
1008-PrivateWindow::checkClear ()
1009-{
1010- if (pendingConfigures.pending ())
1011- {
1012- /* FIXME: This is a hack to avoid performance regressions
1013- * and must be removed in 0.9.6 */
1014- compLogMessage ("core", CompLogLevelWarn, "failed to receive ConfigureNotify event on 0x%x\n",
1015- id);
1016- pendingConfigures.dump ();
1017- pendingConfigures.clear ();
1018- }
1019-
1020- return false;
1021-}
1022-
1023 void
1024 compiz::X11::PendingEventQueue::add (PendingEvent::Ptr p)
1025 {
1026@@ -2464,21 +2163,6 @@
1027 mValueMask (valueMask),
1028 mXwc (*xwc)
1029 {
1030- CompOption::Vector options;
1031- CompOption::Value v;
1032-
1033- options.push_back (CompOption ("window", CompOption::TypeInt));
1034- v.set ((int) w);
1035- options.back ().set (v);
1036- options.push_back (CompOption ("active", CompOption::TypeInt));
1037- v.set ((int) 1);
1038- options.back ().set (v);
1039-
1040- /* Notify other plugins that it is unsafe to change geometry or serverGeometry
1041- * FIXME: That API should not be accessible to plugins, this is a hack to avoid
1042- * breaking ABI */
1043-
1044- screen->handleCompizEvent ("core", "lock_position", options);
1045 }
1046
1047 compiz::X11::PendingConfigureEvent::~PendingConfigureEvent ()
1048@@ -2488,57 +2172,6 @@
1049 void
1050 CompWindow::syncPosition ()
1051 {
1052- gettimeofday (&priv->lastConfigureRequest, NULL);
1053-
1054- unsigned int valueMask = CWX | CWY;
1055- XWindowChanges xwc = XWINDOWCHANGES_INIT;
1056-
1057- if (priv->pendingPositionUpdates && !priv->pendingConfigures.pending ())
1058- {
1059- if (priv->serverFrameGeometry.x () == priv->frameGeometry.x ())
1060- valueMask &= ~(CWX);
1061- if (priv->serverFrameGeometry.y () == priv->frameGeometry.y ())
1062- valueMask &= ~(CWY);
1063-
1064- /* Because CompWindow::move can update the geometry last
1065- * received from the server, we must indicate that no values
1066- * changed, because when the ConfigureNotify comes around
1067- * the values are going to be the same. That's obviously
1068- * broken behaviour and worthy of a FIXME, but requires
1069- * larger changes to the window movement system. */
1070- if (valueMask)
1071- {
1072- priv->serverGeometry.setX (priv->geometry.x ());
1073- priv->serverGeometry.setY (priv->geometry.y ());
1074- priv->serverFrameGeometry.setX (priv->frameGeometry.x ());
1075- priv->serverFrameGeometry.setY (priv->frameGeometry.y ());
1076-
1077- xwc.x = priv->serverFrameGeometry.x ();
1078- xwc.y = priv->serverFrameGeometry.y ();
1079-
1080- compiz::X11::PendingEvent::Ptr pc =
1081- boost::shared_static_cast<compiz::X11::PendingEvent> (compiz::X11::PendingConfigureEvent::Ptr (
1082- new compiz::X11::PendingConfigureEvent (
1083- screen->dpy (), priv->serverFrame, 0, &xwc)));
1084-
1085- priv->pendingConfigures.add (pc);
1086-
1087- /* Got 3 seconds to get its stuff together */
1088- if (priv->mClearCheckTimeout.active ())
1089- priv->mClearCheckTimeout.stop ();
1090- priv->mClearCheckTimeout.start (boost::bind (&PrivateWindow::checkClear, priv),
1091- 2000, 2500);
1092- XConfigureWindow (screen->dpy (), ROOTPARENT (this), valueMask, &xwc);
1093-
1094- if (priv->serverFrame)
1095- {
1096- XMoveWindow (screen->dpy (), priv->wrapper,
1097- priv->serverInput.left, priv->serverInput.top);
1098- sendConfigureNotify ();
1099- }
1100- }
1101- priv->pendingPositionUpdates = false;
1102- }
1103 }
1104
1105 bool
1106@@ -3360,14 +2993,17 @@
1107 {
1108 unsigned int frameValueMask = 0;
1109
1110- /* Immediately sync window position
1111- * if plugins were updating w->geometry () directly
1112- * in order to avoid a race condition */
1113-
1114- window->syncPosition ();
1115-
1116 /* Remove redundant bits */
1117
1118+ xwc->x = valueMask & CWX ? xwc->x : serverGeometry.x ();
1119+ xwc->y = valueMask & CWY ? xwc->y : serverGeometry.y ();
1120+ xwc->width = valueMask & CWWidth ? xwc->width : serverGeometry.width ();
1121+ xwc->height = valueMask & CWHeight ? xwc->height : serverGeometry.height ();
1122+ xwc->border_width = valueMask & CWBorderWidth ? xwc->border_width : serverGeometry.border ();
1123+
1124+ /* FIXME: This is a total fallacy for the reparenting case
1125+ * at least since the client doesn't actually move here, it only
1126+ * moves within the frame */
1127 if (valueMask & CWX && serverGeometry.x () == xwc->x)
1128 valueMask &= ~(CWX);
1129
1130@@ -3434,18 +3070,15 @@
1131 compLogMessage ("core", CompLogLevelWarn, "restack_mode not Above");
1132 }
1133
1134- frameValueMask = valueMask;
1135+ frameValueMask = CWX | CWY | CWWidth | CWHeight | (valueMask & (CWStackMode | CWSibling));
1136
1137- if (frameValueMask & CWX &&
1138- serverFrameGeometry.x () == xwc->x - serverGeometry.border () - serverInput.left)
1139+ if (serverFrameGeometry.x () == xwc->x - serverGeometry.border () - serverInput.left)
1140 frameValueMask &= ~(CWX);
1141
1142- if (frameValueMask & CWY &&
1143- serverFrameGeometry.y () == xwc->y - serverGeometry.border () - serverInput.top)
1144+ if (serverFrameGeometry.y () == xwc->y - serverGeometry.border () - serverInput.top)
1145 frameValueMask &= ~(CWY);
1146
1147- if (frameValueMask & CWWidth &&
1148- serverFrameGeometry.width () == xwc->width + serverGeometry.border () * 2
1149+ if (serverFrameGeometry.width () == xwc->width + serverGeometry.border () * 2
1150 + serverInput.left + serverInput.right)
1151 frameValueMask &= ~(CWWidth);
1152
1153@@ -3455,15 +3088,13 @@
1154
1155 if (shaded)
1156 {
1157- if (frameValueMask & CWHeight &&
1158- serverFrameGeometry.height () == serverGeometry.border () * 2
1159+ if (serverFrameGeometry.height () == serverGeometry.border () * 2
1160 + serverInput.top + serverInput.bottom)
1161 frameValueMask &= ~(CWHeight);
1162 }
1163 else
1164 {
1165- if (frameValueMask & CWHeight &&
1166- serverFrameGeometry.height () == xwc->height + serverGeometry.border () * 2
1167+ if (serverFrameGeometry.height () == xwc->height + serverGeometry.border () * 2
1168 + serverInput.top + serverInput.bottom)
1169 frameValueMask &= ~(CWHeight);
1170 }
1171@@ -3493,12 +3124,9 @@
1172 serverFrameGeometry.setHeight (xwc->height + serverGeometry.border () * 2
1173 + serverInput.top + serverInput.bottom);
1174 }
1175-
1176-
1177+
1178 if (serverFrame)
1179 {
1180- gettimeofday (&lastConfigureRequest, NULL);
1181-
1182 if (frameValueMask)
1183 {
1184 XWindowChanges wc = *xwc;
1185@@ -3514,15 +3142,20 @@
1186 screen->dpy (), priv->serverFrame, frameValueMask, &wc)));
1187
1188 pendingConfigures.add (pc);
1189- if (priv->mClearCheckTimeout.active ())
1190- priv->mClearCheckTimeout.stop ();
1191- priv->mClearCheckTimeout.start (boost::bind (&PrivateWindow::checkClear, priv),
1192- 2000, 2500);
1193
1194 XConfigureWindow (screen->dpy (), serverFrame, frameValueMask, &wc);
1195 }
1196+
1197 valueMask &= ~(CWSibling | CWStackMode);
1198
1199+ /* If the frame has changed position (eg, serverInput.top
1200+ * or serverInput.left have changed) then we also need to
1201+ * update the client and wrapper position */
1202+ if (!(valueMask & CWX))
1203+ valueMask |= frameValueMask & CWX;
1204+ if (!(valueMask & CWY))
1205+ valueMask |= frameValueMask & CWY;
1206+
1207 if (valueMask)
1208 {
1209 xwc->x = serverInput.left;
1210@@ -3710,6 +3343,9 @@
1211 CompWindow::configureXWindow (unsigned int valueMask,
1212 XWindowChanges *xwc)
1213 {
1214+ int dx = valueMask & CWX ? xwc->x - priv->serverGeometry.x () : 0;
1215+ int dy = valueMask & CWY ? xwc->y - priv->serverGeometry.y () : 0;
1216+
1217 if (priv->managed && (valueMask & (CWSibling | CWStackMode)))
1218 {
1219 CompWindowList transients;
1220@@ -3764,6 +3400,16 @@
1221 {
1222 priv->reconfigureXWindow (valueMask, xwc);
1223 }
1224+
1225+ if (!overrideRedirect () && (dx || dy))
1226+ {
1227+ priv->region.translate (dx, dy);
1228+ priv->inputRegion.translate (dx, dy);
1229+ if (!priv->frameRegion.isEmpty ())
1230+ priv->frameRegion.translate (dx, dy);
1231+ moveNotify (dx, dy, priv->nextMoveImmediate);
1232+ priv->nextMoveImmediate = true;
1233+ }
1234 }
1235
1236 int
1237@@ -4315,10 +3961,6 @@
1238 screen->dpy (), serverFrame, valueMask, &lxwc)));
1239
1240 pendingConfigures.add (pc);
1241- if (priv->mClearCheckTimeout.active ())
1242- priv->mClearCheckTimeout.stop ();
1243- priv->mClearCheckTimeout.start (boost::bind (&PrivateWindow::checkClear, priv),
1244- 2000, 2500);
1245 }
1246
1247 /* Below with no sibling puts the window at the bottom
1248@@ -5489,7 +5131,7 @@
1249 m = priv->geometry.x () + offX;
1250 if (m - priv->input.left < (int) s->width () - vWidth)
1251 rv.setX (offX + vWidth);
1252- else if (m + priv->width + priv->input.right > vWidth)
1253+ else if (m + priv->geometry.width () + priv->input.right > vWidth)
1254 rv.setX (offX - vWidth);
1255 else
1256 rv.setX (offX);
1257@@ -5504,7 +5146,7 @@
1258 m = priv->geometry.y () + offY;
1259 if (m - priv->input.top < (int) s->height () - vHeight)
1260 rv.setY (offY + vHeight);
1261- else if (m + priv->height + priv->input.bottom > vHeight)
1262+ else if (m + priv->geometry.height () + priv->input.bottom > vHeight)
1263 rv.setY (offY - vHeight);
1264 else
1265 rv.setY (offY);
1266@@ -6080,7 +5722,7 @@
1267
1268 if (m - priv->output.left < (int) screen->width () - vWidth)
1269 wx = tx + vWidth;
1270- else if (m + priv->width + priv->output.right > vWidth)
1271+ else if (m + priv->geometry.width () + priv->output.right > vWidth)
1272 wx = tx - vWidth;
1273 }
1274
1275@@ -6090,7 +5732,7 @@
1276
1277 if (m - priv->output.top < (int) screen->height () - vHeight)
1278 wy = ty + vHeight;
1279- else if (m + priv->height + priv->output.bottom > vHeight)
1280+ else if (m + priv->geometry.height () + priv->output.bottom > vHeight)
1281 wy = ty - vHeight;
1282 }
1283
1284@@ -6316,9 +5958,6 @@
1285 priv->serverFrameGeometry = priv->frameGeometry = priv->syncGeometry
1286 = priv->geometry = priv->serverGeometry;
1287
1288- priv->width = priv->attrib.width + priv->attrib.border_width * 2;
1289- priv->height = priv->attrib.height + priv->attrib.border_width * 2;
1290-
1291 priv->sizeHints.flags = 0;
1292
1293 priv->recalcNormalHints ();
1294@@ -6340,8 +5979,7 @@
1295
1296 if (priv->attrib.c_class != InputOnly)
1297 {
1298- priv->region = CompRegion (priv->attrib.x, priv->attrib.y,
1299- priv->width, priv->height);
1300+ priv->region = CompRegion (static_cast <CompRect> (priv->serverGeometry));
1301 priv->inputRegion = priv->region;
1302
1303 /* need to check for DisplayModal state on all windows */
1304@@ -6586,8 +6224,6 @@
1305 hints (NULL),
1306 inputHint (true),
1307 alpha (false),
1308- width (0),
1309- height (0),
1310 region (),
1311 wmType (0),
1312 type (CompWindowTypeUnknownMask),
1313@@ -6622,7 +6258,6 @@
1314 pendingUnmaps (0),
1315 pendingMaps (0),
1316 pendingConfigures (screen->dpy ()),
1317- pendingPositionUpdates (false),
1318
1319 startupId (0),
1320 resName (0),
1321@@ -6824,8 +6459,8 @@
1322 priv->frameRegion &= CompRegion (r);
1323 }
1324
1325- x = priv->geometry.x () - priv->input.left;
1326- y = priv->geometry.y () - priv->input.top;
1327+ x = priv->serverGeometry.x () - priv->input.left;
1328+ y = priv->serverGeometry.y () - priv->input.top;
1329
1330 XShapeCombineRegion (screen->dpy (), priv->serverFrame,
1331 ShapeBounding, -x, -y,
1332@@ -6866,6 +6501,11 @@
1333
1334 priv->updateSize ();
1335 priv->updateFrameWindow ();
1336+
1337+ /* Always send a moveNotify
1338+ * whenever the frame extents update
1339+ * so that plugins can re-position appropriately */
1340+ moveNotify (0, 0, true);
1341 }
1342
1343 /* Use b for _NET_WM_FRAME_EXTENTS here because
1344
1345=== modified file 'src/windowgeometry.cpp'
1346--- src/windowgeometry.cpp 2012-01-23 05:44:19 +0000
1347+++ src/windowgeometry.cpp 2012-02-25 14:51:17 +0000
1348@@ -61,22 +61,22 @@
1349 int
1350 CompWindow::width () const
1351 {
1352- return priv->width +
1353+ return priv->geometry.width () +
1354 priv->geometry.border () * 2;
1355 }
1356
1357 int
1358 CompWindow::height () const
1359 {
1360- return priv->height +
1361+ return priv->geometry.height () +
1362 priv->geometry.border () * 2;;
1363 }
1364
1365 CompSize
1366 CompWindow::size () const
1367 {
1368- return CompSize (priv->width + priv->geometry.border () * 2,
1369- priv->height + priv->geometry.border () * 2);
1370+ return CompSize (priv->geometry.width () + priv->geometry.border () * 2,
1371+ priv->geometry.height () + priv->geometry.border () * 2);
1372 }
1373
1374 int

Subscribers

People subscribed via source and target branches