Fixing offscreen GUI issue #11208

pull meshcollider wants to merge 2 commits into bitcoin:master from meshcollider:201709_offscreen_fix changing 1 files +3 −3
  1. meshcollider commented at 8:18 PM on August 31, 2017: contributor

    Fixes #11207

    This still obviously needs to be confirmed as the cause of the issues in question, this is the current assumption

    Edit: c.f. https://github.com/bitcoin/bitcoin/pull/11335

  2. Fixing offscreen GUI issue 776503029d
  3. MarcoFalke added the label GUI on Aug 31, 2017
  4. MarcoFalke added this to the milestone 0.15.0 on Aug 31, 2017
  5. MarcoFalke commented at 8:28 PM on August 31, 2017: member

    Tagged for backport in case there will be another rc

  6. MarcoFalke added the label Needs backport on Aug 31, 2017
  7. jtimon commented at 9:27 PM on August 31, 2017: contributor

    utACK 7765030

  8. in src/qt/guiutil.cpp:862 in 776503029d outdated 
     861 | -    if ((!pos.x() && !pos.y()) || (QApplication::desktop()->screenNumber(parent) == -1))
 862 | -    {
 863 | -        QRect screen = QApplication::desktop()->screenGeometry();
 864 | +    
 865 | +    QRect screen = QApplication::desktop()->screenGeometry();
 866 | +    if ((!pos.x() && !pos.y()) || pos.x() > screen.width() || pos.y() > screen.height() || (QApplication::desktop()->screenNumber(parent) == -1)) {
    promag commented at 9:52 PM on August 31, 2017:

    How about

    if (!screen.contains(parent->geometry()) || QApplication::desktop()->screenNumber(parent) == -1) {

    And I think the first condition is enough, not sure though.

    meshcollider commented at 10:08 AM on September 1, 2017:

    parent->geometry() does not include the window frame, so perhaps frameGeometry()? But yeah I guess a check for full containment makes sense rather than just the point pos()

  9. in src/qt/guiutil.cpp:861 in 776503029d outdated 
     856 | @@ -857,10 +857,9 @@ void restoreWindowGeometry(const QString& strSetting, const QSize& defaultSize,
 857 |  
 858 |      parent->resize(size);
 859 |      parent->move(pos);
 860 | -
 861 | -    if ((!pos.x() && !pos.y()) || (QApplication::desktop()->screenNumber(parent) == -1))
 862 | -    {
 863 | -        QRect screen = QApplication::desktop()->screenGeometry();
 864 | +    
 865 | +    QRect screen = QApplication::desktop()->screenGeometry();
    promag commented at 10:02 PM on August 31, 2017:

    Use availableGeometry().

    meshcollider commented at 10:07 AM on September 1, 2017:

    Yep sounds good. I'll fix it to availableGeometry(screen_number) too now that I think about it. Will push it as a separate commit for review before squashing.

  10. promag commented at 10:07 PM on August 31, 2017: member

    Concept ACK.

  11. achow101 commented at 11:17 PM on August 31, 2017: member

    ACK 776503029d5f9e6976d65aa6d8ee1958f8afef04

    Tested on Windows 10, confirms that this fixes the issue I identified in #11207. However #11171 sounds like it is a different problem.

  12. AllanDoensen commented at 11:00 AM on September 1, 2017: contributor

    I believe this was fixed here : #10156

  13. in src/qt/guiutil.cpp:860 in bf1dcb199d outdated 
     856 | @@ -857,10 +857,10 @@ void restoreWindowGeometry(const QString& strSetting, const QSize& defaultSize,
 857 |  
 858 |      parent->resize(size);
 859 |      parent->move(pos);
 860 | -
 861 | -    if ((!pos.x() && !pos.y()) || (QApplication::desktop()->screenNumber(parent) == -1))
 862 | -    {
 863 | -        QRect screen = QApplication::desktop()->screenGeometry();
 864 | +
    promag commented at 11:10 AM on September 1, 2017:

    Remove whitespace.

  14. in src/qt/guiutil.cpp:863 in bf1dcb199d outdated 
     862 | -    {
 863 | -        QRect screen = QApplication::desktop()->screenGeometry();
 864 | +    
 865 | +    int screen_number = QApplication::desktop()->screenNumber(parent);
 866 | +    QRect screen = QApplication::desktop()->availableGeometry(screen_number);
 867 | +    if ((!pos.x() && !pos.y()) || screen_number == -1 || !screen.contains(parent->frameGeometry())) {
    promag commented at 11:18 AM on September 1, 2017:

    If I understand correctly, !pos.x() && !pos.y() should be replaced with !settings.contains(strSettings + "Pos").

    meshcollider commented at 11:28 AM on September 1, 2017:

    Safer to check after converting it to a point, just to make sure conversion worked properly?

    promag commented at 11:30 AM on September 1, 2017:

    But what if the window position is QPoint(0, 0)?

  15. in src/qt/guiutil.cpp:862 in bf1dcb199d outdated 
     861 | -    if ((!pos.x() && !pos.y()) || (QApplication::desktop()->screenNumber(parent) == -1))
 862 | -    {
 863 | -        QRect screen = QApplication::desktop()->screenGeometry();
 864 | +    
 865 | +    int screen_number = QApplication::desktop()->screenNumber(parent);
 866 | +    QRect screen = QApplication::desktop()->availableGeometry(screen_number);
    promag commented at 11:21 AM on September 1, 2017:

    What if the result of availableGeometry() when screen_number = -1?

    meshcollider commented at 11:26 AM on September 1, 2017:

    Do you mean -1? If the parameter is -1, it returns the default screen

    promag commented at 11:28 AM on September 1, 2017:

    Fixed above comment.

  16. Check screen contains window when restoring geometry 6067244698
  17. luke-jr approved
  18. luke-jr commented at 7:20 AM on September 2, 2017: member

    utACK

  19. luke-jr referenced this in commit 80b0f37ac5 on Sep 2, 2017
  20. luke-jr referenced this in commit 347a7efd29 on Sep 2, 2017
  21. in src/qt/guiutil.cpp:863 in 6067244698 
     857 | @@ -858,9 +858,9 @@ void restoreWindowGeometry(const QString& strSetting, const QSize& defaultSize,
 858 |      parent->resize(size);
 859 |      parent->move(pos);
 860 |  
 861 | -    if ((!pos.x() && !pos.y()) || (QApplication::desktop()->screenNumber(parent) == -1))
 862 | -    {
 863 | -        QRect screen = QApplication::desktop()->screenGeometry();
 864 | +    int screen_number = QApplication::desktop()->screenNumber(parent);
 865 | +    QRect screen = QApplication::desktop()->availableGeometry(screen_number);
 866 | +    if ((!pos.x() && !pos.y()) || screen_number == -1 || !screen.contains(parent->frameGeometry())) {
    jonasschnelli commented at 5:27 PM on September 3, 2017:

    Does this allow a window that is halfway moved out of the screen (I think we should allow this)?

    meshcollider commented at 7:23 PM on September 3, 2017:

    No I don't think it does, but I thought about that and the expected behaviour of most software is that it launches within the screen even if you moved it partially out last time?

  22. sipa commented at 9:08 PM on September 3, 2017: member

    utACK

  23. laanwj commented at 3:56 PM on September 5, 2017: member

    I believe this was fixed here : #10156

    I was about to remark this - there have been a few PRs before in the last few years 'fixing' this issue, but it keeps returning (or wasn't fixed properly after all). So normally we would make sure of this by unit tests but I don't think that's an option here due to the GUI-ness. So I suggest creating a manual testplan before/after, so we can verify that this bug is squashed.

  24. MarcoFalke added this to the milestone 0.15.1 on Sep 5, 2017
  25. MarcoFalke removed this from the milestone 0.15.0 on Sep 5, 2017
  26. jonasschnelli commented at 9:19 PM on September 5, 2017: contributor

    Just tested on macOS and windows 8.1 (VM): Current master OSX: if I move the window outside of the boundary, it will relocate the window to the maximum boundaries without overlapping the visible area. With this PR OX: overlapping the boundaries will always lead to reopen it in the screen center. (I think this is acceptable, but slightly different to the current state).

    Could not reproduce the issue on Windows 8.1 (did run on a very large screen, closes Bitcoin-Qt while having the main window on the right bottom, reduce screen by times 4, re-started Qt and had the window visible)

  27. laanwj commented at 5:32 PM on September 6, 2017: member

    So it's unclear whether this is actually needed. Removing the milestone.

  28. laanwj removed the label Needs backport on Sep 6, 2017
  29. laanwj removed this from the milestone 0.15.1 on Sep 6, 2017
  30. laanwj commented at 8:44 AM on September 15, 2017: member

    Closing in favor of #11335

  31. laanwj closed this on Sep 15, 2017
  32. laanwj referenced this in commit f2e0ff687d on Sep 15, 2017
  33. laanwj referenced this in commit 8cf88b4aae on Sep 25, 2017
  34. meshcollider deleted the branch on Oct 13, 2017
  35. codablock referenced this in commit b6e7a4b612 on Sep 25, 2019
  36. charlesrocket referenced this in commit 5dc1920f75 on Dec 15, 2019
  37. barrystyle referenced this in commit fbc3c58b1d on Jan 22, 2020
  38. andrewtookay referenced this in commit 0cd763d9d4 on Jul 24, 2021
  39. DrahtBot locked this on Sep 8, 2021
Contributors
meshcolliderMarcoFalkejtimonpromagachow101AllanDoensenluke-jrjonasschnellisipalaanwj
Labels
GUI
Linked (view graph)
#11207 [Windows] GUI window can still appear off screen.#11335 Replace save|restoreWindowGeometry with Qt functions
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-13 15:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me