Details

Reviewers

Campbell Barton (campbellbarton)
Sergey Sharybin (sergey)
Brecht Van Lommel (brecht)

Summary

It seems like blender always opens windows like preferences in the top left corner of the screen. And then I immediately have to move it manually to the center of the screen before I start using it due to OCD.

New features:

by default, windows open in the center of the main display.
window positions and state (maximized/minimized) are saved when they are closed.
when opening a window, previous position and state is used.
if there is no previous position or state information, defaults are used.
windows are constrained to opening within a physical display. otherwise, defaults are used. (command line positions currently override this)
for "temp" windows, one of each type can be open at once (render, preferences) - not sure if this will break anything

Overview of code changes:

added GHOST_GetDisplayDimensions() to support multiple displays.
updated getWindowBounds() and createWindow() methods so that when the dimensions returned by getWindowBounds() are used in subsequent createWindow() calls, it creates a window in the same size and position.
removed x, y parameters from WM_window_open_temp(). the user preferred position (the last position a window was closed in) is now used, or the window is positioned centrally in its parent.
added 'type' to wmWindow and a WM_WINDOW_DEFAULT type, previously these types were only used for "temp" windows.

Diff Detail

Repository: rB Blender

Event Timeline

Andrew Williams (sobakasu) created this revision.Feb 1 2019, 1:51 AM

Andrew Williams (sobakasu) marked an inline comment as done.Feb 1 2019, 3:03 AM

Andrew Williams (sobakasu) added inline comments.

source/blender/windowmanager/intern/wm_window.c
933	it seems like this is an invalid assumption - I just realised maximised windows have a smaller title bar in windows. bit of a chicken and egg problem with this code, as you need to have created the window before you can get the title bar size from ghost window.

now saves window positions to disk on exit and restores on start up

Andrew Williams (sobakasu) updated this revision to Diff 13519.Feb 6 2019, 6:27 AM

Andrew Williams (sobakasu) edited the summary of this revision. (Show Details)

fixed comment style and put printfs in if(G.debug) {} blocks

Andrew Williams (sobakasu) edited the summary of this revision. (Show Details)Feb 6 2019, 7:01 AM

Andrew Williams (sobakasu) added a reviewer: Sergey Sharybin (sergey).Feb 6 2019, 7:04 AM

fixed compiler warnings

now working on macOS (tested on mojave)

fixes for X11 and SDL

Andrew Williams (sobakasu) mentioned this in D4323: Add a command line argument to allow starting Blender maximized.Feb 10 2019, 5:14 AM

How is this supposed to work on multi-display Linux? I'm doing blender --factory-startup and the window is opened in a way which makes it to stick to the right edge of my left monitor (the window is vertically aligned on center though).

Didn't test other platforms yet.

In D4291#98131, @Sergey Sharybin (sergey) wrote:

How is this supposed to work on multi-display Linux? I'm doing blender --factory-startup and the window is opened in a way which makes it to stick to the right edge of my left monitor (the window is vertically aligned on center though).

Didn't test other platforms yet.

I've only tested it on ubuntu running on vmware on windows. I will try to set up my computer to run linux natively and look into this

Sebastian Parborg (zeddb) mentioned this in D4332: Default to opening window maximized.Feb 11 2019, 11:16 AM

I'm fine with better default window positions.

However I have doubts about the way window position and size are saved. Currently window layout is saved in the startup file and .blend files. There you can put Windows in specific places with specific contents, and Blender will restore it. If we store the window size and position separately this will get out of sync, it's not obvious that the positions and size for one specific window setup should apply to another.

I would rather keep this state in .blend files. I guess a problem is that we do not save position and size for temporarily windows like render or preferences. However there is a more general problem here. Users also want the preferences to remember the scrolling state or the file browser to remember certain states. So maybe that can be solved somehow without windows.txt?

It would be good to split this into two patches. One for the defaults and another for saving the state.

intern/ghost/intern/GHOST_BoundsTracker.cpp
22 ↗	(On Diff #13574)	What is a "bounds tracker"? Can you document this? Or can this just be part of `GHOST_Window` instead of a separate class?

This revision now requires changes to proceed.Feb 11 2019, 11:45 AM

On Mac we get this already for free - it's built into the system.

I'm not sure about saving inside the blend file - this really depends on the display of the system used, how many displays you have and the size really dictate how the windows should be restored.

So to me it makes sense to store this locally, not in the blend.

@Brecht Van Lommel (brecht) the idea here is to have blender remember the state of the windows on close, not on save.
So I think the easiest way would be to store this info in a separate file (as it is currently in this patch).

Otherwise we would have to save changes to .blend files when the users close the application.

I think the problem here is that many do not know that the .blend files stores this information.
As can be seen by the requests to have blender start maximized. If it was intuitive, people would just realize that they could save the startup file in a maximized state and be done with it.

Andrew Williams (sobakasu) mentioned this in D4339: Window manager: window size/position persistence.Feb 12 2019, 12:04 AM

Andrew Williams (sobakasu) added a child revision: D4339: Window manager: window size/position persistence.

removed the code that saved window position to disk, that is now in D4339
fixed a crash with show drivers editor
area dupli windows now have type WM_TYPE_CUSTOM which is not saved

In D4291#98165, @William Reynish (billreynish) wrote:

On Mac we get this already for free - it's built into the system.

I just tried master on my mac and that doesn't seem to be the case for blender - neither the main window or preferences window remembered where I put them.

I'm not sure about saving inside the blend file - this really depends on the display of the system used, how many displays you have and the size really dictate how the windows should be restored.

I agree, that's why I separated it from the blend file. If I open someone else's blend file, I don't want the preferences window to open where they last closed it, I want it to appear where I put it and the size that I had it last.

In D4291#98131, @Sergey Sharybin (sergey) wrote:

How is this supposed to work on multi-display Linux? I'm doing blender --factory-startup and the window is opened in a way which makes it to stick to the right edge of my left monitor (the window is vertically aligned on center though).

I can't reproduce this problem yet. For me it starts maximised on the main display with --factory-startup. I'm using ubuntu 18.04.2 with gnome 3.28.2. What are you using?

This seems like a heavy solution for a problem that's not that bad, on OSX apparently this isn't needed?
Window managers on Linux can save window locations too (don't know about win32?)

It's adding a lot of code for something we have mostly been able to ignore.
Further, since Blender isn't an application that makes heavy use of many floating windows, it's not such a benefit - given the trade-off of code maintenance/complexity.

OTOH having access to multiple monitors display size seems generally useful and could be split into it's own patch.

Think we could attempt something less involved then this patch proposes, eg:

Store the monitor used for opening a temporary window, so toggling the a window uses that monitor again.

intern/ghost/GHOST_C-api.h
162–164 ↗	(On Diff #13611)	return arguments should be last (after `num`)
intern/ghost/GHOST_Types.h
27–28 ↗	(On Diff #13611)	Should use `GHOST_` prefix for public defines.
intern/ghost/intern/GHOST_SystemSDL.cpp
142 ↗	(On Diff #13611)	Could assert if this fails, caller shouldn't be asking for displays that don't exist.
source/blender/windowmanager/wm_window.h
34–39 ↗	(On Diff #13611)	Prefer existing names, new new name suggests this initializes a window, or is related to `wm_window_match_init`.

Campbell Barton (campbellbarton) requested changes to this revision.Feb 13 2019, 4:52 AM

This revision now requires changes to proceed.Feb 13 2019, 4:52 AM

revert function name change, wm_window_init() -> wm_ghost_init()/_exit()
fix ghost constants, add GHOST_ prefix
update ghost getDisplayDimensions() argument order (return arguments last)
update ghost CMakeLists, Ghost_BoundsTracker only needed for X11 and SDL

Andrew Williams (sobakasu) marked 4 inline comments as done.Feb 13 2019, 10:30 AM

Andrew Williams (sobakasu) added inline comments.

intern/ghost/intern/GHOST_BoundsTracker.cpp
22 ↗	(On Diff #13574)	the bounds tracker code is only used for X11 and SDL. It could be put into GHOST_Window. I have added some more comments to that code.

Campbell Barton (campbellbarton) added inline comments.Feb 15 2019, 9:08 AM

source/blender/makesdna/DNA_windowmanager_types.h
225–226	Think we could try remove this, the data is cache for ghost - if its need we can have an API call to access it. It means the window size is stored in 3 places (4 if you count wmWindowPositions) so ensuring they're valid gets a bit out of hand. We will end up having to update them from ghost before using it, so it seems like it would be better if it's moved out of the window.
source/blender/windowmanager/WM_api.h
136 ↗	(On Diff #13645)	could call `DEFAULT` ? - The window menu has the option `New Window` and `New Main Window` - which doesn't look like it's related to this use of the term Main.

Removed ghost_rect from wmWindow struct
Renamed WM_WINDOW_MAIN to WM_WINDOW_DEFAULT
Added BLI_rcti_inset to simplify some code in wm_window.c

Note:
after removing ghost_rect from wmWindow, I changed some code that was calling WM_check(C) to open ghost windows to call wm_window_ghostwindow_add() directly instead, so that I could specify the size of the window. It still seems to work but it's possible that this could cause problems.

Andrew Williams (sobakasu) marked 2 inline comments as done.Feb 17 2019, 1:32 AM

Andrew Williams (sobakasu) edited the summary of this revision. (Show Details)Feb 17 2019, 9:08 AM

Andrew Williams (sobakasu) marked an inline comment as done.Feb 18 2019, 11:35 AM

Window manager: position windows centrally and restore previous window position and sizes
Needs ReviewPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 13459

source/blender/makesdna/DNA_windowmanager_types.h

source/blender/windowmanager/intern/wm_window.c

Window manager: position windows centrally and restore previous window position and sizesNeeds ReviewPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 13459

source/blender/makesdna/DNA_windowmanager_types.h

source/blender/windowmanager/intern/wm_window.c

Window manager: position windows centrally and restore previous window position and sizes
Needs ReviewPublic
Actions

Revision Contents
Changeset List