ArcticaProject / nx-libs

nx-libs
Other
119 stars 39 forks source link

Resolution in NX Client for Windows on 3.5.9.26 is not correct. #1030

Closed Djelff closed 2 years ago

Djelff commented 2 years ago

It happened after this: https://github.com/uli42/nx-libs/commit/ec521d2960536bd9585853b6ed751a3e352ca811

AllScreens is false by default and the screen is shrunk by 3/4. I changed this behavior in Args.c but I'm not sure if this is the right solution. It is very sad that we cannot determine the type/version of the client.

In addition, the ability to change the screen resolution has disappeared (not critical, but in some cases it was necessary).

nxlibs 3.5.99.25 3 5 99 25 nxlibs 3.5.99.26 3 5 99 26 .

uli42 commented 2 years ago

Can you please specify the problem in more detail? I am unsure what needs to be fixed.

Have you tried the current git HEAD?

Uli

Djelff @.***> schrieb am So., 19. Dez. 2021, 07:47:

It happened after this: @.*** https://github.com/uli42/nx-libs/commit/ec521d2960536bd9585853b6ed751a3e352ca811

AllScreens is falsified by default and the screen is shrunk by 3/4. I changed this behavior in Args.c but I'm not sure if this is the right solution. It is very sad that we cannot determine the type/version of the client.

In addition, the ability to change the screen resolution has disappeared (not critical, but in some cases it was necessary). [image: 3 5 99 25] https://user-images.githubusercontent.com/59478610/146666332-ff9391d5-1e29-450f-95a4-bf8829a3b04d.jpg [image: 3 5 99 26] https://user-images.githubusercontent.com/59478610/146666339-eda497f5-7144-4bb8-b86c-75b25817c58f.jpg .

— Reply to this email directly, view it on GitHub https://github.com/ArcticaProject/nx-libs/issues/1030, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQHBZB2AJ2XC5NTCGP5VELURV5W7ANCNFSM5KLRGJOQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

Djelff commented 2 years ago

Yes, on the current git HEAD. NX Client for Windows only sends geometry at startup, it is not resizable. NX Client for Linux work fine, but is resizable.

More detail:

// nx-libs-3.6.x\nx-X11\programs\Xserver\hw\nxagent\Options.c void nxagentInitOptions(void) ... nxagentOptions.AllScreens = False;

// nx-libs-3.6.x\nx-X11\programs\Xserver\hw\nxagent\Args.c static void nxagentParseSingleOption(char name, char value) ... if (!strcmp(argv[i], "-geometry")) { if (++i < argc) { if (!strcmp(argv[i],"fullscreen") || !strcmp(argv[i],"allscreens")) { nxagentChangeOption(Fullscreen, True); nxagentChangeOption(AllScreens, True); } else if (!strcmp(argv[i],"onescreen")) { nxagentChangeOption(Fullscreen, True); nxagentChangeOption(AllScreens, False); } else { if (nxagentUserGeometry.flag == 0) { // getting screen size from nxclient // after that nxagentOptions.AllScreens remains False nxagentUserGeometry.flag = XParseGeometry(argv[i], &nxagentUserGeometry.X, &nxagentUserGeometry.Y, &nxagentUserGeometry.Width, &nxagentUserGeometry.Height); } }

  if (nxagentUserGeometry.flag || (nxagentOption(Fullscreen)))
  {
return 2;
  }
}
return 0;

}

// nx-libs-3.6.x\nx-X11\programs\Xserver\hw\nxagent\Screen.c Bool nxagentOpenScreen(ScreenPtr pScreen, int argc, char argv[]) ... if (nxagentOption(AllScreens)) { nxagentChangeOption(Width, w); nxagentChangeOption(Height, h); } else { // screen size from nxclient resized to 3/4 nxagentChangeOption(Width, w 3 / 4); nxagentChangeOption(Height, h * 3 / 4); }

uli42 commented 2 years ago

Hm, thanks, I need to further think about this problem. From your mail it is unclear what your fix looks like (ATM I am occupied with other stuff so I cannot compare with the current source code).

I guess all this has to do with Vcxsrv's multiwindow mode which behaves differently than an X11 window manager. I have seen a similar behavior recently.

It is very sad that we cannot determine the type/version of the client.

Well, that's not entirely true, the type of client can be passed in the client= setting in the options file/string (values windows/winnt/solaris/macos/linux). Need to check if x2go handles that correctly. The version of the client can be (roughly) determined via the protocol but I have not looked deeper into that.

Uli

On Mon, Dec 20, 2021 at 6:35 AM Djelff @.***> wrote:

Yes, on the current git HEAD. NX Client for Windows only sends geometry at startup, it is not resizable.

More detail:

// nx-libs-3.6.x\nx-X11\programs\Xserver\hw\nxagent\Options.c void nxagentInitOptions(void) ... nxagentOptions.AllScreens = False;

// nx-libs-3.6.x\nx-X11\programs\Xserver\hw\nxagent\Args.c static void nxagentParseSingleOption(char name, char value) ... if (!strcmp(argv[i], "-geometry")) { if (++i < argc) { if (!strcmp(argv[i],"fullscreen") || !strcmp(argv[i],"allscreens")) { nxagentChangeOption(Fullscreen, True); nxagentChangeOption(AllScreens, True); } else if (!strcmp(argv[i],"onescreen")) { nxagentChangeOption(Fullscreen, True); nxagentChangeOption(AllScreens, False); } else { if (nxagentUserGeometry.flag == 0) { // getting screen size from nxclient // after that nxagentOptions.AllScreens remains False nxagentUserGeometry.flag = XParseGeometry(argv[i], &nxagentUserGeometry.X, &nxagentUserGeometry.Y, &nxagentUserGeometry.Width, &nxagentUserGeometry.Height); } }

if (nxagentUserGeometry.flag || (nxagentOption(Fullscreen))) { return 2; } } return 0;

}

// nx-libs-3.6.x\nx-X11\programs\Xserver\hw\nxagent\Screen.c Bool nxagentOpenScreen(ScreenPtr pScreen, int argc, char argv[]) ... if (nxagentOption(AllScreens)) { nxagentChangeOption(Width, w); nxagentChangeOption(Height, h); } else { // screen size from nxclient resized to 3/4 nxagentChangeOption(Width, w 3 / 4); nxagentChangeOption(Height, h * 3 / 4); }

— Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you commented.Message ID: @.***>

Djelff commented 2 years ago

The fix for NX Client for Windows is very simple `--- Args.c 2021-02-04 05:34:56.000000000 +0300 +++ Args.c 2021-12-20 13:35:56.000000000 +0300 @@ -585,6 +585,7 @@ { if (nxagentUserGeometry.flag == 0) {

But I don’t know how other NX clients will behave.

uli42 commented 2 years ago

Ok, thanks. Just to be sure: The versions before my change were working fine?

I will have a look at this, hopefully in the next few days. Thanks for spotting it!

Uli

On Mon, Dec 20, 2021 at 12:51 PM Djelff @.***> wrote:

The fix for NX Client for Windows is very simple `--- Args.c 2021-02-04 05:34:56.000000000 +0300 +++ Args.c 2021-12-20 13:35:56.000000000 +0300 @@ -585,6 +585,7 @@ { if (nxagentUserGeometry.flag == 0) {

 nxagentChangeOption(AllScreens, True);

 nxagentUserGeometry.flag = XParseGeometry(argv[i],

                                               &nxagentUserGeometry.X,

                                                   &nxagentUserGeometry.Y, `

But I don’t know how other NX clients will behave.

— Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you commented.Message ID: @.***>

Djelff commented 2 years ago

Ok, thanks. Just to be sure: The versions before my change were working fine?

Yes, until https://github.com/uli42/nx-libs/commit/ec521d2960536bd9585853b6ed751a3e352ca811 everything worked well and the geometry changes took place there.

As far as I understand, this fix was made for a reason https://github.com/ArcticaProject/nx-libs/issues/987 but what I suggest should not affect the proposed fix.

uli42 commented 2 years ago

To be sure I am doing the same you do: What's your configuration on the Connection tab of the session in x2goclient? And what kind of session are you running (single application, desktop, published application)? What is your Windows display configuration? Please also post the content of the options file after the session is up (/tmp/(.x2go-/C-.../options.)

Thanks!

Djelff commented 2 years ago

uli42, I was not writing about the x2go client, but about the NX Client for Windows. I tested this on NX Client version 3.5.0-7, this error appears because NX Client specifies screen resolution at startup and it does not change this in the future. The x2go client shouldn't have such problems

P.S. NX Client for Windows is very limited in screen settings. nxforwin

uli42 commented 2 years ago

Right, my fault.. sorry.

Djelff @.***> schrieb am Do., 23. Dez. 2021, 06:31:

uli42, I was not writing about the x2go client, but about the NX Client for Windows. I tested this on NX Client version 3.5.0-7, this error appears because NX Client specifies screen resolution at startup and it does not change this in the future. The x2go client shouldn't have such problems.

— Reply to this email directly, view it on GitHub https://github.com/ArcticaProject/nx-libs/issues/1030#issuecomment-1000054564, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQHBZFY3Q3P276LAEOZH23USKXZXANCNFSM5KLRGJOQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you commented.Message ID: @.***>

uli42 commented 2 years ago

But please provide the options file anyway.

Ulrich Sibiller @.***> schrieb am Do., 23. Dez. 2021, 08:58:

Right, my fault.. sorry.

Djelff @.***> schrieb am Do., 23. Dez. 2021, 06:31:

uli42, I was not writing about the x2go client, but about the NX Client for Windows. I tested this on NX Client version 3.5.0-7, this error appears because NX Client specifies screen resolution at startup and it does not change this in the future. The x2go client shouldn't have such problems.

— Reply to this email directly, view it on GitHub https://github.com/ArcticaProject/nx-libs/issues/1030#issuecomment-1000054564, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQHBZFY3Q3P276LAEOZH23USKXZXANCNFSM5KLRGJOQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you commented.Message ID: @.***>

Djelff commented 2 years ago

The config on which the screenshots were taken in the first post. ubuntu1804vbox.zip

uli42 commented 2 years ago

Well, that only helps a bit, what I really would like to inspect is the options file of a running session. And the commandline of the nxagent process of that session.

Uli

On Thu, Dec 23, 2021 at 9:52 AM Djelff @.***> wrote:

The config on which the screenshots were taken in the first post. ubuntu1804vbox.zip https://github.com/ArcticaProject/nx-libs/files/7767955/ubuntu1804vbox.zip

— Reply to this email directly, view it on GitHub https://github.com/ArcticaProject/nx-libs/issues/1030#issuecomment-1000142708, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQHBZHRBGOBJYGRG3UYSRTUSLPOFANCNFSM5KLRGJOQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you commented.Message ID: @.***>

Djelff commented 2 years ago

I attach the logs, but in the logs what is happening is not visible nx_logs.zip They are exactly the same in nx_libs 3.5.9.25 and 3.5.9.26

I already wrote in the 3rd post what is happening. I will repeat it in a slightly different way, perhaps this will make it clearer.

The sequence leading to screen distortion in 3/4 of the original size is as follows:

Step1: the AllScreens = False flag is set in nx-libs-3.6.x\nx-X11\programs\ Xserver\ hw\nxagent\Options.c

Step2: the -geometry option is parsed in nx-libs-3.6.x\nx-X11\programs\Xserver\hw\nxagent\Args.c, and the geometry comes in fixed and AllScreens is not set to True

Step3: in nx-libs-3.6.x\nx-X11\programs\Xserver\hw\nxagent\Screen.c if AllScreens = False, the screen is set to 3/4 of size from options client.

There are only 3 steps to get 3/4 of the screen size.

I don't really understand what else depends on the AllScreens flag, but it is he who leads to this behavior.

I see 3 possible solutions:

  1. Set AllScreens flag to true in Args.c on arrival of fixed geometry.
  2. Set the AllScreens flag to true by default in Options.c (it shouldn't break anything).
  3. Add the FixedSize flag to Options.c with a default value of False, set it to True in Args.c and do not change the screen upon initialization in Screen.c Solutions 3 seems to be more logical.

P.S. I suppose that the OpenNX on Windows (i participated in its development a bit) will display the same 3/4 as the NXClient or Windows.

uli42 commented 2 years ago

Thanks for the logs. From there I see:

  1. the client passes geometry=1024x768 via the options file
  2. The agent sizes the screen accordingly: Xlib: extension "XINERAMA" missing on display "nx,options=/home/addm/.nx/C-server2c-2000-10011E22C26575EC33DCAD379F656C04/options:2000.0". Info: Screen [0] resized to geometry [1024x768] fullscreen [1].

So from here I don't see a contradiction between what's requested and what is used. Or in other words: not 3/4 issue. Is this your patched version?

As I do not have a setup where I can easily test the Windows NX Client I can either try to set one up or to understand what the nx server (or nxagent) is receiving from the other side. That's why I was asking for the options file and the Windows screen resolution.

Regarding AllScreens: I understood how we can prevent the 3/4 from happening. Setting AllScreens to True is leading to the right code path but - for me - for the false reasons. AllScreens is a synonym for Fullscreen and when the user provides a geometry the nxagent is supposed to open a window with that size and not a fullscreen session. Therefore AllScreens has to be False. So I really need to adapt the code to recognize your situation and act accordingly. And for that I need to fully understand what's coming from the other side. Your solution 3 seems to be the most logical one of the 3 suggestions, I agree. But still I must ensure this will not break other situations...

I will read your logs in detail in a few days (after X-Mas), and ask you to provide one more thing in the meantime (I have not seen this in the logs at a quick glance): What's the result of ps waux|grep nxagent for such a session?

Thanks,

Uli

On Thu, Dec 23, 2021 at 1:36 PM Djelff @.***> wrote:

I attach the logs, but in the logs what is happening is not visible nx_logs.zip They are exactly the same in nx_libs 3.5.9.25 and 3.5.9.26

I already wrote in the 3rd post what is happening. I will repeat it in a slightly different way, perhaps this will make it clearer.

The sequence leading to screen distortion in 3/4 of the original size is as follows:

Step1: the AllScreens = False flag is set in nx-libs-3.6.x\nx-X11\programs\ Xserver\ hw\nxagent\Options.c

Step2: the -geometry option is parsed in nx-libs-3.6.x\nx-X11\programs\Xserver\hw\nxagent\Args.c, and the geometry comes in fixed and AllScreens is not set to True

Step3: in nx-libs-3.6.x\nx-X11\programs\Xserver\hw\nxagent\Screen.c if AllScreens = False, the screen is set to 3/4 of size from options client.

There are only 3 steps to get 3/4 of the screen size.

I don't really understand what else depends on the AllScreens flag, but it is he who leads to this behavior.

I see 3 possible solutions:

Set AllScreens flag to true in Args.c on arrival of fixed geometry. Set the AllScreens flag to true by default in Options.c (it shouldn't break anything). Add the FixedSize flag to Options.c with a default value of False, set it to True in Args.c and do not change the screen upon initialization in Screen.c Solutions 3 seems to be more logical.

P.S. I suppose that the OpenNX on Windows (i participated in its development a bit) will display the same 3/4 as the NXClient or Windows.

— Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you commented.Message ID: @.***>

Djelff commented 2 years ago

Oops, indeed the log was from an already patched version. Here is the log from the original 3.5.99.26 session.zip The resizing is captured in it.

nxagentParseSingleOption: Processing option 'geometry' = '1024x768'. ... Info: Screen [0] resized to geometry [768x576] fullscreen [1].

root@server2c:/usr/src/arctica/3.5.99.26# ps waux|grep nxagent addm 30815 0.1 0.5 169284 32848 ? S 08:45 0:01 /usr/bin/nxagent -persistent -D -name NX - addm@server2c:2000 - ubuntu1804vbox (GPL Edition) -option /home/addm/.nx/C-server2c-2000-A9BF3B44204391DFAB1931D9FA415C13/options -nolisten tcp -dpi 96 :2000`

And a CustomSize_patch.zip to the 3.5.99.26 with the addition of the CustomSize flag, this name is probably clearer. Tested it, it works.

uli42 commented 2 years ago

I have now checked this and I think the problem is that nxwin is not offering a window manager. In that case nxagent will automatically activate FullScreen but not AllScreens. So I have added setting AllScreens, too. Works for me, please check.

Regarding the resolution selection dropdown: As long as the command line option +rrxinerama is used (which is the default) nxagent will only know one resolution which will be adjusted dynamically. So this is "works as intended". Is this behaviour a problem? (You can disable rrxinerama by adding AGENT_EXTRA_OPTIONS_X=-rrxinerama to /etc/nxserver/node.conf.d/11-nxagent.conf)

Djelff commented 2 years ago

I confirm. This fix works well. Thanks, uli42!