feat!: fix appearance of --frame transparent and deprecate neovide_background_color.

[crupest]

Fix #2094

This pr does following changes:

1. fix the bug in --frame transparent. Before this pr, the titlebar floats on the head of content. This pr adds a top padding of titlebar height, so the appearance looks better.
2. deprecated the neovide_background_color option because the color of titlebar now honors neovide_transparency. An error message will be printed if user uses it.
3. code refactor: draw_background code merges into the feature code of this pr. And docs are updated.

And here are some other related changes:

1. The light/dark theme of the window will be set based on the background color. So we can get the titlebar text in the right color (white/black).

This pr is almost done but still has some details to discuss. If you want to experience this right now, you can download it from the CI at https://github.com/crupest/neovide/actions/runs/7626642491 (currently based on 3e575fc on main branch). Or just checkout the branch of this pr and cargo run --release -- --frame transparent.

[crupest]

@fredizzimo I have to say, things are much simpler than I think. We don't have to draw the title bar ourselves.

Here is a style mask on macos NSWindowStyleMaskFullSizeContentView, which makes a effect like content view spreads to the whole window and the title bar floats on the top.

So things get simple. We always specify NSWindowStyleMaskFullSizeContentView to get full control of the window and use setTitlebarAppearsTransparent_ to make the title bar have no background if there is one. When we don't want a title bar, just don't specify NSWindowStyleMaskTitled. When we want a title bar, specify NSWindowStyleMaskTitled and related masks, make a top padding and it's done.

Here is the result.

Then the problem becomes how do we adjust old configs. I think we can deprecate all related macos specific configs in the doc and try to leave them there if possible so people won't get surprised that old configs not work. I want to deprecate the two macos specific transparent and buttonless options of --frame and silently make them work like borderless since I think these two ones are meaningless. And I also want to deprecate neovide_background_color but leave it still work since it does not complicates many things. How do you think? Is there anything I didn't notice?

[fredizzimo]

@crupest, that's great news!

We actually discussed the deprecation of neovide_background_color yesterday on Discord.

I think we should just show a message with the link to the appropriate issue and documentation here, for people that tries to use it. The error_msg! macro in error_handling.rs shows a mesage using the standard Neovim message system, so that could be used.

[crupest]

@fredizzimo My network environment does not allow me to access discord. I'm sorry I can't synchronize with you guys there.

I have merged draw_background into my macos_feature. I'll follow your suggestion to use neovim message to show the error. What about the --frame options? Maybe you can discuss on this topic to get what most people would like.

I think I'll have plenty of time to deal with this this weekend. During this period, I'll also complete the documentation change in the pr.

[fredizzimo]

@crupest, I'm not a macOS user, so I cant tell how useful or useless the options are. But by reading the description I do think they are quite useless, especially if the transparency follows neovide transparency automatically. So from my point of view you can go ahead remove them. Maybe the same strategy could be applied to that too, with a message referring to an issue, so that we possibly can add them back based on feedback.

[crupest]

OK. That's great! I'll complete the rest works soon. Best regards!

[crupest]

@fredizzimo Almost everything is done except one thing. macos seems not to deliver mouse events on title bar to system if it is transparent, which means if you click on title bar (except the title bar text), the mouse events are passed to neovide and the window does not zoom as expected. However, window move works but it also trigger neovide mouse handling at the same time. I tried to hack on MouseManager but it looks like it would cause a great damage to current codes. Do you have any idea about this?

Here are the screenshots of the final effects.

Here is the bug mentioned above.

Screen.Recording.2023-12-09.at.20.55.17-1.mov

[fredizzimo]

I don't know if it's possible, but maybe it's possible to make the text field as wide as the titlebar? Even if the actual visible text doesn't take up all the space.

Or some other invisible title bar elements?

It would be rather easy to ignore all events in a certain area in the mouse_manager, but I think there will be problems when the mouse up and mouse down are in different areas.

[crupest]

That's a good idea. I'll try to put a native invisible control there and handle the zoom.

[crupest]

@fredizzimo It's a little awkward for me to misunderstand a lot of things. I should have tried the transparent and buttonless options before I did all these. But luckily I have learned a lot of things. I want to share them here before I go to rest today.

Here are the our implementations,

1. --frame none = Borderless. Borderless window has no round corner and suffers bug macOS cannot close window without frame #2087. This is useful for tiling window manager as the issue author said.
2. --frame buttonless = MY --frame none = Titled | FullSizeContentView with invisible title bar. Different to borderless, this has round corner and does not have above bug.
3. --frame transparent = MY --frame full without top padding = Titled | FullSizeContentView | [window buttons flags] . So the window buttons are float on the content, which makes it ugly. So I guess nobody use this.

So everything is just a composition of complicated things. I think we have to sort these again. I think we can leave 3 final choices:

1. borderless (no round corner) and fix macOS cannot close window without frame #2087
2. borderless (with round corner), already works now
3. decorated, and fix the title bar issue mentioned above

I will revise my codes to reflect these new options tomorrow. As for the title bar problem, I have investigated a long time. The way of putting a native control is infeasible. We have so little ability to interact with native platform, especially for macos, which use obj-c but not pure-c api. And it requires to use obj-c api to do the event handling and other related things. We can use the rust library objc but it only makes the code ugly and unmaintainable. (Maybe there is other library?) Logically, in this case, title bar is under our control and we then have the duty to manage that area. But it in turn breaks our current codes a lot.

I'll keep searching for solutions tomorrow. Just to leave a note here. I'm really new to macos developing. And if somebody has any experience on this, I'll be very glad to learn from you!

UPDATE:
Also have to mention, borderless window with no round corner is not movable or resizable. And borderless window with round corner can move and resize. This matches other platform's behaviors, I think.

[fredizzimo]

No, problems, this is complicated, and I don't think anyone actually understands it, that's why we have these issues opened for months without any comments about implementation ideas. I have problems even imaging the concepts since I don't use macOS.

As for the objc, winit uses https://github.com/madsmtm/objc2, so I guess that's about as good as it gets. And talking about winit, it's possible that some of these things might make more sense to implement there, or at least port it there later.

Edit: I forgot to say, from what I have seen, the macOS documentation itself is some of the worst I have seen...

[crupest]

@fredizzimo I have a good news to share with you. The method of using a stub view at titlebar works perfectly. I have already pushed the working codes. I'll do code clean tomorrow and then I think we can merge. Cheers🍺.

[crupest]

And I will also try to improve macos specific code, use better apis and make code as clean as I can do.

[crupest]

Edit: I forgot to say, from what I have seen, the macOS documentation itself is some of the worst I have seen...

Exactly. Far worse than M$. I develop totally via trials.

[crupest]

@fredizzimo Coming again. After struggling for several days to try, I failed and gave up to find a way to layout title bar stub completely in macos codes. I still need to use winit resize event to update title bar stub's position and size. It makes the code kind of unclean but I can't find a better way. The biggest problem is winit always pacics here in whatever way I try:

eovide panicked with the message 'called `Result::unwrap()` on an `Err` value: PoisonError { .. }'. (File: /Users/crupest/.cargo/registry/src/index.crates.io-6f17d22bba15001f/winit-0.29.4/src/platform_impl/macos/app_state.rs; Line: 383, Column: 33)

I'm not sure whether there is a more gentle way to handle this. However, current code works well in function. Do you think we can try more about making the code neater or we just stop here.

Here is the final effects. Just ignore the lag of resizing. It's an old problem.

Screen.Recording.2023-12-16.at.00.17.51.mov

[fredizzimo]

@crupest, I did not take a too detailed look at the code, but it looks OK to me and we can always improve it later. Even small glitches are OK at this point, since it should be strictly better than what we had before, as long as it does not crash.

@madsmtm, @kchibisov, do you have some advice from the Winit side? The idea here is to support transparent backgrounds and titlebars, by drawing them ourselves. Previously we used a custom background color for the whole window, but that was interfering with the transparency of the Neovim background.

[kchibisov]

@fredizzimo winit has option to do transparent decorations or borderless, so you can offset the title bar, and you still draw into it (alacritty has transparent setting on macOS).

It's not like you need to draw them on your own entirely, you can just clear and then update viewport?

[fredizzimo]

@crupest, I guess you tried some approaches using those options before starting to work on this approach?

Personally, I'm quite blind on this, since I don't have a macOS computer to test on, and I'm just going on what's been reported in different issues here.

But yes, now I saw this old issue, especially this image #1298 (comment), and that does indeed look like exactly what we want. We just need to offset the content, which is already possible through a user option g:neovide_padding_top, but that should probably be automatic in this case.

[crupest]

You are right. A top padding is enough to make the appearance good. But there are several extra points of my codes.

1. I removed transparent frame option because it has no top padding and makes no actual sense then.

2. I write a way to correctly, maybe, get the title bar height.

3. The appearance looks good. But the double click on title bar to zoom doesn't work. I don't know if winit handles this well if just to use winit api to set the window style. Maybe I'll try it tomorrow.

Besides, there are some clean work. Like move the codes of previous draw background and update docs.

[crupest]

It's not like you need to draw them on your own entirely, you can just clear and then update viewport?

Actually I don't draw the title bar by ourselves now. The solution is dropped after I find the way to add a top padding. I lined out the original solution and point out the comment of the new solution.

[kchibisov]

Yeah, but it's pretty much the same with transparent style, no? The only issue is that you need to add handling for clicks and padding yourself, but winit has all the functions available for stuff like that, including moving.

[crupest]

To be clear, actually the core solution of my codes is exactly adding a top padding. And there are the other points mentioned above.

[polachok]

Works fine for me. Not a fan of the animation while entering/exiting fullscreen, but it can be fixed later.

[falcucci]

@crupest everything works fine, congrats!

The only exception is a changed that has been made and I didn't see in the first look: the title is hidden by default on upstream when used --frame transparent.

But by now you have removed it here which is bothering once the users had it hidden always by default.

my suggestion is to create a setting to toggle it as the user wishes.

After all, this is the last request I've made to finally approve the PR.

[crupest]

Works fine for me. Not a fan of the animation while entering/exiting fullscreen, but it can be fixed later.

@polachok That's kind of annoying indeed. But it may take some time for me to find a way of handling it better.

MacOS automatically zooms the window content when toggling fullscreen, and then the new padding is painted. So the visual effect is a little bad. Maybe we have to find a way to update the padding and draw the surface before the zoom happens. Or we have to make a smooth animation so it looks better.

my suggestion is to create a setting to toggle it as the user wishes.

@falcucci I'm not sure about this problem. I was assuming that --frame transparent means that the titlebar has a transparent background and everything is the same as not specifying it. And the visibility of titlebar text is another feature. I can implement it in this pr. But I'm afraid that it will make this pr too huge and hard to maintain.

[falcucci]

@crupest I am sure about this problem: a default feature has been changed and we are deprecating it in this pr.

I see two ways to handle:

• we add a toggle since we shouldn't deprecate the hidden title instead of making it visible as default. (this should be the starting change and then proceed with everything else related to title bar);
• we add a setting to toggle it in another PR before the release if possible (this should be a minor task);

[crupest]

@falcucci Well, I think the former way is better. Since we are going to make it anyway, let's just make them together instead of creating another one immediately after this. But it may take some time for me to hack into the settings part.

[fredizzimo]

Could you take screenshots of all four frame variations, so it's easer for us that are not on Mac to judge?

I think the title can be hidden by just setting it to empty inside Neovim, so we don't necessarily need to do anything

[falcucci]

@crupest sure, sounds good. I can help creating the toggle PR, but I am not sure if today I can do that (far from computer currently, maybe tomorrow).

@fredizzimo you still plan to release today?

[fredizzimo]

Maybe, but we can delay it until tomorrow. But we should not delay it too long, since the release is long overdue, we get a new bugreport about the startup freeze about every other day.

[crupest]

Default:

With --frame none:

With --frame buttonless:

With --frame transparent:

I think the title can be hidden by just setting it to empty inside Neovim, so we don't necessarily need to do anything

That's an interesting idea. But I can't find the option right now. Would you mind telling me?

[falcucci]

@fredizzimo it's just the title being displayed or not.

I'm not sure if a neovim config would do that but I am curious to try since this is a winit platform specific property.

[crupest]

I'm not sure if a neovim config would do that but I am curious to try since this is a winit platform specific property.

Looks like the options are title and titlestring. But I failed to make it empty because neovim seems to not allow an empty value. If you set it to empty string, neovim will use a default one.

[falcucci]

@fredizzimo @crupest let's merge this and i'll handle to toggle the title hidden option in another PR, I'll try to do it before going to the theater today.

[fredizzimo]

I realized, I made a mistake when reviewing this PR

• fix: prevent empty window title #2126

I thought the override only applied when the user don't set it to empty himself. At least Ithat's the way I think it should work, "Neovide" should only be the default

[crupest]

let's merge this and i'll handle to toggle the title hidden option in another PR, I'll try to do it before going to the theater today.

That would be great!

[falcucci]

#2319 the PR has been created.

[fredizzimo]

Thank you! Let's fix further problems separately if they come up.

[falcucci]

🎉💯