goneovim icon indicating copy to clipboard operation
goneovim copied to clipboard

After maximize window does not restore to normal size

Open damanis opened this issue 2 years ago • 14 comments

How to reproduce

  • Open goneovim window, for example 100 columns x 50 lines
  • Maximize window horizontally, vertically or full
  • Unmaximize window Expected behavior: window restore to previous size, 100 cols x 50 lines Actual behavior: window size is same as in maximized state.

damanis avatar Jun 18 '22 18:06 damanis

@damanis Hi, Thanks for the issue report.

What specific means are you using for each Window operation in the reproduction procedure you provided? I understand that some window operations use the interface provided by Goneovim (e.g. GonvimMaximize), while others are platform-specific (e.g. Win key + directional key in Windows, Window Manager in Linux).

akiyosi avatar Jun 19 '22 06:06 akiyosi

@akiyosi I configure shortcuts in window manager (WindowMaker). But same effect with wmctrl: wmctrl -i -r <win_id> -b "add,maximized_horz"

damanis avatar Jun 19 '22 07:06 damanis

After GonvimMaximize a window restores properly. In both cases, changing window state by GonvimMaxmize and by window manager, xprop returns same window property _NET_WM_STATE(ATOM) = _NET_WM_STATE_MAXIMIZED_HORZ, _NET_WM_STATE_MAXIMIZED_VERT But in GonvimMaxmize Qt WindowState() returns 2, while after change by window manager WindowState() returns 0.

damanis avatar Sep 10 '22 11:09 damanis

The function func (e *Editor) resizeMainWindow() is called before window state is changed, so e.window.WindowState() == core.Qt__WindowMaximized is false and geometry updated. Seems, QResizeEvent does not provide enough data and need to use ConnectChangeEvent() to handle maximize/unmaximize by window manager.

damanis avatar Sep 10 '22 14:09 damanis

@damanis As you point out, ConnectResizeEvent does not have sufficient information to address this issue. Also, since the WindowStateChange event is also fired asynchronously, it is possible that the event notification may not have been received yet at the time resizeMainWindow() is executed.

I' ll try to create a test binary that handles the state based on WindowStateChange as a trial.

By the way, am I correct in my understanding that you have WindowGeometryBasedOnFontmetrics enabled?

akiyosi avatar Sep 10 '22 15:09 akiyosi

@akiyosi In my tests ConnectResizeEvent is coming before WindowStateChange event. WindowGeometryBasedOnFontmetrics is enabled to define window size by set lines and columns.

damanis avatar Sep 10 '22 16:09 damanis

@damanis I have tested on my machine and as you have confirmed, WindowStateChange seems to be emitted after ResizeEvent. This means that it is difficult to simply determine if a given ResizeEvent is accompanied by a WindowStateChange event when it is emitted. One solution, for example, it is possible to ignore ResizeEvent emitted after a GonvimMaximized command is executed until a WindowStateChange event is emitted. However, this solution will not work for events emitted outside of the Goneovim application, such as those controlled by wmctrl or similar.

Do you have any ideas?

akiyosi avatar Sep 12 '22 13:09 akiyosi

@akiyosi Now I use GonvimMaximized to maximize and wmctrl to unmaximize. I will try some options, but usually I can debug on week-end (Fri-Sat) only.

damanis avatar Sep 12 '22 13:09 damanis

@akiyosi

Do you have any ideas?

I have one idea, but it may be platform depended. The resize callback instead of check window.state() can check main window flags _NET_WM_STATE_* set by window manager (e.g., via wmctrl). These flags from FreeDesktop standard, so should be some API. So, in Linux it should work. I think, something like this should be in Windows and MAC, but I have no details. It's possible, Qt provides an API, so this will work on all platforms. I think, need to protect only maximise state. If I don't mistake, full screen is not part of standard.

damanis avatar Sep 13 '22 17:09 damanis

@akiyosi I tried add timer after WindowGeometryBasedOnFontmetrics checking and put rest code to goroutine. It work isn't good: sometimes work, sometimes doesn't and sometimes throw error "QObject::setParent: Cannot set parent, new parent is in a different thread". If Qt has API to send event, it may help: in resizeMainWindow() add timer that will send event 'resize', then process the event. Between set timer and timer event the main window will be maximized and resizeByTimer() will work properly.

damanis avatar Sep 17 '22 11:09 damanis

@akiyosi QT does have closed issue about inconsistent window state in resizeEvent: QTBUG-30085. The main reason of undefined window state: 'it depends very much on the underlying windowing system'. The simple workaround is save old size in resizeEvent, then use it if maximized event appears. Another way - handle changeEvent.

damanis avatar Dec 03 '22 11:12 damanis

@damanis Thanks for this info. I may not be understanding the workaround correctly, but I think the way to save the old window size when emitting a resizeevent does not solve the problem, as the animation during window maximization causes a number of resizeevents to be emitted.

I tried add timer after WindowGeometryBasedOnFontmetrics checking and put rest code to goroutine. It work isn't good: sometimes work, sometimes doesn't and sometimes throw error "QObject::setParent: Cannot set parent, new parent is in a different thread". If Qt has API to send event, it may help: in resizeMainWindow() add timer that will send event 'resize', then process the event. Between set timer and timer event the main window will be maximized and resizeByTimer() will work properly.

Could you provide me with the above fixes, if possible? This way seems to me to be the most practical.

akiyosi avatar Dec 04 '22 13:12 akiyosi

@damanis I was inspired with your idea and I think we are getting closer to a solution to this problem. I have pushed one branch to improve this problem.

https://github.com/akiyosi/goneovim/pull/426

akiyosi avatar Dec 05 '22 13:12 akiyosi

@akiyosi I think, this solution should work:

  • add struct to store current and previous window size struct { current_size; previous_size; maximized: bool; }

  • in resize handler copy current_size to previous_size (in addition to current code)

  • in maximize handler set maximized to true

  • in next resize or unmaximize restore window size to previous_size if maximized is true. Set maximized to false.

damanis avatar Jul 07 '23 20:07 damanis