Merge lp:~cyphermox/unity/multimonitor into lp:unity

Proposed by Mathieu Trudel-Lapierre
Status: Merged
Merged at revision: 893
Proposed branch: lp:~cyphermox/unity/multimonitor
Merge into: lp:unity
Diff against target: 111 lines (+70/-0)
2 files modified
src/unityshell.cpp (+64/-0)
src/unityshell.h (+6/-0)
To merge this branch: bzr merge lp:~cyphermox/unity/multimonitor
Reviewer Review Type Date Requested Status
Sam Spilsbury (community) Needs Fixing
Alex Launi (community) Approve
Review via email: mp+50234@code.launchpad.net

Description of the change

I've been testing this code on a laptop with an external screen and it seems to work fine with most resolution changes (all secondary screen res changes, most res changes of the primary screen with the notable exception of 800x600 by my test on my hardware).

This has already been working just fine when first starting unity, whichever resolution that happens to be with.

To post a comment you must log in.
Revision history for this message
Alex Launi (alexlauni) wrote :

This looks pretty good to me. I can't test until nvidia drivers, but the code looks clean.

review: Approve
Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Hi Mathieu,

Great initiative here - just nitpicking a few things before we merge this:

1) You've got merge markers in the source (eg <<<<<<< TREE MERGE SOURCE), you need to remove them or it won't build :p
2) You have strutHackTimeout in UnityScreen::Relayout - this means that on init you'll have strutHackTimeout being called twice. Remove it from UnityScreen::Relayout
3) s/g_warning/g_debug/ The screen size change is *not* a warning
4) you've got:

+ g_signal_connect_swapped (gdk_screen_get_default (), "monitors-changed", G_CALLBACK (&UnityScreen::Relayout), (void*) this);
+ g_signal_connect_swapped (gdk_screen_get_default (), "size-changed", G_CALLBACK (&UnityScreen::Relayout), (void*) this);

That is going to cause UnityScreen::Relayout to be called twice in cases where the monitors changed and the size changed. That's bad

5) This is probably going to be a point of contention between me and the other developers, but I'd actually prefer it if we hooked up this ::outputChangeNotify wrappable function in compiz. The reason why is because outputs are user-configurable in compiz in case your graphics driver is horribly broken or you need to do multihead testing. In addition, it means that we're reacting to the output change once the window manager has done everything it needs to do, meaning racy weirdness when compiz reshuffles windows about to handle the output change and we've already moved before that.

review: Needs Fixing
Revision history for this message
Mirco Müller (macslow) wrote :

I fixed all but 5.) of the issues Sam raised. I've pushed the corrected branch (and re-merged with trunk) to lp:~canonical-dx-team/unity/unity.fix-675862

This is to avoid messing with other people's personal branches.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/unityshell.cpp'
--- src/unityshell.cpp 2011-02-21 13:21:49 +0000
+++ src/unityshell.cpp 2011-02-21 17:09:08 +0000
@@ -38,6 +38,7 @@
38#include <dbus/dbus-glib.h>38#include <dbus/dbus-glib.h>
39#include <glib/gi18n-lib.h>39#include <glib/gi18n-lib.h>
40#include <gtk/gtk.h>40#include <gtk/gtk.h>
41#include <gdk/gdk.h>
4142
42#include <core/atoms.h>43#include <core/atoms.h>
4344
@@ -543,6 +544,64 @@
543 }544 }
544}545}
545546
547void
548UnityScreen::Relayout ()
549{
550 GdkScreen *scr;
551 GdkRectangle rect;
552 nux::Geometry lCurGeom, pCurGeom;
553 gint primary_monitor;
554
555 scr = gdk_screen_get_default ();
556 primary_monitor = gdk_screen_get_primary_monitor (scr);
557 gdk_screen_get_monitor_geometry (scr, primary_monitor, &rect);
558
559 pCurGeom = panelWindow->GetGeometry();
560 lCurGeom = launcherWindow->GetGeometry();
561
562 panelWindow->EnableInputWindow(false);
563 launcherWindow->EnableInputWindow(false);
564
565 panelView->SetMaximumWidth(rect.width);
566 launcher->SetMaximumHeight(rect.height - pCurGeom.height);
567
568 g_warning ("setting to primary screen rect: x=%d y=%d w=%d h=%d",
569 rect.x, rect.y, rect.width, rect.height );
570
571 panelWindow->SetGeometry(nux::Geometry(rect.x,
572 rect.y,
573 rect.width,
574 pCurGeom.height));
575 panelView->SetGeometry(nux::Geometry(rect.x,
576 rect.y,
577 rect.width,
578 pCurGeom.height));
579
580 launcherWindow->SetGeometry(nux::Geometry(rect.x,
581 rect.y + pCurGeom.height,
582 lCurGeom.width,
583 rect.height - pCurGeom.height));
584 launcher->SetGeometry(nux::Geometry(rect.x,
585 rect.y + pCurGeom.height,
586 lCurGeom.width,
587 rect.height - pCurGeom.height));
588
589 panelWindow->EnableInputWindow(true);
590 launcherWindow->EnableInputWindow(true);
591
592 strutHackTimeout(this);
593}
594
595gboolean
596UnityScreen::RelayoutTimeout (gpointer data)
597{
598 UnityScreen *uScr = (UnityScreen*) data;
599
600 uScr->Relayout();
601
602 return FALSE;
603}
604
546/* Handle changes in the number of workspaces by showing the switcher605/* Handle changes in the number of workspaces by showing the switcher
547 * or not showing the switcher */606 * or not showing the switcher */
548bool607bool
@@ -651,6 +710,10 @@
651710
652 g_timeout_add (0, &UnityScreen::initPluginActions, this);711 g_timeout_add (0, &UnityScreen::initPluginActions, this);
653 g_timeout_add (5000, (GSourceFunc) write_logger_data_to_disk, NULL);712 g_timeout_add (5000, (GSourceFunc) write_logger_data_to_disk, NULL);
713
714 g_signal_connect_swapped (gdk_screen_get_default (), "monitors-changed", G_CALLBACK (&UnityScreen::Relayout), (void*) this);
715 g_signal_connect_swapped (gdk_screen_get_default (), "size-changed", G_CALLBACK (&UnityScreen::Relayout), (void*) this);
716
654 END_FUNCTION ();717 END_FUNCTION ();
655}718}
656719
@@ -749,6 +812,7 @@
749 self->launcher->SetLaunchAnimation (Launcher::LAUNCH_ANIMATION_PULSE);812 self->launcher->SetLaunchAnimation (Launcher::LAUNCH_ANIMATION_PULSE);
750 self->launcher->SetUrgentAnimation (Launcher::URGENT_ANIMATION_WIGGLE);813 self->launcher->SetUrgentAnimation (Launcher::URGENT_ANIMATION_WIGGLE);
751 g_timeout_add (2000, &UnityScreen::strutHackTimeout, self);814 g_timeout_add (2000, &UnityScreen::strutHackTimeout, self);
815 g_timeout_add (2000, &UnityScreen::RelayoutTimeout, self);
752816
753 END_FUNCTION ();817 END_FUNCTION ();
754}818}
755819
=== modified file 'src/unityshell.h'
--- src/unityshell.h 2011-02-21 11:34:16 +0000
+++ src/unityshell.h 2011-02-21 17:09:08 +0000
@@ -148,6 +148,12 @@
148 void148 void
149 onRedrawRequested ();149 onRedrawRequested ();
150150
151 void
152 Relayout ();
153
154 static gboolean
155 RelayoutTimeout (gpointer data);
156
151 static void157 static void
152 launcherWindowConfigureCallback(int WindowWidth, int WindowHeight, nux::Geometry& geo, void* user_data);158 launcherWindowConfigureCallback(int WindowWidth, int WindowHeight, nux::Geometry& geo, void* user_data);
153159