Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XGServerWindow.m small fixes #25

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
2020-03-11 Sergii Stoian <[email protected]>

* Source/x11/XGServerWindow.m (_createAppIconPixmaps): accept
WRaster context depth 24.
(styleoffsets::::::): do not guess offsets for miniwindow and
appicon they are equal to 0.0.

2020-03-12 Sergii Stoian <[email protected]>

* Source/art/GNUmakefile.preamble,
Expand Down
19 changes: 15 additions & 4 deletions Source/x11/XGServerEvent.m
Original file line number Diff line number Diff line change
Expand Up @@ -1977,10 +1977,21 @@ - (NSEvent *)_handleTakeFocusAtom: (XEvent)xEvent
events */
if ([NSApp isHidden])
{
/* This often occurs when hidding an app, since a bunch of
windows get hidden at once, and the WM is searching for a
window to take focus after each one gets hidden. */
NSDebugLLog(@"Focus", @"WM take focus while hiding");
if (generic.wm & XGWM_WINDOWMAKER)
{
/* If window receives WM_TAKE_FOCUS and application is in hidden
state - it's time to unhide. There's no other method to
tell us to unhide. */
NSDebugLLog(@"Focus", @"WM take focus while hidden - unhiding.");
[NSApp unhide:nil];
}
else
{
/* This often occurs when hidding an app, since a bunch of
windows get hidden at once, and the WM is searching for a
window to take focus after each one gets hidden. */
NSDebugLLog(@"Focus", @"WM take focus while hiding");
}
}
else if (cWin->ignore_take_focus == YES)
{
Expand Down
5 changes: 3 additions & 2 deletions Source/x11/XGServerWindow.m
Original file line number Diff line number Diff line change
Expand Up @@ -2283,7 +2283,8 @@ - (void) styleoffsets: (float *) l : (float *) r : (float *) t : (float *) b

if ((style & NSIconWindowMask) || (style & NSMiniWindowMask))
{
style = NSBorderlessWindowMask;
*l = *r = *t = *b = 0.0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you think this required? Shouldn't we first check what is already stored in the known offsets?
There may be window managers that put a border even on these windows.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are 2 things here:

  1. It's a special windows: miniwindow and appicon. If window manager defines offsets for such type of windows user should better disable appearance of them via defaults.
  2. Simple application has 2 windows which produce "styleoffsets ... guessing offsets" messages: appicon and window with ID == 0. I see exacly 2 messages at application start. It means that code at line range 2295-2306 doesn't work either. Finally in WindowMaker we will end up with offsets 1.0 and that's wrong for miniwindow and appicon. For other WMs it will be 4.0. Is it what we need for our appicon and miniwindows?

In general I think that code below NSLog line is wrong and useless. Why offset for EWMH-compliant WM is 4.0? Why titlebar and resizebar sizes fixed and defined to that values?

return;
}

/* Next try to get the offset information that we have obtained from
Expand Down Expand Up @@ -2841,7 +2842,7 @@ - (int) _createAppIconPixmaps
height = [rep pixelsHigh];
colors = [rep samplesPerPixel];

if (rcontext->depth != 32)
if (rcontext->depth != 32 && rcontext->depth != 24)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did put this new test here on purpose as your code did not work for Riccardo. Please have him test this change first. We do not need a regression shortly before a release.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, no problem.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this specific change on both the problematic T23 16bit as well as the Letux400 (16 bit too) with no adverse effects.
However, I noticed one big thing: possibly due to this skipping, on the Letux app mini windows are not displayed anymore. Like battery monitor is unable to display the battery, only the icon is displayed. I fear this is because all the code is skipped because now it works only in certain conditions, while before it worked.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify, the change that Sergii made tries to allow for more systems to use this new code. It was my change to check for the depth that shut of Letux. I did this because you got a segmentation fault with the new code. If you don't get a segmentation fault now things are improved and Sergii's change to allow for depth 24 may be correct. But it is more likely that one of the other checks I added still blocks the call. You should check for that first.

{
NSLog(@"Unsupported context depth %d", rcontext->depth);
return 0;
Expand Down