Skip to content
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

Save and restore window position and maximized state #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

M-a-r-k
Copy link
Owner

@M-a-r-k M-a-r-k commented Nov 14, 2022

This fixes a couple of annoyances:

  • Ghidra did not restore the maximized window state
  • Window size and position could be moved if Ghidra was exited with window close to edge of screen

This removes a couple of calls to WindowUtilities.ensureOnScreen(). Ideally that function would be adjusted somehow instead.

This fixes a couple of annoyances:
 - Ghidra did not restore the maximized window state
 - Window size and position could be moved if Ghidra was exited with window close to edge of screen
@justanotheranonymoususer

Why not submit the PR to the original repo at https://github.com/NationalSecurityAgency/ghidra?

@M-a-r-k
Copy link
Owner Author

M-a-r-k commented Nov 14, 2022

Why not submit the PR to the original repo at https://github.com/NationalSecurityAgency/ghidra?

I'm a github noob, plus I doubt they would merge it as-is due to my commenting out calls to ensureOnScreen().

@justanotheranonymoususer

I'm a github noob

So what? If the fix is not completely correct, somebody who knows the code better will tell you how to improve it. Worst case, the PR will be closed.

due to my commenting out calls to ensureOnScreen()

Just remove this code? :)

@M-a-r-k
Copy link
Owner Author

M-a-r-k commented Nov 19, 2022

The main code change could probably be simplified to unconditionally call frame.setExtendedState(). I originally thought that maybe there might be some GUI flickering from calling it when not necessary, but I doubt that's actually true. Also, handle the case where state is iconified:

JFrame frame = windowWrapper.getParentFrame();
int state = frame.getExtendedState();		// Current state (may be maximized or iconified)

// We want to save un-maximized/un-iconified dimensions
// frame.setExtendedState(state & ~JFrame.MAXIMIZED_BOTH & ~JFrame.ICONIFIED);
frame.setExtendedState(JFrame.NORMAL);	// There are only three state bits & we want to clear them so this should be OK
Rectangle r = frame.getBounds();
frame.setExtendedState(state);		// Revert to original state 

root.setAttribute("X_POS", "" + r.x);
root.setAttribute("Y_POS", "" + r.y);
root.setAttribute("WIDTH", "" + r.width);
root.setAttribute("HEIGHT", "" + r.height);
root.setAttribute("EX_STATE", "" + state);	// No need to call frame.getExtendedState() again

@dragonmacher
Copy link

dragonmacher commented Nov 21, 2022

The code at line 490 may fix the issue. I'd like to know if you undo all changes, except for reading the extended state and then setting it after the call to frame.setBound(), whether that fixes your original issue. It seems to work for me locally.

I'm assuming that no matter the bounds you set, restoring the extended state will then maximize the window, which is the effect you are trying to achieve, I believe.

@M-a-r-k
Copy link
Owner Author

M-a-r-k commented Nov 21, 2022

I'm not set up to build Ghidra here right now. But anyway...

You mean restore the calls to WindowUtilities.ensureOnScreen()? And remove frame.setExtendedState(JFrame.NORMAL) before frame.getBounds(), remove frame.setExtendedState(state) after?

Restoring extended state should maximize the window. But I think when a window is maximized, its bounds correspond to a full-screen window. If you save the bounds of a maximized window, the un-maximized bounds are probably lost. That's what happens with Ghidra currently. If you exit Ghidra with a window maximized, next launch the window appears un-maximized but occupies most of the screen regardless of its previous un-maximized size/position.

Suppose you have a window occupying the left third of the screen, then maximize it. Later you exit Ghidra. On launching Ghidra the next time, you would want:

  • the window to open maximized
  • restoring the window (i.e. un-maximizing, click button near top right to the left of close button) causes it to occupy the left third of the screen like it did before

@dragonmacher
Copy link

I suppose there are many issues that you are trying to fix. My suggestion earlier was really just add the call to frame.setExtendedState(state) to restoreFromXml(), as you had done, but to omit the other changes.

When I tested this suggestion, it achieved the effect of having Ghidra open with the Code Browser tool fully maximized, which it was not doing before the change.

It seems like you would also like the bounds to be saved as well, in such a way that 'unmaximizing' the window will restore the previous 'unmaximized' bounds. It makes sense that this behavior is not working under certain systems, due to the functionality inside of WindowUtilities.ensureOnScreen()

In thinking about this, it seems like the most direct solution would be to make the first simple change. Then, we could provide an option to disable the ensureOnScreen() behavior. This would allow users in your environment to turn off that feature, relying strictly on the saved bounds and state. If users ever found themselves with lost windows, then they could turn this option back on long enough to fix the issue.

@M-a-r-k
Copy link
Owner Author

M-a-r-k commented Nov 23, 2022

Perhaps an alternative behavior for ensureOnScreen() would help? Instead of trying to make every window entirely on-screen, just ensure it is not completely off-screen. Then it wouldn't mess with window positions in the usual case, but the user could still move a misplaced window if needed. (On Windows, Alt-Space then select Move from the window menu.)

So maybe replace

if (visibleScreenBounds.contains(bounds)) {
    return; // the given shape is completely on the screen 
}

with

if (visibleScreenBounds.intersects(bounds)) {
    return; // the given shape is not completely off-screen 
}

Also, call ensureOnScreen() with the non-maximized bounds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants