subclass windows in macos so they can be made resizable even with no decorations #408

sodiumjoe · 2018-02-19T01:01:27Z

The subclassing logic was copied from servo's fork of glutin:
https://github.com/servo/glutin/blob/63026a0f4c8ba8bdc40787f8305ab008e5eb45e3/src/api/cocoa/mod.rs#L418

fixes rust-windowing#368 The subclassing logic was copied from servo's fork of glutin: https://github.com/servo/glutin/blob/63026a0f4c8ba8bdc40787f8305ab008e5eb45e3/src/api/cocoa/mod.rs#L418

paulrouget · 2018-02-19T02:18:03Z

src/platform/macos/window.rs

+            decl.add_method(sel!(canBecomeMainWindow), yes as extern fn(&Object, Sel) -> BOOL);
+            decl.add_method(sel!(canBecomeKeyWindow), yes as extern fn(&Object, Sel) -> BOOL);
+            decl.add_method(sel!(mouseDownCanMoveWindow), yes as extern fn(&Object, Sel) -> BOOL);
+            decl.add_method(sel!(isMovableByWindowBackground), yes as extern fn(&Object, Sel) -> BOOL);


Are mouseDownCanMoveWindow and isMovableByWindowBackground necessary?

Does the window move when you click and drag anywhere in the window? If so, is that the behavior you want. And is this the behavior we want when there is a titlebar?

Great point, and I also see that there's a separate platform attribute movable_by_window_background that already handles that independently. I'll remove those.

paulrouget · 2018-02-19T05:43:54Z

examples/window.rs


 fn main() {
    let mut events_loop = winit::EventsLoop::new();

    let _window = winit::WindowBuilder::new()
        .with_title("A fantastic window!")
+        .with_decorations(false)


Not sure we want that in the example.

yup, that was just from my testing, I've removed it

paulrouget · 2018-02-19T05:59:29Z

The window is not movable at that point.

I'm not 100% sure what kind of behavior we want to achieve here. Like… how will drag-to-move work, or should we keep the window buttons, the rounded corner and the shadow.

But maybe adding these masks will make the window look better?

NSBorderlessWindowMask
NSTitledWindowMask
NSResizableWindowMask
NSFullSizeContentViewWindowMask
NSClosableWindowMask
NSMiniaturizableWindowMask

sodiumjoe · 2018-02-19T06:11:05Z

To be clear, this behavior is with an explicit with_decorations(false). I have it running this way in a local build of alacritty, and it's exactly what I want. I think granular control of those masks makes more sense as platform attributes. I use a separate program to move/resize windows (hammerspoon), and I like the square corners and no border look.

how will drag-to-move work

I've tested with

        .with_movable_by_window_background(true)
        .with_decorations(false)

and that creates an undecorated window that is movable by dragging anywhere in the window.

Maybe it makes sense for most or all of those separate masks to be granularly configurable as separate platform attribute methods, with with_decorations(false) resolving to some sane default preset combination of those methods?

I'd be happy to add that to this PR if that's the direction we want to go. I think it makes the configuration story for downstream projects a little complicated, though. E.g., currently alacritty just exposes a platform-agnostic boolean decorations field. I'm not sure how alacritty would expose these platform specific attributes through its config yaml file, or if @jwilm would event want to expose that level of granularity. I'll let him weigh in if he'd like.

paulrouget · 2018-02-19T06:24:53Z

So there are 3 different setups:

titlebar + window controls + round borders
window controls + round borders
nothing + no round borders

winit only have one switch. So we need to pick between window controls or nothing.

Let's go for the current setup (nothing).

If anyone wants to add back the window controls, they can use WindowExt::get_nswindow() to reset the mask. In Alacritty, you could expose these 3 options.

paulrouget · 2018-02-19T06:34:31Z

You moved from NSFullSizeContentViewWindowMask to NSBorderlessWindowMask and removed the closable and miniaturizable masks. Can you explain exactly why these were necessary?

sodiumjoe · 2018-02-19T07:25:26Z

NSFullSizeContentViewWindowMask to NSBorderlessWindowMask

My understanding of NSFullSizeContentViewWindowMask is that it's for extending the content view into the title bar area. In my testing, it doesn't appear to have any effect when there is no title bar. The closable and miniaturizable masks also don't appear to have any effect when there's no title bar. If they were there for some reason I missed, I'd be happy to add them back.

sodiumjoe · 2018-02-19T07:29:59Z

So I think this PR achieves:

nothing + no round borders

While #389 would allow:

window controls + round borders

And I think it makes sense to call "nothing + no round borders" with_decorations(false).

paulrouget · 2018-02-19T07:33:08Z

Ok. That works for me.

@tomaka happy with these changes?

Osspial · 2018-02-21T22:05:26Z

Are we sure this behavior is something we want implicitly true with the with_decorations flag? There are perfectly valid reasons to create an unresizable borderless window, such as with a right-click menu, and I don't want to lose the ability to do that on OSX. This is also inconsistent with other platforms - I know resizing a borderless window isn't allowed in Windows, at least right now, and I don't think it's allowed on Linux.

sodiumjoe · 2018-02-21T22:18:47Z

@Osspial fair question. I guess it depends on what decorations means to wininit. I interpreted it to refer purely to the existence of visual elements, not behavior.

Also, iiuc, #389 would offer more granular controls over the masks such that you could still achieve non-resizeable borderless windows.

paulrouget · 2018-02-22T05:16:08Z

Are we sure this behavior is something we want implicitly true
I don't want to lose the ability to do that on OSX

Both scenario are covered. Both scenario are valid. It's a matter of what we do by default, what the expectation is.

I would recommend to land this as it is, and in #389 add enough documentation / examples to explain how to achieve both behaviors.

sodiumjoe · 2018-02-24T22:28:44Z

Anything blocking this from merging? @tomaka ?

Osspial · 2018-02-24T22:47:27Z

I'm okay with adding an option to make borderless windows resizable, but my biggest issue is that it both changes the current default behavior and makes the default behavior inconsistent across platforms. Also, I could be reading the diff incorrectly, but there doesn't seem to be a way to configure it to use the current behavior, which seems like an important thing to have.

EDIT: @sodiumjoe you mentioned that #389 would add the ability to create the current behavior. How so? The documentation doesn't suggest that it does, but I can't say I'm versed enough in OSX platform development to tell if the underlying code lets you do this.

sodiumjoe · 2018-02-24T22:58:01Z

My understanding is that borderless windows on windows and linux are resizable through other means, just not by dragging the borders (because the borders aren't there). Please correct me if I'm wrong.

If this is the case, then I think neither behavior is perfectly consistent across platforms. In fact, I'm not sure it's possible to achieve this in macos (i.e. borderless windows that are not resizeable by dragging the edges of the window, but resizeable through other means, like a window manager).

The more granular control is represented by #389, not this PR.

Osspial · 2018-02-25T00:00:42Z

Testing it, it looks like borderless windows on Windows are completely unresizable (or at least, I can't figure out a way to do it, even with keyboard shortcuts).

On Linux, it seems to depend on the window manager. I tested in with Xfce, and was able to resize borderless windows with Alt+Right-Click, but it still couldn't be resized with border dragging. Despite this, the behavior is still mostly consistent across platforms - borderless windows can't be resized by dragging on the edges.

Ralith · 2018-02-25T02:57:54Z

Note that under X11, it is technically impossible for a client to prevent the window manager from resizing its windows however it likes, so the existence of the possibility under some particular state shouldn't be read into too heavily.

sodiumjoe · 2018-02-25T03:50:32Z

.with_decorations(false) (current):

platform	resizable by dragging	resizable by window manager	borders
windows	no	no	no
linux	no	yes	no
macos	no	no	no

.with_decorations(false) (proposed in this PR):

platform	resizable by dragging	resizable by window manager	borders
windows	no	no	no
linux	no	yes	no
macos	yes	yes	no

@Osspial I can see your point about it being slightly more consistent behavior across
platforms. However, it still seems to me that it depends on what the meaning of
"decorations" is. I interpret it to refer purely to aesthetic concerns. For a
flag that concerns the behavior that you're talking about, I think a different
flag that refers specifically to the behavior would be more appropriate, e.g.
something like .resizable(bool).

You're right that #389 doesn't have the settings for controlling this specifically, but I think it shows us how to use the window builder extension to expose all the granular settings that we would need to to allow it.

sodiumjoe · 2018-03-06T06:54:13Z

@tomaka would love your input on this. If this is not acceptable as is, I would like to get started on the WindowBuilderExt work to get the resizable window with no titlebar that I want.

tomaka · 2018-03-06T08:40:43Z

Sorry, it's just that I don't really have a good opinion on this.

sodiumjoe · 2018-03-07T00:51:38Z

@tomaka Hmmm, ok, so whom do I need to convince to get this merged or rejected?

oschrenk · 2018-03-07T07:23:31Z

I'm just a user but I sure do hope that this get's merged as I would love to have borderless windows that I can move around and resize (with external programs in macOS).

On that note @sodiumjoe If you could cut a release of your alacritty (with this change) that would be amazing.

sodiumjoe · 2018-03-08T05:57:48Z

@oschrenk you just need to clone alacritty, glutin, and winit, then reference your local copy of winit from glutin, and glutin from alacritty:

glutin's Cargo.toml's [dependencies] block:

winit = { path = "../winit" }

in alacritty's Cargo.toml's [dependencies] block:

glutin = { path = "../glutin" }

I've been using this build of alacritty since I started this PR and really enjoying it.

paulrouget · 2018-03-08T06:44:11Z

Maybe split this PR in 2. The WinitWindow subclass is necessary no matter what.

Then as what to do with the mask, it doesn't really matter, you can always do that via WindowExt::get_nswindow() I believe.

sodiumjoe · 2018-03-10T06:28:26Z

I made a PR to @robertgzr 's branch for #389 to add another WindowBuilderExt method to hide the titlebar, but keep resizability: robertgzr#1

But it depends on this one getting merged first.

sodiumjoe · 2018-03-11T20:25:21Z

@tomaka I believe all the objections to this PR have been resolved.

sodiumjoe · 2018-03-20T04:37:47Z

@tomaka anything else necessary here?

tomaka · 2018-03-23T09:24:20Z

Needs a rebase or a merge, and I'll merge!

sodiumjoe · 2018-03-23T18:07:51Z

@tomaka merged, thanks!

mitchmindtree · 2018-03-26T09:10:28Z

I just tried testing this on macos and this seems to break the multiwindow.rs example (I use this quite a lot downstream). Here's the backtrace:

   Compiling winit v0.11.2 (file:///Users/joshuabatty/Documents/Code/rust/winit)
    Finished dev [unoptimized + debuginfo] target(s) in 1.81 secs
     Running `target/debug/examples/multiwindow`
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', src/libcore/option.rs:335:21
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::panicking::default_hook::{{closure}}
             at src/libstd/sys_common/backtrace.rs:68
             at src/libstd/sys_common/backtrace.rs:57
             at src/libstd/panicking.rs:381
   2: std::panicking::default_hook
             at src/libstd/panicking.rs:397
   3: std::panicking::begin_panic
             at src/libstd/panicking.rs:577
   4: std::panicking::begin_panic
             at src/libstd/panicking.rs:538
   5: std::panicking::try::do_call
             at src/libstd/panicking.rs:522
   6: std::panicking::try::do_call
             at src/libstd/panicking.rs:498
   7: <core::ops::range::Range<Idx> as core::fmt::Debug>::fmt
             at src/libcore/panicking.rs:71
   8: <core::ops::range::Range<Idx> as core::fmt::Debug>::fmt
             at src/libcore/panicking.rs:51
   9: <core::option::Option<T>>::unwrap
             at /Users/travis/build/rust-lang/rust/src/libcore/macros.rs:20
  10: winit::platform::platform::window::Window2::create_window
             at src/platform/macos/window.rs:443
  11: winit::platform::platform::window::Window2::new
             at src/platform/macos/window.rs:321
  12: winit::platform::platform::Window::new
             at src/platform/macos/mod.rs:32
  13: <core::option::Option<T> as core::ops::try::Try>::into_result
             at src/window.rs:119
  14: winit::window::<impl winit::Window>::new
             at ./src/window.rs:135
  15: multiwindow::main
             at examples/multiwindow.rs:7
  16: std::rt::lang_start::{{closure}}
             at /Users/travis/build/rust-lang/rust/src/libstd/rt.rs:74
  17: std::panicking::try::do_call
             at src/libstd/rt.rs:59
             at src/libstd/panicking.rs:480
  18: panic_unwind::dwarf::eh::read_encoded_pointer
             at src/libpanic_unwind/lib.rs:101
  19: std::sys_common::bytestring::debug_fmt_bytestring
             at src/libstd/panicking.rs:459
             at src/libstd/panic.rs:365
             at src/libstd/rt.rs:58
  20: std::rt::lang_start
             at /Users/travis/build/rust-lang/rust/src/libstd/rt.rs:74
  21: multiwindow::main

macos 10.12.4

sodiumjoe · 2018-03-27T07:51:55Z

I can repro, I'll check it out

sodiumjoe · 2018-03-27T08:06:34Z

@mitchmindtree fixed, can you verify?

mitchmindtree

Just tested the multiwindow example again and it seems to be working fine! Also checked the events that are being emitted and they seem to still look good too.

sodiumjoe · 2018-03-28T03:10:15Z

@mitchmindtree awesome, thanks for the quick verification! Anything else I need to do to get this merged?

sodiumjoe · 2018-04-02T22:02:44Z

@tomaka just my weekly check-in on this PR 😄

francesca64 · 2018-04-02T22:24:27Z

@sodiumjoe this should have a CHANGELOG entry.

sodiumjoe · 2018-04-02T22:28:58Z

@francesca64 done!

francesca64 · 2018-04-02T22:45:30Z

The change should go under the Unreleased heading
Please try to better conform to the style of the rest of the CHANGELOG (i.e. don't add those additional line breaks, and in general)

sodiumjoe · 2018-04-02T22:48:22Z

@francesca64 Ok, how does it look now?

francesca64 · 2018-04-02T23:11:31Z

@sodiumjoe Looks good, thanks!

While I'm able to merge this, I'm not sure how comfortable @tomaka is with me merging things. With how busy he is, I want to do as much to help out with winit as I can, so I'm hoping for some clarification on his policy.

...that said, since he already approved this, I'll merge it.

…pcwalton Make the fields of `TextMetrics` lazily calculated, and add an API that eliminates double layouts. This adds an extension to the HTML canvas API that allows you to pass the `TextMetrics` object returned by `measure_text()` to `fill_text()` and/or `stroke_text()` to draw the measured text without laying it out again. It improves performance on the `canvas_nanovg` demo.

* feat: support theme on macOS * Update support-theme-on-macos.md Co-authored-by: Ngo Iok Ui (Wu Yu Wei) <wusyong9104@gmail.com>

make windows without decorations resizable and movable in macos

78b2441

fixes rust-windowing#368 The subclassing logic was copied from servo's fork of glutin: https://github.com/servo/glutin/blob/63026a0f4c8ba8bdc40787f8305ab008e5eb45e3/src/api/cocoa/mod.rs#L418

sodiumjoe mentioned this pull request Feb 19, 2018

[Feature] Add setting to hide titlebar alacritty/alacritty#168

Closed

paulrouget reviewed Feb 19, 2018

View reviewed changes

remove isMovableByWindowBackground and mouseDownCanMoveWindow

af27f5d

paulrouget reviewed Feb 19, 2018

View reviewed changes

revert example changes

bcb7737

robertgzr mentioned this pull request Feb 19, 2018

[macOS] WindowBuilderExt additions for the titlebar #389

Closed

remove resizable mask from decoration: false

23f594f

This was referenced Mar 17, 2018

Allow changing window title bar color in macOS alacritty/alacritty#686

Closed

323 windowbuilder ext macos #423

Merged

Merge branch 'master' into 368-resize-no-decorations-macos

c169567

avoid duplicate class declarations

fc15524

mitchmindtree approved these changes Mar 27, 2018

View reviewed changes

sodiumjoe mentioned this pull request Apr 1, 2018

[META] Feature parity between platforms #252

Closed

sodiumjoe added 2 commits April 2, 2018 15:27

update changelog

a2e457a

Merge branch 'master' into 368-resize-no-decorations-macos

e7e5d97

fix changelog

bc86ed7

changelog whitespace

ab38e93

francesca64 merged commit be6d2ed into rust-windowing:master Apr 2, 2018

francesca64 mentioned this pull request Jun 5, 2018

Decoration free windows can't be resized after glutin 0.16 alacritty/alacritty#1345

Closed

futurepaul mentioned this pull request Mar 11, 2020

implement 'show titlebar' window option linebender/druid#634

Open

madsmtm pushed a commit to madsmtm/winit that referenced this pull request Jun 11, 2022

feat: support theme on macOS (rust-windowing#408)

8af4d8f

* feat: support theme on macOS * Update support-theme-on-macos.md Co-authored-by: Ngo Iok Ui (Wu Yu Wei) <wusyong9104@gmail.com>

subclass windows in macos so they can be made resizable even with no decorations #408

subclass windows in macos so they can be made resizable even with no decorations #408

Conversation

sodiumjoe commented Feb 19, 2018 • edited Loading

paulrouget Feb 19, 2018

Choose a reason for hiding this comment

sodiumjoe Feb 19, 2018

Choose a reason for hiding this comment

paulrouget Feb 19, 2018

Choose a reason for hiding this comment

sodiumjoe Feb 19, 2018

Choose a reason for hiding this comment

paulrouget commented Feb 19, 2018

sodiumjoe commented Feb 19, 2018 • edited Loading

paulrouget commented Feb 19, 2018

paulrouget commented Feb 19, 2018

sodiumjoe commented Feb 19, 2018

sodiumjoe commented Feb 19, 2018 • edited Loading

paulrouget commented Feb 19, 2018

Osspial commented Feb 21, 2018 • edited Loading

sodiumjoe commented Feb 21, 2018

paulrouget commented Feb 22, 2018

sodiumjoe commented Feb 24, 2018

Osspial commented Feb 24, 2018 • edited Loading

sodiumjoe commented Feb 24, 2018

Osspial commented Feb 25, 2018

Ralith commented Feb 25, 2018

sodiumjoe commented Feb 25, 2018

sodiumjoe commented Mar 6, 2018

tomaka commented Mar 6, 2018

sodiumjoe commented Mar 7, 2018

oschrenk commented Mar 7, 2018

sodiumjoe commented Mar 8, 2018

paulrouget commented Mar 8, 2018

sodiumjoe commented Mar 10, 2018

sodiumjoe commented Mar 11, 2018

sodiumjoe commented Mar 20, 2018

tomaka commented Mar 23, 2018

sodiumjoe commented Mar 23, 2018

mitchmindtree commented Mar 26, 2018 • edited Loading

sodiumjoe commented Mar 27, 2018

sodiumjoe commented Mar 27, 2018

mitchmindtree left a comment

Choose a reason for hiding this comment

sodiumjoe commented Mar 28, 2018

sodiumjoe commented Apr 2, 2018

francesca64 commented Apr 2, 2018

sodiumjoe commented Apr 2, 2018

francesca64 commented Apr 2, 2018

sodiumjoe commented Apr 2, 2018 • edited Loading

francesca64 commented Apr 2, 2018

sodiumjoe commented Feb 19, 2018 •

edited

Loading

sodiumjoe commented Feb 19, 2018 •

edited

Loading

sodiumjoe commented Feb 19, 2018 •

edited

Loading

Osspial commented Feb 21, 2018 •

edited

Loading

Osspial commented Feb 24, 2018 •

edited

Loading

mitchmindtree commented Mar 26, 2018 •

edited

Loading

sodiumjoe commented Apr 2, 2018 •

edited

Loading