Merge lp:~smspillaz/compiz-core/compiz-core.cleanup-modifierhandler into lp:compiz-core/0.9.5

[Sam Spilsbury]

Clean up the ModifierHandler class , remove #define, remove duplication etc etc

[Alan Griffiths (alan-griffiths)]

yes

review: Approve

[Daniel van Vugt (vanvugt)]

Changed status back to Needs review, while the prereq branch "Needs fixing".

[Daniel van Vugt (vanvugt)]

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

review: Needs Resubmitting

Unmerged revisions

2953. By Sam Spilsbury on 2012-01-24

Clean up ModifierHandler

2952. By Sam Spilsbury on 2012-01-24

Break the dependency of CompAction and CompScreen - use ModifierHandler instead

2951. By Sam Spilsbury on 2012-01-24

Assorted cleanups - remove header dependencies from ModifierHandler and PropertyWriter

=== modified file 'include/core/modifierhandler.h'
--- include/core/modifierhandler.h	2012-01-24 08:30:14 +0000
+++ include/core/modifierhandler.h	2012-01-24 08:30:15 +0000
@@ -30,39 +30,15 @@
 #define _COMPIZ_MODIFIERHANDLER_H
 
 #include <X11/Xlib-xcb.h>
-
-const unsigned int CompModAlt = 0;
-const unsigned int CompModMeta = 1;
-const unsigned int CompModSuper  = 2;
-const unsigned int CompModHyper  = 3;
-const unsigned int CompModModeSwitch = 4;
-const unsigned int CompModNumLock = 5;
-const unsigned int CompModScrollLock = 6;
-const unsigned int CompModNum = 7;
-
-const unsigned int CompAltMask = (1 << 16);
-const unsigned int CompMetaMask = (1 << 17);
-const unsigned int CompSuperMask = (1 << 18);
-const unsigned int CompHyperMask = (1 << 19);
-const unsigned int CompModeSwitchMask = (1 << 20);
-const unsigned int CompNumLockMask = (1 << 21);
-const unsigned int CompScrollLockMask = (1 << 22);
-const unsigned int CompNoMask = (1 << 25);
-
-/**
- * Toplevel class which provides access to display
- * level modifier information
- */
-class ModifierHandler
+#include <string>
+
+namespace compiz
 {
-    public:
-
-	ModifierHandler (Display *);
-	~ModifierHandler ();
-
+    namespace modifiers
+    {
 	typedef enum
 	{
-	    Alt = 1,
+	    Alt = 0,
 	    Meta,
 	    Super,
 	    Hyper,
@@ -84,53 +60,76 @@
 	    NoMask         = (1 << 25),
 	} ModMask;
 
-    public:
-
-
-	/**
-	 * Takes an X11 Keycode and returns a bitmask
-	 * with modifiers that have been pressed
-	 */
-	unsigned int keycodeToModifiers (int keycode);
-
-	/**
-	 * Updates X11 Modifier mappings
-	 */
-	bool updateModifierMappings ();
-
-	/**
-	 * Takes a virtual modMask and returns a real modifier mask
-	 * by removing unused bits
-	 */
-	unsigned int virtualToRealModMask (unsigned int modMask);
-
-	/**
-	 * Returns a bit modifier mask for a Motifier enum
-	 */
-	unsigned int modMask (Modifier);
-
-	/**
-	 * Returns a const bit modifier mask for what should be ignored
-	 */
-	unsigned int ignoredModMask ();
-
-	/**
-	 * Returns a const XModifierKeymap for compiz
-	 */
-	const XModifierKeymap * modMap ();
-
-	static ModifierHandler *
-	Default ();
-
-	static void
-	SetDefault (ModifierHandler *);
-    private:
-
-	ModMask    mModMask[ModNum];
-	unsigned int    mIgnoredModMask;
-	XModifierKeymap *mModMap;
-
-	Display    *mDisplay;
-};
+	/**
+	 * Toplevel class which provides access to display
+	 * level modifier information
+	 */
+	class ModifierHandler
+	{
+	    public:
+
+		ModifierHandler (Display *);
+		~ModifierHandler ();
+
+	    public:
+
+
+		/**
+		 * Takes an X11 Keycode and returns a bitmask
+		 * with modifiers that have been pressed
+		 */
+		unsigned int keycodeToModifiers (int keycode);
+
+		/**
+		 * Updates X11 Modifier mappings
+		 */
+		bool updateModifierMappings ();
+
+		/**
+		 * Takes a virtual modMask and returns a real modifier mask
+		 * by removing unused bits
+		 */
+		unsigned int virtualToRealModMask (unsigned int modMask);
+
+		/**
+		 * Takes a key string and returns a keycode
+		 */
+		int stringToKeycode (const std::string &) const;
+
+		/**
+		 * Takes a keycode and returns a key string
+		 */
+		std::string keycodeToString (int keycode) const;
+
+		/**
+		 * Returns a bit modifier mask for a Motifier enum
+		 */
+		unsigned int modMask (Modifier);
+
+		/**
+		 * Returns a const bit modifier mask for what should be ignored
+		 */
+		unsigned int ignoredModMask ();
+
+		/**
+		 * Returns a const XModifierKeymap for compiz
+		 */
+		const XModifierKeymap * modMap ();
+
+		static ModifierHandler *
+		Default ();
+
+		static void
+		SetDefault (ModifierHandler *);
+	    private:
+
+		ModMask    mModMask[ModNum];
+		unsigned int    mIgnoredModMask;
+		XModifierKeymap *mModMap;
+
+		Display    *mDisplay;
+	};
+    }
+}
 
 #endif

=== modified file 'include/core/screen.h'
--- include/core/screen.h	2012-01-24 08:30:14 +0000
+++ include/core/screen.h	2012-01-24 08:30:15 +0000
@@ -65,7 +65,6 @@
 extern bool       debugOutput;
 
 extern CompScreen   *screen;
-extern ModifierHandler *modHandler;
 
 extern int lastPointerX;
 extern int lastPointerY;

=== modified file 'plugins/resize/src/resize.cpp'
--- plugins/resize/src/resize.cpp	2012-01-18 16:26:45 +0000
+++ plugins/resize/src/resize.cpp	2012-01-24 08:30:15 +0000
@@ -32,8 +32,11 @@
 #include <X11/cursorfont.h>
 
 #include <core/atoms.h>
+#include <core/modifierhandler.h>
 #include "resize.h"
 
+using namespace compiz::modifiers;
+
 COMPIZ_PLUGIN_20090315 (resize, ResizePluginVTable)
 
 void
@@ -1713,11 +1716,11 @@
     if (valueMask & ResizeModeShiftMask)
 	*mask |= ShiftMask;
     if (valueMask & ResizeModeAltMask)
-	*mask |= CompAltMask;
+	*mask |= AltMask;
     if (valueMask & ResizeModeControlMask)
 	*mask |= ControlMask;
     if (valueMask & ResizeModeMetaMask)
-	*mask |= CompMetaMask;
+	*mask |= MetaMask;
 }
 
 void

=== modified file 'src/action.cpp'
--- src/action.cpp	2012-01-24 08:24:18 +0000
+++ src/action.cpp	2012-01-24 08:30:15 +0000
@@ -33,10 +33,15 @@
 #include "privateaction.h"
 #include "privatescreen.h"
 
+<<<<<<< TREE
+=======
+using namespace compiz;
+
+>>>>>>> MERGE-SOURCE
 struct _Modifier {
     const char *name;
     int        modifier;
-} modifiers[] = {
+} allModifiers[] = {
     { "<Shift>",      ShiftMask		 },
     { "<Control>",    ControlMask	 },
     { "<Mod1>",	      Mod1Mask		 },
@@ -44,14 +49,14 @@
     { "<Mod3>",	      Mod3Mask		 },
     { "<Mod4>",	      Mod4Mask		 },
     { "<Mod5>",	      Mod5Mask		 },
-    { "<Alt>",	      CompAltMask        },
-    { "<Meta>",	      CompMetaMask       },
-    { "<Super>",      CompSuperMask      },
-    { "<Hyper>",      CompHyperMask	 },
-    { "<ModeSwitch>", CompModeSwitchMask }
+    { "<Alt>",	      modifiers::AltMask		 },
+    { "<Meta>",	      modifiers::MetaMask           },
+    { "<Super>",      modifiers::SuperMask          },
+    { "<Hyper>",      modifiers::HyperMask	         },
+    { "<ModeSwitch>", modifiers::ModeSwitchMask     }
 };
 
-#define N_MODIFIERS (sizeof (modifiers) / sizeof (struct _Modifier))
+#define N_MODIFIERS (sizeof (allModifiers) / sizeof (struct _Modifier))
 
 struct _Edge {
     const char *name;
@@ -74,8 +79,8 @@
 
     for (unsigned int i = 0; i < N_MODIFIERS; i++)
     {
-	if (modMask & modifiers[i].modifier)
-	    binding += modifiers[i].name;
+	if (modMask & allModifiers[i].modifier)
+	    binding += allModifiers[i].name;
     }
 
     return binding;
@@ -88,8 +93,8 @@
 
     for (unsigned int i = 0; i < N_MODIFIERS; i++)
     {
-	if (str.find (modifiers[i].name) != std::string::npos)
-	    mods |= modifiers[i].modifier;
+	if (str.find (allModifiers[i].name) != std::string::npos)
+	    mods |= allModifiers[i].modifier;
     }
 
     return mods;
@@ -156,7 +161,7 @@
     CompString   sStr;
     unsigned int mods;
     size_t       pos, start = 0;
-    KeySym	 keysym;
+    int          keycode;
 
     if (!screen)
 	return false;
@@ -184,31 +189,25 @@
     }
 
     sStr   = str.substr (start);
-    keysym = XStringToKeysym (sStr.c_str ());
+    keycode = modifiers::ModifierHandler::Default ()->stringToKeycode (sStr);
 
-    if (keysym != NoSymbol)
+    if (!keycode)
     {
-	KeyCode keycode;
-
-	keycode = XKeysymToKeycode (screen->dpy (), keysym);
-	if (keycode)
+	if (sStr.compare (0, 2, "0x") == 0)
 	{
-	    mKeycode   = keycode;
+	    mKeycode   = strtol (sStr.c_str (), NULL, 0);
 	    mModifiers = mods;
 
 	    return true;
 	}
-    }
-
-    if (sStr.compare (0, 2, "0x") == 0)
-    {
-	mKeycode   = strtol (sStr.c_str (), NULL, 0);
-	mModifiers = mods;
-
-	return true;
-    }
-
-    return false;
+	else
+	    return false;
+    }
+
+    mModifiers = mods;
+    mKeycode = keycode;
+
+    return true;
 }
 
 CompString
@@ -223,13 +222,9 @@
 
     if (mKeycode != 0)
     {
-	KeySym keysym;
-	char   *keyname;
-
-	keysym  = XKeycodeToKeysym (screen->dpy (), mKeycode, 0);
-	keyname = XKeysymToString (keysym);
-
-	if (keyname)
+	std::string keyname (modifiers::ModifierHandler::Default ()->keycodeToString (mKeycode));
+
+	if (!keyname.empty ())
 	    binding += keyname;
 	else
 	    binding += compPrintf ("0x%x", mKeycode);

=== modified file 'src/event.cpp'
--- src/event.cpp	2012-01-24 08:30:14 +0000
+++ src/event.cpp	2012-01-24 08:30:15 +0000
@@ -37,10 +37,13 @@
 #include <X11/extensions/Xfixes.h>
 
 #include <core/atoms.h>
+#include <core/modifierhandler.h>
 #include "privatescreen.h"
 #include "privatewindow.h"
 #include "privatestackdebugger.h"
 
+using namespace compiz::modifiers;
+
 bool
 PrivateWindow::handleSyncAlarm ()
 {
@@ -80,8 +83,10 @@
     return false;
 }
 
-#define REAL_MOD_MASK (ShiftMask | ControlMask | Mod1Mask | Mod2Mask | \
-		       Mod3Mask | Mod4Mask | Mod5Mask | CompNoMask)
+const unsigned int REAL_MOD_MASK = (ShiftMask | ControlMask |
+				    Mod1Mask | Mod2Mask |
+				    Mod3Mask | Mod4Mask |
+				    Mod5Mask | compiz::modifiers::NoMask);
 
 static bool
 isCallBackBinding (CompOption	           &option,

=== modified file 'src/modifierhandler.cpp'
--- src/modifierhandler.cpp	2012-01-24 08:30:14 +0000
+++ src/modifierhandler.cpp	2012-01-24 08:30:15 +0000
@@ -31,11 +31,21 @@
 #include <X11/keysym.h>
 #include <cstring>
 
+using namespace compiz;
+
+typedef modifiers::ModifierHandler ModifierHandler;
+
 namespace
 {
+
 const unsigned int virtualModMask[7] = {
-    CompAltMask, CompMetaMask, CompSuperMask, CompHyperMask,
-    CompModeSwitchMask, CompNumLockMask, CompScrollLockMask
+    modifiers::AltMask,
+    modifiers::MetaMask,
+    modifiers::SuperMask,
+    modifiers::HyperMask,
+    modifiers::ModeSwitchMask,
+    modifiers::NumLockMask,
+    modifiers::ScrollLockMask
 };
 const int maskTable[8] = {
     ShiftMask, LockMask, ControlMask, Mod1Mask,
@@ -61,11 +71,11 @@
 bool
 ModifierHandler::updateModifierMappings ()
 {
-    unsigned int    modMask[CompModNum];
+    unsigned int    modMask[modifiers::ModNum];
     int		    minKeycode, maxKeycode, keysymsPerKeycode = 0;
     KeySym*         key;
 
-    for (unsigned int i = 0; i < CompModNum; i++)
+    for (unsigned int i = 0; i < modifiers::ModNum; i++)
 	modMask[i] = 0;
 
     XDisplayKeycodes (mDisplay, &minKeycode, &maxKeycode);
@@ -104,42 +114,42 @@
 		if (keysym == XK_Alt_L ||
 		    keysym == XK_Alt_R)
 		{
-		    modMask[CompModAlt] |= mask;
+		    modMask[modifiers::Alt] |= mask;
 		}
 		else if (keysym == XK_Meta_L ||
 			 keysym == XK_Meta_R)
 		{
-		    modMask[CompModMeta] |= mask;
+		    modMask[modifiers::Meta] |= mask;
 		}
 		else if (keysym == XK_Super_L ||
 			 keysym == XK_Super_R)
 		{
-		    modMask[CompModSuper] |= mask;
+		    modMask[modifiers::Super] |= mask;
 		}
 		else if (keysym == XK_Hyper_L ||
 			 keysym == XK_Hyper_R)
 		{
-		    modMask[CompModHyper] |= mask;
+		    modMask[modifiers::Hyper] |= mask;
 		}
 		else if (keysym == XK_Mode_switch)
 		{
-		    modMask[CompModModeSwitch] |= mask;
+		    modMask[modifiers::ModeSwitch] |= mask;
 		}
 		else if (keysym == XK_Scroll_Lock)
 		{
-		    modMask[CompModScrollLock] |= mask;
+		    modMask[modifiers::ScrollLock] |= mask;
 		}
 		else if (keysym == XK_Num_Lock)
 		{
-		    modMask[CompModNumLock] |= mask;
+		    modMask[modifiers::NumLock] |= mask;
 		}
 	    }
 	}
 
-	for (unsigned int i = 0; i < CompModNum; i++)
+	for (unsigned int i = 0; i < modifiers::ModNum; i++)
 	{
 	    if (!modMask[i])
-		modMask[i] = CompNoMask;
+		modMask[i] = modifiers::NoMask;
 	}
 
 	if (memcmp (modMask, mModMask, sizeof (modMask)))
@@ -147,8 +157,8 @@
 	    memcpy (mModMask, modMask, sizeof (modMask));
 
 	    mIgnoredModMask = LockMask |
-		(modMask[CompModNumLock]    & ~CompNoMask) |
-		(modMask[CompModScrollLock] & ~CompNoMask);
+		(modMask[modifiers::NumLock]    & ~modifiers::NoMask) |
+		(modMask[modifiers::ScrollLock] & ~modifiers::NoMask);
 
 	    if (key)
 		XFree (key);
@@ -163,7 +173,7 @@
 unsigned int
 ModifierHandler::virtualToRealModMask (unsigned int modMask)
 {
-    for (unsigned int i = 0; i < CompModNum; i++)
+    for (unsigned int i = 0; i < modifiers::ModNum; i++)
     {
 	if (modMask & virtualModMask[i])
 	{
@@ -194,6 +204,29 @@
     return mods;
 }
 
+int
+ModifierHandler::stringToKeycode (const std::string &str) const
+{
+    KeySym keysym = XStringToKeysym (str.c_str ());
+
+    if (keysym != NoSymbol)
+	return static_cast <int> (XKeysymToKeycode (mDisplay, keysym));
+
+    return 0;
+}
+
+std::string
+ModifierHandler::keycodeToString (int keycode) const
+{
+    KeySym keysym;
+    std::string keyname;
+
+    keysym  = XKeycodeToKeysym (mDisplay, keycode, 0);
+    keyname = std::string (XKeysymToString (keysym));
+
+    return keyname;
+}
+
 ModifierHandler *
 ModifierHandler::Default ()
 {
@@ -212,7 +245,7 @@
     mModMap (0),
     mDisplay (display)
 {
-    for (int i = 0; i < ModNum; i++)
+    for (int i = 0; i < modifiers::ModNum; i++)
 	mModMask[i] = NoMask;
 }
 

=== modified file 'src/privatescreen.h'
--- src/privatescreen.h	2012-01-24 08:30:14 +0000
+++ src/privatescreen.h	2012-01-24 08:30:15 +0000
@@ -407,7 +407,7 @@
 	std::list< CompWatchFd * > watchFds;
 	CompWatchFdHandle        lastWatchFdHandle;
 
-	ModifierHandler          *modHandler;
+	compiz::modifiers::ModifierHandler          *modHandler;
 
 	std::map<CompString, CompPrivate> valueMap;
 

=== modified file 'src/screen.cpp'
--- src/screen.cpp	2012-01-24 08:30:14 +0000
+++ src/screen.cpp	2012-01-24 08:30:15 +0000
@@ -61,6 +61,8 @@
 #include "privateaction.h"
 #include "privatestackdebugger.h"
 
+using namespace compiz::modifiers;
+
 bool inHandleEvent = false;
 
 bool screenInitalized = false;
@@ -3069,7 +3071,7 @@
 
 
 
-    if (!(mask & CompNoMask))
+    if (!(mask & compiz::modifiers::NoMask))
     {
 	if (!grabUngrabKeys (mask, key.keycode (), true))
 	    return false;
@@ -3103,7 +3105,7 @@
 
 	    it = keyGrabs.erase (it);
 
-	    if (!(mask & CompNoMask))
+	    if (!(mask & compiz::modifiers::NoMask))
 		grabUngrabKeys (mask, key.keycode (), false);
 	}
     }
@@ -3118,7 +3120,7 @@
 
     for (it = keyGrabs.begin (); it != keyGrabs.end (); it++)
     {
-	if (!((*it).modifiers & CompNoMask))
+	if (!((*it).modifiers & compiz::modifiers::NoMask))
 	{
 	    grabUngrabKeys ((*it).modifiers,
 			    (*it).keycode, true);

=== modified file 'src/window.cpp'
--- src/window.cpp	2012-01-24 08:30:14 +0000
+++ src/window.cpp	2012-01-24 08:30:15 +0000
@@ -46,6 +46,8 @@
 #include "privatescreen.h"
 #include "privatestackdebugger.h"
 
+using namespace compiz::modifiers;
+
 PluginClassStorage::Indices windowPluginClassIndices (0);
 
 unsigned int
@@ -5872,7 +5874,7 @@
 	{
 	    unsigned int mods = ModifierHandler::Default ()->virtualToRealModMask (bind.modifiers);
 
-	    if (mods & CompNoMask)
+	    if (mods & compiz::modifiers::NoMask)
 		continue;
 
 	    for (unsigned int ignore = 0;