New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Layout does not get preserved between multiple launches of git-cola #848

Closed
jakubklos77 opened this Issue Jun 28, 2018 · 7 comments

Comments

Projects
None yet
2 participants
@jakubklos77

jakubklos77 commented Jun 28, 2018

I have tried almost anything but each time I run git-cola the layout configuration gets resized a few pixels.
It does not really matter if I use the Lock Layout option in the View menu.

The vertical splitter jumps a few pixels to the right and so do the horizontal splitters (more to the top)
The vertical splitter keeps doing this until it is in the center of the window.

Testing the current git master. What can I do to help to fix this issue?
Thank you

@jakubklos77

This comment has been minimized.

Show comment
Hide comment
@jakubklos77

jakubklos77 Jul 11, 2018

I wanted to debug this and while testing I have figured out the issue occurs only when the window is maximazed. If it's not maximized/fullscreen then it works properly. Although between restarts of "git-cola" the window slightly moves to the left a few pixels until it gets aligned with x=0. Somebody should look at this.
Thank you

jakubklos77 commented Jul 11, 2018

I wanted to debug this and while testing I have figured out the issue occurs only when the window is maximazed. If it's not maximized/fullscreen then it works properly. Although between restarts of "git-cola" the window slightly moves to the left a few pixels until it gets aligned with x=0. Somebody should look at this.
Thank you

@davvid

This comment has been minimized.

Show comment
Hide comment
@davvid

davvid Jul 12, 2018

Member

Thanks for the heads-up. What OS are you using? If Linux, what window manager? I'll see if I can reproduce it.

To help debug it, the code that applies the saved window settings are the apply_state() functions in cola/widgets/main.py and the base MainWindowMixin and WidgetMixin classes in cola/widgets/standard.py.

We're using Qt's built-in showMaximized() and restoreState() functions to store and apply the dock widget sizes, so it's possible we're running into a bug in Qt or the style engine that's being used (it's happened in the past on a few occasions).

One tweak worth trying is running the base class WidgetMixin.apply_state() after the stuff done in the MainWindowMixin's apply_state() (rather than at the very beginning, as is currently done).

It's possible that could work around it, since we'd be maximizing after restoring the state, outside of Qt's control. We'll see... it's hard to know without knowing more details.

Member

davvid commented Jul 12, 2018

Thanks for the heads-up. What OS are you using? If Linux, what window manager? I'll see if I can reproduce it.

To help debug it, the code that applies the saved window settings are the apply_state() functions in cola/widgets/main.py and the base MainWindowMixin and WidgetMixin classes in cola/widgets/standard.py.

We're using Qt's built-in showMaximized() and restoreState() functions to store and apply the dock widget sizes, so it's possible we're running into a bug in Qt or the style engine that's being used (it's happened in the past on a few occasions).

One tweak worth trying is running the base class WidgetMixin.apply_state() after the stuff done in the MainWindowMixin's apply_state() (rather than at the very beginning, as is currently done).

It's possible that could work around it, since we'd be maximizing after restoring the state, outside of Qt's control. We'll see... it's hard to know without knowing more details.

@jakubklos77

This comment has been minimized.

Show comment
Hide comment
@jakubklos77

jakubklos77 Jul 12, 2018

Thank you David.
Yes, running on Linux. I use the gala window manager (Elementary OS) but have just tested Ubuntu 18.04 (gnome window manager I believe) and the behavior is the same.

I have also tried the tweak as you mentioned above and it also does not seem to help.

I started git-cola with the default settings and moved the vertical splitter to the left (it was in the center by default). Then restarted and it does not stay in position.

I haven't tested it on Windows as I abandonded that OS some time ago :)

Thank you

jakubklos77 commented Jul 12, 2018

Thank you David.
Yes, running on Linux. I use the gala window manager (Elementary OS) but have just tested Ubuntu 18.04 (gnome window manager I believe) and the behavior is the same.

I have also tried the tweak as you mentioned above and it also does not seem to help.

I started git-cola with the default settings and moved the vertical splitter to the left (it was in the center by default). Then restarted and it does not stay in position.

I haven't tested it on Windows as I abandonded that OS some time ago :)

Thank you

@davvid

This comment has been minimized.

Show comment
Hide comment
@davvid

davvid Jul 13, 2018

Member

Me too ;-) thanks for the verification.

I'm able to reproduce the issue now. I see why I wasn't able to before -- my layout was always within the margins where the widget sizes wouldn't trigger it. I think I have a solution that should solve this nicely. Thanks for the heads-up.

Today I learned that I shouldn't try to implement my own window maximized and sizing logic -- just let Qt's saveGeometry / restoreGeometry handle it. We have custom logic for restoring window sizes, and that seems to be the core issue.

Member

davvid commented Jul 13, 2018

Me too ;-) thanks for the verification.

I'm able to reproduce the issue now. I see why I wasn't able to before -- my layout was always within the margins where the widget sizes wouldn't trigger it. I think I have a solution that should solve this nicely. Thanks for the heads-up.

Today I learned that I shouldn't try to implement my own window maximized and sizing logic -- just let Qt's saveGeometry / restoreGeometry handle it. We have custom logic for restoring window sizes, and that seems to be the core issue.

@jakubklos77

This comment has been minimized.

Show comment
Hide comment
@jakubklos77

jakubklos77 Jul 13, 2018

Glad you got it reproduced. Good job.
Love git-cola a lot :). I used to do all the git work in bash but git-cola makes my work so much faster.
Thank you

jakubklos77 commented Jul 13, 2018

Glad you got it reproduced. Good job.
Love git-cola a lot :). I used to do all the git work in bash but git-cola makes my work so much faster.
Thank you

davvid added a commit to davvid/git-cola that referenced this issue Jul 13, 2018

qtutils: add hide_dock() helper
Related-to: #848
Signed-off-by: David Aguilar <davvid@gmail.com>

davvid added a commit to davvid/git-cola that referenced this issue Jul 13, 2018

main: use a better default layout
Improve and simplify the default layout to the status, commit, branch,
and diff widgets.  Use the new hide_dock() function to make sure this is
handled consistenly.

Related-to: #848
Signed-off-by: David Aguilar <davvid@gmail.com>

davvid added a commit to davvid/git-cola that referenced this issue Jul 13, 2018

widgets: honor old geometry settings when new settings are not present
Allow upgrading existing users's window sizes by checking for the old
settings keys when the 'geometry' key does not exist.

Related-to: #848
Signed-off-by: David Aguilar <davvid@gmail.com>

@davvid davvid closed this in 8fa926b Jul 13, 2018

@davvid

This comment has been minimized.

Show comment
Hide comment
@davvid

davvid Jul 13, 2018

Member

This should be much better behaved now. One issue remains, but it's a tiny one.

On the very first launch, when no settings are present, the middle splitter can't be resized. Once we restart the app, this issue goes away, and everything works as expected. That is a Qt5-only bug. I tested Qt4 and it does not exhibit that behavior:

env QT_API=pyqt4 ./bin/git-cola

This lines up with some bugs that have been submitted against Qt, and explains why we didn't see this in the past:

https://bugreports.qt.io/browse/QTBUG-65592

https://stackoverflow.com/questions/48119969/qdockwidget-splitter-jumps-when-qmainwindow-resized

In any case, this should be a non-issue in practice now. Qt4 should be fine, and Qt5 will be fine once you start up and shutdown the app once with the new code.

Member

davvid commented Jul 13, 2018

This should be much better behaved now. One issue remains, but it's a tiny one.

On the very first launch, when no settings are present, the middle splitter can't be resized. Once we restart the app, this issue goes away, and everything works as expected. That is a Qt5-only bug. I tested Qt4 and it does not exhibit that behavior:

env QT_API=pyqt4 ./bin/git-cola

This lines up with some bugs that have been submitted against Qt, and explains why we didn't see this in the past:

https://bugreports.qt.io/browse/QTBUG-65592

https://stackoverflow.com/questions/48119969/qdockwidget-splitter-jumps-when-qmainwindow-resized

In any case, this should be a non-issue in practice now. Qt4 should be fine, and Qt5 will be fine once you start up and shutdown the app once with the new code.

@jakubklos77

This comment has been minimized.

Show comment
Hide comment
@jakubklos77

jakubklos77 Jul 13, 2018

I confirm the fix. It is working now. Thank you

jakubklos77 commented Jul 13, 2018

I confirm the fix. It is working now. Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment