Generate screen modes at the exact requested size and refresh rate

server/pkg/xorg/xorg.c

@@ -303,7 +303,7 @@ void XGetScreenConfigurations() {
 
 // Inspired by https://github.com/raboof/xrandr/blob/master/xrandr.c
 // width and height are in/out: on return they contain the actual dimensions
-// of the created mode (libxcvt may round width to a multiple of 8).
+// of the created mode.
 void XCreateScreenMode(int *width, int *height, short rate) {
   Display *display = getXDisplay();
   Window root = DefaultRootWindow(display);
@@ -331,22 +331,27 @@ void XCreateScreenMode(int *width, int *height, short rate) {
 
 // Inspired by https://fossies.org/linux/xwayland/hw/xwayland/xwayland-cvt.c
 XRRModeInfo *XCreateScreenModeInfo(int hdisplay, int vdisplay, short vrefresh) {
-#ifdef _LIBCVT_H_
-  struct libxcvt_mode_info *mode_info;
-
-  // get screen mode from libxcvt, if available
-  // NOTE: libxcvt may round hdisplay up to a multiple of 8 (CVT convention).
-  // We use the actual output dimensions for both the mode name and geometry
-  // so that they are consistent.
-  mode_info = libxcvt_gen_mode_info(hdisplay, vdisplay, vrefresh, false, false);
-
   char name[128];
-  snprintf(name, sizeof name, "%dx%d_%d", mode_info->hdisplay, mode_info->vdisplay, vrefresh);
+  snprintf(name, sizeof name, "%dx%d_%d", hdisplay, vdisplay, vrefresh);
   XRRModeInfo *modeinfo = XRRAllocModeInfo(name, strlen(name));
 
-  modeinfo->width      = mode_info->hdisplay;
-  modeinfo->height     = mode_info->vdisplay;
-  modeinfo->dotClock   = mode_info->dot_clock * 1000;
+  modeinfo->width  = hdisplay;
+  modeinfo->height = vdisplay;
+
+#ifdef _LIBCVT_H_
+  struct libxcvt_mode_info *mode_info;
+
+  // libxcvt rounds hdisplay down to a multiple of 8 (CVT convention). Generate
+  // the timings for the width rounded up instead, so the requested width fits
+  // inside the active area and the mode keeps the exact requested size.
+  int hdisplay_cvt = (hdisplay + 7) & ~7;
+  mode_info = libxcvt_gen_mode_info(hdisplay_cvt, vdisplay, vrefresh, false, false);
+
+  // libxcvt also rounds the pixel clock down to a multiple of 250 kHz. On small
+  // modes that lowers the refresh rate enough for X to round it to vrefresh - 1
+  // (e.g. 1008x384 at 60 Hz becomes 59.45 Hz), and XRRSetScreenConfigAndRate
+  // then rejects vrefresh with BadValue. Use the exact clock for vrefresh.
+  modeinfo->dotClock   = (unsigned long) mode_info->htotal * mode_info->vtotal * vrefresh;
   modeinfo->hSyncStart = mode_info->hsync_start;
   modeinfo->hSyncEnd   = mode_info->hsync_end;
   modeinfo->hTotal     = mode_info->htotal;
@@ -356,14 +361,6 @@ XRRModeInfo *XCreateScreenModeInfo(int hdisplay, int vdisplay, short vrefresh) {
   modeinfo->modeFlags  = mode_info->mode_flags;
 
   free(mode_info);
-#else
-  // fallback to a simple mode without refresh rate
-  char name[128];
-  snprintf(name, sizeof name, "%dx%d_%d", hdisplay, vdisplay, vrefresh);
-  XRRModeInfo *modeinfo = XRRAllocModeInfo(name, strlen(name));
-
-  modeinfo->width = hdisplay;
-  modeinfo->height = vdisplay;
 #endif
 
   return modeinfo;

server/pkg/xorg/xorg.go

@@ -239,6 +239,32 @@ func ChangeScreenSize(s types.ScreenSize) (types.ScreenSize, error) {
 	return s, err
 }
 
+type screenModeInfo struct {
+	Width, Height                int
+	HSyncStart, HSyncEnd, HTotal int
+	VSyncStart, VSyncEnd, VTotal int
+	DotClock                     uint64
+}
+
+// newScreenModeInfo returns the mode XCreateScreenMode would create, without
+// adding it to the X server. Tests use it because they cannot call cgo directly.
+func newScreenModeInfo(width, height int, rate int16) screenModeInfo {
+	info := C.XCreateScreenModeInfo(C.int(width), C.int(height), C.short(rate))
+	defer C.XRRFreeModeInfo(info)
+
+	return screenModeInfo{
+		Width:      int(info.width),
+		Height:     int(info.height),
+		HSyncStart: int(info.hSyncStart),
+		HSyncEnd:   int(info.hSyncEnd),
+		HTotal:     int(info.hTotal),
+		VSyncStart: int(info.vSyncStart),
+		VSyncEnd:   int(info.vSyncEnd),
+		VTotal:     int(info.vTotal),
+		DotClock:   uint64(info.dotClock),
+	}
+}
+
 func GetScreenSize() types.ScreenSize {
 	mu.Lock()
 	defer mu.Unlock()

server/pkg/xorg/xorg_test.go

@@ -0,0 +1,42 @@
+package xorg
+
+import "testing"
+
+// refreshRate rounds the way the X server does when it lists the rates of a
+// mode (RRVerticalRefresh), which is what XRRSetScreenConfigAndRate matches.
+func (m screenModeInfo) refreshRate() int {
+	dots := uint64(m.HTotal) * uint64(m.VTotal)
+	return int((m.DotClock + dots/2) / dots)
+}
+
+func TestNewScreenModeInfo(t *testing.T) {
+	// Sizes whose libxcvt timings run slightly below the requested rate
+	// (e.g. 1008x384@60 at 59.45 Hz), widths that are not a multiple of 8, and
+	// the 1360x768 size that libxcvt widens to 1366.
+	sizes := [][2]int{
+		{1008, 384}, {976, 384}, {1024, 384}, {1112, 384}, {1200, 384},
+		{1008, 368}, {788, 300}, {630, 240}, {379, 480}, {1003, 500},
+		{1205, 600}, {1360, 768}, {1366, 768}, {64, 64}, {390, 844},
+		{1000, 384}, {1920, 1080}, {3840, 2160},
+	}
+	rates := []int16{10, 25, 30, 59, 60}
+
+	for _, size := range sizes {
+		for _, rate := range rates {
+			m := newScreenModeInfo(size[0], size[1], rate)
+
+			if m.Width != size[0] || m.Height != size[1] {
+				t.Errorf("%dx%d@%d: mode is %dx%d", size[0], size[1], rate, m.Width, m.Height)
+			}
+			if got := m.refreshRate(); got != int(rate) {
+				t.Errorf("%dx%d@%d: X lists the mode at %d Hz", size[0], size[1], rate, got)
+			}
+			if m.Width > m.HSyncStart || m.HSyncStart > m.HSyncEnd || m.HSyncEnd > m.HTotal {
+				t.Errorf("%dx%d@%d: invalid horizontal timings %+v", size[0], size[1], rate, m)
+			}
+			if m.Height > m.VSyncStart || m.VSyncStart > m.VSyncEnd || m.VSyncEnd > m.VTotal {
+				t.Errorf("%dx%d@%d: invalid vertical timings %+v", size[0], size[1], rate, m)
+			}
+		}
+	}
+}