godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
91.54k stars 21.26k forks source link

Editor's Popup overlay not hiding/freeing #33572

Closed henriiquecampos closed 4 years ago

henriiquecampos commented 5 years ago

Godot version: 3.2.beta1

OS/device including version: Solus 4.0 Gnome 3.34.1

Issue description: Some times when closing a popup dialog the overlay (the one that makes the background darker) doesn't disappear and it also consume inputs, making so that is impossible to even save some progress (unless it is saveable through the exiting dialog).

image of the overlay covering the screen

Steps to reproduce:

ps: it may have to do with the usage of tablets (I'm using a wacom tablet) and I thin @NathanLovato had similar issues as well, dunno if he was using a tablet, tho.

akien-mga commented 5 years ago

@YeldhamDev Dimming issues never end...

YeldhamDev commented 5 years ago

Considering that I've been using the master branch in my projects and never encountered this, I am going to assume that indeed this may be related touch inputs. And since I have none to test the editor with, this creates a problem for me to even start debugging it...

YeldhamDev commented 5 years ago

This is the section that turns dimming on and off. Could you check if those conditionals fire when opening and closing dialogs in your touch device, and if you can reproduce it with a mouse? https://github.com/godotengine/godot/blob/953f37f49b4d71b064935ff7e910bad829bfd17a/scene/gui/dialogs.cpp#L241-L251

henriiquecampos commented 5 years ago

@YeldhamDev I don't have a touch device, the tablet I mention is a wacom tablet with pen and stuff...

I just tested and it also happens with mouse. And god...I'm having this non stop every time

YeldhamDev commented 5 years ago

@henriiquecampos I see. Could you describe the action that makes this occur more frequently? Because I'm completely unable to reproduce this...

henriiquecampos commented 5 years ago

Everything that pops a dialog, really. Lemme see if I reproduce it consistently. It mostly happen when these dialogues pop from a dropdown menu interaction, but it also happens otherwise.

Here are 3 examples, but honestly I could keep going, there are at least 6 more that I can reproduce consistently:

  1. Try to delete a folder on the Filesystem using the drop down menu 01
  2. Try to delete a node in the SceneTree using the drop down menu 02
  3. Try to save a branch scene that is an instance of another scene using the drop down menu 03
YeldhamDev commented 5 years ago

I'm sorry, but I can't reproduce any of those. Everything works correctly on my end.

nathanfranke commented 5 years ago

I've randomly encountered this also, albeit randomly Ubuntu 19.10 eoan x86_64 Linux 5.3.0-19-generic

Can someone check the source for this? Closing a popup sometimes shouldn't get rid of the dark overlay (If it is a submenu of another popup) So there may be a logic error

henriiquecampos commented 5 years ago

Something interesting that I noticed: if we delete using DELETE and pressing ENTER or RETURN to confirm the deletion, this bug doesn't happen

capnm commented 5 years ago

Personally, I think dimming is a terribly implemented 'anti-feature', and now it's not even possible to disable it :-/

(If you build Godot yourself, you can apply our patch that completely disables dimming)

diff --git a/editor/editor_node.cpp b/editor/editor_node.cpp
index ef382c0a1..1558c1cd5 100644
--- a/editor/editor_node.cpp
+++ b/editor/editor_node.cpp
@@ -5194,6 +5194,12 @@ void EditorNode::_open_imported() {
 }

 void EditorNode::dim_editor(bool p_dimming, bool p_force_dim) {
+
+       // disable dimming
+       dimmed = false;
+       gui_base->set_modulate(Color(1, 1, 1));
+       return;
+
        // Dimming can be forced regardless of the editor setting, which is useful when quitting the editor.
        if ((p_force_dim || EditorSettings::get_singleton()->get("interface/editor/dim_editor_on_dialog_popup")) && p_dimming) {
                dimmed = true;
diff --git a/editor/project_manager.cpp b/editor/project_manager.cpp
index ab62a59be..a19144dd8 100644
--- a/editor/project_manager.cpp
+++ b/editor/project_manager.cpp
@@ -1815,6 +1815,9 @@ void ProjectManager::_notification(int p_what) {

 void ProjectManager::_dim_window() {

+       // disable dimming
+       return;
+
        // This method must be called before calling `get_tree()->quit()`.
        // Otherwise, its effect won't be visible
Calinou commented 5 years ago

@capnm Doesn't disabling Interface > Editor > Dim Editor On Dialog Popup do the job? If not, that should be fixed.

I personally like editor dimming as it makes it easier for the user to notice a popup has opened. Back in the Godot 2.x days, I occasionally missed popups due to this.

capnm commented 5 years ago

@capnm Doesn't disabling Interface > Editor > Dim Editor On Dialog Popup do the job? If not, that should be fixed.

→ // Dimming can be forced regardless of the editor setting, which is useful when quitting the editor. Previously, it was possible to completely disable it.

I personally like editor dimming as it makes it easier for the user to notice a popup has opened …

Maybe the default window frame could be made darker?

Most of the people (including myself) I asked, do not like that. It looks like something went wrong and just scares my kids… It's not an essential feature, so I'd rather recommend removing it from the editor (instead of wasting time to fix it).

Calinou commented 5 years ago

→ // Dimming can be forced regardless of the editor setting, which is useful when quitting the editor. Previously, it was possible to completely disable it.

That's something entirely different :slightly_smiling_face:

It's a special case of dimming that's displayed only when quitting the editor (or project manager). I first added it in #29636 so that the editor doesn't feel "stuck" when you close it. (It can take a few seconds to close on a slow machine, but even on a fast machine, it's never really instant.)

Since the editor is no longer responsive when you start quitting it, I don't see a point in making this kind of dimming toggleable.

Maybe the default window frame could be made darker?

We can't make it too dark without going into "pure black" territory, which would look bad due to the contrast rate being very high. Also, if the background color is very dark, it means we can't make any elements darker (which is another reason most dark themes don't go below #222222 for their base color).

henriiquecampos commented 5 years ago

Dimming is essential for UX, pop ups lose all their weight without dimming the background.

If the setting is not disabling it, it is an issue itself, but removing the dimming isn't going to improve the editor

capnm commented 5 years ago

Since the editor is no longer responsive when you start quitting it, I don't see a point in making this kind of dimming toggleable.

The big flicker with overhead projector in a darkened room is I think more annoying, a few seconds of unresponsive editor does not really bother so much.

(I could be biased because I already had to deal with children with epileptic seizures, so I try to avoid such flashing effects as much as possible. This seems to be a godot speciality, no other software we use – WS Code, Krita, Gimp, Blender, Darktable, … does this.)

We can't make it too dark without going into "pure black" territory,…

Then maybe a brighter edge or something in the sense that the border becomes more visible? At the moment it is not easy to see where the dialogue lies:

Screenshot

(Advertising :1st_place_medal: I'd be really happy if anyone could spend his energy to fix #27669 That's something I have no idea how to fight against ;-)

henriiquecampos commented 5 years ago

A quick update on this: I tried to turn off the dimming in the Editor Settings and now the editor freezes on the examples I mentioned...seems like the problem is a bit deeper indeed

henriiquecampos commented 5 years ago

@akien-mga I just saw we have a beta2, this issue is definitely critical and, as I tested, it still happens in 3.2.beta2.

I think there can't be a stable release with that bug still around, I really mean it, it's impossible to work, and this is potentially a regression from 3.1.1, which this bug doesn't happen, at least not that frequently (it happen once in a while, but currently it is just impossible to work)

akien-mga commented 4 years ago

Might be fixed by #34770. Can anyone confirm in the current master branch / upcoming 3.2 beta 5?

YeldhamDev commented 4 years ago

@henriiquecampos Ping.

henriiquecampos commented 4 years ago

Ohh, sorry. Yeah I've been testing the last 4 days or so and didn't notice it happening, seems pretty much fixed at least for the cases I mentioned, thanks a lot :D