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

Fill the windows in the examples with a solid color #2812

Merged
merged 7 commits into from
Jun 19, 2023

Conversation

notgull
Copy link
Member

@notgull notgull commented May 20, 2023

  • Tested on all platforms changed (only tested on X11)
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

When you run the examples for winit, the contents of the window will be undefined depending on the platform. To the uninformed user, this appears to be a bug in some cases. In the past we've gotten quite a few issues because of this (#2807 comes to mind) that aren't issues at all.

This PR fills the windows with a solid gray background in order to make sure that there is something inside of the window.

Before (X11):

image

After (X11):

image

@fredizzimo
Copy link

I think this should be merged sooner rather than later. Currently the examples are unusable on Wayland, you can't even activate the window.

If users encounters bugs in Neovide, I would sometimes like to have the repeat the bug using one of the Winit examples, but if the examples don't work, that's not possible.

@notgull, Could you update to the latest master and also fix the new key_binding.rs example? That's a new example and does not have the fix.

@kchibisov
Copy link
Member

That's true, though there's always glutin with window example, which I use as a testing or alacritty branches which I always open to target latest winit dev, since that's what I use myself as a maintainer to test winit API.

@notgull notgull force-pushed the softbuffer-fill-em-up branch from 1c3d5a7 to 03d544a Compare June 3, 2023 01:48
@notgull notgull mentioned this pull request Jun 3, 2023
5 tasks
@notgull
Copy link
Member Author

notgull commented Jun 3, 2023

I've rebased on the latest master. So far softbuffer doesn't support Android (rust-windowing/softbuffer#44) or iOS (rust-windowing/softbuffer#43) yet, so I've just made fill_window a no-op on those platforms. The Windows CI errors are due to winit using windows-sys version 0.45 and should be fixed by #2842

Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

All examples must handle Resize events, otherwise Wayland won't fly.

Probably macOS won't fly as well.

@kchibisov
Copy link
Member

kchibisov commented Jun 3, 2023

All examples must handle Resize events, otherwise Wayland won't fly.

Probably macOS won't fly as well.

Actually, you do Resize(inner_size), but the thing is that it's not really resized on Wayland, even when winit is telling the right sizes. I'd assume we'd need a new release of softbuffer?

it also busy drawing on Wayland even when you do wait and remove any requests for redraw, so I'd assume it's not working.

cc @ids1024

examples/util/fill.rs Outdated Show resolved Hide resolved
@ids1024
Copy link
Member

ids1024 commented Jun 4, 2023

At this point we might as well do another release of softbuffer, from the current master, instead of having things relying on 0.2. It's a breaking release, and it will probably have another breaking release for other things, but that's not really an issue at this stage.

@kchibisov
Copy link
Member

At this point we might as well do another release of softbuffer, from the current master, instead of having things relying on 0.2. It's a breaking release, and it will probably have another breaking release for other things, but that's not really an issue at this stage.

That would be nice, given that it's un-usable in the current state on Wayland from what I've seen in this PR. The example on softbuffer master does work fine.

@notgull notgull force-pushed the softbuffer-fill-em-up branch from f6a2a16 to 7ca3974 Compare June 4, 2023 23:20
@notgull
Copy link
Member Author

notgull commented Jun 5, 2023

Now cargo-deny is flagging the memmap2 dependency. Shouldn't be a real issue.

@kchibisov
Copy link
Member

Could you add memmap2 into deny and open an issue on client-toolkit(I can do this myself if you don't want to)?

@kchibisov kchibisov merged commit b2a46d0 into rust-windowing:master Jun 19, 2023
@notgull notgull deleted the softbuffer-fill-em-up branch June 19, 2023 18:48
strohel pushed a commit to tonarino/winit that referenced this pull request Aug 27, 2023
strohel pushed a commit to tonarino/winit that referenced this pull request Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants