Skip to content

Commit abe9ac5

Browse files
fix: remove unnecessary screen width rounding in ChangeScreenSize (#11)
## Summary - Removes the Go-level `s.Width = s.Width - (s.Width % 8)` rounding in `ChangeScreenSize` that was silently truncating requested widths (e.g. 390 → 384) - Makes `XCreateScreenMode` return the actual dimensions that `libxcvt` produced via in/out pointer parameters, so the Go caller uses the correct width for `XSetScreenConfiguration` - Fixes the mode name in `XCreateScreenModeInfo` to use `libxcvt`'s actual output dimensions instead of the original request, preventing name/geometry mismatches ## Context The rounding was originally added to match `libxcvt`'s CVT convention of producing widths as multiples of 8. However, CVT is a timing standard for generating timing parameters for physical monitors — not a hard requirement of X.Org or the Xorg dummy driver used for virtual displays. The rounding made it impossible to use non-multiple-of-8 resolutions like 390x844 for mobile viewports, even when valid modelines existed in `xorg.conf`, because Neko would turn 390 into 384 before ever searching for a mode. Now the resolution flow is: 1. **Exact match** — if a pre-defined modeline exists in `xorg.conf` for the exact requested dimensions (e.g. `390x844`), `XSetScreenConfiguration` finds and uses it directly. 2. **Dynamic creation** — if no match, `XCreateScreenMode` dynamically creates a new mode via `libxcvt` and reports the actual dimensions back to the caller. ## Verified Tested end-to-end in a headful `chromium-headful` container: - Requested 390x844@25Hz via the kernel-images API - xrandr confirmed display at exactly `390x844_25` (using a manually crafted modeline in `xorg.conf` that bypasses CVT rounding) - CDP verification confirmed `screen.width=390`, `screen.height=844`, `innerWidth=390`, `innerHeight=807` (37px app-mode title bar) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches XRandR mode selection/creation and changes the resolution-setting flow, which could affect display configuration behavior across environments, but the change is localized to the Xorg wrapper. > > **Overview** > `ChangeScreenSize` no longer truncates widths to a multiple of 8 before attempting to apply a screen mode; it now tries to set the exact requested dimensions first and only creates a new mode if no existing mode matches. > > `XCreateScreenMode` is updated to take width/height as in/out pointers so the caller receives libxcvt’s *actual* generated dimensions, and `XCreateScreenModeInfo` now names modes using those actual dimensions to avoid name/geometry mismatches. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 3f90ca6. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 5073274 commit abe9ac5

File tree

3 files changed

+28
-13
lines changed

3 files changed

+28
-13
lines changed

server/pkg/xorg/xorg.c

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -302,12 +302,18 @@ void XGetScreenConfigurations() {
302302
}
303303

304304
// Inspired by https://github.com/raboof/xrandr/blob/master/xrandr.c
305-
void XCreateScreenMode(int width, int height, short rate) {
305+
// width and height are in/out: on return they contain the actual dimensions
306+
// of the created mode (libxcvt may round width to a multiple of 8).
307+
void XCreateScreenMode(int *width, int *height, short rate) {
306308
Display *display = getXDisplay();
307309
Window root = DefaultRootWindow(display);
308310

309311
// create new mode info
310-
XRRModeInfo *mode_info = XCreateScreenModeInfo(width, height, rate);
312+
XRRModeInfo *mode_info = XCreateScreenModeInfo(*width, *height, rate);
313+
314+
// write back the actual dimensions that were created
315+
*width = mode_info->width;
316+
*height = mode_info->height;
311317

312318
// create new mode
313319
RRMode mode = XRRCreateMode(display, root, mode_info);
@@ -325,16 +331,19 @@ void XCreateScreenMode(int width, int height, short rate) {
325331

326332
// Inspired by https://fossies.org/linux/xwayland/hw/xwayland/xwayland-cvt.c
327333
XRRModeInfo *XCreateScreenModeInfo(int hdisplay, int vdisplay, short vrefresh) {
328-
char name[128];
329-
snprintf(name, sizeof name, "%dx%d_%d", hdisplay, vdisplay, vrefresh);
330-
XRRModeInfo *modeinfo = XRRAllocModeInfo(name, strlen(name));
331-
332334
#ifdef _LIBCVT_H_
333335
struct libxcvt_mode_info *mode_info;
334336

335337
// get screen mode from libxcvt, if available
338+
// NOTE: libxcvt may round hdisplay up to a multiple of 8 (CVT convention).
339+
// We use the actual output dimensions for both the mode name and geometry
340+
// so that they are consistent.
336341
mode_info = libxcvt_gen_mode_info(hdisplay, vdisplay, vrefresh, false, false);
337342

343+
char name[128];
344+
snprintf(name, sizeof name, "%dx%d_%d", mode_info->hdisplay, mode_info->vdisplay, vrefresh);
345+
XRRModeInfo *modeinfo = XRRAllocModeInfo(name, strlen(name));
346+
338347
modeinfo->width = mode_info->hdisplay;
339348
modeinfo->height = mode_info->vdisplay;
340349
modeinfo->dotClock = mode_info->dot_clock * 1000;
@@ -349,6 +358,10 @@ XRRModeInfo *XCreateScreenModeInfo(int hdisplay, int vdisplay, short vrefresh) {
349358
free(mode_info);
350359
#else
351360
// fallback to a simple mode without refresh rate
361+
char name[128];
362+
snprintf(name, sizeof name, "%dx%d_%d", hdisplay, vdisplay, vrefresh);
363+
XRRModeInfo *modeinfo = XRRAllocModeInfo(name, strlen(name));
364+
352365
modeinfo->width = hdisplay;
353366
modeinfo->height = vdisplay;
354367
#endif

server/pkg/xorg/xorg.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -201,9 +201,6 @@ func ChangeScreenSize(s types.ScreenSize) (types.ScreenSize, error) {
201201
mu.Lock()
202202
defer mu.Unlock()
203203

204-
// round width to 8, because of Xorg
205-
s.Width = s.Width - (s.Width % 8)
206-
207204
// if rate is 0, set it to 60
208205
if s.Rate == 0 {
209206
s.Rate = 60
@@ -212,16 +209,21 @@ func ChangeScreenSize(s types.ScreenSize) (types.ScreenSize, error) {
212209
// convert variables to C types
213210
c_width, c_height, c_rate := C.int(s.Width), C.int(s.Height), C.short(s.Rate)
214211

215-
// if screen configuration already exists, just set it
212+
// try to set the screen configuration with the exact requested dimensions
216213
status := C.XSetScreenConfiguration(c_width, c_height, c_rate)
214+
215+
// if no existing mode matches, dynamically create one via libxcvt
217216
if status != C.RRSetConfigSuccess {
218-
// create new screen configuration
219-
C.XCreateScreenMode(c_width, c_height, c_rate)
217+
C.XCreateScreenMode(&c_width, &c_height, c_rate)
220218

221219
// screen configuration should exist now, set it
222220
status = C.XSetScreenConfiguration(c_width, c_height, c_rate)
223221
}
224222

223+
// update s with the actual dimensions that were set
224+
s.Width = int(c_width)
225+
s.Height = int(c_height)
226+
225227
var err error
226228

227229
// if screen configuration was not set successfully, return error

server/pkg/xorg/xorg.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ void XKey(KeySym keysym, int down);
3939
Status XSetScreenConfiguration(int width, int height, short rate);
4040
void XGetScreenConfiguration(int *width, int *height, short *rate);
4141
void XGetScreenConfigurations();
42-
void XCreateScreenMode(int width, int height, short rate);
42+
void XCreateScreenMode(int *width, int *height, short rate);
4343
XRRModeInfo *XCreateScreenModeInfo(int hdisplay, int vdisplay, short vrefresh);
4444

4545
void XSetKeyboardModifier(unsigned char mod, int on);

0 commit comments

Comments
 (0)