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

Console windows won't swap #711

Closed
rastkov opened this issue Mar 8, 2021 · 4 comments
Closed

Console windows won't swap #711

rastkov opened this issue Mar 8, 2021 · 4 comments
Labels
bug Confirmed bug in MaxTo.
Milestone

Comments

@rastkov
Copy link

rastkov commented Mar 8, 2021

Describe the bug
Console windows like CMD, PowerShell and WSL won't swap with other console windows.

To Reproduce

  1. Open multiple windows (notepad, CMD, powershell, windows explorer)
  2. Try to swap between windows (cmd with powershell, cmd with notepad, etc)
  3. Open two CMS windows (or two powershell windows) and try swapping them

Expected behavior

Console windows should be able to swap with other open console windows.

System information:

  • Windows version: Windows version: Windows 10 Version 1909 (OS Build 18363.1256)
  • MaxTo version: 2.2.0-beta2

Additional context
I'm actually not sure if this is a bug or not. I wasn't able to find any related information or Issues for Console Windows and the MaxTo Compability for Console windows that is always enabled.

Looking into the build-in Recipe for Swapping windows, it uses the Move window ingredient with the "Swap" option enabled. I don't know if it somehow uses SetWindowPos in the background that the Compatibility is mentioning and that would be ignored in this case because of the compatibility.

@vegardlarsen vegardlarsen added the bug Confirmed bug in MaxTo. label Mar 11, 2021
@vegardlarsen
Copy link
Member

Console windows are "weird" in a way, as they do not retain the size they are resized to. Therefore, MaxTo will consider them to be outside of a region (because their sizes doesn't match exactly with the region), and won't consider them as "swappable". This should be possible to fix, and I'm adding this to the backlog.

@vegardlarsen
Copy link
Member

Just as a note, this also happens if you attempt to swap Notepad and CMD, when Notepad is the focused (i.e. driving) window.

@vegardlarsen
Copy link
Member

Digging into this, this is turning into quite a more interesting case than I thought it would be. The problem here is that MaxTo compares the position of windows to the position of the region it is looking to swap windows with. Command prompt windows never match exactly with that size, so they aren't considered candidates for swapping.

To solve this, we need to tag windows that have been maximized to regions with some sort of identifier so we know which region they are in. This has been something I have wanted to do for a long time; as it would solve a range of bugs (but may also introduce a few new ones). The one limitation of that identifier is that it has to be a single int value.

I was able to eliminate this error by using the GetHashCode method of System.Drawing.Rectangle to get a int hash to mark windows that are maximized to a region (when it is maximized), and comparing that to the int hash of the region we are looking for. This seems to work well; except for one "little" issue: it can fail at literally any time.

The problem is that the information in a rectangle contains 128 bits, while an int is only 32 bits. This means that you can actually end up in the position where two windows have completely different positions, but the same hash (known as a hash collision).

However, there is some evidence suggesting a Windows desktop does not support any resolutions above 32Kx32K (probably being constrained to a signed 16-bit value somewhere), but I have not been able to confirm that information. If it is true, however, it allows us to shrink the size of a rectangle down to 4x16 bits = 64 bits, which gets us much closer to something that fits into a single int.

So it looks to me like that is a dead end.

Let's look at a more pragmatic approach to doing this. Let's say we continue to use the GetHashCode method to retrieve a value. That leaves us with a list of candidates. If we then compare the coordinates of its top left corner with the top left corner of the reference region, that should also match if we are looking at the correct region.

So our code moves from the original:

var windowsInDestination = this._windowManager.DesktopWindows
    .Where(w => w.Position == chosenRectangle)
    .ToList();

to

var windowsInDestination = this._windowManager.DesktopWindows
    .Where(w => w.GetProperty(WindowProperties.MaximizedToRegion) == chosenRectangle.GetHashCode())
    .Where(w => w.Position.Location == chosenRectangle.Location)
    .ToList();

This will at least reduce drastically the chance of a hash collision, if not completely eliminate it.

@vegardlarsen
Copy link
Member

After a little bit of refactoring, this logic was moved to an extension method that better handles some edge cases (with comments this time):

public static bool IsInRegion(this ISystemWindow window, Rectangle region)
{
    // This has been extracted into an extension method as ConHost windows (e.g. command
    // prompt, PowerShell, etc) resizes after being placed in a region. This only affects
    // their size, not their position (i.e. the top left coordinates remain the same).
    // So instead of a direct comparison on their position, we first check if the hash
    // of the region they have been placed in matches, if not, we fall back to comparing
    // exact positions.

    var rectangleMatches = region == window.Position;

    var windowRegionHash = window.GetProperty(WindowProperties.MaximizedToRegion);
    if (windowRegionHash == 0)
    {
        // GetProp may lead to few false positives, as the hash can theoretically be 0,
        // and the return value from GetProperty is 0 when nothing is found, so in those cases
        // we fall back to a direct comparison of their positions.
        return rectangleMatches;
    }

    // If the stored hash matches, and the top left coordinates match, the window is 
    // considered to be in the region. In the case of console windows, their size may
    // change, but the top left will always match if it hasn't been moved.
    var hashAndLocationMatches = region.GetHashCode() == windowRegionHash
        && window.Position.Location == region.Location;

    return hashAndLocationMatches || rectangleMatches;
}

@vegardlarsen vegardlarsen added this to the 2.2.0 milestone Apr 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug in MaxTo.
Projects
None yet
Development

No branches or pull requests

2 participants