Inset Mac OS traffic lights

[haasal]

What kind of change does this PR introduce?

• Bugfix
• Feature
• Docs
• New Binding issue [feat] Allowing for native inset OS X traffic lights on NSWindow tauri#4789
• Code style update
• Refactor
• Build-related changes
• Other, please describe:

Does this PR introduce a breaking change?

• Yes, and the changes were approved in issue #___
• No

Checklist

• When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
• A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
• I have added a convincing reason for adding this feature, if necessary

Other information

Important: There still exist artifacts when resizing the window because the traffic lights are set just after the window has already been drawn.

[morajabi]

Thanks for this awesome PR! I had tried doing this only to fail at the "resize" artifacts: #494 Reading your solution made it clear. I should close that issue then.

[PhotonQuantum]

This feature is fantastic!

However, I noticed a strange behavior: the hover effect still shows when I move the cursor to the "original" position of traffic lights. The electron implementation doesn't seem like have this behavior. I dug into its code but failed to find a solution.

Update: strangely, the problem goes away if I resize the window.

[tr3ysmith]

Hey what's the status of this? I really would like to be able to apply inset on the traffic lights without work arounds

[haasal]

Hi, me too. But I know really little about Tao. If this gets merged then we still have to implement it somehow into tauri.
Could someone review this please?

[wusyong]

The implementation seems fine to me. But I'm not sure if it's good to merge now.
@amrbashir Do you think winit can accept this?
Btw you can get the raw window handle to change it yourself if you can't wait for it any longer.

[amrbashir]

there is a good chance they may not accept but lets go ahead and merge this and if they didn't accept it upstream, we can implement it directly in tauri

[haasal]

I'm sorry this has taken so long. I didn't really have time to work on this. I will document the function and delete the example these days. I hope it's not to late to merge?

[amrbashir]

Looks fine to me, however I'll hand this to @wusyong and @JonasKruckenberg (since you were interested in this)

[amrbashir]

Please rebase the PR to fix the conflicts and add a change file

[JonasKruckenberg]

I'll review this tomorrow, it's a super important imo and this or has been open wayy too long 😉 let's get this over the finish line

[haasal]

Maybe it's important to add that I couldn't test the changes since my MacBook broke 🥺. If one of you could do that it would be amazing

[JonasKruckenberg]

After testing this PR now, I feel LogicalSize makes no sense here, the way you currently call the method is as such

                                      // WTH means width here? and height?
.with_traffic_light_inset(LogicalSize { width: 10, height: 20 });

[JonasKruckenberg]

Okay a few other observations:

1. While this does successfully inset the traffic lights, it does not increase the draggable area size
2. When resizing the window, it looses the inset

The second one really is a dealbreaker right now, I guess we need to implement a delegate and stuff just like electron does as well

[haasal]

Okay a few other observations:

1. While this does successfully inset the traffic lights, it does not increase the draggable area size

2. When resizing the window, it looses the inset

The second one really is a dealbreaker right now, I guess we need to implement a delegate and stuff just like electron does as well

I don't know if you fixed it already in that commit, but when I tested it with my supplied example the inset was persistent during resizing of the window.

[JonasKruckenberg]

I don't know if you fixed it already in that commit, but when I tested it with my supplied example the inset was persistent during resizing of the window.

Okay yes my bad, calling the set_traffic_light_inset method after the RedrawRequested event got fired fixes the artifacts.

My only question @haasal is why the with_traffic_light_inset method exists if it's not really doing what the user expects. This feels like misleading api design

[JonasKruckenberg]

@amrbashir I added back the example, since it's indeed needed to use the method correctly (it's not enough to just call it once but it needs to be called after RedrawRequested)

[haasal]

Yeah that was the reason I made the example. I already tried implementing this in Tauri itself using the raw window handle but I couldn't get rid of these artifacts. Do you think this method can now be used in tauri without causing artifacts?

[haasal]

My only question @haasal is why the with_traffic_light_inset method exists if it's not really doing what the user expects. This feels like misleading api design

I don't know how to implement it instead. This whole thing gave me big headaches because I couldn't find a way to set the inset without having to call the method in some weird way. If you have any idea how to do this without having to call the method after RedrawRequested it would be great!

[amrbashir]

I still don't see why the example is needed since tao should already handle that internally and not require users to keep calling it in RedrawRequested

[JonasKruckenberg]

I still don't see why the example is needed since tao should already handle that internally and not require users to keep calling it in RedrawRequested

then we should change it to behave that way

[haasal]

I really would like to get this over with but as said earlier I don't really know how. If you know how to directly inject something into the "window draw pipeline" (I.e. how to automatically call my inset-method/function without requiring the user to do some weird setup) I could take a look at it

[amrbashir]

@haasal you need add an inset field on the ViewState struct here

tao/src/platform_impl/macos/view.rs

Line 54 in c5d606d

pub(super) struct ViewState {

and your new function set_traffic_light_inset sets this field, then in draw_rect

tao/src/platform_impl/macos/view.rs

Line 382 in c5d606d

extern "C" fn draw_rect(this: &Object, _sel: Sel, rect: NSRect) {

you have access to the state where you can read it and re-position the tarffic lights

[haasal]

@wusyong I didn't do anything like this before and I don't want to mess up even worse. Do you mean pulling from upstream onto my dev branch and then merging with all of my contributions squashed into one commit? Wouldn't this negate all my commit messages?

[amrbashir]

I guess it is ready for merge now, @wusyong would you mind testing and merge?