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

Proposed by Sam Spilsbury on 2012-03-31
Status: Merged
Merged at revision: 3086
Proposed branch: lp:~smspillaz/compiz-core/compiz-core.fix_969101
Merge into: lp:compiz-core
Diff against target: 1083 lines (+784/-123)
8 files modified
plugins/decor/CMakeLists.txt (+5/-1)
plugins/decor/src/clip-groups/CMakeLists.txt (+62/-0)
plugins/decor/src/clip-groups/include/clip-groups.h (+100/-0)
plugins/decor/src/clip-groups/src/clip-groups.cpp (+76/-0)
plugins/decor/src/clip-groups/tests/CMakeLists.txt (+14/-0)
plugins/decor/src/clip-groups/tests/clip-groups/src/test-decor-clip-groups.cpp (+395/-0)
plugins/decor/src/decor.cpp (+92/-121)
plugins/decor/src/decor.h (+40/-1)
To merge this branch: bzr merge lp:~smspillaz/compiz-core/compiz-core.fix_969101
Reviewer Review Type Date Requested Status
Daniel van Vugt Approve on 2012-04-05
Alan Griffiths 2012-03-31 Approve on 2012-04-02
Review via email: mp+100270@code.launchpad.net

This proposal supersedes a proposal from 2012-03-30.

Description of the change

== Problem ==

See LP#969101 - DecorWindow::computeShadowRegion is called like 20000 times per 800 moves, which is bad and not necessary considering what it is used for. Also the implementation kinda sucked

== Solution ==

Refactor shadow clipping into a clip group and clippable interfaces. Clip groups represent a single "layer" for shadowing purposes where all the shadows are clipped against the union of the input rects of the windows belonging to that layer. Only update items in the clip group when a relevant one has changed.

MatchedDecorClipGroup etc provide a "guarded" entrance so that only relevant windows are let in.

== Tests ==

Unit tests included.

To post a comment you must log in.
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

The killer: tests are not added to "make test"

Also, pure virtual functions should be private (not protected).

And there's no point to making destructors pure => "virtual ~DecorClippableInterface () {}"

Comment only: I (and almost everyone except Herb) don't see the point of NVPI.

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

Tests OK. No obvious regressions.

However it does terrify me to be changing such a vast (and important) region of code so close to release. I can't retain confidence in the release if we merge this before precise, so I'll leave the decision to someone else.

review: Abstain
Daniel van Vugt (vanvugt) wrote :

Approved, on the condition that it doesn't get merged till 0.9.8.

Sam Spilsbury (smspillaz) wrote :

1) Its fully tested
2) It vastly improves performance
3) The vast majority of the diff size is in the tests
4) It improves code design
5) Its blocking another very important piece of code from landing

I think those are plenty good reasons to merge this /before/ precise

Took it for a test spin here and it works very well. Tried all sorts of weird window combinations with maximaized, semi-maximized, etc. All worked.

This branch seems to have as a side effect also fixing the bug where lingering shadow outlines would be briefly left behind when scrubbing panel app/indicator menus very fast. Only tested this on fast (sandybridge) hardware though, and not on my slower netbook. Is this correct or am I dreaming things up?

Alan Griffiths (alan-griffiths) wrote :

I agree with Sam that the "after" code is cleaner and that the majority of the diff is low risk (adding tests & build script).

I'd quibble with making destructors pure, and NVPI but it seems to run fine.

Cleaner, unit tested code... what's not to like?

review: Approve
Sam Spilsbury (smspillaz) wrote :

> Took it for a test spin here and it works very well. Tried all sorts of weird
> window combinations with maximaized, semi-maximized, etc. All worked.
>
> This branch seems to have as a side effect also fixing the bug where lingering
> shadow outlines would be briefly left behind when scrubbing panel
> app/indicator menus very fast. Only tested this on fast (sandybridge) hardware
> though, and not on my slower netbook. Is this correct or am I dreaming things
> up?

That is a bug in the animation plugin. Not sure what to do about it really.

Daniel van Vugt (vanvugt) wrote :

Confirmed this performance improvement seems to unblock the issue with:
lp:~smspillaz/compiz-core/compiz-core.work_923683

Daniel van Vugt (vanvugt) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/decor/CMakeLists.txt'
2--- plugins/decor/CMakeLists.txt 2011-01-25 00:31:51 +0000
3+++ plugins/decor/CMakeLists.txt 2012-03-31 03:35:24 +0000
4@@ -3,7 +3,11 @@
5 include (CompizPlugin)
6 include (CompizCommon)
7
8-compiz_plugin(decor PLUGINDEPS composite opengl LIBRARIES decoration)
9+include_directories (${CMAKE_CURRENT_SOURCE_DIR}/src/clip-groups/include/)
10+
11+compiz_plugin(decor PLUGINDEPS composite opengl LIBRARIES decoration compiz_decor_clip_groups)
12+
13+add_subdirectory (src/clip-groups)
14
15 if (COMPIZ_BUILD_WITH_RPATH AND NOT COMPIZ_DISABLE_PLUGIN_DECOR)
16
17
18=== added directory 'plugins/decor/src/clip-groups'
19=== added file 'plugins/decor/src/clip-groups/CMakeLists.txt'
20--- plugins/decor/src/clip-groups/CMakeLists.txt 1970-01-01 00:00:00 +0000
21+++ plugins/decor/src/clip-groups/CMakeLists.txt 2012-03-31 03:35:24 +0000
22@@ -0,0 +1,62 @@
23+pkg_check_modules (
24+ GLIBMM
25+ REQUIRED
26+ glibmm-2.4 glib-2.0
27+)
28+
29+INCLUDE_DIRECTORIES (
30+ ${CMAKE_CURRENT_SOURCE_DIR}/include
31+ ${CMAKE_CURRENT_SOURCE_DIR}/src
32+
33+ ${compiz_SOURCE_DIR}/src/point/include
34+ ${compiz_SOURCE_DIR}/src/rect/include
35+ ${compiz_SOURCE_DIR}/src/window/geometry/include
36+ ${compiz_SOURCE_DIR}/src/window/geometry-saver/include
37+ ${compiz_SOURCE_DIR}/src/window/extents/include
38+ ${compiz_SOURCE_DIR}/include
39+
40+ ${Boost_INCLUDE_DIRS}
41+
42+ ${GLIBMM_INCLUDE_DIRS}
43+)
44+
45+LINK_DIRECTORIES (${GLIBMM_LIBRARY_DIRS})
46+
47+SET (
48+ PUBLIC_HEADERS
49+)
50+
51+SET (
52+ PRIVATE_HEADERS
53+ ${CMAKE_CURRENT_SOURCE_DIR}/include/clip-groups.h
54+)
55+
56+SET(
57+ SRCS
58+ ${CMAKE_CURRENT_SOURCE_DIR}/src/clip-groups.cpp
59+)
60+
61+ADD_LIBRARY(
62+ compiz_decor_clip_groups STATIC
63+
64+ ${SRCS}
65+
66+ ${PUBLIC_HEADERS}
67+ ${PRIVATE_HEADERS}
68+)
69+
70+if (COMPIZ_BUILD_TESTING)
71+ADD_SUBDIRECTORY( ${CMAKE_CURRENT_SOURCE_DIR}/tests )
72+endif (COMPIZ_BUILD_TESTING)
73+
74+SET_TARGET_PROPERTIES(
75+ compiz_decor_clip_groups PROPERTIES
76+ PUBLIC_HEADER "${PUBLIC_HEADERS}"
77+)
78+
79+TARGET_LINK_LIBRARIES(
80+ compiz_decor_clip_groups
81+
82+ compiz_core
83+ ${GLIBMM_LIBRARIES}
84+)
85
86=== added directory 'plugins/decor/src/clip-groups/include'
87=== added file 'plugins/decor/src/clip-groups/include/clip-groups.h'
88--- plugins/decor/src/clip-groups/include/clip-groups.h 1970-01-01 00:00:00 +0000
89+++ plugins/decor/src/clip-groups/include/clip-groups.h 2012-03-31 03:35:24 +0000
90@@ -0,0 +1,100 @@
91+/*
92+ * Copyright (C) 2001 Havoc Pennington
93+ * Copyright (C) 2002, 2003 Red Hat, Inc.
94+ * Copyright (C) 2003 Rob Adams
95+ * Copyright (C) 2005 Novell, Inc.
96+ *
97+ * This program is free software; you can redistribute it and/or
98+ * modify it under the terms of the GNU General Public License as
99+ * published by the Free Software Foundation; either version 2 of the
100+ * License, or (at your option) any later version.
101+ *
102+ * This program is distributed in the hope that it will be useful, but
103+ * WITHOUT ANY WARRANTY; without even the implied warranty of
104+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
105+ * General Public License for more details.
106+ *
107+ * You should have received a copy of the GNU General Public License
108+ * along with this program; if not, write to the Free Software
109+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
110+ * 02111-1307, USA.
111+ */
112+
113+#ifndef _COMPIZ_DECOR_CLIP_GROUPS_H
114+#define _COMPIZ_DECOR_CLIP_GROUPS_H
115+
116+#include <vector>
117+#include <core/rect.h>
118+#include <core/region.h>
119+#include <core/match.h>
120+
121+namespace compiz
122+{
123+ namespace decor
124+ {
125+ class DecorClipGroupInterface;
126+
127+ class DecorClippableInterface
128+ {
129+ public:
130+
131+ virtual ~DecorClippableInterface () = 0;
132+ void updateShadow (const CompRegion &r) { doUpdateShadow (r); }
133+ void setOwner (DecorClipGroupInterface *i) { doSetOwner (i); }
134+ bool matches (const CompMatch &m) { return doMatches (m); }
135+ const CompRegion & outputRegion () { return getOutputRegion (); }
136+ const CompRegion & inputRegion () { return getInputRegion (); }
137+ void updateGroupShadows () { doUpdateGroupShadows (); }
138+
139+ private:
140+
141+ virtual void doUpdateShadow (const CompRegion &) = 0;
142+ virtual void doSetOwner (DecorClipGroupInterface *i) = 0;
143+ virtual bool doMatches (const CompMatch &m) = 0;
144+ virtual const CompRegion & getOutputRegion () = 0;
145+ virtual const CompRegion & getInputRegion () = 0;
146+ virtual void doUpdateGroupShadows () = 0;
147+ };
148+
149+ class DecorClipGroupInterface
150+ {
151+ public:
152+
153+ virtual ~DecorClipGroupInterface () = 0;
154+
155+ bool pushClippable (DecorClippableInterface *dc) { return doPushClippable (dc); }
156+ bool popClippable (DecorClippableInterface *dc) { return doPopClippable (dc); }
157+ void regenerateClipRegion () { doRegenerateClipRegion (); }
158+ const CompRegion & clipRegion () { return getClipRegion (); }
159+ void updateAllShadows () { return doUpdateAllShadows (); }
160+
161+ private:
162+
163+ virtual bool doPushClippable (DecorClippableInterface *dc) = 0;
164+ virtual bool doPopClippable (DecorClippableInterface *dc) = 0;
165+ virtual void doRegenerateClipRegion () = 0;
166+ virtual const CompRegion & getClipRegion () = 0;
167+ virtual void doUpdateAllShadows () = 0;
168+ };
169+
170+ namespace impl
171+ {
172+ class GenericDecorClipGroup :
173+ public DecorClipGroupInterface
174+ {
175+ private:
176+
177+ bool doPushClippable (DecorClippableInterface *dc);
178+ bool doPopClippable (DecorClippableInterface *dc);
179+ void doRegenerateClipRegion ();
180+ const CompRegion & getClipRegion ();
181+ void doUpdateAllShadows ();
182+
183+ std::vector <DecorClippableInterface *> mClippables;
184+ CompRegion mRegion;
185+ };
186+ }
187+ }
188+}
189+
190+#endif
191
192=== added directory 'plugins/decor/src/clip-groups/src'
193=== added file 'plugins/decor/src/clip-groups/src/clip-groups.cpp'
194--- plugins/decor/src/clip-groups/src/clip-groups.cpp 1970-01-01 00:00:00 +0000
195+++ plugins/decor/src/clip-groups/src/clip-groups.cpp 2012-03-31 03:35:24 +0000
196@@ -0,0 +1,76 @@
197+#include "clip-groups.h"
198+#include <boost/foreach.hpp>
199+#include <algorithm>
200+
201+#ifndef foreach
202+#define foreach BOOST_FOREACH
203+#endif
204+
205+using namespace compiz::decor;
206+using namespace compiz::decor::impl;
207+
208+DecorClippableInterface::~DecorClippableInterface () {}
209+DecorClipGroupInterface::~DecorClipGroupInterface () {}
210+
211+bool
212+GenericDecorClipGroup::doPushClippable (DecorClippableInterface *dc)
213+{
214+ std::vector <DecorClippableInterface *>::iterator it = std::find (mClippables.begin (),
215+ mClippables.end (),
216+ dc);
217+
218+ if (it == mClippables.end ())
219+ {
220+ mClippables.push_back (dc);
221+ regenerateClipRegion ();
222+ dc->setOwner (this);
223+
224+ return true;
225+ }
226+
227+ return false;
228+}
229+
230+bool
231+GenericDecorClipGroup::doPopClippable (DecorClippableInterface *dc)
232+{
233+ std::vector <DecorClippableInterface *>::iterator it = std::find (mClippables.begin (),
234+ mClippables.end (),
235+ dc);
236+
237+ if (it != mClippables.end ())
238+ {
239+ dc->setOwner (NULL);
240+ dc->updateShadow (emptyRegion);
241+ mClippables.erase (it);
242+ regenerateClipRegion ();
243+
244+ return true;
245+ }
246+
247+ return false;
248+}
249+
250+void
251+GenericDecorClipGroup::doRegenerateClipRegion ()
252+{
253+ mRegion -= infiniteRegion;
254+
255+ foreach (DecorClippableInterface *clippable, mClippables)
256+ {
257+ mRegion += clippable->inputRegion ();
258+ }
259+}
260+
261+const CompRegion &
262+GenericDecorClipGroup::getClipRegion ()
263+{
264+ return mRegion;
265+}
266+
267+void
268+GenericDecorClipGroup::doUpdateAllShadows ()
269+{
270+ foreach (DecorClippableInterface *clippable, mClippables)
271+ clippable->updateShadow (mRegion);
272+}
273
274=== added directory 'plugins/decor/src/clip-groups/tests'
275=== added file 'plugins/decor/src/clip-groups/tests/CMakeLists.txt'
276--- plugins/decor/src/clip-groups/tests/CMakeLists.txt 1970-01-01 00:00:00 +0000
277+++ plugins/decor/src/clip-groups/tests/CMakeLists.txt 2012-03-31 03:35:24 +0000
278@@ -0,0 +1,14 @@
279+include_directories(${CMAKE_CURRENT_SOURCE_DIR})
280+
281+add_executable (compiz_test_decor_clip_groups
282+ ${CMAKE_CURRENT_SOURCE_DIR}/clip-groups/src/test-decor-clip-groups.cpp)
283+
284+target_link_libraries (compiz_test_decor_clip_groups
285+ compiz_decor_clip_groups
286+ ${GTEST_BOTH_LIBRARIES}
287+ ${GMOCK_LIBRARY}
288+ ${GMOCK_MAIN_LIBRARY}
289+ ${CMAKE_THREAD_LIBS_INIT} # Link in pthread.
290+ )
291+
292+gtest_add_tests (compiz_test_decor_clip_groups "" ${CMAKE_CURRENT_SOURCE_DIR}/clip-groups/src/test-decor-clip-groups.cpp)
293
294=== added directory 'plugins/decor/src/clip-groups/tests/clip-groups'
295=== added directory 'plugins/decor/src/clip-groups/tests/clip-groups/src'
296=== added file 'plugins/decor/src/clip-groups/tests/clip-groups/src/test-decor-clip-groups.cpp'
297--- plugins/decor/src/clip-groups/tests/clip-groups/src/test-decor-clip-groups.cpp 1970-01-01 00:00:00 +0000
298+++ plugins/decor/src/clip-groups/tests/clip-groups/src/test-decor-clip-groups.cpp 2012-03-31 03:35:24 +0000
299@@ -0,0 +1,395 @@
300+/*
301+ * Copyright © 2012 Canonical Ltd.
302+ *
303+ * Permission to use, copy, modify, distribute, and sell this software
304+ * and its documentation for any purpose is hereby granted without
305+ * fee, provided that the above copyright notice appear in all copies
306+ * and that both that copyright notice and this permission notice
307+ * appear in supporting documentation, and that the name of
308+ * Canonical Ltd. not be used in advertising or publicity pertaining to
309+ * distribution of the software without specific, written prior permission.
310+ * Canonical Ltd. makes no representations about the suitability of this
311+ * software for any purpose. It is provided "as is" without express or
312+ * implied warranty.
313+ *
314+ * CANONICAL, LTD. DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
315+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN
316+ * NO EVENT SHALL CANONICAL, LTD. BE LIABLE FOR ANY SPECIAL, INDIRECT OR
317+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
318+ * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
319+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
320+ * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
321+ *
322+ * Authored by: Sam Spilsbury <sam.spilsbury@canonical.com>
323+ */
324+
325+#include <gtest/gtest.h>
326+#include <gmock/gmock.h>
327+#include <iostream>
328+#include "clip-groups.h"
329+
330+using ::testing::Invoke;
331+using ::testing::_;
332+using ::testing::StrictMock;
333+
334+using namespace compiz::decor;
335+
336+class CompDecorClipGroupsTest :
337+ public ::testing::Test
338+{
339+};
340+
341+class FakeDecorClippable :
342+ public DecorClippableInterface
343+{
344+ public:
345+ FakeDecorClippable (const CompRegion &inputRegion,
346+ const CompRegion &outputRegion,
347+ DecorClippableInterface *parent,
348+ bool matches) :
349+ mInputRegion (inputRegion),
350+ mOutputRegion (outputRegion),
351+ mParent (parent),
352+ mOwner (NULL),
353+ mMatches (matches)
354+ {
355+ }
356+
357+ ~FakeDecorClippable ()
358+ {
359+ if (mOwner)
360+ mOwner->popClippable (mParent);
361+ }
362+
363+ const CompRegion &
364+ getShadowRegion ()
365+ {
366+ return mShadowRegion;
367+ }
368+
369+ private:
370+
371+ void doUpdateShadow (const CompRegion &r)
372+ {
373+ mShadowRegion = mOutputRegion.intersected (r - mInputRegion);
374+ }
375+
376+ void doSetOwner (DecorClipGroupInterface *i)
377+ {
378+ mOwner = i;
379+ }
380+
381+ bool doMatches (const CompMatch &m)
382+ {
383+ return mMatches;
384+ }
385+
386+ const CompRegion & getOutputRegion () { return mOutputRegion; }
387+ const CompRegion & getInputRegion () { return mInputRegion; }
388+
389+ void doUpdateGroupShadows ()
390+ {
391+ if (mOwner)
392+ mOwner->updateAllShadows ();
393+ }
394+
395+ CompRegion mInputRegion;
396+ CompRegion mOutputRegion;
397+
398+ CompRegion mShadowRegion;
399+
400+ DecorClippableInterface *mParent;
401+ DecorClipGroupInterface *mOwner;
402+
403+ bool mMatches;
404+};
405+
406+class MockDecorClippable :
407+ public DecorClippableInterface
408+{
409+ public:
410+
411+ MOCK_METHOD1 (doUpdateShadow, void (const CompRegion &));
412+ MOCK_METHOD1 (doSetOwner, void (DecorClipGroupInterface *));
413+ MOCK_METHOD1 (doMatches, bool (const CompMatch &));
414+ MOCK_METHOD0 (getOutputRegion, const CompRegion & ());
415+ MOCK_METHOD0 (getInputRegion, const CompRegion & ());
416+ MOCK_METHOD0 (doUpdateGroupShadows, void ());
417+
418+ void Delegate (DecorClippableInterface &other)
419+ {
420+ ON_CALL (*this, doUpdateShadow (_)).WillByDefault (Invoke (&other, &DecorClippableInterface::updateShadow));
421+ ON_CALL (*this, doSetOwner (_)).WillByDefault (Invoke (&other, &DecorClippableInterface::setOwner));
422+ ON_CALL (*this, doMatches (_)).WillByDefault (Invoke (&other, &DecorClippableInterface::matches));
423+ ON_CALL (*this, getOutputRegion ()).WillByDefault (Invoke (&other, &DecorClippableInterface::outputRegion));
424+ ON_CALL (*this, getInputRegion ()).WillByDefault (Invoke (&other, &DecorClippableInterface::inputRegion));
425+ ON_CALL (*this, doUpdateGroupShadows ()).WillByDefault (Invoke (&other, &DecorClippableInterface::updateGroupShadows));
426+ }
427+};
428+
429+void PrintTo(const CompRegion &reg, ::std::ostream *os)
430+{
431+ const CompRect &br = reg.boundingRect ();
432+ *os << "Bounding Rect " << br.x () << " " << br.y () << " " << br.width () << " " << br.height () << std::endl;
433+ const CompRect::vector &rv = reg.rects ();
434+ for (CompRect::vector::const_iterator it = rv.begin ();
435+ it != rv.end ();
436+ it++)
437+ {
438+ const CompRect &r = *it;
439+ *os << " - Rect : " << r.x () << " " << r.y () << " " << r.width () << " " << r.height () << std::endl;
440+ }
441+}
442+
443+TEST_F(CompDecorClipGroupsTest, TestPushClippable)
444+{
445+ impl::GenericDecorClipGroup cg;
446+ StrictMock <MockDecorClippable> mdc;
447+ FakeDecorClippable fdc (CompRegion (50, 50, 100, 100),
448+ CompRegion (0, 0, 100, 100),
449+ &mdc,
450+ true);
451+
452+ mdc.Delegate (fdc);
453+
454+ EXPECT_CALL (mdc, getInputRegion ());
455+ EXPECT_CALL (mdc, doSetOwner (_)).Times (2);
456+ EXPECT_CALL (mdc, doUpdateShadow (_));
457+
458+ cg.pushClippable (&mdc);
459+}
460+
461+TEST_F(CompDecorClipGroupsTest, TestPushClippableUpdatesRegion)
462+{
463+ impl::GenericDecorClipGroup cg;
464+ StrictMock <MockDecorClippable> mdc;
465+ FakeDecorClippable fdc (CompRegion (50, 50, 100, 100),
466+ CompRegion (0, 0, 100, 100),
467+ &mdc,
468+ true);
469+
470+ mdc.Delegate (fdc);
471+
472+ EXPECT_CALL (mdc, getInputRegion ()).Times (2);
473+ EXPECT_CALL (mdc, doSetOwner (_)).Times (2);
474+ EXPECT_CALL (mdc, doUpdateShadow (_));
475+
476+ cg.pushClippable (&mdc);
477+
478+ EXPECT_EQ (cg.clipRegion (), mdc.inputRegion ());
479+}
480+
481+MATCHER_P (CompRegionEq, other, "")
482+{
483+ return (arg ^ other).isEmpty ();
484+}
485+
486+TEST_F(CompDecorClipGroupsTest, TestPush2ClippableUpdatesRegion)
487+{
488+ impl::GenericDecorClipGroup cg;
489+ StrictMock <MockDecorClippable> mdc;
490+ FakeDecorClippable fdc (CompRegion (50, 50, 100, 100),
491+ CompRegion (0, 0, 100, 100),
492+ &mdc,
493+ true);
494+
495+ mdc.Delegate (fdc);
496+
497+ StrictMock <MockDecorClippable> mdc2;
498+ FakeDecorClippable fdc2 (CompRegion (250, 250, 100, 100),
499+ CompRegion (200, 200, 100, 100),
500+ &mdc2,
501+ true);
502+
503+ mdc2.Delegate (fdc2);
504+
505+ EXPECT_CALL (mdc, getInputRegion ()).Times (4);
506+ EXPECT_CALL (mdc, doSetOwner (&cg));
507+ EXPECT_CALL (mdc, doSetOwner (NULL));
508+ EXPECT_CALL (mdc, doUpdateShadow (_)).Times (1);
509+
510+ EXPECT_CALL (mdc2, getInputRegion ()).Times (2);
511+ EXPECT_CALL (mdc2, doSetOwner (&cg));
512+ EXPECT_CALL (mdc2, doSetOwner (NULL));
513+ EXPECT_CALL (mdc2, doUpdateShadow (_)).Times (1);
514+
515+ cg.pushClippable (&mdc);
516+ cg.pushClippable (&mdc2);
517+
518+ CompRegion accumulated;
519+
520+ accumulated += mdc.inputRegion ();
521+ accumulated += mdc2.inputRegion ();
522+
523+ EXPECT_THAT (cg.clipRegion (), CompRegionEq (accumulated));
524+}
525+
526+TEST_F(CompDecorClipGroupsTest, TestPush3ClippableUpdatesRegion)
527+{
528+ impl::GenericDecorClipGroup cg;
529+ StrictMock <MockDecorClippable> mdc;
530+ FakeDecorClippable fdc (CompRegion (50, 50, 100, 100),
531+ CompRegion (0, 0, 100, 100),
532+ &mdc,
533+ true);
534+
535+ mdc.Delegate (fdc);
536+
537+ StrictMock <MockDecorClippable> mdc2;
538+ FakeDecorClippable fdc2 (CompRegion (150, 150, 100, 100),
539+ CompRegion (100, 100, 100, 100),
540+ &mdc2,
541+ true);
542+
543+ mdc2.Delegate (fdc2);
544+
545+ StrictMock <MockDecorClippable> mdc3;
546+ FakeDecorClippable fdc3 (CompRegion (250, 250, 100, 100),
547+ CompRegion (200, 200, 100, 100),
548+ &mdc3,
549+ true);
550+
551+ mdc3.Delegate (fdc3);
552+
553+ EXPECT_CALL (mdc, getInputRegion ()).Times (6);
554+ EXPECT_CALL (mdc, doSetOwner (&cg));
555+ EXPECT_CALL (mdc, doSetOwner (NULL));
556+ EXPECT_CALL (mdc, doUpdateShadow (_)).Times (1);
557+
558+ EXPECT_CALL (mdc2, getInputRegion ()).Times (4);
559+ EXPECT_CALL (mdc2, doSetOwner (&cg));
560+ EXPECT_CALL (mdc2, doSetOwner (NULL));
561+ EXPECT_CALL (mdc2, doUpdateShadow (_)).Times (1);
562+
563+ EXPECT_CALL (mdc3, getInputRegion ()).Times (2);
564+ EXPECT_CALL (mdc3, doSetOwner (&cg));
565+ EXPECT_CALL (mdc3, doSetOwner (NULL));
566+ EXPECT_CALL (mdc3, doUpdateShadow (_)).Times (1);
567+
568+ cg.pushClippable (&mdc);
569+ cg.pushClippable (&mdc2);
570+ cg.pushClippable (&mdc3);
571+
572+ CompRegion accumulated;
573+
574+ accumulated += mdc.inputRegion ();
575+ accumulated += mdc2.inputRegion ();
576+ accumulated += mdc3.inputRegion ();
577+
578+ EXPECT_THAT (cg.clipRegion (), CompRegionEq (accumulated));
579+}
580+
581+TEST_F(CompDecorClipGroupsTest, TestPush2ClippableUpdatesShadow)
582+{
583+ impl::GenericDecorClipGroup cg;
584+ StrictMock <MockDecorClippable> mdc;
585+ FakeDecorClippable fdc (CompRegion (50, 50, 25, 25),
586+ CompRegion (25, 25, 100, 100),
587+ &mdc,
588+ true);
589+
590+ mdc.Delegate (fdc);
591+
592+ StrictMock <MockDecorClippable> mdc2;
593+ FakeDecorClippable fdc2 (CompRegion (75, 75, 50, 50),
594+ CompRegion (25, 25, 100, 100),
595+ &mdc2,
596+ true);
597+
598+ mdc2.Delegate (fdc2);
599+
600+ EXPECT_CALL (mdc, getInputRegion ()).Times (4);
601+ EXPECT_CALL (mdc, doSetOwner (&cg));
602+ EXPECT_CALL (mdc, doSetOwner (NULL));
603+ EXPECT_CALL (mdc, doUpdateShadow (_)).Times (2);
604+
605+ EXPECT_CALL (mdc2, getInputRegion ()).Times (2);
606+ EXPECT_CALL (mdc2, doSetOwner (&cg));
607+ EXPECT_CALL (mdc2, doSetOwner (NULL));
608+ EXPECT_CALL (mdc2, doUpdateShadow (_)).Times (2);
609+
610+ cg.pushClippable (&mdc);
611+ cg.pushClippable (&mdc2);
612+
613+ cg.updateAllShadows ();
614+
615+ CompRegion accumulated;
616+
617+ accumulated += mdc.inputRegion ();
618+ accumulated += mdc2.inputRegion ();
619+
620+ EXPECT_THAT (cg.clipRegion (), CompRegionEq (accumulated));
621+
622+ CompRegion fdcRegion (75, 75, 50, 50);
623+
624+ EXPECT_THAT (fdc.getShadowRegion (), CompRegionEq (fdcRegion));
625+
626+ CompRegion fdc2Region (50, 50, 25, 25);
627+
628+ EXPECT_THAT (fdc2.getShadowRegion (), CompRegionEq (fdc2Region));
629+}
630+
631+TEST_F(CompDecorClipGroupsTest, TestPush3ClippableUpdatesRegionPop1)
632+{
633+ impl::GenericDecorClipGroup cg;
634+ StrictMock <MockDecorClippable> mdc;
635+ FakeDecorClippable fdc (CompRegion (50, 50, 100, 100),
636+ CompRegion (0, 0, 100, 100),
637+ &mdc,
638+ true);
639+
640+ mdc.Delegate (fdc);
641+
642+ StrictMock <MockDecorClippable> mdc2;
643+ FakeDecorClippable fdc2 (CompRegion (150, 150, 100, 100),
644+ CompRegion (100, 100, 100, 100),
645+ &mdc2,
646+ true);
647+
648+ mdc2.Delegate (fdc2);
649+
650+ StrictMock <MockDecorClippable> mdc3;
651+ FakeDecorClippable fdc3 (CompRegion (250, 250, 100, 100),
652+ CompRegion (200, 200, 100, 100),
653+ &mdc3,
654+ true);
655+
656+ mdc3.Delegate (fdc3);
657+
658+ EXPECT_CALL (mdc, getInputRegion ()).Times (7);
659+ EXPECT_CALL (mdc, doSetOwner (&cg));
660+ EXPECT_CALL (mdc, doSetOwner (NULL));
661+ EXPECT_CALL (mdc, doUpdateShadow (_)).Times (1);
662+
663+ EXPECT_CALL (mdc2, getInputRegion ()).Times (5);
664+ EXPECT_CALL (mdc2, doSetOwner (&cg));
665+ EXPECT_CALL (mdc2, doSetOwner (NULL));
666+ EXPECT_CALL (mdc2, doUpdateShadow (_)).Times (1);
667+
668+ EXPECT_CALL (mdc3, getInputRegion ()).Times (2);
669+ EXPECT_CALL (mdc3, doSetOwner (&cg));
670+ EXPECT_CALL (mdc3, doSetOwner (NULL));
671+ EXPECT_CALL (mdc3, doUpdateShadow (_)).Times (1);
672+
673+ cg.pushClippable (&mdc);
674+ cg.pushClippable (&mdc2);
675+ cg.pushClippable (&mdc3);
676+
677+ CompRegion accumulated;
678+
679+ accumulated += mdc.inputRegion ();
680+ accumulated += mdc2.inputRegion ();
681+ accumulated += mdc3.inputRegion ();
682+
683+ EXPECT_THAT (cg.clipRegion (), CompRegionEq (accumulated));
684+
685+ cg.popClippable (&mdc3);
686+
687+ accumulated = CompRegion ();
688+
689+ accumulated += mdc.inputRegion ();
690+ accumulated += mdc2.inputRegion ();
691+
692+ EXPECT_THAT (cg.clipRegion (), CompRegionEq (accumulated));
693+}
694+
695
696=== modified file 'plugins/decor/src/decor.cpp'
697--- plugins/decor/src/decor.cpp 2012-03-22 17:00:51 +0000
698+++ plugins/decor/src/decor.cpp 2012-03-31 03:35:24 +0000
699@@ -42,6 +42,57 @@
700
701 COMPIZ_PLUGIN_20090315 (decor, DecorPluginVTable)
702
703+MatchedDecorClipGroup::MatchedDecorClipGroup (const CompMatch &match) :
704+ mMatch (match)
705+{
706+}
707+
708+bool
709+MatchedDecorClipGroup::doPushClippable (DecorClippableInterface *dc)
710+{
711+ if (dc->matches (mMatch))
712+ return mClipGroupImpl.pushClippable (dc);
713+
714+ return false;
715+}
716+
717+void
718+DecorWindow::doUpdateShadow (const CompRegion &reg)
719+{
720+ shadowRegion = outputRegion () - (reg - inputRegion ());
721+}
722+
723+void
724+DecorWindow::doSetOwner (DecorClipGroupInterface *i)
725+{
726+ mClipGroup = i;
727+}
728+
729+bool
730+DecorWindow::doMatches (const CompMatch &m)
731+{
732+ return const_cast <CompMatch &> (m).evaluate (window) && !window->invisible ();
733+}
734+
735+const CompRegion &
736+DecorWindow::getOutputRegion ()
737+{
738+ return mOutputRegion;
739+}
740+
741+const CompRegion &
742+DecorWindow::getInputRegion ()
743+{
744+ return mInputRegion;
745+}
746+
747+void
748+DecorWindow::doUpdateGroupShadows ()
749+{
750+ if (mClipGroup)
751+ mClipGroup->updateAllShadows ();
752+}
753+
754 /* From core */
755
756 /*
757@@ -69,89 +120,6 @@
758 return false;
759 }
760
761-/*
762- * DecorWindow::computeShadowRegion
763- *
764- * This function computes the current clip region for the
765- * shadow that should be draw on glDraw.
766- *
767- * Make shadows look nice, don't paint shadows on top of
768- * things they don't make sense on top of, eg, menus
769- * need shadows but they don't need to be painted when
770- * another menu is adjacent and covering the shadow
771- * region. Also panel shadows are nice, but not
772- * when they obscure client window shadows
773- *
774- * We need to use the current clip region here
775- * and take an intersection of that to ensure
776- * that we don't unintentionally expand the clip
777- * region that core already reduced by doing
778- * occlusion detection
779- */
780-void
781-DecorWindow::computeShadowRegion ()
782-{
783- shadowRegion = CompRegion (window->outputRect ());
784-
785- if (window->type () == CompWindowTypeDropdownMenuMask ||
786- window->type () == CompWindowTypePopupMenuMask)
787- {
788- /* Other transient menus should clip
789- * this menu's shadows, also the panel
790- * which is a transient parent should
791- * too */
792-
793- CompWindowList::iterator it = std::find (screen->windows ().begin (),
794- screen->windows ().end (),
795- window);
796-
797- for (it--; it != screen->windows ().end (); it--)
798- {
799- if (!(*it)->isViewable ())
800- continue;
801-
802- if (!((*it)->type () == CompWindowTypeDropdownMenuMask ||
803- (*it)->type () == CompWindowTypePopupMenuMask ||
804- (*it)->type () == CompWindowTypeDockMask))
805- continue;
806-
807- /* window needs to be a transient parent */
808- if (!isAncestorTo (window, (*it)))
809- continue;
810-
811- CompRegion inter (shadowRegion);
812- inter &= (*it)->borderRect ();
813-
814- if (!inter.isEmpty ())
815- shadowRegion -= inter;
816- }
817-
818- /* If the region didn't change, then it is safe to
819- * say that that this window was probably the first
820- * menu in the "chain" of dropdown menus that comes
821- * from a menu-bar - in that case there isn't any
822- * window that the shadow would necessarily occlude
823- * here so clip the shadow to the top of the input
824- * rect.
825- *
826- * FIXME: We need a better way to detect exactly
827- * where the menubar is for the dropdown menu,
828- * that will look a lot better.
829- */
830- if (window->type () == CompWindowTypeDropdownMenuMask &&
831- shadowRegion == CompRegionRef (window->outputRect ().region ()))
832- {
833- CompRect area (window->outputRect ().x1 (),
834- window->outputRect ().y1 (),
835- window->outputRect ().width (),
836- window->inputRect ().y1 () -
837- window->outputRect ().y1 ());
838-
839- shadowRegion -= area;
840- }
841- }
842-}
843-
844 /*
845 * DecorWindow::glDraw
846 *
847@@ -1508,7 +1476,7 @@
848 if (dScreen->cmActive)
849 {
850 cWindow->damageOutputExtents ();
851- computeShadowRegion ();
852+ updateGroupShadows ();
853 }
854
855 /* Determine how much we moved the window for the old
856@@ -2183,7 +2151,17 @@
857 break;
858 }
859
860+ /* For non-switcher windows we need to update the decoration
861+ * anyways, since the window is unmapped. Also need to
862+ * update the shadow clip regions for panels and other windows */
863+ update (true);
864+ if (dScreen->mMenusClipGroup.pushClippable (this))
865+ updateGroupShadows ();
866+
867+ break;
868+
869 case CompWindowNotifyUnmap:
870+ {
871
872 /* When the switcher is unmapped, it has no frame window
873 * so the frame window for it needs to unmapped manually */
874@@ -2198,14 +2176,15 @@
875 * anyways, since the window is unmapped. Also need to
876 * update the shadow clip regions for panels and other windows */
877 update (true);
878- if (dScreen->cmActive)
879- {
880- foreach (CompWindow *cw, DecorScreen::get (screen)->cScreen->getWindowPaintList ())
881- {
882- DecorWindow::get (cw)->computeShadowRegion ();
883- }
884- }
885+
886+ /* Preserve the group shadow update ptr */
887+ DecorClipGroupInterface *clipGroup = mClipGroup;
888+
889+ if (dScreen->mMenusClipGroup.popClippable (this))
890+ if (clipGroup)
891+ clipGroup->updateAllShadows ();
892 break;
893+ }
894 case CompWindowNotifyUnreparent:
895 {
896 /* Compiz detaches the frame window from
897@@ -2397,18 +2376,6 @@
898 if (w)
899 DecorWindow::get (w)->update (true);
900 }
901- /* On a transient change, we need to recompute shadow regions
902- * for eg, menus */
903- else if (event->xproperty.atom == XA_WM_TRANSIENT_FOR)
904- {
905- if (cmActive)
906- {
907- foreach (CompWindow *cw, cScreen->getWindowPaintList ())
908- {
909- DecorWindow::get (cw)->computeShadowRegion ();
910- }
911- }
912- }
913 else
914 {
915 if (event->xproperty.window == screen->root ())
916@@ -2736,14 +2703,11 @@
917 }
918 updateReg = true;
919
920- if (dScreen->cmActive)
921- {
922- foreach (CompWindow *cw,
923- DecorScreen::get (screen)->cScreen->getWindowPaintList ())
924- {
925- DecorWindow::get (cw)->computeShadowRegion ();
926- }
927- }
928+ mInputRegion.translate (dx, dy);
929+ mOutputRegion.translate (dx, dy);
930+
931+ if (dScreen->cmActive && mClipGroup)
932+ updateGroupShadows ();
933
934 window->moveNotify (dx, dy, immediate);
935 }
936@@ -2796,14 +2760,10 @@
937 updateDecorationScale ();
938 updateReg = true;
939
940- if (dScreen->cmActive)
941- {
942- foreach (CompWindow *cw,
943- DecorScreen::get (screen)->cScreen->getWindowPaintList ())
944- {
945- DecorWindow::get (cw)->computeShadowRegion ();
946- }
947- }
948+ mInputRegion = CompRegion (window->inputRect ());
949+ mOutputRegion = CompRegion (window->outputRect ());
950+ if (dScreen->cmActive && mClipGroup)
951+ updateGroupShadows ();
952
953 window->resizeNotify (dx, dy, dwidth, dheight);
954 }
955@@ -2971,7 +2931,8 @@
956 0,
957 None,
958 boost::shared_array <decor_quad_t> (NULL),
959- 0))
960+ 0)),
961+ mMenusClipGroup (CompMatch ("type=Dock | type=DropdownMenu | type=Menu | type=PopupMenu"))
962 {
963 supportingDmCheckAtom =
964 XInternAtom (s->dpy (), DECOR_SUPPORTING_DM_CHECK_ATOM_NAME, 0);
965@@ -3045,7 +3006,10 @@
966 unshading (false),
967 shading (false),
968 isSwitcher (false),
969- frameExtentsRequested (false)
970+ frameExtentsRequested (false),
971+ mClipGroup (NULL),
972+ mOutputRegion (window->outputRect ()),
973+ mInputRegion (window->inputRect ())
974 {
975 WindowInterface::setHandler (window);
976
977@@ -3074,6 +3038,10 @@
978 }
979
980 window->resizeNotifySetEnabled (this, true);
981+
982+ if (!window->invisible ())
983+ if (dScreen->mMenusClipGroup.pushClippable (this))
984+ updateGroupShadows ();
985 }
986
987 /*
988@@ -3090,6 +3058,9 @@
989 if (wd)
990 WindowDecoration::destroy (wd);
991
992+ if (mClipGroup)
993+ mClipGroup->popClippable (this);
994+
995 decor.mList.clear ();
996 }
997
998
999=== modified file 'plugins/decor/src/decor.h'
1000--- plugins/decor/src/decor.h 2012-02-29 08:38:22 +0000
1001+++ plugins/decor/src/decor.h 2012-03-31 03:35:24 +0000
1002@@ -33,6 +33,8 @@
1003 #include <core/atoms.h>
1004 #include <core/windowextents.h>
1005
1006+#include <clip-groups.h>
1007+
1008 #include "decor_options.h"
1009
1010 #define DECOR_SCREEN(s) DecorScreen *ds = DecorScreen::get(s)
1011@@ -50,6 +52,27 @@
1012 #define DECOR_ACTIVE 1
1013 #define DECOR_NUM 2
1014
1015+using namespace compiz::decor;
1016+
1017+class MatchedDecorClipGroup :
1018+ public DecorClipGroupInterface
1019+{
1020+ public:
1021+
1022+ MatchedDecorClipGroup (const CompMatch &match);
1023+
1024+ private:
1025+
1026+ bool doPushClippable (DecorClippableInterface *dc);
1027+ bool doPopClippable (DecorClippableInterface *dc) { return mClipGroupImpl.popClippable (dc); }
1028+ void doRegenerateClipRegion () { return mClipGroupImpl.regenerateClipRegion (); }
1029+ const CompRegion & getClipRegion () { return mClipGroupImpl.clipRegion (); }
1030+ void doUpdateAllShadows () { return mClipGroupImpl.updateAllShadows (); }
1031+
1032+ impl::GenericDecorClipGroup mClipGroupImpl;
1033+ CompMatch mMatch;
1034+};
1035+
1036 class DecorTexture {
1037
1038 public:
1039@@ -204,13 +227,16 @@
1040 std::map<Window, DecorWindow *> frames;
1041
1042 CompTimer decoratorStart;
1043+
1044+ MatchedDecorClipGroup mMenusClipGroup;
1045 };
1046
1047 class DecorWindow :
1048 public WindowInterface,
1049 public CompositeWindowInterface,
1050 public GLWindowInterface,
1051- public PluginClassHandler<DecorWindow,CompWindow>
1052+ public PluginClassHandler<DecorWindow,CompWindow>,
1053+ public DecorClippableInterface
1054 {
1055 public:
1056 DecorWindow (CompWindow *w);
1057@@ -260,6 +286,15 @@
1058 static bool matchState (CompWindow *w, unsigned int decorState);
1059 static bool matchActions (CompWindow *w, unsigned int decorActions);
1060
1061+ private:
1062+
1063+ void doUpdateShadow (const CompRegion &);
1064+ void doSetOwner (DecorClipGroupInterface *i);
1065+ bool doMatches (const CompMatch &m);
1066+ const CompRegion & getOutputRegion ();
1067+ const CompRegion & getInputRegion ();
1068+ void doUpdateGroupShadows ();
1069+
1070 public:
1071
1072 CompWindow *window;
1073@@ -296,6 +331,10 @@
1074 bool isSwitcher;
1075
1076 bool frameExtentsRequested;
1077+
1078+ DecorClipGroupInterface *mClipGroup;
1079+ CompRegion mOutputRegion;
1080+ CompRegion mInputRegion;
1081 };
1082
1083 class DecorPluginVTable :

Subscribers

People subscribed via source and target branches