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

Added configurable window modes #322

Merged
merged 1 commit into from
Jun 11, 2021

Conversation

James-Allenby
Copy link
Collaborator

@James-Allenby James-Allenby commented Mar 5, 2021

References issue #291

This commit adds a new TOML variable which changes the window mode to use on start-up.

The TOML configuration file uses strings to indicate which mode should be used

Feedback welcome of course

@James-Allenby James-Allenby changed the title Added configurable window modes [DO NOT MERGE] Added configurable window modes Mar 5, 2021
@IAmNotHanni IAmNotHanni self-assigned this Mar 5, 2021
@IAmNotHanni IAmNotHanni linked an issue Mar 5, 2021 that may be closed by this pull request
include/inexor/vulkan-renderer/wrapper/window.hpp Outdated Show resolved Hide resolved
configuration/renderer.toml Outdated Show resolved Hide resolved
IAmNotHanni
IAmNotHanni previously approved these changes Mar 5, 2021
Copy link
Member

@IAmNotHanni IAmNotHanni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work. Welcome aboard!

@James-Allenby James-Allenby changed the title [DO NOT MERGE] Added configurable window modes Added configurable window modes Mar 6, 2021
src/vulkan-renderer/application.cpp Outdated Show resolved Hide resolved
src/vulkan-renderer/wrapper/window.cpp Outdated Show resolved Hide resolved
Copy link
Member

@yeetari yeetari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work :)

@James-Allenby
Copy link
Collaborator Author

James-Allenby commented Mar 11, 2021

One more thing before I'm happy to merge this in.. Should the default window mode be windowed instead of fullscreen in the configuration toml?

@IceflowRE
Copy link
Member

One more thing before I'm happy to merge this in.. Should the default window mode be windowed instead of fullscreen in the configuration toml?

for the final game fullscreen for sure, like which game starts in window? :D
Oh also pls change your commit naming ^^ like [config] Add default window mode or so

IceflowRE
IceflowRE previously approved these changes Mar 12, 2021
@IceflowRE
Copy link
Member

Who ever is merging, squash it ^^

@IceflowRE IceflowRE added the org:ready to merge pull request or PR from an issue is ready to merge label Mar 26, 2021
@IAmNotHanni
Copy link
Member

The item "Ability to change window mode during runtime" requires some deeper work in the rendergraph since we don't need to recompile it entirely when invalidating the swapchain (see ticket #291 and #288). I will move this to a ticket and merge this pull request.

@IAmNotHanni
Copy link
Member

IAmNotHanni commented Mar 29, 2021

I reset all reviews to make sure we don't forget about the squash before we merge :)
Edit: Oh, the current settings allow it to be merged without new review anyways. Whatever..

@IceflowRE IceflowRE dismissed stale reviews from IAmNotHanni and themself March 29, 2021 17:35

Squash required.

@James-Allenby
Copy link
Collaborator Author

I can squash it! I was thinking of naming the commit [settings] add configurable window modes. I just need one more approval on the PR

@IceflowRE
Copy link
Member

I can squash it! I was thinking of naming the commit [settings] add configurable window modes. I just need one more approval on the PR

[settings] Add configurable window modes

@IceflowRE
Copy link
Member

Oof, that not what i meant to do with update branch.... :D anyway can you rebase and squash?

@James-Allenby James-Allenby force-pushed the james/window-mode branch 2 times, most recently from 4b9ab3a to 317a7e7 Compare June 11, 2021 13:08
@James-Allenby James-Allenby merged commit b10f8d7 into inexorgame:master Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat:rendering rendering feat:settings settings org:ready to merge pull request or PR from an issue is ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement fullscreen toggle
4 participants