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

Fix Fancy Zones editor location calculation #520

Closed

Conversation

brenca
Copy link

@brenca brenca commented Oct 17, 2019

Summary of the Pull Request

I noticed that while the new Fancy Zones worked on my primary monitor, the editor window was halfway off the screen on the secondary one (it's to the left, Windows refers to it with negative coordinates). I've found the error in the calculations, fixed it and here we are I guess.

References

This is related to Fancy Zones' multi-monitor support.

PR Checklist

  • I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Detailed Description of the Pull Request / Additional comments

The current code uses the unscaled top/left coordinates of the monitor, and then adds the difference between the work area and the monitor rect left/top coordinates (this is to compensate for the taskbar, since it can be on any edge).
We should instead scale the monitor top/left coordinates as well, and then add the difference between the two areas. I added the std::abs calls to mark that we need to add the absolute difference there, but the calculations should always yield positive values anyway if I'm correct.

Validation Steps Performed

I opened the build with the fix included, and the positioning of the editor window is now correct, and the zones work correctly.
To validate the fix you could:

  • Open the Fancy Zones tab
  • Click Edit Zones
  • See that the default zone covers the entire work area of the window (in older versions, it was offset to the left, and wouldn't be able to reach the right or bottom side).

@enricogior
Copy link
Contributor

Hi @brenca,
sorry for the long delay. It's great to get this kind of contributions!
Hopefully today I'll have the time to test it.
Thanks!

@enricogior
Copy link
Contributor

Hi @brenca,
I'm trying to reproduce the bug using a second monitor, it's a 4K monitor scaled at 150%. I moved the monitor to the left of the primary monitor but I can't see anything off when I open the FZ Editor.
Can you please describe your monitors configuration in details so I may be able to reproduce the same scenario?
A screenshot showing what is off would also help. Thank you.

@brenca
Copy link
Author

brenca commented Oct 22, 2019

Hey @enricogior!
Thanks for the check/review! I'll attach some screenshots below.
image
image
The screenshot is from master - I'm seeing the editor window offset to the left and top. Both of my monitors are 1440p with 1.25 scaling factor, but the left one is rotated as you can see from the screenshot. After this fix, everything shows up in the correct location. Since this problem might come from calculations with negative coordinates, you could try positioning the left monitor to have at least a part above the primary monitor's top left corner, but that's mostly a guess at how to repro this.

@brenca
Copy link
Author

brenca commented Oct 22, 2019

(If you are wondering the whole bunch of modifications that are uncommitted are just project settings to compile on VS 2017, I was too lazy to upgrade to 2019 :) I don't see how that might have caused the issue tho.)

@enricogior
Copy link
Contributor

Thanks for the screenshots. I put my monitor on the left in portrait mode and with 125% scaling to replicate your configuration but I still don't get anything off.
In your screenshot I see the editor for the Custom layouts, in that case it's resized but not re-centered, that is by design. You should check if it's centered when showing the default templates.

display

screen

@brenca
Copy link
Author

brenca commented Oct 22, 2019

It's not recentered even with the default templates, and the issue goes a bit deeper than that - I would have settled with an uncentered editor and probably just posted an issue if that was the case. Sadly, the editor being offset also means that I can't adjust the zones to their proper places (it won't let me move the zones into the non-editor space you see on my screenshot).

@brenca
Copy link
Author

brenca commented Oct 22, 2019

Also, it's not just that the custom layouts are not re-centered, the whole editor window that should cover the monitor is offset to the left and to the top and it also extends over the bounds of the monitor. The size is OK, the position is not. Furthermore, if I edit the zones with the editor from master, the zones get the proper coordinates (so they won't show up where the editor shows them, but in their proper places).

@enricogior
Copy link
Contributor

There was another bug that was effecting the zones placement and resizing, that has been fixed.
Can you give it a try to the current master and see if that makes a difference in your case?
I would also suggest to update to VS 2019, it's much better! And that will remove one variable from the equation.
I'm not saying your fix doesn't have merit, I just want to make sure we have the full picture and we can proceed with certainty.

@brenca
Copy link
Author

brenca commented Oct 22, 2019

I tried master, but it didn't fix this for me. I'll see if I can upgrade to VS2019, I have to keep a certain version because of my work.
Can you test if the my version breaks the layout on your computer?

@brenca
Copy link
Author

brenca commented Oct 22, 2019

Alright, the problem is hiding somewhere else. I'll take a stab at fixing the underlaying issue. You can repro what I'm seeing by having your primary monitor scaled to 1.25 as well (or anything else should work also that's not 1.0).

@enricogior
Copy link
Contributor

Got it!
Scaling to 125% the first monitor as well shows the bug, it's not effecting the editor window position, but it's effecting the zones position as you previously described.
I'm going to try your fix , thanks!

@enricogior
Copy link
Contributor

Btw, you can install VS 2019 side by side with VS 2017 ;)

@enricogior
Copy link
Contributor

OK, I can confirm that the fix works if both monitors are at 125%, but if the primary monitor is at 100% the zones are now shifted to the right.

@brenca brenca force-pushed the brenca/fix-multi-monitor-sizing branch from 8fc7ae2 to 610a599 Compare October 24, 2019 21:04
@brenca
Copy link
Author

brenca commented Oct 24, 2019

@enricogior I've updated the branch with another way to address this - could you please check if this breaks anything? It does seem to work properly here, but I'd love another set of eyes to test this since I was pretty sure the last time as well :D

@enricogior
Copy link
Contributor

Great! I'll give it a try later and let you know. Thanks!

@enricogior
Copy link
Contributor

Unfortunately it only worked when both monitors have 125% scaling, but if the second monitor (the one on the left) is at 100% scaling while the primary monitor is at 125% scaling, the editor is centered on the second monitor but the zones are shifted to the right and span to the primary monitor.

@brenca
Copy link
Author

brenca commented Oct 24, 2019

Ah, heck, I forgot to test that scenario, I'll take a look at what's going on. (Sorry, I can only work at this during the evenings, and I get a bit tired)

@brenca
Copy link
Author

brenca commented Oct 24, 2019

I'll also borrow my sister's monitor to test some theories (namely if this is related to the primary monitor's scaling and the monitor you are on, or if three monitors would screw things up even more)

@nickalbrecht
Copy link

nickalbrecht commented Oct 30, 2019

I also noticed that the calculated space for the zone(s) to use are off when using multiple monitors because it's using the primary monitors's amount of work space after accounting for the start menu. But the start menu may not be in the same location on all monitors. If it is, it may not be the same size on all monitors. So this should be calculated per monitor.

This is after testing with the 0.12 as well as it's initial behavior with 0.11

@enricogior
Copy link
Contributor

@brenca
just to let you know that we have looked into this issue and it turned out that is not a simple error in the application when calculating the coordinates but a more in depth issue with some APIs that we are using that are not DPI aware. We are reviewing the current code to migrate it to use newer APIs that are fully DPI aware.

@enricogior
Copy link
Contributor

@brenca I'm closing this PR since it's superseded by #657
Thanks for you contribution.

@enricogior enricogior closed this Nov 7, 2019
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.

None yet

3 participants