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 688252 - Maximum 3D size limitations
Maximum 3D size limitations
Status: RESOLVED FIXED
Product: gnome-session
Classification: Core
Component: gnome-session
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Session Maintainers
Session Maintainers
Depends on:
Blocks: 646280
 
 
Reported: 2012-11-13 13:54 UTC by Bastien Nocera
Modified: 2012-11-13 17:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gnome-session-check-accelerated: Use glib (1.20 KB, patch)
2012-11-13 13:56 UTC, Bastien Nocera
committed Details | Review
gnome-session-check-accelerated: Use gbooleans for retval (5.49 KB, patch)
2012-11-13 13:56 UTC, Bastien Nocera
reviewed Details | Review
gnome-session-check-accelerated: Better kcmdline parsing (2.22 KB, patch)
2012-11-13 13:56 UTC, Bastien Nocera
committed Details | Review
gnome-session-check-accelerated: Use constant for specific sizes (1.18 KB, patch)
2012-11-13 13:56 UTC, Bastien Nocera
committed Details | Review
gnome-session-check-accelerated: Set _GNOME_MAX_SCREEN_SIZE on the root window (3.43 KB, patch)
2012-11-13 13:56 UTC, Bastien Nocera
committed Details | Review
gnome-session-check-accelerated: Use gbooleans for retval (6.57 KB, patch)
2012-11-13 15:10 UTC, Bastien Nocera
none Details | Review
gnome-session-check-accelerated: Use gbooleans for retval (6.93 KB, patch)
2012-11-13 15:11 UTC, Bastien Nocera
needs-work Details | Review
gnome-session-check-accelerated: Simplify _has_extension() (2.58 KB, patch)
2012-11-13 15:17 UTC, Bastien Nocera
needs-work Details | Review

Description Bastien Nocera 2012-11-13 13:54:33 UTC
+++ This bug was initially created as a clone of Bug #646280 +++

The Intel GM945 chipset is very common, and has a limitation that the maximum 3D rendered area is 2048 pixels wide. Which isn't wide enough to handle a 1024x600 netbook + a 1280x1024 monitor.

Mesa is pretty good about automatically falling back to software rendering, so the observed effect is that if you plug in such a monitor, things become dead slow and your system becomes unusable.

See https://bugzilla.redhat.com/show_bug.cgi?id=678791

Here's a plan for how we should handle this case:

 * gnome-session-check-accelerated should query GL_MAX_RENDERBUFFER_SIZE_EXT and set the result as a property on the root window. ( _GNOME_MAX_SCREEN_SIZE)

 * When in non-fallback mode, the xrandr code in gnome-settings-daemon should check this property and then only pick monitor configurations that fit within GL_MAX_RENDERBUFFER_SIZE_EX. Ordering of the initial default configuration should be:

 - As currently, if it fits
 - Mirrored displays if that would result in a resolution of at least 1024x600
 - Otherwise, only the primary monitor with the secondary output off

 * The panel should respect the same limitations, so if you try to enable the one monitor, then the other monitor turns off and the primary switches to the on monitor.

The reasons for doing the query in gnome-session-check-accelerated are:

 - It already has the code to set up a glx context and query stuff from it
 - It's very important that this code doesn't run unless 
   gnome-session-check-accelerated passed, since GLX crashing on
   initialization is reasonable common and we want to handle it gracefully.
Comment 1 Bastien Nocera 2012-11-13 13:55:01 UTC
$ DISPLAY=:0 xprop -root | grep _GNOME
_GNOME_BACKGROUND_REPRESENTATIVE_COLORS(STRING) = "rgb(109,167,185)"
_GNOME_SESSION_ACCELERATED(CARDINAL) = 1
$ ./gnome-session-check-accelerated-helper 
$ DISPLAY=:0 xprop -root | grep _GNOME
_GNOME_MAX_SCREEN_SIZE(CARDINAL) = 16384
_GNOME_BACKGROUND_REPRESENTATIVE_COLORS(STRING) = "rgb(109,167,185)"
_GNOME_SESSION_ACCELERATED(CARDINAL) = 1
Comment 2 Bastien Nocera 2012-11-13 13:56:37 UTC
Created attachment 228887 [details] [review]
gnome-session-check-accelerated: Use glib
Comment 3 Bastien Nocera 2012-11-13 13:56:39 UTC
Created attachment 228888 [details] [review]
gnome-session-check-accelerated: Use gbooleans for retval

Instead of ints, with zero sometimes for failure, sometimes
for success.
Comment 4 Bastien Nocera 2012-11-13 13:56:42 UTC
Created attachment 228889 [details] [review]
gnome-session-check-accelerated: Better kcmdline parsing
Comment 5 Bastien Nocera 2012-11-13 13:56:45 UTC
Created attachment 228890 [details] [review]
gnome-session-check-accelerated: Use constant for specific sizes
Comment 6 Bastien Nocera 2012-11-13 13:56:48 UTC
Created attachment 228891 [details] [review]
gnome-session-check-accelerated: Set _GNOME_MAX_SCREEN_SIZE on the root window

Compute this from the minimum of the maximum texture size and the
maximum renderbuffer size.  gnome-settings-daemon may use this to avoid
setting mode configurations that exceed the screen dimensions.
Comment 7 Colin Walters 2012-11-13 14:27:38 UTC
Review of attachment 228887 [details] [review]:

I typically just jump straight to gio-unix-2.0, but this is fine too.
Comment 8 Colin Walters 2012-11-13 15:03:35 UTC
Review of attachment 228888 [details] [review]:

This one might just be me reading the patch in splinter wrong, but:

::: tools/gnome-session-check-accelerated-helper.c
@@ +344,1 @@
         return 1;

return ret; ?
Comment 9 Colin Walters 2012-11-13 15:05:02 UTC
Review of attachment 228889 [details] [review]:

Sure...
Comment 10 Bastien Nocera 2012-11-13 15:09:28 UTC
Review of attachment 228888 [details] [review]:

::: tools/gnome-session-check-accelerated-helper.c
@@ +344,1 @@
         return 1;

_has_extension() didn't get ported. Will update patch now.
Comment 11 Bastien Nocera 2012-11-13 15:10:08 UTC
Created attachment 228901 [details] [review]
gnome-session-check-accelerated: Use gbooleans for retval

Instead of ints, with zero sometimes for failure, sometimes
for success.
Comment 12 Bastien Nocera 2012-11-13 15:11:45 UTC
Created attachment 228903 [details] [review]
gnome-session-check-accelerated: Use gbooleans for retval

Instead of ints, with zero sometimes for failure, sometimes
for success.
Comment 13 Bastien Nocera 2012-11-13 15:17:14 UTC
Created attachment 228904 [details] [review]
gnome-session-check-accelerated: Simplify _has_extension()
Comment 14 Colin Walters 2012-11-13 15:36:22 UTC
Review of attachment 228890 [details] [review]:

Looks fine.
Comment 15 Colin Walters 2012-11-13 16:22:52 UTC
Review of attachment 228903 [details] [review]:

I think this is all right...man, the old code was a mess!
Comment 16 Colin Walters 2012-11-13 16:25:46 UTC
Review of attachment 228904 [details] [review]:

::: tools/gnome-session-check-accelerated-helper.c
@@ +329,1 @@
                 return FALSE;

Er...shouldn't this still be if (!extension_list) ?

Looks to me like we'll just always return false now.

Actually, you could just drop his bit of code, and rely on g_strsplit () returning an empty list I think.

@@ +329,2 @@
                 return FALSE;
+        g_return_val_if_fail (extension != NULL, TRUE);

Style: better to do g_return_if_fail() as the very first thing in a function.
Comment 17 Colin Walters 2012-11-13 16:28:06 UTC
Review of attachment 228891 [details] [review]:

Looks reasonable.  The code that sets the property could use a comment like /* Will be read by gnome-settings-daemon */ or something.
Comment 18 Bastien Nocera 2012-11-13 17:30:44 UTC
(In reply to comment #16)
> Review of attachment 228904 [details] [review]:
> 
> ::: tools/gnome-session-check-accelerated-helper.c
> @@ +329,1 @@
>                  return FALSE;
> 
> Er...shouldn't this still be if (!extension_list) ?
> 
> Looks to me like we'll just always return false now.

Yep, I broke that. There was also a bit more unported "gboolean" code in another call to that function, fixed in the commit.

> Actually, you could just drop his bit of code, and rely on g_strsplit ()
> returning an empty list I think.

Still need to check for extension_list != NULL though.

> @@ +329,2 @@
>                  return FALSE;
> +        g_return_val_if_fail (extension != NULL, TRUE);
> 
> Style: better to do g_return_if_fail() as the very first thing in a function.

Well, that means changing the retval if extension is NULL and extension_list is too. Not that it makes that much difference in reality...

(In reply to comment #17)
> Review of attachment 228891 [details] [review]:
> 
> Looks reasonable.  The code that sets the property could use a comment like /*
> Will be read by gnome-settings-daemon */ or something.

Done.
Comment 19 Bastien Nocera 2012-11-13 17:39:54 UTC
Attachment 228887 [details] pushed as 0a8a017 - gnome-session-check-accelerated: Use glib
Attachment 228889 [details] pushed as 97088bd - gnome-session-check-accelerated: Better kcmdline parsing
Attachment 228890 [details] pushed as 63e765c - gnome-session-check-accelerated: Use constant for specific sizes
Attachment 228891 [details] pushed as 313def1 - gnome-session-check-accelerated: Set _GNOME_MAX_SCREEN_SIZE on the root window