Merge lp:~smspillaz/compiz-wall-plugin/compiz-wall-plugin.fix_939228 into lp:compiz-wall-plugin

[Sam Spilsbury]

== Problem ==

Another arm of bug 939228 : if a window spanned two monitors it would shift around on viewport changes

== Solution ==

Take all workareas into account when calculating window shift

== test ==

included.

[Daniel van Vugt (vanvugt)]

Deduplicated bugs. This is now bug 755842.

[Daniel van Vugt (vanvugt)]

Linking CXX executable compiz_test_wall_offset_movement
/usr/bin/ld: cannot find -lcompiz_core
collect2: ld returned 1 exit status
make[2]: *** [src/offset_movement/tests/compiz_test_wall_offset_movement] Error 1
make[1]: *** [src/offset_movement/tests/CMakeFiles/compiz_test_wall_offset_movement.dir/all] Error 2
make: *** [all] Error 2

review: Needs Fixing

[Sam Spilsbury (smspillaz)]

Curious. It compiles fine here and

pkg_check_modules (
  GLIBMM
  REQUIRED
  glibmm-2.4 glib-2.0 compiz
)

INCLUDE_DIRECTORIES (
  ${CMAKE_CURRENT_SOURCE_DIR}/include
  ${CMAKE_CURRENT_SOURCE_DIR}/src

  ${CMAKE_SOURCE_DIR}/include

  ${Boost_INCLUDE_DIRS}

  ${GLIBMM_INCLUDE_DIRS}
)

LINK_DIRECTORIES (${GLIBMM_LIBRARY_DIRS})

^ should take care of finding the right directory for libcompiz_core . I suspect your install is broken. What is the output of make when CMAKE_VERBOSE_MAKEFILE is turned on ?

[Daniel van Vugt (vanvugt)]

Looks OK and seems to work.

review: Approve

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2012-04-24 08:39:54 +0000
+++ CMakeLists.txt	2012-04-28 19:08:18 +0000
@@ -2,4 +2,7 @@
 
 include (CompizPlugin)
 
-compiz_plugin (wall PLUGINDEPS composite opengl mousepoll PKGDEPS cairo cairo-xlib-xrender LIBRARIES dl)
+add_subdirectory (src/offset_movement)
+include_directories (src/offset_movement/include)
+
+compiz_plugin (wall PLUGINDEPS composite opengl mousepoll PKGDEPS cairo cairo-xlib-xrender LIBRARIES compiz_wall_offset_movement dl)

=== added directory 'src/offset_movement'
=== added file 'src/offset_movement/CMakeLists.txt'
--- src/offset_movement/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ src/offset_movement/CMakeLists.txt	2012-04-28 19:08:18 +0000
@@ -0,0 +1,57 @@
+pkg_check_modules (
+  GLIBMM
+  REQUIRED
+  glibmm-2.4 glib-2.0 compiz
+)
+
+INCLUDE_DIRECTORIES (  
+  ${CMAKE_CURRENT_SOURCE_DIR}/include
+  ${CMAKE_CURRENT_SOURCE_DIR}/src
+
+  ${CMAKE_SOURCE_DIR}/include
+    
+  ${Boost_INCLUDE_DIRS}
+  
+  ${GLIBMM_INCLUDE_DIRS}
+)
+
+LINK_DIRECTORIES (${GLIBMM_LIBRARY_DIRS}) 
+
+SET ( 
+  PUBLIC_HEADERS 
+)
+
+SET ( 
+  PRIVATE_HEADERS
+  ${CMAKE_CURRENT_SOURCE_DIR}/include/offset-movement.h
+)
+
+SET( 
+  SRCS 
+  ${CMAKE_CURRENT_SOURCE_DIR}/src/offset-movement.cpp
+)
+
+ADD_LIBRARY( 
+  compiz_wall_offset_movement STATIC
+  
+  ${SRCS}
+  
+  ${PUBLIC_HEADERS}
+  ${PRIVATE_HEADERS}
+)
+
+if (COMPIZ_BUILD_TESTING)
+ADD_SUBDIRECTORY( ${CMAKE_CURRENT_SOURCE_DIR}/tests )
+endif (COMPIZ_BUILD_TESTING)
+
+SET_TARGET_PROPERTIES(
+  compiz_wall_offset_movement PROPERTIES
+  PUBLIC_HEADER "${PUBLIC_HEADERS}"
+)
+
+TARGET_LINK_LIBRARIES(
+  compiz_wall_offset_movement
+
+  compiz_core
+  ${GLIBMM_LIBRARIES}
+)

=== added directory 'src/offset_movement/include'
=== added file 'src/offset_movement/include/offset-movement.h'
--- src/offset_movement/include/offset-movement.h	1970-01-01 00:00:00 +0000
+++ src/offset_movement/include/offset-movement.h	2012-04-28 19:08:18 +0000
@@ -0,0 +1,41 @@
+/**
+ *
+ * Compiz wall plugin
+ *
+ * offset-movement.h
+ *
+ * Copyright (c) 2006 Robert Carr <[e-mail]>
+ *
+ * Authors:
+ * Robert Carr <[e-mail]>
+ * Dennis Kasprzyk <[e-mail]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ **/
+
+#ifndef _COMPIZ_WALL_OFFSET_MOVEMENT_H
+#define _COMPIZ_WALL_OFFSET_MOVEMENT_H
+
+#include <core/region.h>
+#include <core/rect.h>
+#include <core/point.h>
+
+namespace compiz
+{
+    namespace wall
+    {
+	CompPoint movementWindowOnScreen (const CompRect &serverBorderRect,
+					  const CompRegion &screenRegion);
+    }
+}
+
+#endif

=== added directory 'src/offset_movement/src'
=== added file 'src/offset_movement/src/offset-movement.cpp'
--- src/offset_movement/src/offset-movement.cpp	1970-01-01 00:00:00 +0000
+++ src/offset_movement/src/offset-movement.cpp	2012-04-28 19:08:18 +0000
@@ -0,0 +1,62 @@
+/**
+ *
+ * Compiz wall plugin
+ *
+ * offset-movement.c
+ *
+ * Copyright (c) 2006 Robert Carr <[e-mail]>
+ *
+ * Authors:
+ * Robert Carr <[e-mail]>
+ * Dennis Kasprzyk <[e-mail]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ **/
+
+#include "offset-movement.h"
+
+CompPoint
+compiz::wall::movementWindowOnScreen (const CompRect &serverBorderRect,
+				      const CompRegion &screenRegion)
+{
+    CompRegion sbrRegion (static_cast <const CompRect &> (serverBorderRect));
+
+    /* If the window would be partially offscreen
+     * after it was moved then we should move it back
+     * so that it is completely onscreen, since we moved
+     * from mostly offscreen on B to mostly onscreen on A,
+     * the user should be able to see their selected window */
+    CompRegion inter = sbrRegion.intersected (screenRegion);
+    CompRegion rem = sbrRegion - screenRegion;
+
+    int dx = 0;
+    int dy = 0;
+
+    for (std::vector <CompRect>::const_iterator it = rem.rects ().begin ();
+	 it != rem.rects ().end ();
+	 it++)
+    {
+	const CompRect &r = *it;
+
+	if (r.x1 () >= inter.boundingRect ().x2 ())
+	    dx -= r.width ();
+	else if (r.x2 () <= inter.boundingRect ().x1 ())
+	    dx += r.width ();
+
+	if (r.y1 () >= inter.boundingRect ().y2 ())
+	    dy -= r.height ();
+	else if (r.y2 () <= inter.boundingRect ().y1 ())
+	    dy += r.height ();
+    }
+
+    return CompPoint (dx, dy);
+}

=== added directory 'src/offset_movement/tests'
=== added file 'src/offset_movement/tests/CMakeLists.txt'
--- src/offset_movement/tests/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ src/offset_movement/tests/CMakeLists.txt	2012-04-28 19:08:18 +0000
@@ -0,0 +1,58 @@
+# Build Google Test and make its headers known
+find_package (GTest)
+
+if (NOT GTEST_FOUND)
+
+    # Check for google test and build it locally
+    set (GTEST_ROOT_DIR
+	"/usr/src/gtest" # Default value, adjustable by user with e.g., ccmake
+	CACHE
+	PATH
+	"Path to Google test srcs"
+    )
+
+    find_path (GTEST_INCLUDE_DIR gtest/gtest.h)
+
+    if (GTEST_INCLUDE_DIR)
+	#FIXME - hardcoded is bad!
+	add_subdirectory (${GTEST_ROOT_DIR}
+		      gtest)
+    endif(GTEST_INCLUDE_DIR)
+
+    set (GTEST_BOTH_LIBRARIES gtest gtest_main)
+    set (GTEST_FOUND TRUE)
+
+endif (NOT GTEST_FOUND)
+
+pkg_check_modules (
+  COMPIZ
+  REQUIRED
+  glibmm-2.4 glib-2.0 compiz
+)
+
+find_library (GMOCK_LIBRARY gmock)
+find_library (GMOCK_MAIN_LIBRARY gmock_main)
+
+if (NOT GMOCK_LIBRARY OR NOT GMOCK_MAIN_LIBRARY OR NOT GTEST_FOUND)
+  message ("Google Mock and Google Test not found - cannot build tests!")
+  set (COMPIZ_BUILD_TESTING OFF)
+endif (NOT GMOCK_LIBRARY OR NOT GMOCK_MAIN_LIBRARY OR NOT GTEST_FOUND)
+
+include_directories (${GTEST_INCLUDE_DIRS})
+include_directories(${CMAKE_CURRENT_SOURCE_DIR})
+include_directories (${COMPIZ_INCLUDE_DIRS})
+
+link_directories (${COMPIZ_LIBRARY_DIRS})
+
+add_executable (compiz_test_wall_offset_movement
+                ${CMAKE_CURRENT_SOURCE_DIR}/test-wall-offset-movement.cpp)
+
+target_link_libraries (compiz_test_wall_offset_movement
+                       compiz_wall_offset_movement
+                       ${GTEST_BOTH_LIBRARIES}
+		       ${GMOCK_LIBRARY}
+		       ${GMOCK_MAIN_LIBRARY}
+		       ${CMAKE_THREAD_LIBS_INIT} # Link in pthread. 
+                       )
+
+gtest_add_tests (compiz_test_wall_offset_movement "" ${CMAKE_CURRENT_SOURCE_DIR}/test-wall-offset-movement.cpp)

=== added file 'src/offset_movement/tests/test-wall-offset-movement.cpp'
--- src/offset_movement/tests/test-wall-offset-movement.cpp	1970-01-01 00:00:00 +0000
+++ src/offset_movement/tests/test-wall-offset-movement.cpp	2012-04-28 19:08:18 +0000
@@ -0,0 +1,97 @@
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+#include "offset-movement.h"
+
+class WallOffsetMovementTest :
+    public ::testing::Test
+{
+};
+
+TEST(WallOffsetMovementTest, TestOffsetRight)
+{
+    CompRect sbr (750, 0, 500, 500);
+    CompRegion sr (0, 0, 1000, 1000);
+
+    CompPoint offset = compiz::wall::movementWindowOnScreen (sbr, sr);
+
+    EXPECT_EQ (offset, CompPoint (-250, 0));
+}
+
+TEST(WallOffsetMovementTest, TestOffsetLeft)
+{
+    CompRect sbr (-250, 0, 500, 500);
+    CompRegion sr (0, 0, 1000, 1000);
+
+    CompPoint offset = compiz::wall::movementWindowOnScreen (sbr, sr);
+
+    EXPECT_EQ (offset, CompPoint (250, 0));
+}
+
+TEST(WallOffsetMovementTest, TestOffsetTop)
+{
+    CompRect sbr (0, -250, 500, 500);
+    CompRegion sr (0, 0, 1000, 1000);
+
+    CompPoint offset = compiz::wall::movementWindowOnScreen (sbr, sr);
+
+    EXPECT_EQ (offset, CompPoint (0, 250));
+}
+
+TEST(WallOffsetMovementTest, TestOffsetBottom)
+{
+    CompRect sbr (0, 750, 500, 500);
+    CompRegion sr (0, 0, 1000, 1000);
+
+    CompPoint offset = compiz::wall::movementWindowOnScreen (sbr, sr);
+
+    EXPECT_EQ (offset, CompPoint (0, -250));
+}
+
+TEST(WallOffsetMovementTest, TestOffsetRightMMSlice)
+{
+    CompRect sbr (750, 0, 500, 500);
+    CompRegion sr (0, 0, 1000, 1000);
+
+    sr -= CompRegion (400, 0, 200, 0);
+
+    CompPoint offset = compiz::wall::movementWindowOnScreen (sbr, sr);
+
+    EXPECT_EQ (offset, CompPoint (-250, 0));
+}
+
+TEST(WallOffsetMovementTest, TestOffsetLeftMMSlice)
+{
+    CompRect sbr (-250, 0, 500, 500);
+    CompRegion sr (0, 0, 1000, 1000);
+
+    sr -= CompRegion (400, 0, 200, 0);
+
+    CompPoint offset = compiz::wall::movementWindowOnScreen (sbr, sr);
+
+    EXPECT_EQ (offset, CompPoint (250, 0));
+}
+
+TEST(WallOffsetMovementTest, TestOffsetTopMMSlice)
+{
+    CompRect sbr (0, -250, 500, 500);
+    CompRegion sr (0, 0, 1000, 1000);
+
+    sr -= CompRegion (400, 0, 200, 0);
+
+    CompPoint offset = compiz::wall::movementWindowOnScreen (sbr, sr);
+
+    EXPECT_EQ (offset, CompPoint (0, 250));
+}
+
+TEST(WallOffsetMovementTest, TestOffsetBottomMMSlice)
+{
+    CompRect sbr (0, 750, 500, 500);
+    CompRegion sr (0, 0, 1000, 1000);
+
+    sr -= CompRegion (400, 0, 200, 0);
+
+    CompPoint offset = compiz::wall::movementWindowOnScreen (sbr, sr);
+
+    EXPECT_EQ (offset, CompPoint (0, -250));
+}

=== modified file 'src/wall.cpp'
--- src/wall.cpp	2011-10-11 13:28:07 +0000
+++ src/wall.cpp	2012-04-28 19:08:18 +0000
@@ -620,50 +620,29 @@
 	    XWindowChanges xwc;
 	    unsigned int   mask = 0;
 
-	    ws->moveViewport (-dx, -dy, false);
+	    /* If changing viewports fails we should not
+	     * move the client window */
+	    if (!ws->moveViewport (-dx, -dy, false))
+	    {
+		window->activate ();
+		return;
+	    }
+
 	    ws->focusDefault = false;
 
-	    CompWindow::Geometry sbr (window->serverBorderRect ().x (),
-				      window->serverBorderRect ().y (),
-				      window->serverBorderRect ().width (),
-				      window->serverBorderRect ().height (),
-				      0);
-	    CompRegion sbrRegion (sbr);
-	    int        output = screen->outputDeviceForGeometry (sbr);
-	    CompRegion outputRegion (screen->outputDevs ()[output].workArea ());
-
-	    /* If the window would be partially offscreen
-	     * after it was moved then we should move it back
-	     * so that it is completely onscreen, since we moved
-	     * from mostly offscreen on B to mostly onscreen on A,
-	     * the user should be able to see their selected window */
-	    CompRegion inter = sbrRegion.intersected (outputRegion);
-	    CompRegion rem = sbrRegion - outputRegion;
-
-	    foreach (const CompRect &r, rem.rects ())
-	    {
-		if (r.x1 () >= inter.boundingRect ().x2 ())
-		{
-		    xwc.x = window->serverGeometry ().x () - r.width ();
-		    mask |= CWX;
-		}
-		else if (r.x2 () <= inter.boundingRect ().x1 ())
-		{
-		    xwc.x = window->serverGeometry ().x () + r.width ();
-		    mask |= CWX;
-		}
-
-		if (r.y1 () >= inter.boundingRect ().y2 ())
-		{
-		    xwc.y = window->serverGeometry ().y () - r.height ();
-		    mask |= CWY;
-		}
-		else if (r.y2 () <= inter.boundingRect ().y1 ())
-		{
-		    xwc.y = window->serverGeometry ().y () + r.height ();
-		    mask |= CWY;
-		}
-	    }
+	    CompRegion screenRegion;
+
+	    foreach (const CompOutput &o, screen->outputDevs ())
+		screenRegion += o.workArea ();
+
+	    CompPoint d = compiz::wall::movementWindowOnScreen (window->serverBorderRect (),
+								screenRegion);
+
+	    mask |= d.x () !=0 ? CWX : 0;
+	    mask |= d.y () !=0 ? CWY : 0;
+
+	    xwc.x = window->serverGeometry ().x () + dx;
+	    xwc.y = window->serverGeometry ().y () + dy;
 
 	    window->configureXWindow (mask, &xwc);
 	}
@@ -1515,7 +1494,7 @@
 
     if (!moving && !showPreview && grabIndex)
     {
-	screen->removeGrab (grabIndex, NULL);
+	screen->removeGrab (static_cast <CompScreen::GrabHandle> (grabIndex), NULL);
 	grabIndex = 0;
     }
 

=== modified file 'src/wall.h'
--- src/wall.h	2012-04-24 09:17:57 +0000
+++ src/wall.h	2012-04-28 19:08:18 +0000
@@ -31,6 +31,8 @@
 #include <cairo-xlib-xrender.h>
 #include <cairo.h>
 
+#include "offset-movement.h"
+
 #include "wall_options.h"