[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] RFR: 8239894: Xserver crashes when the wrong high refresh rate is used
From:       Alexander Zuev <kizune () openjdk ! java ! net>
Date:       2021-01-21 5:18:54
Message-ID: HnCFoyf-0VvZ46Aw23I_p_ubNAeVt_z42rqDTm2PXHM=.ff3e2764-878d-4a5a-b835-e99aabe98178 () github ! com
[Download RAW message or body]

On Tue, 19 Jan 2021 00:57:31 GMT, Sergey Bylokhov <serb@openjdk.org> wrote:

> We use two different approaches to listing the display modes on the system:
> - For single-screen configuration use version 1.1 protocol \
>                 XRRGetScreenInfo/XRRConfigRates/XRRConfigSizes
> - For multi-screen configuration use version 1.2 protocol: \
> XRRGetScreenResources/XRRGetOutputInfo/XRRModeInfo 
> Unfortunately, the current code does not work for some specific double scan modes, \
> it doubles the real refresh rate. If this refresh rate will be passed to the \
> Xserver later it could hang/crash. 
> The same bug existed in xrandr utility before version 1.4:
> https://bugs.freedesktop.org/show_bug.cgi?id=37043
> https://gitlab.freedesktop.org/xorg/app/xrandr/commit/7fd4f18b649f22fad4dbf9fc64b69b3e7f172207
>  
> As suggested in the bug report I have tried to unify the code for single and \
> multiscreen systems and use updated code currently used on the multiscreen. 
> But unfortunately, I have found that the multi-screen code is even more broken. The \
> problematic code is this: 
> int scr = 0, out = 0;
> if (usingXinerama && XScreenCount(awt_display) > 0) {
> out = screen;
> } else {
> scr = screen;
> }
> The idea of the code is: if the passed screen is Xinerama index used by the java2d, \
> then the "scr" will be zero, and "out" will be the "virtual" index for that screen. \
> If the screen is X11 screen then "src" will be the correct screen and "out" will be \
> zero. 
> The problem is that the code assumes that xrandr expect the "virtual" index as the \
> "out" parameter, but this is not. This parameter is the index of the "output" such \
> as DP1, DP2, other ports/etc. 
> So sometimes the code works when the first screen is connected to the first port on \
> the video card and the second screen to the second port, otherwise, we just report \
> data for the wrong screen or report nothing. 
> I did not found a reliable way to map Xinerama srceens to the xrandr. It is better \
> to rework the java2d to use the xrandr instead of xinerama. 
> As a fix, I suggest dropping the code which uses the incorrect mapping \
> screen->output. This includes the code related to the display modes added in the \
> JDK-8167486 + JDK-8022810, and also the code related to the calculation of scale \
> factor from the "ubuntu property" which does not work on the recent ubuntu version \
> after migration to gnome3 added in the JDK-8149115 
> I will file a separate two bugs to reimplement this functionality, most probably \
> the "display modes" will be implementing when we fully migrate to the xrandr, and \
> calculation of the scale factor will be implemented via the gtk library.

I guess not having this functionality at all is better than having a code that hangs. \
Can we have these new bugs about re-implementing this functionality linked in the \
initial bug so they can be tracked easier?

-------------

Marked as reviewed by kizune (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2136


[prev in list] [next in list] [prev in thread] [next in thread]