onflapp / gs-desktop

GNUstep Desktop
https://onflapp.github.io/gs-desktop/index.html
MIT License
102 stars 7 forks source link

Display and Screen Preferences glitches after changing resolution. Also - images dissappear. #27

Closed mclarenlabs closed 3 months ago

mclarenlabs commented 9 months ago

I recently set up gs-desktop on a new laptop, and one of the first things I did was use the nextspace/Preferences application to set screen resolutions for both the internal and external monitor. The application worked somewhat, but did not behave very well. After changing the resolution of the internal monitor, the images in the App disappeared and the widgets did not update correctly. I decided to investigate.

(FWIW: I like to change screen resolutions depending on whether I am at home or somewhere else.)

I think I have fixed some of the problems.

There were three main causes

With these three changes, both the "Display" and "Screens" modules of the nextspace/Preferences application work smoothly for me.

I found a good reference for the original notification architecture involving WorkspaceManager sending the distributed notification is in the following commit.

https://github.com/onflapp/nextspace/commit/62e30521934ae04dbe10e2fd0ea46738a9d6cb89

I'm not sure if the notification fix I suggested to OSEScreen.m is the right long-term approach, or if an approach whereby the window manager (WindowMaker) is changed to send a DistributedNotification to all of the running applications. But since WindowMaker is not a gnustep application, it would be difficult to get it to send the required notification.

Rather than prepare pull requests with these changes, I'm going to share my results here to get some feedback.

I'm pretty happy with the result. The Preferences Application now remembers resolution settings for the configuration where I have two monitors (an internal and an external) and a different configuration where I only have the internal monitor. It's very nice now.

-Tom

The Diffs to the Three Files appears below.


================================================================

$ git diff OSEScreen.m
diff --git a/Frameworks/SystemKit/OSEScreen.m b/Frameworks/SystemKit/OSEScreen.mindex 7a656030..f320bec3 100644
--- a/Frameworks/SystemKit/OSEScreen.m
+++ b/Frameworks/SystemKit/OSEScreen.m
@@ -306,13 +306,23 @@ static OSEScreen *systemScreen = nil;

   useAutosave = NO;

+#define NEXTSPACE_WORKSPACE_MANAGER_IS_USED 0
+#if NEXTSPACE_WORKSPACE_MANAGER_IS_USED
   // Workspace Manager notification sent as a reaction to XRRScreenChangeNotify   [[NSDistributedNotificationCenter defaultCenter]
     addObserver:self
        selector:@selector(randrScreenDidChange:)
            name:OSEScreenDidChangeNotification
          object:nil];
-
+#else
+  // Catch the notification from XGServerEvent.m directly and update randr
+  // resources.
+  [[NSNotificationCenter defaultCenter]
+    addObserver:self
+       selector:@selector(randrScreenDidChange:)
+          name:NSApplicationDidChangeScreenParametersNotification
+         object:nil];
+#endif
   return self;
 }

================================================================

$ diff Display.m ~/git/nextspace/Applications/Preferences/Modules/Display/Display.m
301,308c301,303
<
<   // TOM: perform asynchronously so this method can return
<   // [self setResolution];
<   [self performSelector:@selector(setResolution)
<            withObject:nil
<            afterDelay:0.1];
<
<
---
>
>   [self setResolution];
>
318,322c313
<   // TOM: perform asynchronously so this method can return
<   // [self setResolution];
<   [self performSelector:@selector(setResolution)
<            withObject:nil
<            afterDelay:0.1];
---
>   [self setResolution];

================================================================

$ cd git/gnustep-gui
$ git diff
diff --git a/Source/NSImage.m b/Source/NSImage.m
index 071af32d..99e797d0 100644
--- a/Source/NSImage.m
+++ b/Source/NSImage.m
@@ -533,7 +533,9 @@ repd_for_rep(NSArray *_reps, NSImageRep *rep)
   //_flags.syncLoad = NO;
   _reps = [[NSMutableArray alloc] initWithCapacity: 2];
   ASSIGN(_color, clearColor);
-  _cacheMode = NSImageCacheDefault;
+  // TOM:
+  // _cacheMode = NSImageCacheDefault;
+  _cacheMode = NSImageCacheNever;

   return self;
 }
@@ -2253,6 +2255,31 @@ iterate_reps_for_types(NSArray* imageReps, SEL method)
 }
onflapp commented 9 months ago

brilliant! This was bothering me for sometime too.

Looks like good solution. GSDE fork of WindowMaker has simple GNUstep bridge that could be used to received/send notification. In addition, it has dbus interface. I'll think of the best way to incorporate your changes into the main code ASAP.

Many thanks Tom!

onflapp commented 9 months ago

I integrated your fixes and added some of my own. Most of the applications work now. GWorkspace still has problems with small icons. I'm still investigating how to solve that problem.

mclarenlabs commented 9 months ago

When you integrated the changes - do the Display and Screen Preferences modules work better for you too?

mclarenlabs commented 9 months ago

One other change related to screen size changes. In OSEScreen.m, when the screen size changes, it is either BIGGER or SMALLER.

The BIGGER part is here

  if (newPixSize.width > sizeInPixels.width &&
      newPixSize.height > sizeInPixels.height) {
    NSDebugLLog(@"Screen", @"OSEScreen: set new BIGGER screen size: START");
    XRRSetScreenSize(xDisplay, xRootWindow,
                     (int)newPixSize.width, (int)newPixSize.height,
                     (int)mmSize.width, (int)mmSize.height);

and the SMALLER part is here

  // If new screen size is SMALLER - set new screen size here
  if (newPixSize.width < sizeInPixels.width ||
      newPixSize.height < sizeInPixels.height) {
    NSDebugLLog(@"Screen", @"OSEScreen: set new SMALLER screen size: START");

But, If the screen size is LARGER in height but EQUAL in width, or EQUAL in height but LARGER in width, then no change happens. We also do not want to trigger a screen resize if the old and new are exactly EQUAL, so I changed the BIGGER part to look like this

  if ((newPixSize.width > sizeInPixels.width &&
       newPixSize.height > sizeInPixels.height) ||
      (newPixSize.width > sizeInPixels.width &&
       newPixSize.height == sizeInPixels.height) ||
      (newPixSize.width == sizeInPixels.width &&
       newPixSize.height > sizeInPixels.height)) {
    NSDebugLLog(@"Screen", @"OSEScreen: set new BIGGER screen size: START");
    XRRSetScreenSize(xDisplay, xRootWindow,
                     (int)newPixSize.width, (int)newPixSize.height,
                     (int)mmSize.width, (int)mmSize.height);

and that behaves better when only one of the dimensions gets larger.

I have not tested this change in a VM, FYI.

onflapp commented 3 months ago

should be fixed now. Just make sure you use gs-workspace and gnustep-gui forks GSDE comes with. The changes were not merged upstream yet.