Patch for XrandR.java

[Matthias]

ok - next try (first try was killed by this great forum software!)

The following "xrandr -q" output causes a ArrayIndexOutOfBoundsException in XrandR.java:

Screen 0: minimum 320 x 200, current 3360 x 1050, maximum 8192 x 8192
VGA-0 connected 1680x1050+0+0 (normal left inverted right x axis y axis) 434mm x 270mm
   1680x1050      60.0*+
   1280x1024      75.0     60.0  
   1152x864       75.0  
   1024x768       75.1     60.0  
   800x600        75.0     60.3  
   640x480        75.0     60.0  
   720x400        70.1  
LVDS connected (normal left inverted right x axis y axis)
   1680x1050      60.1 +
   1400x1050      60.0  
   1280x1024      59.9  
   1440x900       59.9  
   1280x960       59.9  
   1280x854       59.9  
   1280x800       59.8  
   1280x720       59.9  
   1152x768       59.8  
   1024x768       59.9  
   800x600        59.9  
   848x480        59.7  
   720x480        59.7  
   640x480        59.4  
S-video disconnected (normal left inverted right x axis y axis)
DVI-0 connected 1680x1050+1680+0 (normal left inverted right x axis y axis) 434mm x 270mm
   1680x1050      59.9*+
   1280x1024      75.0     60.0  
   1152x864       75.0  
   1024x768       75.1     60.0  
   800x600        75.0     60.3  
   640x480        75.0     60.0  
   720x400        70.1

The attached patch should fix that

[Matthias]

patch:

Index: src/java/org/lwjgl/opengl/XRandR.java
===================================================================
--- src/java/org/lwjgl/opengl/XRandR.java	(revision 3339)
+++ src/java/org/lwjgl/opengl/XRandR.java	(working copy)
@@ -39,6 +39,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 /**
@@ -82,10 +83,10 @@
 						name = sa[0];
 
 						// record the current config
-						currentList.add(new Screen(name, sa[2]));
+						parseScreen(currentList, name, sa[2]);
 					} else if (Pattern.matches("\\d*x\\d*", sa[0])) {
 						// found a new mode line
-						possibles.add(new Screen(name, sa[0]));
+						parseScreen(possibles, name, sa[0]);
 					}
 				}
 
@@ -179,7 +180,43 @@
 		// clone the array to prevent held copies being altered
 		return (Screen[]) ((Screen[]) screens.get(name)).clone();
 	}
+    
+    private static final Pattern SCREEN_PATTERN1 = Pattern.compile("^(\\d+)x(\\d+)\\+(\\d+)\\+(\\d+)$");
+    private static final Pattern SCREEN_PATTERN2 = Pattern.compile("^(\\d+)x(\\d+)$");
 
+    /**
+     * Parses a screen configuration and adds it to the list if it's valid.
+     * 
+     * @param list
+     *           the list to add the Screen to if it's valid 
+     * @param name
+     *           the name of this screen                          
+	 * @param conf
+	 *           config string, format either widthxheight or
+	 *           widthxheight+xPos+yPos
+	 */             
+    private static void parseScreen(List /* <Screen> */ list, String name, String what) {
+        Matcher m = SCREEN_PATTERN1.matcher(what);
+        if(!m.matches()) {
+            m = SCREEN_PATTERN2.matcher(what);
+            if(!m.matches()) {
+                System.out.println("Did not match: " + what);
+                return;
+            }
+        }
+        int width = Integer.parseInt(m.group(1));
+        int height = Integer.parseInt(m.group(2));
+        int xpos, ypos;
+        if(m.groupCount() > 3) {
+            xpos = Integer.parseInt(m.group(3));
+            ypos = Integer.parseInt(m.group(4));
+        } else {
+            xpos = 0;
+            ypos = 0;
+        }
+        list.add(new Screen(name, width, height, xpos, ypos));
+    }
+    
 	/**
 	 * Encapsulates the configuration of a monitor. Resolution is
 	 * fixed, position is mutable
@@ -213,24 +250,12 @@
 		 */
 		public int			yPos	= 0;
 
-		/**
-		 * @param name
-		 *           name of the screen
-		 * @param conf
-		 *           config string, format either widthxheight or
-		 *           widthxheight+xPos+yPos
-		 */
-		private Screen(String name, String conf) {
+		private Screen(String name, int width, int height, int xPos, int yPos) {
 			this.name = name;
-
-			String[] sa = conf.split("\\D");
-			width = Integer.parseInt(sa[0]);
-			height = Integer.parseInt(sa[1]);
-
-			if (sa.length > 2) {
-				xPos = Integer.parseInt(sa[2]);
-				yPos = Integer.parseInt(sa[3]);
-			}
+			this.width = width;
+			this.height = height;
+			this.xPos = xPos;
+			this.yPos = yPos;
 		}
 
 		private void getArgs(List/* <String> */argList) {

[kappa]

committed.

[spasi]

I encountered this as well the other day (had my secondary monitor connected but disabled). But I fixed it by simply not adding connected & disabled screens to the currentList. Also, I don't think SCREEN_PATTERN2 will ever match anything, will it? And shouldn't we replace that System.out.println with LWJGLUtil.log?

[Matthias]

The code is also used to parse part of the mode list which only has widthxheight and the 2nd pattern will match this.

[princec]

(Just an aside: maybe I'm being a bit of a luddite but I wonder whether it might not be simpler to actually link to libxrandr and talk to it directly?)

Cas

[ryanm]

Yep, that would be the ideal solution. It just requires that someone actually do it.
The scope of XRandR.java is very limited - it only exists to properly restore the screen configuration on shutdown on linux systems with multiple monitors. All mode switching is already done through libxrandr.
Until LWJGL moves to support multiple displays, I reckon it'd be a lot of effort for little gain to do this stuff in native code.

Find attached a slightly different version of XRandR. My Eclipse has auto-formatted from the svn version, so a diff won't be terribly illuminating, but the changes are:

• Catches Throwable in populate() - any problems in parsing the xrandr output and we revert to LWJGL's regular behaviour
• Nothing going to System.out or System.err - any exceptions or unexpected output go to LWJGLUtil.log()

[kappa]

ryanm just downloaded your patch, seems like its incomplete and that the bottom part of the src code is missing.

could you re-upload please, also do you include matthiasm's fixes too?

[ryanm]

I downloaded from the svn and then made changes, so It does include Matthias's stuff. Don't know what happened with the attachment, try this one

edit: that one does the same thing. Very odd. Get it here

[kappa]

same problem, still 8.4kb big.

you could try embedded it in the actual post.

or hop on to #lwjgl on freenode.net (most dev's hang out there) and just pastebin it on irc.

[ryanm]

The linked version didn't work for you? Is the internet allergic to this file or something? How about this?

[kappa]

The linked version didn't work for you? Is the internet allergic to this file or something? How about this?

sorry not sure how I missed the linked version, just saw another patch attached to the forum post. But the link version does work.

you missed some casting in the your code which was causing it to fail when compiling, anyhow changed that.

Patch committed.

[ryanm]

You must have caught me in the period between:

• discovering that simply repeating past actions doesn't give different results
• hosting the file elsewhere.