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

[MWF] Implement multi-display support on Linux (Fixes #325669) #943

Merged
merged 3 commits into from May 16, 2014

Conversation

ermshiperete
Copy link
Contributor

The number of displays and sizes can be retrieved with the help of
libXinerama. The implementation was done in a way that is backwards
compatible, so on non-Linux systems or when libXinerama is not
available the behavior will be the same as before.

Also fixed the implementation of FormStartPosition.CenterScreen to
bring up the form centered on the current display. This fixes
Novell bug#325669 on Linux
(https://bugzilla.novell.com/show_bug.cgi?id=325669).

} else {
displaySize = Screen.PrimaryScreen.WorkingArea.Size;
}
this.Location = new Point(displaySize.Width / 2 - w / 2, displaySize.Height / 2 - h / 2);
Copy link
Contributor

Choose a reason for hiding this comment

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

This should account for the work area's top-left corner, not assume it's (0,0).

@ermshiperete
Copy link
Contributor Author

@madewokherd: thanks for your review comments. I updated the patch.

}
}

public static Rectangle WorkingArea {
get {
return XplatUI.WorkingArea;
// TODO: this doesn't account for taskbar, launcher etc.
return Screen.PrimaryScreen.Bounds;
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the problem with XplatUI.WorkingArea, or Screen.PrimaryScreen.WorkingArea? Then we could implement it in backends later without touching this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

XplatUI.WorkingArea returns the working area of all screens (at least for X11). But Screen.PrimaryScreen.WorkingArea would be better than ...Bounds. I'll change that.

@madewokherd
Copy link
Contributor

Hmm, I didn't know we had the ability to (badly) query workarea on X11. It'd be nice to not break the part of it that works, although properly solving it is more work.

Maybe for now you could have XplatUIX11.AllScreens intersect the screen bounds with _NET_WORKAREA to calculate each screen's work area? That's the best we can do without _UNITY_NET_WORKAREA_REGION anyway.

@ermshiperete
Copy link
Contributor Author

I don't see why fixing XplatUIX11.WorkingArea should be part of this change. I think using Screen.PrimaryScreen.WorkingArea in SystemInformation.WorkingArea gives us what MSDN documentation states ("WorkingArea always returns the work area of the primary monitor."). IMO additional improvements should be part of separate patch sets.

(Just as some background information: we have a custom mono version with additional bugfixes; most of them got attached to issues in bugzilla but many never got applied. I'm currently going through all of our patches in preparation for updating to Mono 3.x. Unfortunately I don't have time to make extra nice-to-have changes since I need to get back to my main project)

@ermshiperete
Copy link
Contributor Author

The fix for bug #15462 (commit de64851) isn't really part of implementing multi-display support, but it only works correctly if these changes got merged, that's why I added it to this pull request.

@monoadmin
Copy link

Need maintainer approval to build this pull request.

[MonoTODO]
internal override Screen[] AllScreens {
get {
// To support multiples, we need to use GetMonitorInfo API on Win32
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are going to be calling this method, we should not be throwing an exception.

So let us return null, to indicate "Do not have support for this", and deal with that in the upper layers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, in both XplatUIWin32.cs and XplatUICarbon.cs

ermshiperete and others added 3 commits May 16, 2014 16:21
The number of displays and sizes can be retrieved with the help of
libXinerama. The implementation was done in a way that is backwards
compatible, so on non-Linux systems or when libXinerama is not
available the behavior will be the same as before.

Also fixed the implementation of FormStartPosition.CenterScreen to
bring up the form centered on the current display (with the current
display being either the screen the top left corner of the owner
form is on, or if there is no owner the screen that has the mouse
pointer). This fixes Novell bug #325669 on Linux
(https://bugzilla.novell.com/show_bug.cgi?id=325669).
The Screen.FromRectangle method was too simple minded.  It did not try to
find the best match for a screen when the system has more than one screen.
The new implementation makes at least some effort to find the closest
screen with the most overlap with the rectangle.
The fix mimics the behavior of Windows/.NET.
migueldeicaza added a commit that referenced this pull request May 16, 2014
[MWF] Implement multi-display support on Linux (Fixes #325669)
@migueldeicaza migueldeicaza merged commit 39ad7e3 into mono:master May 16, 2014
@ermshiperete ermshiperete deleted the bug-novell-325669 branch May 16, 2014 16:12
Red54 pushed a commit to Red54/mono that referenced this pull request Mar 15, 2019
…-crash-on-invalid-method-header

Return NULL so client can raise exception rather than asserting on invalid method header (case 1032601)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants