After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 98698 - List windows from only one xinerama monitor
List windows from only one xinerama monitor
Status: RESOLVED FIXED
Product: libwnck
Classification: Core
Component: general
2.8.x
Other Linux
: High enhancement
: ---
Assigned To: libwnck maintainers
libwnck maintainers
Depends on:
Blocks:
 
 
Reported: 2002-11-16 12:42 UTC by Jürg Billeter
Modified: 2004-12-22 21:47 UTC
See Also:
GNOME target: 2.8.x
GNOME version: 2.7/2.8


Attachments
Patch to extend libwnck with multi-head support (5.96 KB, patch)
2002-11-16 12:45 UTC, Jürg Billeter
none Details | Review
Patch to provide multimonitor support in libwnck (5.35 KB, patch)
2003-02-06 12:07 UTC, Jürg Billeter
none Details | Review
Patch against libwnck-2.3.1 w/automatic monitor detection (4.62 KB, patch)
2003-06-05 12:26 UTC, Jürg Billeter
none Details | Review
Patch against libwnck-2.5.1 (2.27 KB, patch)
2003-12-25 15:42 UTC, Jürg Billeter
none Details | Review
Patch with leak fix and correct applet dragging behaviour (3.55 KB, patch)
2003-12-26 00:14 UTC, Jürg Billeter
none Details | Review
Patch with fix for off-monitor window centers and _slightly_ enhanced speed (4.17 KB, patch)
2003-12-26 11:42 UTC, Jürg Billeter
none Details | Review
Rediff against current cvs, added changelog entry (4.76 KB, patch)
2004-09-17 21:23 UTC, Jürg Billeter
none Details | Review

Description Jürg Billeter 2002-11-16 12:42:04 UTC
I have a dual-head setup with xinerama. On each head I have added a gnome
panel with a window-list applet. It's not so pleasant to view all windows
on each head. Therefore libwnck should have an option to restrict the
window list to one head.

I'm new in coding for gnome (linux generally) but I've tried to
implement something like that for my own purpose. It would be nice to have
this extension in CVS.
Comment 1 Jürg Billeter 2002-11-16 12:45:21 UTC
Created attachment 12339 [details] [review]
Patch to extend libwnck with multi-head support
Comment 2 Havoc Pennington 2002-11-16 17:14:32 UTC
Thanks, I think this is a reasonable idea as panels are per-monitor.
I don't think it's necessary to make it an option right away; we could
try just doing it this way by default, and if that fails be even more
clever and do it this way if there's a tasklist on each monitor, 
and do it the old way if there's only a single tasklist. That way 
stuff just works.

A couple changes I would make to the patch:

 - "multihead" normally refers to multiple screens, not to xinerama. 
   the xinerama heads are "monitors" and the "real" heads are 
   "screens" (where "screen" means "separate root window"). 
   Anyhow I think an s/head/monitor/ would clarify things
   for future maintenance.

 - the Xinerama checks and usage are Linux-specific, you have to do 
   more complicated configure.in stuff, see GDK or metacity. However
   the easy course of action is to just use GDK 2.1.x for this, 
   i.e. gdk_screen_get_num_monitors() and so on. IMO that's the 
   way to go, then it will automatically work on Solaris etc.

Thanks again
Comment 3 Jürg Billeter 2002-11-16 23:26:17 UTC
I wanted to do it without an option but I couldn't figure out how to 
get the current monitor of the widget. Perhaps you have me a clue on 
this?

Sorry for the wrong name, the reason is the DualHead feature of 
Matrox - since then I call it head.

Thanks for the tip regarding the Linux-specificity, I'll try to 
implement it using GDK 2.1.
Comment 4 Havoc Pennington 2002-11-16 23:30:10 UTC
The simplest way to get the current monitor is probably to 
use gdk_screen_get_monitor_at_point() or whatever it's called.
Comment 5 Jürg Billeter 2002-11-16 23:34:46 UTC
My problem is how to get the point on the screen within libwnck, not 
how to get the right monitor for a defined point. Perhaps that's a 
trivial question but I'm new to GTK programming as I've stated 
before.
Comment 6 Havoc Pennington 2002-11-17 00:03:33 UTC
The hacky thing is gdk_window_get_origin(tasklist->window) probably,
though that requires the applet to be realized and in-place before 
it works obviously.

A nicer solution might be to have the panel tell applets somehow 
what monitor the panel is on, then have the applet pass the 
monitor number to libwnck. That would probably be more robust 
and less annoying to implement.
Comment 7 Jürg Billeter 2003-02-06 12:06:44 UTC
I've now changed the patch as proposed. It refers to monitors 
instead of heads and it doesn't rely anymore on Linux/Xinerama.

The monitor number has still to be passed as an argument, as I can't 
figure a way out of this. But in my gnome-panel patch (Bug 98700), I 
found a solution, how to recognize the active monitor. This 
information gets passed to libwnck.
Comment 8 Jürg Billeter 2003-02-06 12:07:56 UTC
Created attachment 14155 [details] [review]
Patch to provide multimonitor support in libwnck
Comment 9 Jürg Billeter 2003-03-05 19:19:58 UTC
Would be nice if someone could review my patch and if it's ok, 
commit it.
Comment 10 Jürg Billeter 2003-06-05 12:24:58 UTC
I've improved my patch. Now it's got an automatic monitor detection,
so there is no need for the gnome-panel patch anymore.
Also updated to apply cleanly on libwnck-2.3.1.
Comment 11 Jürg Billeter 2003-06-05 12:26:26 UTC
Created attachment 17166 [details] [review]
Patch against libwnck-2.3.1 w/automatic monitor detection
Comment 12 Jürg Billeter 2003-06-11 08:51:40 UTC
Havoc, me again ;) Would be nice to see a comment here, too.
Comment 13 Jürg Billeter 2003-06-20 07:00:19 UTC
*ping*
Comment 14 Jürg Billeter 2003-12-25 15:41:51 UTC
I've updated my patch. It now applies cleanly on libwnck-2.5.1 and cvs
head.

Would be nice to get some feedback.
Comment 15 Jürg Billeter 2003-12-25 15:42:33 UTC
Created attachment 22708 [details] [review]
Patch against libwnck-2.5.1
Comment 16 Havoc Pennington 2003-12-25 16:31:19 UTC
Are you leaking a region object here?

on_active_monitor = gdk_region_point_in (gdk_region_rectangle (
+                                 &tasklist->priv->monitor_geometry),
+                                 x + w / 2, y + h / 2);


Does the patch work properly if you drag the tasklist applet from one
monitor to another? it looks like monitor_num may not be updated in
that case.

Thanks
Comment 17 Jürg Billeter 2003-12-26 00:13:17 UTC
I've fixed the region leak; I didn't look at my old code well enough...

You were right that it didn't work properly when dragging the tasklist
applet from one monitor to another. I had only tested the dragging
with the test-tasklist application, there it worked because the test
application window does not skip the taskbar.

I've now implemented an additional check and now applet dragging works
properly. I hope the implementation is acceptable, I haven't got a
better idea.
Comment 18 Jürg Billeter 2003-12-26 00:14:42 UTC
Created attachment 22711 [details] [review]
Patch with leak fix and correct applet dragging behaviour
Comment 19 Havoc Pennington 2003-12-26 05:04:06 UTC
Re: the region code, why create a region at all now that I think about
it; that's somewhat expensive. Something like 

#define POINT_IN_RECT(xcoord, ycoord, rect) \
 ((xcoord) >= (rect).x &&                   \
  (xcoord) <  ((rect).x + (rect).width) &&  \
  (ycoord) >= (rect).y &&                   \
  (ycoord) <  ((rect).y + (rect).height))

Usage of gdk_screen_get_monitor_at_window() should be avoided if we
can, as it will be a round-trip network request to the X server. It
would be better to use cached positions of things on the client side.
gdk_window_get_position() uses the client-side position without a
round trip.

I'm not sure I understand why the tasklist being skip_tasklist
matters, what am I missing?
Comment 20 Havoc Pennington 2003-12-26 05:08:24 UTC
problem though, gdk_window_get_position doesn't return the position in
root window coordinates. so the round trip may be needed, or else some
relatively elaborate solution...
Comment 21 Jürg Billeter 2003-12-26 11:39:51 UTC
POINT_IN_RECT: Sure, will be more efficient. There is no
gdk_rectangle_point_in() but a gdk_region_point_in(), so I thought
that's the way to go. I've changed that.
Unfortunately it can't be used in wnck_tasklist_include_window. I had
to replace my buggy code there with gdk_screen_get_monitor_at_point()
(no round trip), the reason is that windows could get lost if their
center is not on any monitor.

I don't know a possibility to avoid gdk_screen_get_monitor_at_window()
but I have limited its use in wnck_tasklist_window_changed_geometry()
by an additional condition. But gdk_screen_get_monitor_at_window()
does only use a round trip to get the window coordinates and caches
the xinerama monitors, at least not more than one round trip per call.

tasklist and skip_tasklist: Let's have a look at the moment when the
tasklist changes monitor in two situations
- The simple case of test-tasklist (no relevant skip_tasklist): Before
the monitor change the test-tasklist itself is shown on the tasklist.
During the monitor change libwnck gets a geometry_changed signal with
win_task != NULL but !show (because test-tasklist is off-monitor), so
it will re-generate the task list.
- The case of gnome-panel (has skip_tasklist): Before the monitor
change the panel / tasklist itself is not shown on the tasklist.
During the monitor change libwnck gets a geometry_changed signal with
win_task == NULL and !show, so it _won't_ re-generate the task list.
But if we integrate my extra check, it will correctly re-generate the
task list.
Comment 22 Jürg Billeter 2003-12-26 11:42:31 UTC
Created attachment 22718 [details] [review]
Patch with fix for off-monitor window centers and _slightly_ enhanced speed
Comment 23 Kjartan Maraas 2004-04-18 21:17:05 UTC
This looks like it should be looked at again for 2.7.x.
Comment 24 Jürg Billeter 2004-04-18 21:42:46 UTC
Would be nice to get this finally in (or at least a reason why not). The
supplied patch still applies as of libwnck 2.6.0.1.
Comment 25 Josh Hansen 2004-08-10 23:11:10 UTC
2.7.x is now open. Perhaps now is the time to merge this patch?
Comment 26 Havoc Pennington 2004-08-12 02:41:51 UTC
Comment on attachment 22718 [details] [review]
Patch with fix for off-monitor window centers and _slightly_ enhanced speed

The latest patch looks good to me. Mark is it OK to commit with current freeze
rules?
Comment 27 Elijah Newren 2004-08-13 04:18:17 UTC
I'm cc'ing Mark so that he will see Havoc's question in the last comment.
Comment 28 Mark McLoughlin 2004-08-16 10:03:04 UTC
Its not really a feature, there's no impact on docs or translators and it looks
like a relatively safe patch. No problems with comitting it...
Comment 29 Jürg Billeter 2004-09-17 20:36:47 UTC
As this hasn't been committed before 2.8 even though you wanted to, could you
please commit it now, so this gets in at last for 2.9/2.10?
Comment 30 Elijah Newren 2004-09-17 21:03:16 UTC
Jürg: Mark and Havoc both said it'd be okay and wouldn't break freezes.  Do you
have cvs commit access?  If not, I'll go ahead and commit it for you on the
stable branch so it'll be in Gnome 2.8.1.
Comment 31 Jürg Billeter 2004-09-17 21:23:08 UTC
Created attachment 31657 [details] [review]
Rediff against current cvs, added changelog entry

I don't have cvs commit access, so it'd be nice if you could commit the patch.
I've rediffed against current cvs and added a short changelog entry. Thx
Comment 32 Elijah Newren 2004-09-17 22:18:32 UTC
committed.
Comment 33 J.H.M. Dassen (Ray) 2004-10-04 21:07:05 UTC
This change appears to cause some problems, outlined in
http://bugs.debian.org/274890 .
Comment 34 Elijah Newren 2004-10-04 21:25:50 UTC
Yes, same issue was pointed out in bug 154040.  Havoc, Mark: Should we revert
until Jurg can provide an updated patch that fixes all cases?  
Comment 35 Mark McLoughlin 2004-10-05 05:50:49 UTC
No, I don't think its a huge issue - one that I'd like to see fixed, but there's
no real need to revert.
Comment 36 Rick Schippers 2004-10-26 07:53:36 UTC
This change is really annoying when using twinview on nvidia cards. Please
create an option so you can switch back to the old behaviour.
Comment 37 Havoc Pennington 2004-10-26 17:25:04 UTC
What mode are you using twinview in? Just like regular xinerama, or some sort of
zoom/clone/app-exclusive/etc. setup?

An option isn't as good as just detecting the active setup and doing the right
thing (for example basing behavior on number of tasklist applets active)
Comment 38 Lars Hamann 2004-10-26 18:36:32 UTC
I'm using plain Nvidia-Twinview with XineramaInfo.
I.e.:

    Option "TwinView"
    Option "ConnectedMonitor" "CRT, CRT"
    Option "TwinViewOrientation" "RightOf"
    Option "SecondMonitorHorizSync" "30-96"
    Option "SecondMonitorVertRefresh" "50-160"
    Option "MetaModes" "1400x1050, 1400x1050"

That's it.

The right thing(tm) is to be consisten in the first place.
As I already mentioned in Bug 154040, Window List is the
only applet which distinguishes monitors of the same screen.
This simply doesn't feels right. I can't set different backgrounds
on different monitors but I've to run two panels to see all
windows?
If you don't want to present additional options, then you should
revert this patch.
Comment 39 Rob Adams 2004-10-27 20:19:16 UTC
For a while now I've been noticing that sometime windows seem to disappear from
my taskbar applet.  It wasn't until a few moments ago I realized it was now only
showing me the windows from one of the xinerama screens.

Frankly, I can't imagine why I would want that.  Now I'd have to add a panel on
the other xinerama just so I can have those windows show up.

I could almost imagine this being a preference, which is perhaps smart enough to
only appear if you actually have a real xinerama setup, but really, it seems to
me this should simply be reverted.
Comment 40 Elijah Newren 2004-10-27 20:31:46 UTC
There's a patch in bug 154041 to fix the "slowly disappearing" windows aspect. 
That is supposed to make it so that the system immediately only shows windows
from one of the xinerama screens.

I don't use Xinerama myself (though I really wish I had the money for more
monitors so I could use it...), but I have to agree with Rob.  If the default is
to only have one panel (as I've been led to believe from what I've read/heard),
why do we keep a patch that breaks that default?  Users are unlikely to realize
that they have to add another panel on the other screen, but if they don't do so
then they can't make use of them other than manually.  In constrast, reverting
this patch provides no more problems than having windows listed once on each
monitor.

(See also bug 154040)