nsSreenManagerGTK should use GTK APIs instead of Xinerama

NEW
Unassigned

Status

()

P5
normal
3 years ago
7 months ago

People

(Reporter: m_kato, Unassigned)

Tracking

Trunk
All
Linux
Points:
---

Firefox Tracking Flags

(firefox47 affected)

Details

(Whiteboard: tpi:+)

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
To get screen size / available size per monitor, we still use xinerama.  But it can replace with the following GTK3 APIs

- gdk_screen_get_monitor_geometry
- gdk_screen_get_monitor_workarea (GTK3.4)
- gdk_screen_get_primary_monitor

Also, current implementation doesn't get correct workarea when using dual monitors.  So it can resolve it using GTK APIs
(Reporter)

Comment 1

3 years ago
Created attachment 8730598 [details]
MozReview Request: Bug 1252408 - nsScreenMangerGtk should use GTK API instead of Xinerama. r?karlt

Curent code by Xinerama doesn't get vaild workarea size if using dual monitor.  GTK3 has APIs to get workarea size and current screen size per monitor.  So we should use it instead of Xinerama.  Even if GTK2, it can get current screen size per monitor by GTK APIs.

Review commit: https://reviewboard.mozilla.org/r/40037/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/40037/
Attachment #8730598 - Flags: review?(karlt)
Comment on attachment 8730598 [details]
MozReview Request: Bug 1252408 - nsScreenMangerGtk should use GTK API instead of Xinerama. r?karlt

https://reviewboard.mozilla.org/r/40037/#review37123

Nice, thanks!

Looking at GDK sources, it seems that monitors-changed won't necessarily be received when _NET_WORKAREA changes, so we do need to continue to use root_window_event_filter(), as retained with this patch.

::: widget/gtk/nsScreenGtk.cpp:197
(Diff revision 1)
> -#ifdef MOZ_X11
>  void
> -nsScreenGtk :: Init (XineramaScreenInfo *aScreenInfo)
> +nsScreenGtk :: Init (GdkScreen *aScreen, gint aIndex, gint aTotal)
>  {
> -  nsIntRect xineRect(aScreenInfo->x_org, aScreenInfo->y_org,
> -                     aScreenInfo->width, aScreenInfo->height);
> +#if (MOZ_WIDGET_GTK == 2)
> +  // GTK 3.4 early doesn't have gdk_screen_get_monitor_workarea

Probably the best way to say this is "gdk_screen_get_monitor_workarea() was introduced in GTK 3.4." or "GTK versions prior to 3.4 don't have gdk_screen_get_monitor_workarea()."

::: widget/gtk/nsScreenGtk.cpp:207
(Diff revision 1)
> +  mRect = nsIntRect(rect.x * scale, rect.y * scale,
> +                    rect.width * scale, rect.height * scale);
> +
> +#if (MOZ_WIDGET_GTK == 3)
> +  gdk_screen_get_monitor_workarea(aScreen, aIndex, &rect);
> +  mAvailRect = nsIntRect(rect.x * scale, rect.y * scale,

Maybe use m(Avail)Rect.SetRect().

I don't mind.
Attachment #8730598 - Flags: review?(karlt) → review+
Makoto:  do you want to try to fix/land this old patch?
Flags: needinfo?(m_kato)
Priority: -- → P5
Whiteboard: tpi:+

Updated

7 months ago
Flags: needinfo?(m_kato)
You need to log in before you can comment on or make changes to this bug.