godot
godot copied to clipboard
Make `PopupMenu/Panel` shadows properly visible again
This PR does the following:
- Make
PopupMenu/Panelnodes have spacing around them to show the shadows of their theming, as well as repositioning them accordingly with their shadows' size when popping up. - Make
PopupMenu/Panelnodes be transparent by default. Not only this is necessary for the shadows, but also for rounded corners. It slightly breaks compatibility, as a default value is changed. But I think it's non-intrusive enough to be acceptable. - Fix some bugs related to content scaling, and submenu positioning.
Closes https://github.com/godotengine/godot-proposals/issues/7532. Closes https://github.com/godotengine/godot-proposals/issues/8738.
Test Project: shadow-of-the-popups.zip
I'd say this also closes https://github.com/godotengine/godot-proposals/issues/8738 (by superseding it).
Doesn't look right with the default theme on mac 🙃
@passivestar Could you enable display/window/per_pixel_transparency/allowed in the project settings?
@passivestar Could you enable display/window/per_pixel_transparency/allowed in the project settings?
yeah that works
I've added a warning that appears when a PopupMenu/Panel has a theme that needs per pixel transparency but it isn't enabled.
Note that currently it always appears when running the game because the default project theme has rounded corners. If this is merged, it will need to be modified.
We probably can enable transparency for the editor by default, it may increase VRAM usage a bit, but should not have any significant performance impact on modern systems.
Also, I'll take a look if we can test if transparency is really supported and working (checks if compositor is active and surface/front buffer have correct format), and will add a method to the display server.
@bruvzg Nice! But I think having the warning is a good thing nonetheless.
Themed the menu to take advantage of this PR, makes a huge difference imo
As I understand transparency needs to be an editor setting because we probably don't want a project setting to affect editor theming. Even better if enabled by default like @bruvzg suggested
P.S. Clicking on the popup menu shadow doesn't close the popup which is a bit annoying with a large shadow
P.S. Clicking on the popup menu shadow doesn't close the popup which is a bit annoying with a large shadow
Got you covered. :wink:
Updated to use the new is_window_transparency_available() function.
Still needs transparency to be enabled in the project settings to work but also it looks like things broke since the last time I tested this PR:
- Popup menus now have scrollbars
- Submenu is drawn at a weird distance
- When shadow is enabled it's only drawn in corners inside of the bounding box of the menu
| No shadow | Shadow |
|---|---|
Same with the default theme:
Got you covered. 😉
Can't test if clicking on shadows to close the menu is fixed because there's no shadow to click :)
@passivestar Indeed, it seems that this PR rotted after a while. Sadly, I don't have any time to work on this at all currently.
@passivestar Welp, I'm a masochist, so I worked on it anyways. Please test again.
@Geometror Enable display/window/per_pixel_transparency/allowed in the Project Settings, the shadows shall appear in the running game and in the editor when you restart. A warning about it appears when you run without the setting being enabled.
I actually tested with that setting enabled and disabled. Transparency wasn't the problem, it's that the shadow is cropped.
How does the shadows look in your end in with my test project?
Well, they are not there :)
Are you sure you didn't change anything? I'm on GNU/Linux (Wayland) and the shadows work fine. Maybe try rebasing it to master in your end?
The test project on my end (transparency is not enabled in the project so I had to enable it):
Ok, I finally got it to work. Sorry, I assumed this project setting was turned on in the test project :)
However, this only works for me when using the compatibility backend (both Linux (Wayland) and Windows 11), despite per pixel transparency being supported (https://github.com/godotengine/godot/pull/91333#issuecomment-2506386147)
So it looks like if DisplayServer::get_singleton()->is_window_transparency_available() doesn't determine correctly whether it is available.
@Geometror Huh, all backends work for me.
Anyways, CC @bruvzg, since you were the one who implemented it.
Should also work on macOS (with any renderer) and on Windows with D3D 12, but Vulkan depend on the driver and NVIDIA broke it some time ago.
@Geometror Changes made.
@Geometror @AThousandShips Changes made.
This fix doesn't work properly when display/window/subwindows/embed_subwindows is true (not just shadows, but almost all theme settings).
It seems that for native windows, we need another set of code to handle the theme
@molingyu Both embedded and non-embedded works fine here. Did you enable display/window/per_pixel_transparency/allowed?
@molingyu Both embedded and non-embedded works fine here. Did you enable
display/window/per_pixel_transparency/allowed?嵌入式和非嵌入式在这里都工作正常。您是否启用了display/window/per_pixel_transparency/allowed?
No.
I used the Test Project provided by PR and the shading is still incorrect.
My environment: windows 10
I tried enabling display/window/per_pixel_transparency/allowed.
Shadows that extend beyond the bounds are still clipped(I used the build and test projects provided by the pr action and only changed the shadow to blue to make it easier to spot.).
By the way, my editor also became abnormally transparent.
Additional information: My graphics card is AMD Radeon(TM) Graphics
Godot v4.4.dev (dc78b4c5d) - Windows 10.0.19045 - Multi-window, 2 monitors - Vulkan (Forward+) - integrated AMD Radeon(TM) Graphics (Advanced Micro Devices, Inc.; 27.20.15026.8004) - AMD Ryzen 5 5500U with Radeon Graphics (12 threads)
I checked with other people using Windows and they have no issues.
So this may be caused by the graphics card?
At this point, pr should be fine. Could the problem come from somewhere else?
cc @YeldhamDev
It looks like the issue has not been fixed in the Forward+ / Vulkan (Windows) backend when display/window/subwindows/embed_subwindows is set to false. I also get the following warning messages when launching the project under the abovementioned configuration:
W 0:00:00:0760 PopupMenu::_notification: The current theme styles PopupMenu to have shadows and/or rounded corners, but those won't display correctly if 'display/window/per_pixel_transparency/allowed' isn't enabled in the Project Settings, nor if it isn't supported.
<C++ Source> scene\gui\popup_menu.cpp:1113 @ PopupMenu::_notification()
W 0:00:00:0762 PopupPanel::_notification: The current theme styles PopupPanel to have shadows and/or rounded corners, but those won't display correctly if 'display/window/per_pixel_transparency/allowed' isn't enabled in the Project Settings, nor if it isn't supported.
<C++ Source> scene\gui\popup.cpp:348 @ PopupPanel::_notification()
To clarify, I do have the per_pixel_transparency enabled in the project setting, so it's somewhat not recognized by the backend.
Edit: I also ran the test on a macOS machine, and the issue in forward+ seems to be exclusive to the Windows platform.
Windows & Open GL:
Godot v4.4.dev (f7f6432af) - Windows 10.0.19044 - Multi-window, 4 monitors - OpenGL 3 (Compatibility) - NVIDIA GeForce RTX 4060 Ti (NVIDIA; 32.0.15.6603) - AMD Ryzen 9 5900X 12-Core Processor (24 threads)
| Embed | Not Embed |
|---|---|
Windows & Vulkan:
Godot v4.4.dev (f7f6432af) - Windows 10.0.19044 - Multi-window, 4 monitors - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 4060 Ti (NVIDIA; 32.0.15.6603) - AMD Ryzen 9 5900X 12-Core Processor (24 threads)
| Embed | !! Not Embed !! |
|---|---|
macOS & Open GL:
Godot v4.4.dev (f7f6432af) - macOS 15.1.1 - Multi-window, 1 monitor - OpenGL 3 (Compatibility) - Apple M4 - Apple M4 (10 threads)
| Embed | Not Embed |
|---|---|
macOS & Metal
Godot v4.4.dev (f7f6432af) - macOS 15.1.1 - Multi-window, 1 monitor - Metal (Forward+) - integrated Apple M4 (Apple9) - Apple M4 (10 threads)
| Embed | Not Embed |
|---|---|
@Delsin-Yu The warning comes from display/window/per_pixel_transparency/allowed not being enabled by default. We should probably make so that this is always enabled for the editor itself as well in another PR.
And about it not working on some backends, it's unrelated to the PR itself, and on @bruvzg's ballpark.