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

Add fullscreen shortcut #26

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Sophira
Copy link
Contributor

@Sophira Sophira commented Jun 10, 2023

NOTE: This is a draft pull request and not ready to be pulled in just yet, see below! I'm posting this in order to ask for advice on how this works.

I'm working on making it possible to have a key shortcut to enter fullscreen mode, as suggested at https://stardot.org.uk/forums/viewtopic.php?p=395344#p395344 . (I've chosen CTRL-PGDN for this but am happy to go for another if wanted.) The docs that I can find for wxWidgets suggest that this can be done by adding the key shortcut to the label of the menu item and that this should be all that's necessary to work.

In order to make this work, I fixed a small typo in the Makefile that was causing the wx-resources.cc file to not be regenerated (and removed the file from the distribution so that it's rebuilt on compile) [edit: This is now a separate pull request, #42], but even with this fixed there the key shortcut appears not to work; it's displayed properly, but pressing the key shortcut doesn't work, even when input isn't grabbed. I don't know enough about wxWidgets to know why this might be the case, unfortunately. I'll try to work more on it but I was wondering in the meantime if you (or anybody) know why this might be failing?

Thank you for any help anybody can give!

@Sophira
Copy link
Contributor Author

Sophira commented Jun 10, 2023

(note that the Makefile that's currently in the repository hasn't been updated as these files should be removed from the git repository - see pull request #8. I'm happy to update the Makefile if wanted but these files in the git repo are pretty outdated as I recall.)

@pdjstone
Copy link
Contributor

Input is handled through SDL events, which is probably why the wxWidgets shortcuts aren't working. Interestingly, in the Linux code in wx-sdl2.c, there is a shortcut to enter fullscreen using RWIN+ENTER (on my laptop it's Right Ctrl + Enter):

		/*Toggle fullscreen with RWIN-Enter (Alt-Enter, Cmd-Enter),
		  or enter by selecting Fullscreen from the menu.*/
		if (win_dofullscreen ||
			(key[KEY_RWIN] && key[KEY_ENTER] && !fullscreen)
		)
		{
			win_dofullscreen = 0;

			SDL_RaiseWindow(sdl_main_window);
			SDL_SetWindowFullscreen(sdl_main_window, SDL_WINDOW_FULLSCREEN_DESKTOP);
			sdl_enable_mouse_capture();
			fullscreen = 1;
		}

This code isn't in wx-win32.c:

		if (win_dofullscreen)
		{
			win_dofullscreen = 0;

			SetMenu(ghwnd, 0);
			SDL_RaiseWindow(sdl_main_window);
			SDL_SetWindowFullscreen(sdl_main_window, SDL_WINDOW_FULLSCREEN_DESKTOP);
			mouse_capture_enable();
			fullscreen = 1;

		}

but it should be easy to change that if to match the Linux version. I'm not set up to compile Arculator on Windows, so I'm not able to test this.

@Sophira
Copy link
Contributor Author

Sophira commented Jun 10, 2023

Oh, interesting! I use Linux myself and didn't know about this. (For me, it's RWin + Enter.) That said, the Linux fullscreen thing appears to continually toggle on and off each frame(?) depending on the state of the key currently, rather than only occurring on keydown. This feels like something worth fixing and adding to the Windows build, but I don't have the build environment in order to be able to make the Win32 build so I can't test it.

The said, I'll see what I can do to get the Linux part working first and then see what I can do for Windows.

@Sophira
Copy link
Contributor Author

Sophira commented Jun 10, 2023

Annoyingly, RWIN isn't a key that can be displayed in the accelerator part of the menu (see https://docs.wxwidgets.org/3.0/classwx_menu_item.html#a8b0517fb35e3eada66b51568aa87f261 ) but honestly I don't think you can use a Windows key as part of a shortcut in Windows anyway; I think those are reserved for the OS. That being the case, we should probably think of an appropriate key shortcut; I'd personally recommend the key shortcut I had at first, CTRL+PGDN. I'll keep going based on that.

@Sophira
Copy link
Contributor Author

Sophira commented Jun 10, 2023

Does anybody know of a keyboard tester program I could run on the RISC OS side? I'm retooling to use SDL events and I'd like to make sure that everything stays the same on the RISC OS side as it was before.

@sjnewbury
Copy link
Contributor

Does anybody know of a keyboard tester program I could run on the RISC OS side? I'm retooling to use SDL events and I'd like to make sure that everything stays the same on the RISC OS side as it was before.

You could knock something up with SDL since your brain is operating in SDL mode ;-) SDL does support RISC OS!

@IJeffray
Copy link
Contributor

Does anybody know of a keyboard tester program I could run on the RISC OS side?

REPEAT SYS 6,121 TO ,X : PRINT X : UNTIL .

@sjnewbury
Copy link
Contributor

Can I request the hotkey combination be user configurable. My HP Pavilion laptop is rather lacking on the key front front, missing RWIN, RCTRL, MENU and BREAK!?! That latter missing key is really annoying for Acorn emulation, I can tell you..!

A typo in the Makefile meant that it was impossible for the
wx-resources.cc file to be recreated during compile; this fixes that and
removes the file from the distribution.
@Sophira
Copy link
Contributor Author

Sophira commented Jan 22, 2024

As I am submitting the "Fix typo to allow wx-resources creation" part of this bug in a separate pull request (#42), I will force-push the code currently here to remove that commit.

[edit: On second thoughts, I rebased this pull request to be on top of the commit that the other pull request uses, since of course it's still necessary.]

@Sophira Sophira force-pushed the fullscreen-shortcut branch from 6e35a39 to 69892ff Compare January 22, 2024 05:14
CTRL+PGDN is a fairly unused key combination and it's fairly close to
the existing CTRL+END shortcut.
@Sophira Sophira force-pushed the fullscreen-shortcut branch from 69892ff to 497e326 Compare January 22, 2024 19:37
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.

4 participants