`get_current_monitor()` panics on Wayland. #793

icefoxen · 2019-02-13T18:53:44Z

Basically, this function does a some_vec.last().unwrap() of a Vec that can be null. The vec appears to be acquired from sctk::surface::get_outputs() here: https://github.com/Smithay/client-toolkit/blob/2c28ada5c7dd067ac1245382bc371cc933d4c416/src/surface.rs#L109 . As far as I can tell, the outputs vec is created empty and only has things added to it on SurfaceUserData::enter(), but I don't know enough about the Wayland protocol to understand what's actually going on there.

The text was updated successfully, but these errors were encountered:

elinorbgr · 2019-02-13T21:02:04Z

Welp.

If this Vec is empty, it means that the surface is currently not being displayed on any monitor, likely because the method was called early in the init process and the window is not currently displayed yet.

I'm not sure what would be the best thing to do here, given the API winit enforces. Would it make sens to take advantage of the pending breaking changes to change the prototype of get_current_motinor() to return an Option<MonitorId> ? (or even to return a Vec<MonitorId> tbh)

Osspial · 2019-02-13T21:52:31Z

I'd be fine with either returning Option<MonitorId> or Vec<MonitorId>.

If we go with Vec<MonitorId> we should consider if we define an order for the returned monitors (say, order it by the area of intersection of the window of each monitor). That should be possible on win32, but I don't know what the story on that looks like.

elinorbgr · 2019-02-13T21:54:40Z

order it by the area of intersection of the window of each monitor

That would not be possible for wayland. The only order that can be implemented there would be « the order in which the window entered the monitors », which is hardly useful.

Osspial · 2019-02-13T22:04:36Z

Thanks, good to know. Is there any way to find out a window's primary monitor on Wayland? I ask because on systems where different monitors have different DPIs, it can be useful to know which monitor the system considers to be the main monitor for DPI scaling, which I'd like to support if possible (on Windows that's done through the MonitorFromWindow function). If we can expose that alongside a list of all monitors the window overlaps with that would be awesome.

elinorbgr · 2019-02-13T22:17:57Z

There is no concept of "primary monitor" in wayland. Regarding DPI, what happens is that the client app actually tells the server that it is drawing its contents using a given dpi factor, and the server takes care of upscaling or downscaling if that does not match the dpi factor of the monitor. So what winit does currently is it takes the max() of the dpi factors of the monitors the window is currently displayed on, and lets the server down scale if necessary. Le 13 février 2019 23:04:37 GMT+01:00, Osspial <notifications@github.com> a écrit :

…

Thanks, good to know. Is there any way to find out a window's primary monitor on Wayland? I ask because on systems where different monitors have different DPIs, it can be useful to know which monitor the system considers to be the main monitor for DPI scaling, which I'd like to support if possible (on Windows that's done through the [`MonitorFromWindow`](https://docs.microsoft.com/en-us/windows/desktop/api/winuser/nf-winuser-monitorfromwindow) function). If we can expose that alongside a list of all monitors the window overlaps with that would be awesome. -- You are receiving this because you commented. Reply to this email directly or view it on GitHub: #793 (comment)

icefoxen · 2019-04-11T14:50:10Z

I need to solve this for ggez. Is there a default I can replace the unwrap() with? Or if it returns Option<MonitorId>, any ideas when this breaking change will result in a release?

Osspial · 2019-04-25T02:47:19Z

My guess is that we'd wrap that breaking change into EL2.0, or a release shortly afterwords.

aleksander · 2019-07-09T06:17:22Z

Does anyone work on this bug? I want to try to fix it.

elinorbgr · 2019-07-09T06:44:03Z

I don't think anyone is carrently working on it, so please do!

Luraktinus · 2019-12-10T18:18:51Z

any news?

betseg · 2020-05-19T04:34:06Z

nothing?

icefoxen · 2020-07-19T18:10:36Z

Well, still fails on my system using winit 0.19.5

kchibisov · 2020-07-19T21:17:16Z

I'm working on that issue right now during some other rework, just could take a bit of time for me to finish.

kchibisov · 2020-08-18T02:16:13Z

The main issue here is that returning Vec<MonitorHandle> won't help to start window in a fullscreen, since this is why this function(get_current_monitor) is being used. We should start return Vec<MonitorHandle>, since it makes a bit more sense, but it's other story.

winit uses Option<Fullscreen> to detect on which monitor to set fullscreen, and on None it unsets it, however since there's no current monitor when users create a window, they can't make a fullscreen. On Wayland, there's an option to let compositor decide on which monitor to enter fullscreen. So, if winit's API will expose that in set_fullscreen and with_fullscreen calls it could help a lot for use cases where get_current_monitor will return empty Vec.

I'm not sure how parameters to for such calls could be changed. I'm a bit afraid of adding things like unset_fullscreen and just reworking the current API internally, since it doesn't solve a problem with with_fullscreen. We can pass something like Option<Option<Fullscreen>> for both calls, but I'm not sure that it's something nice(but it'll work).

If anyone has a suggestion on how API to enter fullscreen could look like wrt indication of system picking a monitor for you let me know. I kind of think that Option<Option<Fullscreen>> is the simplest solution here, but it'll look a bit weird.

kchibisov · 2020-08-20T00:02:46Z

Just thinking more about that fullscreen startup problem on Wayland. I feel like we can change the Fullscreen enum from

#[derive(Clone, Debug, PartialEq)]
pub enum Fullscreen {
    Exclusive(VideoMode),
    Borderless(MonitorHandle),
}

To

#[derive(Clone, Debug, PartialEq)]
pub enum Fullscreen {
    Exclusive(VideoMode),
    Borderless(Option<MonitorHandle>),
}

The None will be treated like 'let the system decide', which will most of the time result in a current monitor being picked on at least Wayland. If the platform doesn't have a concept of 'let the system decide' like on Wayland, it may fallback to picking current monitor by itself.

I'm not sure what to do about Exclusive fullscreen, since Wayland doesn't have such things at all, so having it under Option isn't something I can speak for.

What to do you think @chrisduerr , @murarth , @Osspial, @ryanisaacg , @francesca64 ?

bestouff · 2020-08-20T10:20:37Z

I'd say this calls for a third enum variant.

kchibisov · 2020-08-20T10:22:33Z

I'm not sure about the third one, since it's actually a Borderless, it's not like give me random fullscreen with a random video mode, it's exactly give me a fullscreen on a random screen with Borderless. So that's why it's an option, since the intent is to get Borderless fullscreen.

chrisduerr · 2020-08-20T10:50:24Z

Having a None parameter to refer to just a random default value is relatively consistent with the rest of the API of both winit and glutin I'd say. I would prefer Borderless(None) over some new enum variant.

francesca64 · 2020-08-20T20:07:20Z

I'd also prefer Borderless(None) over having some third variant. Users already have plenty of tools for working with Option, and it expresses the meaning correctly.

bestouff · 2020-08-21T08:38:08Z

Ok ok, I get it :)

On certain platforms window couldn't be on any monitor resulting in failures of `current_monitor` function on them, since they might return specific monitor. Such issue was happening on Wayland, since the window isn't on any monitor, unless the user has drawn something into it. Returning `impl Iterator<Item = MonitorHandle>` will give ability to expose such behavior, as well as providew information about all current monitors, since window could be on both at the same time. Fixes rust-windowing#793.

kchibisov · 2020-08-21T13:42:45Z

I've opened #1670 and #1671 to address raised issues.

On certain platforms window couldn't be on any monitor resulting in failures of `current_monitor` function on them, since they might return specific monitor. Such issue was happening on Wayland, since the window isn't on any monitor, unless the user has drawn something into it. Returning `impl Iterator<Item = MonitorHandle>` will give ability to expose such behavior, as well as providew information about all current monitors, since window could be on both at the same time. Fixes rust-windowing#793.

On certain platfroms window couldn't be on any monitor resulting in failures of `current_monitor` function on such, since they can't return any monitor at all. Such issue was happening on Wayland, since the window isn't on any monitor, unless the user has drawn something into it. Returning `Option<MonitorHandle>` will give an ability to handle such situations gracefully by properly indicating that there's no current monitor. Fixes rust-windowing#793.

On certain platforms window couldn't be on any monitor resulting in failures of 'current_monitor' function. Such issue was happening on Wayland, since the window isn't on any monitor, unless the user has drawn something into it. Returning 'Option<MonitorHandle>' will give an ability to handle such situations gracefully by properly indicating that there's no current monitor. Fixes rust-windowing#793.

On certain platforms window couldn't be on any monitor resulting in failures of 'current_monitor' function. Such issue was happening on Wayland, since the window isn't on any monitor, unless the user has drawn something into it. Returning 'Option<MonitorHandle>' will give an ability to handle such situations gracefully by properly indicating that there's no current monitor. Fixes #793.

On Linux/Wayland, it seems that sometimes `window.current_monitor()` can return `None` when nothing has been drawn to the screen yet. When this happens, simply skip most of the logic and draw an empty frame to the screen. The next time `view()` is called, the `current_monitor()` method will then return the correct value. (See rust-windowing/winit#793 for some details.).

icefoxen mentioned this issue Feb 13, 2019

Panic on Option::unwrap() ggez/ggez#579

Closed

Osspial added DS - wayland S - api Design and usability labels Feb 13, 2019

elinorbgr mentioned this issue Feb 13, 2019

Replace Option<T> in Window function return types with T #794

Closed

chrisduerr mentioned this issue Aug 11, 2019

Unable to launch alacritty with Fullscreen startup mode on Wayland alacritty/alacritty#2714

Closed

icefoxen mentioned this issue Oct 27, 2019

unable to run examples in archlinux ggez/ggez#698

Closed

parasyte mentioned this issue Nov 2, 2019

window.current_monitor() panics on Wayland when the window is hidden #1257

Closed

goddessfreya added C - needs investigation Issue must be confirmed and researched B - bug Dang, that shouldn't have happened labels Nov 6, 2019

mkroening mentioned this issue May 27, 2020

Linux: Starting under Wayland iceiix/stevenarella#316

Closed

eirikblekesaune mentioned this issue Jun 21, 2020

panic when calling Window::current_monitor() on wayland nannou-org/nannou#618

Open

kchibisov self-assigned this Jul 19, 2020

francesca64 added C - needs discussion Direction must be ironed out and removed C - needs investigation Issue must be confirmed and researched labels Aug 20, 2020

kchibisov mentioned this issue Aug 21, 2020

Rework Fullscreen::Borderless enum variant #1670

Merged

kchibisov mentioned this issue Aug 21, 2020

Make current_monitor return Option<MonitorHandle> #1671

Merged

kchibisov closed this as completed in #1671 Sep 7, 2020

0x777 mentioned this issue Oct 7, 2020

demo panics on wayland servo/pathfinder#444

Open

abusch mentioned this issue Oct 26, 2021

Fix window_coordinates example crash on Linux/Wayland nannou-org/nannou#802

Merged

Diegovsky mentioned this issue Dec 5, 2021

[Feature Proposal] Attach an empty buffer on new wayland surfaces #2089

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`get_current_monitor()` panics on Wayland. #793

`get_current_monitor()` panics on Wayland. #793

icefoxen commented Feb 13, 2019

elinorbgr commented Feb 13, 2019

Osspial commented Feb 13, 2019

elinorbgr commented Feb 13, 2019

Osspial commented Feb 13, 2019

elinorbgr commented Feb 13, 2019 via email

icefoxen commented Apr 11, 2019

Osspial commented Apr 25, 2019

aleksander commented Jul 9, 2019

elinorbgr commented Jul 9, 2019 via email

Luraktinus commented Dec 10, 2019

betseg commented May 19, 2020

icefoxen commented Jul 19, 2020

kchibisov commented Jul 19, 2020

kchibisov commented Aug 18, 2020 •

edited

Loading

kchibisov commented Aug 20, 2020 •

edited

Loading

bestouff commented Aug 20, 2020

kchibisov commented Aug 20, 2020

chrisduerr commented Aug 20, 2020

francesca64 commented Aug 20, 2020

bestouff commented Aug 21, 2020

kchibisov commented Aug 21, 2020

get_current_monitor() panics on Wayland. #793

get_current_monitor() panics on Wayland. #793

Comments

icefoxen commented Feb 13, 2019

elinorbgr commented Feb 13, 2019

Osspial commented Feb 13, 2019

elinorbgr commented Feb 13, 2019

Osspial commented Feb 13, 2019

elinorbgr commented Feb 13, 2019 via email

icefoxen commented Apr 11, 2019

Osspial commented Apr 25, 2019

aleksander commented Jul 9, 2019

elinorbgr commented Jul 9, 2019 via email

Luraktinus commented Dec 10, 2019

betseg commented May 19, 2020

icefoxen commented Jul 19, 2020

kchibisov commented Jul 19, 2020

kchibisov commented Aug 18, 2020 • edited Loading

kchibisov commented Aug 20, 2020 • edited Loading

bestouff commented Aug 20, 2020

kchibisov commented Aug 20, 2020

chrisduerr commented Aug 20, 2020

francesca64 commented Aug 20, 2020

bestouff commented Aug 21, 2020

kchibisov commented Aug 21, 2020

`get_current_monitor()` panics on Wayland. #793

`get_current_monitor()` panics on Wayland. #793

kchibisov commented Aug 18, 2020 •

edited

Loading

kchibisov commented Aug 20, 2020 •

edited

Loading