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

Switch behaviour of "Fillscreen mode" to a true borderless window #467

Merged
merged 1 commit into from Aug 14, 2018

Conversation

Projects
None yet
2 participants
@mickelson
Owner

mickelson commented Aug 11, 2018

  • possible alternative to: #462
@oomek

This comment has been minimized.

Collaborator

oomek commented Aug 11, 2018

Discussion continued from #462
Oh the irony. All that time I was thinking that this 1 pixel frame in Fill Screen mode was neccessary for the multi monitor mode to work, so I wasn't touching it. I'm absolutely for the changes you've made, but because we have now 2 modes with the same functionality I propose scrapping Fill Screen and Window (no border) and replacing them with Borderless Window as it's named in any other app. This may cause some issues though with reading configuration and prevent AM from launching, so it needs to be replaced there as well. Sorry for stating the obvoius if I did.

@oomek

This comment has been minimized.

Collaborator

oomek commented Aug 11, 2018

Btw, on Windows 10 there is no true fullscreen anymore, so Fullscreen mode is in fact a borderless window.
There is also one more thing I've just discovered, on nvidia cards there is an excessive cpu usage in fullscreen/borderless of 50% as the cpu waits in an acive loop for the swapbuffers (or any other gl blocking command that the driver decided to wait for vsync on) It can be squished down to 25% by disabling the Threaded optimizations in the driver, but in my opinion it's still too much. Im trying now to figure out how to fix it. Threaded Optimizations can be disabled with nvapi so it's doable, but comparing to windowed which takes 5-7% of cpu it's still too much. AMD cards wait for vblank in different way, so there is no issues on them. I was trying glFlush() but nvidia has decided to randomly ignore this command a while ago in one of their driver optimizations.

@oomek

This comment has been minimized.

Collaborator

oomek commented Aug 11, 2018

I would also like to mention here my pull request #461
that adds a manifest to the windows build which is neccessary if the user has set windows DPI to any other value than 100%. When it's set to 125% for example and the exe has no manifest with DpiAware = true Windows will prevent Fullscreen/Borderless optimizations, so both modes will run like an ordinary window.

@oomek

This comment has been minimized.

Collaborator

oomek commented Aug 12, 2018

Ok, this is weird. Please check the following issue #468

@mickelson mickelson merged commit f7b70ac into master Aug 14, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@mickelson mickelson deleted the window_change branch Aug 14, 2018

@oomek

This comment has been minimized.

Collaborator

oomek commented Aug 14, 2018

I remember what you said about Window (No Border) mode allowing the user to change the window position by modifying the window.am file, but it is only relevant to multimonitor being set to on isn't it?
If so, your commit still does not solve the situation when the user has set in the prevoius build the window mode to Fill Screen and then Window (No Border) and then updated to the latest version. In this situation the screen will still be displayed with the -1,-1 offset and +2, +2 size. There should be only one window mode Borderless Window in my opinion and if the multimonitor is set to on AM should load the window position from window.am file, but if multimonitor is set to off it should act as the current Fill Screen mode. Tell me if my thinking is correct or I'm still wrong :)

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