Page MenuHome
Create Task
Maniphest T76418

After render you want to save image, but saving window it is hidden below render windows when Blender.app in fullscreen mode
Closed, ResolvedPublic
Actions

Description

System Information
Operating system: Mac OS Catalina 10.15.4 (latest)
Graphics card: Intel HD 6000

Blender Version
Broken: 2.82a, 2.83 beta

Short description of error
It is ONLY testable on Mac OS when Blender.app in fullscreen mode!!! (if not in fullscreen -- everything works ok)
After you render image, and if your image covers your screen -- you get result in "Render window". And when you click "Save" or "Save as" - this "Save as" window appears BEHIND "render window". So you can't really see it before you move away "render window"

Exact steps for others to reproduce the error

  1. Open Blender, GO IN FULLSCREEN MODE for app
  2. Default startup file is fine
  3. Click "render image"
  4. See result in "render window"
  5. Click "save as"
  6. "Save as" doesn't appears before you move "render window"

Video: https://youtu.be/Ywd0C2qj-_s
Image:

Revisions and Commits

rB Blender
rB796412dca0a5 UI: Allow registering File Browser as child of other child windows

Related Objects

Event Timeline

Volodymyr (linosia) created this task.May 4 2020, 9:01 PM
Volodymyr (linosia) added a comment.May 4 2020, 9:03 PM

Also tested with latest 2.83 beta as of 4-th of May

Volodymyr (linosia) added projects: Platform: macOS, User Interface.May 4 2020, 9:04 PM
Volodymyr (linosia) updated the task description. (Show Details)
Nelson Fritsch (polarlighthouse) added a subscriber: Nelson Fritsch (polarlighthouse).EditedMay 4 2020, 11:41 PM

I am 99% sure this can't be addressed due to the macOS policy: fullscreen apps only allow one active window.

The only way this would work would be a native safe file dialogue (not a window).

You might find a workaround editing your config to not use secondary windows when on render and file browsers.

Yevgeny Makarov (jenkm) added subscribers: Julian Eisel (Severin), Brecht Van Lommel (brecht), Yevgeny Makarov (jenkm).May 5 2020, 9:17 AM

Another manifestation but the reason is the same as in T74101:

  • File Browser is parented to main Blender window.

For quick test, I just deleted the addChildWindow code in GHOST_WindowCocoa.mm, and this fixed both issues.

I don't know if parenting is necessary at all, but at least you can make an exception for fullscreen mode.

@Julian Eisel (Severin) @Brecht Van Lommel (brecht)

Julian Eisel (Severin) added a subscriber: William Reynish (billreynish).May 6 2020, 11:53 PM

The issue is that macOS handles additional windows weirdly in fullscreen mode: They are placed in front of child windows of the fullscreen window.

I see two ways to address this. One is to allow the filebrowser to be attached to temporary windows (preferences, render view, driver editor). So far we only attach it to main windows. This solution would affect all platforms, but might actually be a sensible thing to do, it simplifies a few things. Also addresses T72693.

diff --git a/source/blender/windowmanager/intern/wm.c b/source/blender/windowmanager/intern/wm.c
index 54e6735175d..29b02e5e2ea 100644
--- a/source/blender/windowmanager/intern/wm.c
+++ b/source/blender/windowmanager/intern/wm.c
@@ -359,7 +359,7 @@ void wm_add_default(Main *bmain, bContext *C)
   WorkSpaceLayout *layout = BKE_workspace_layout_find_global(bmain, screen, &workspace);
 
   CTX_wm_manager_set(C, wm);
-  win = wm_window_new(bmain, wm, NULL);
+  win = wm_window_new(bmain, wm, NULL, false);
   win->scene = CTX_data_scene(C);
   STRNCPY(win->view_layer_name, CTX_data_view_layer(C)->name);
   BKE_workspace_active_set(win->workspace_hook, workspace);
diff --git a/source/blender/windowmanager/intern/wm_event_system.c b/source/blender/windowmanager/intern/wm_event_system.c
index 560581a56e2..a60298b55de 100644
--- a/source/blender/windowmanager/intern/wm_event_system.c
+++ b/source/blender/windowmanager/intern/wm_event_system.c
@@ -3416,19 +3416,13 @@ void WM_event_add_fileselect(bContext *C, wmOperator *op)
   wmWindowManager *wm = CTX_wm_manager(C);
   wmWindow *win = CTX_wm_window(C);
   const bool is_temp_screen = WM_window_is_temp_screen(win);
-  const bool opens_window = (U.filebrowser_display_type == USER_TEMP_SPACE_DISPLAY_WINDOW);
-  /* Don't add the file handler to the temporary window if one is opened, or else it owns the
-   * handlers for itself, causing dangling pointers once it's destructed through a handler. It has
-   * a parent which should hold the handlers itself. */
-  ListBase *modalhandlers = (is_temp_screen && opens_window) ? &win->parent->modalhandlers :
-                                                               &win->modalhandlers;
 
   /* Close any popups, like when opening a file browser from the splash. */
-  UI_popup_handlers_remove_all(C, modalhandlers);
+  UI_popup_handlers_remove_all(C, &win->modalhandlers);
 
   if (!is_temp_screen) {
     /* only allow 1 file selector open per window */
-    LISTBASE_FOREACH_MUTABLE (wmEventHandler *, handler_base, modalhandlers) {
+    LISTBASE_FOREACH_MUTABLE (wmEventHandler *, handler_base, &win->modalhandlers) {
       if (handler_base->type == WM_HANDLER_TYPE_OP) {
         wmEventHandler_Op *handler = (wmEventHandler_Op *)handler_base;
         if (handler->is_fileselect == false) {
@@ -3470,7 +3464,7 @@ void WM_event_add_fileselect(bContext *C, wmOperator *op)
   handler->context.area = CTX_wm_area(C);
   handler->context.region = CTX_wm_region(C);
 
-  BLI_addhead(modalhandlers, handler);
+  BLI_addhead(&win->modalhandlers, handler);
 
   /* check props once before invoking if check is available
    * ensures initial properties are valid */
diff --git a/source/blender/windowmanager/intern/wm_window.c b/source/blender/windowmanager/intern/wm_window.c
index b1eee7509b7..da45e7f533c 100644
--- a/source/blender/windowmanager/intern/wm_window.c
+++ b/source/blender/windowmanager/intern/wm_window.c
@@ -286,14 +286,15 @@ static int find_free_winid(wmWindowManager *wm)
 }
 
 /* don't change context itself */
-wmWindow *wm_window_new(const Main *bmain, wmWindowManager *wm, wmWindow *parent)
+wmWindow *wm_window_new(const Main *bmain, wmWindowManager *wm, wmWindow *parent, bool dialog)
 {
   wmWindow *win = MEM_callocN(sizeof(wmWindow), "window");
 
   BLI_addtail(&wm->windows, win);
   win->winid = find_free_winid(wm);
 
-  win->parent = (parent && parent->parent) ? parent->parent : parent;
+  /* Dialogs may have a child window as parent. Otherwise, a child must not be a parent too. */
+  win->parent = (!dialog && parent && parent->parent) ? parent->parent : parent;
   win->stereo3d_format = MEM_callocN(sizeof(Stereo3dFormat), "Stereo 3D Format (window)");
   win->workspace_hook = BKE_workspace_instance_hook_create(bmain);
 
@@ -307,8 +308,9 @@ wmWindow *wm_window_copy(Main *bmain,
                          const bool duplicate_layout,
                          const bool child)
 {
+  const bool is_dialog = GHOST_IsDialogWindow(win_src->ghostwin);
   wmWindow *win_parent = (child) ? win_src : win_src->parent;
-  wmWindow *win_dst = wm_window_new(bmain, wm, win_parent);
+  wmWindow *win_dst = wm_window_new(bmain, wm, win_parent, is_dialog);
   WorkSpace *workspace = WM_window_get_active_workspace(win_src);
   WorkSpaceLayout *layout_old = WM_window_get_active_layout(win_src);
   WorkSpaceLayout *layout_new;
@@ -417,7 +419,6 @@ void wm_quit_with_optional_confirmation_prompt(bContext *C, wmWindow *win)
 void wm_window_close(bContext *C, wmWindowManager *wm, wmWindow *win)
 {
   wmWindow *win_other;
-  const bool is_dialog = (G.background == false) ? GHOST_IsDialogWindow(win->ghostwin) : false;
 
   /* First check if there is another main window remaining. */
   for (win_other = wm->windows.first; win_other; win_other = win_other->next) {
@@ -431,20 +432,11 @@ void wm_window_close(bContext *C, wmWindowManager *wm, wmWindow *win)
     return;
   }
 
-  /* Close child windows and bring windows back to front that dialogs have pushed behind the main
-   * window. */
-  LISTBASE_FOREACH (wmWindow *, iter_win, &wm->windows) {
+  /* Close child windows */
+  LISTBASE_FOREACH_MUTABLE (wmWindow *, iter_win, &wm->windows) {
     if (iter_win->parent == win) {
       wm_window_close(C, wm, iter_win);
     }
-    else {
-      if (G.background == false) {
-        if (is_dialog && iter_win != win && iter_win->parent &&
-            (GHOST_GetWindowState(iter_win->ghostwin) != GHOST_kWindowStateMinimized)) {
-          wm_window_raise(iter_win);
-        }
-      }
-    }
   }
 
   bScreen *screen = WM_window_get_active_screen(win);
@@ -834,7 +826,7 @@ wmWindow *WM_window_open(bContext *C, const rcti *rect)
 {
   wmWindowManager *wm = CTX_wm_manager(C);
   wmWindow *win_prev = CTX_wm_window(C);
-  wmWindow *win = wm_window_new(CTX_data_main(C), wm, win_prev);
+  wmWindow *win = wm_window_new(CTX_data_main(C), wm, win_prev, false);
 
   win->posx = rect->xmin;
   win->posy = rect->ymin;
@@ -905,7 +897,7 @@ wmWindow *WM_window_open_temp(bContext *C,
 
   /* add new window? */
   if (win == NULL) {
-    win = wm_window_new(bmain, wm, win_prev);
+    win = wm_window_new(bmain, wm, win_prev, dialog);
 
     win->posx = rect.xmin;
     win->posy = rect.ymin;
diff --git a/source/blender/windowmanager/wm_window.h b/source/blender/windowmanager/wm_window.h
index 45cfe1431d7..5ca5711b4f2 100644
--- a/source/blender/windowmanager/wm_window.h
+++ b/source/blender/windowmanager/wm_window.h
@@ -33,7 +33,10 @@ void wm_ghost_exit(void);
 void wm_get_screensize(int *r_width, int *r_height);
 void wm_get_desktopsize(int *r_width, int *r_height);
 
-wmWindow *wm_window_new(const struct Main *bmain, wmWindowManager *wm, wmWindow *parent);
+wmWindow *wm_window_new(const struct Main *bmain,
+                        wmWindowManager *wm,
+                        wmWindow *parent,
+                        bool dialog);
 wmWindow *wm_window_copy(struct Main *bmain,
                          wmWindowManager *wm,
                          wmWindow *win_src,

Alternatively we could do a macOS-only solution of not setting the child-window relationship for fullscreen-windows, as @Yevgeny Makarov (jenkm) suggested. Note however that this would break when changing the fullscreen-state while the filebrowser is open. So a bit of extra code during fullscreen-state changes would be needed to prevent that (very doable still).

@Brecht Van Lommel (brecht) & @William Reynish (billreynish) what do you think?

Note that we should really work on a better design to handle different kinds of windows. See T69819: Secondary window improvements and D7497: WIP: Window Creation Refactor.

Ankit Meel (ankitm) changed the task status from Needs Triage to Confirmed.May 8 2020, 12:01 PM
Brecht Van Lommel (brecht) added a comment.May 8 2020, 3:46 PM

I'm fine with attaching the file browser to temporary windows, that's how I think it should work.

Brecht Van Lommel (brecht) added a comment.May 8 2020, 3:47 PM

Changing this behavior, and doing D7497 alongside it may be better left for 2.90. Since it's hard to ensure this will not break anything, especially if it affects Windows and Linux too.

Harley Acheson (harley) added a subscriber: Harley Acheson (harley).EditedMay 8 2020, 6:52 PM

Personally I worry about D7497 unless we do that in concert with changing some dialogs, like QuitSave to using real OS windows. Otherwise that would be the next complaint, that the "QuitSave" was hidden behind some other window.

Edit: I must have been asleep when I wrote the above, because Damn, that sounds dumb in the context of this thread. I should have just written "yes, I agree with Brecht". LOL

Julian Eisel (Severin) closed this task as Resolved by committing rB796412dca0a5: UI: Allow registering File Browser as child of other child windows.May 20 2020, 5:10 PM
Julian Eisel (Severin) claimed this task.
Julian Eisel (Severin) added a commit: rB796412dca0a5: UI: Allow registering File Browser as child of other child windows.