SDL2 -- sdl1 is dead - Githubissues

maxwell-kalin commented 3 years ago

SDL 1.2 is no longer supported and no longer works on many modern systems, please update your codebase!

ColinPitrat commented 3 years ago

I already looked at it in the past and there are a few complicated things.

There's a migration guide here: https://wiki.libsdl.org/MigrationGuide

Creating a sdl2 branch to work on it incrementally.

ColinPitrat commented 3 years ago

Some considerations:

HW video plugins won't be different from corresponding SW ones anymore
BPP seems to be broken for any value different from 32, unclear why
Not clear whether width & height (only used for FS) are still useful with SDL_WINDOW_FULLSCREEN_DESKTOP
We may want to get rid of OpenGL video scaling
We can easily make the window resizable
Keyboard mappings need to be fixed & verified (this will probably require a distributed test plan ...)

ColinPitrat commented 3 years ago

A small status update:

Some cleanup has indeed been done in video plugins: hw and sw are now the same. For compatibility with old configs, we have to keep both entries though (pointing to the same code). Would be nice to version the configuration and support updating it?
BPP works fine (well, sort of), it just required some additional logic to ensure the surface and the renderer had the same pixel format
width & height are now only used with OpenGL video scaling, which is still useful as it's the only plugin that supports scaline, remanency, etc ...
The mouse handling needs to be fixed in fullscreen mode (scaling is not working). Once fixed, this should allow to easily make the window resizable. Not sure if we want that though. But we could support higher scaling (x3, x4) easily.
Keyboard mappings will need careful testing. We may want to rely on scancode rather than keycode?
SDL_WINDOWEVENT_TAKE_FOCUS values have been added in SDL 2.0.5 but are not supported on AppVeyor. We need to add conditions in the code for SDL version.
Copy/paste now works in memory editor. We may want a different button than the current "dump to stdout"
Some cleanup needs to be done

Once mouse handling, SDL version specifics and cleanup are done, this can be merged, although I'd really like some keyboard testing to be done before.

ColinPitrat commented 3 years ago

I tested as much as I could. I'll now ask watchers of the git repo if they can help with the testing.

I prepared a test plan document: https://docs.google.com/document/d/1fFm98ip9TLvsiOsE4WjHMwRYU54N5G5xGGylQyXVZuQ/edit#

And a sheet for the results: https://docs.google.com/spreadsheets/d/1xjtXU4BfGO81EnDbc0tHcNQ5xkHejkOa2wXc-1zshbg/edit#gid=0

@trappedinspacetime @scautura @Ahmedcity @arm-in @Ygarr @bodhi-baum @zackizacki

Would you have some time to test the SDL2 version of Caprice32? Instructions are in the doc. Don't feel forced to test everything, even partial or unstructured testing is useful as long as you tell me what you tested. I'm particularly interested by the keyboard layout & vkeyboard testing either with non-french layouts or under windows (because I only tested french under linux).

ColinPitrat commented 3 years ago

Note: I plan to merge this next week (beginning of April). Would be nice if some testing can occur before that (but it's fine to have feedback & bugs opened after too).

maxwell-kalin commented 3 years ago

i will do some testing thank you for working so hard

ColinPitrat commented 3 years ago

Adding mentions to a few followers who may also be able to help testing.

Past contributors: @sebhz @FloLore @cpcbegin @cpcitor @nudgegoonies @bignaux @pvaret @zackizacki @MartianGirl

From France (for french keyboard under Windows): @Ryusennin @YouMakeTech @gaillarddamien @fphenix @farvardin

From Germany (for german keyboard): @raldus @sielenk @mdm @bfxdev @bodhi-baum

From UK (for UK keyboard): @renaudguerin @Fistynuts

From Spain (for spanish keyboard): @javierlomas @8bitDave @franselles

Would you be able to help testing the SDL2 version of Caprice32? NOTE: This is not yet on the master branch, it's a separate sdl2 branch, see the doc in my previous comment (https://github.com/ColinPitrat/caprice32/issues/185#issuecomment-807251992) for details.

Do not feel forced to contribute nor to strictly follow the test plan (any testing with feedback is helpful, especially around keyboard layouts).

farvardin commented 3 years ago

@ColinPitrat I'm not using windows, but I've tested the sdl2 port, compiling from the source.

There is a problem with the key layout (azerty), the letters are ok ("azerty"), but the numbers are not:

without shifting, I get 123456.... with shouldn't be the case

cpitrat: pressed: 1(49: SDLK_1)
cpitrat: pressed: 2(50: SDLK_2)
cpitrat: pressed: 3(51: SDLK_3)
cpitrat: pressed: 4(52: SDLK_4)
cpitrat: pressed: 5(53: SDLK_5)
cpitrat: pressed: 6(54: SDLK_6)
cpitrat: pressed: 7(55: SDLK_7)

I've changed /temp/github/caprice32/cap32.cfg from kbd_layout=keymap_en.map to kbd_layout=keymap_fr.map but it doesn't change anything.

With shift, I get wrong characters: !@#$%....

(edit: in the option, I can change to "french CPC". But anyway, the behavior seems to be reversed, I still get 1,2,3 without shift.

Also the option interface is really tiny (same as in the previous version). Here is a screenshot:

sielenk commented 3 years ago

Testing on an Aspire Notebook with german keyboard running a Gentoo system.

[...]$ uname -a 
Linux aspire 5.4.97-gentoo-x86_64 #1 SMP Sun Mar 14 17:50:10 CET 2021 x86_64 Genuine Intel(R) CPU U4100 @ 1.30GHz GenuineIntel GNU/Linux
[...]$ git describe 
v4.5.0-277-g74bf2aa
[...]$

The build of the sdl2 branch worked without a problem. The resulting binary starts as expected. But I do get some weird effects with the keyboard (`kbd_layout=keymap_de_linux.map` and `keyboard=0`): pressed key	output on CPC
Z	`y` (lower case)
Shift+Z	`Y` (upper case)
Y	`X` (upper case)
Shift+Y	`Z` (upper case)
X	`Z` (upper case)
Shift+X	`X` (upper case)
Shift+3	`#` (should be `§`)
Shift+7	`'` (back-tick?, should be `/`)
Shift+0	`_` (should be `=`)
Plus	`[`
Shift+Plus	`{` (should be `*`)
Hash	`]` (should be `#`)
Shift+Hash	`}` (should be `'`)
Shift+Comma	`<` (should be `;`)
Shift+Dot	`>` (should be `:`)
Dash	`/` (should be `-`)
Shift+Dash	`?` (should be `_`)

The remaining letters are OK (upper and lower case). As are the numbers. Other keys like the umlauts are not mapped at all.

I tried changing/updating the keymap file but it had no effect. I verified with strace that it is being read, though.

cpcitor commented 3 years ago

Thanks @ColinPitrat for notifying and asking for feedback. Added a tab in the spreadsheet.

There are things that always bugged me with keyboard handling in caprice32.

map file selection should be automatic

Why should user have to choose "PC keyboard layout"? Plus the offered choices mention operating system.

This feels awkward. Should the end user be exposed to those combinations, when the OS and probably keyboard layout can be probed or at least guessed from hints. Most probably, many users won't bother and just consider the thing broken.

As a convenience, cpc-dev-tool-chain selects it automatically based on probing the operating system keymap, see https://github.com/cpcitor/cpc-dev-tool-chain/blob/master/tool/caprice32/caprice32_local_adjust.sh . This currently covers only part of the use cases but could be a basis for improvement.

map files feel like they replicate known information

IMHO there are basically two ways to handle emulated keyboard. Arnoldemu can do both and calls these:

"translated" (what caprice32 does, send to CPC the characters meant by user)
and "positional" (what cpcec does, disregard actual keymap and use only SDL_ScanCode ever).

I have had a look again at the map files. It feels awful that one even have to consider OS difference. Isn't SDL supposed to abstract OS away? Perhaps SDL was fixed in the meantime and some differences between win and linux variants are no longer needed. What happens on Mac?

map files are defined based on SDL_Keycode, so they can only be broken if the file chosen does not match the OS keyboard layout. Had they been defined based on SDL_Scancode they might have been more robust (or not, I'm not naive enough to claim so).

Anyway, since caprice32 chooses the "translated" way, in most keyboard events the needed information to propagate to the CPC is conveyed by the character generated by the key. That character is available in the SDL_TEXTINPUT events, right?

Couldn't keyboard handling be simplified using SDL_TEXTINPUT? All key combinations that generate characters (even complicated like AltGr 0 for the @) would be covered by SDL_TEXTINPUT events immediately. The number of necessary mappings to cover the other cases would be much fewer. Perhaps some of the *map could then be merged because they would no longer be different?

You can answer "no, it's a mess". ;-) I actually had a look at the .map files, and I'd not be surprised.

ColinPitrat commented 3 years ago

Indeed, keyboard handling in caprice32 is a mess.

I agree we shouldn't have two settings (CPC keyboard layout + PC keyboard layout). And I'd love to support both translated & positional modes.

The reason for the win/linux keymaps to exist is that indeed SDL1 was buggy. I suspect the different mappings may not be needed anymore with SDL2, that's one thing I'd like to determine through the testing.

The scancode may be interesting in SDL2. In SDL1, it was documented to be hardware specific and that it shouldn't be used in general (https://www.libsdl.org/release/SDL-1.2.15/docs/html/sdlkeysym.html).

The idea of auto-detecting the right keymap is good. A 2 minutes search allowed me to find this which provides some interesting idea to do it from the code in a platform independent way: https://gist.github.com/g2p/8597984

The SDL_TEXTINPUT is an interesting idea but, as you said, it doesn't cover all keys so it's a bit messy. In SDL1 there was the unicode field which could have been used for keys that had it, but it's not available anymore with SDL2. In SDL2, there's the key name (https://wiki.libsdl.org/SDL_GetKeyName) which could be interesting, but is meant for human consumption so it could be unstable. Probably not a great idea to rely on it.

I opened https://github.com/ColinPitrat/caprice32/issues/189 to track this.

ColinPitrat commented 3 years ago

@farvardin Thanks, I'll have a look into the numbers issue. On my side (Linux, French mapping for linux, CPC keyboard = 1) I have the digits if I press Shift+digit, special char otherwise (e.g & if I press 1 without shift).

Also the option interface is really tiny (same as in the previous version). Here is a screenshot: This is a long lasting problem. The GUI is at the maximum size for "half size" video plugins but is too small on modern displays. The problem is that it's drawn on the emulator output surface, not on the final video surface. This should be changed.

I opened https://github.com/ColinPitrat/caprice32/issues/191 to track this.

ColinPitrat commented 3 years ago

@sielenk Did you try the SDL1 version (checkout branch master instead of sdl2)? (No need to if you didn't, it's just by curiosity. I suspect this layout actually never worked ...)

Also, what log do you get (in the console) when you try to make a question mark?

Can you try the new layout I just committed on sdl2 branch?

ColinPitrat commented 3 years ago

@MartianGirl In the tests result, you mention that scr_width & scr_height don't work anymore in fullscreen for OpenGL. This is a choice I made when coding it: SDL 2 supports using the current resolution, avoiding a switch. It seemed to me that this would be advantageous. I was planning to just drop scr_width & scr_height (and probably scr_bpp too).

Do you see any reason why this would be an issue and why someone would prefer the old behaviour?

ColinPitrat commented 3 years ago

@Fistynuts you mention bicubic fails on SDL1. Do you have more details on the failure? Is it just speed? This video plugin is known to be very slow even on modern hardware, I'm considering removing it completely ...

MartianGirl commented 3 years ago

Hi Colin,

My usual setup is: scr_width=1440 scr_height=1080

This gives me a 4:3 aspect ratio in a 1920x1080 screen. Otherwise, I'd have to force the 4:3 aspect ratio using the screen menu.

I think it's fine removing these settings, but I think it's also good to have some means of specifying the aspect ratio. (In my ZX Spectrum emulator, SpecIde, I'm just forcing 4:3 for all fullscreen modes; it looks weird to me having 16:9 for a 8-bit screen) :)

On Mon, Mar 29, 2021, 22:31 Colin Pitrat @.***> wrote:

@MartianGirl https://github.com/MartianGirl In the tests result, you mention that scr_width & scr_height don't work anymore in fullscreen for OpenGL. This is a choice I made when coding it: SDL 2 supports using the current resolution, avoiding a switch. It seemed to me that this would be advantageous. I was planning to just drop scr_width & scr_height (and probably scr_bpp too).

Do you see any reason why this would be an issue and why someone would prefer the old behaviour?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ColinPitrat/caprice32/issues/185#issuecomment-809691268, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABS3BW3QJG5KG5LFKSIEODTTGDWRLANCNFSM4ZUYMLFA .

sielenk commented 3 years ago

@sielenk Did you try the SDL1 version (checkout branch master instead of sdl2)? (No need to if you didn't, it's just by curiosity. I suspect this layout actually never worked ...)

On the master branch (v4.5.0-230-g001bd66) I get the same behavior as described in my last post regarding the letters.

Also, what log do you get (in the console) when you try to make a question mark?

When pressing the question mark, I get

cpitrat: pressed: Left Shift(1073742049: SDLK_LSHIFT)
cpitrat: pressed: ß(223: )

on the Linux console but no reaction in the emulator window. Similar with the right shift key.

Can you try the new layout I just committed on sdl2 branch?

Yes, I can: With the sdl2 updates (v4.5.0-284-g559aa79) all the letters work correctly, upper and lower case. 👍🏻

ColinPitrat commented 3 years ago

@sielenk Thanks, question mark should be fixed too now.

ColinPitrat commented 3 years ago

@MartianGirl Great, I have added a setting (on by default) to preserve aspect ratio with letter boxing (black bands on right & left), so this should be fine.

cpcbegin commented 3 years ago

Testing on a i7, NVIDIA Corporation GF119 [GeForce GT 610] with Ubuntu Mate 20.04, Spanish keyboard.

Fullscreen:

I use two monitors extended and when I select fullscreen switch to mirror with the resolution of the worst of them, only returns to previous configuration when I reset the computer.

Keyboard (layout=keymap_es_linux.map)

Configured on Spanish does NOT run these keys: ñ(241: ) (1073741824: ) ç(231: ) º(186: ) ¡(161: ) 9(57: SDLK_9) ...... SHIFT + )

ColinPitrat commented 3 years ago

@cpcbegin Sorry for this painful display bug. I don't have the same issue on my side although I do have a bug. When I switch to fullscreen while the window is on the primary display, everything works fine, it goes to fullscreen with the resolution of the primary. When I switch to fullscreen while the window is on the secondary display, it goes fullscreen with the resolution of the primary so I only get the top-left part of the screen.

The bug you describe looks similar to a SDL1.2 issue for which the workaround was to specify SDL_VIDEO_FULLSCREEN_DISPLAY environment variable (https://forum.xfce.org/viewtopic.php?id=14677). Are you sure you tested with SDL2 version (sdl2 branch)?

Edit: I tried the SDL1.2 version (from master branch) and I do have the same issue as you do: when I switch to fullscreen the displays are mirrored and mess-up with my resolution. I recover it with xrandr --output HDMI2 --right-of eDP1 --mode 1920x1080 (would need to be adapted to your case of course). So this renforces my suspicion that you may have tried the SDL 1.2 version, not the SDL2 one.

cpcbegin commented 3 years ago

I'm checking SDL2 version, I compile it with this commands:

git clone https://github.com/ColinPitrat/caprice32 caprice32sdl2
cd caprice32sdl2
git branch sdl2 origin/sdl2
git checkout sdl2
make

If I type git branch I get this:

  master
* sdl2

But now I delete cap32.cfg, reconfigure again and show ok ???

But mencioned keys still not work, but now shift+8 & shift+9 shows (

ColinPitrat commented 3 years ago

Weird but good to know the problem is gone. I attempted to fix the keymap.

As far as I know, there's no ç, ° or ¡ on the spanish CPC (the ç is only in the French charset): https://www.cpcwiki.eu/index.php/File:Charset_spanish_gray.gif https://www.cpcwiki.eu/imgs/c/c2/Spanish_6128_Keyboard_Robcfg.jpg

(from https://www.cpcwiki.eu/index.php/Keyboard_Versions)

sielenk commented 3 years ago

In `v4.5.0-292-gcab2138` the special characters work as far as I can see. But some letters are broken again: pressed key	output on CPC
Z	upper case X - not OK
Shift+Z	upper case Z - OK
X	upper case Z - not OK
Shift+X	upper case X - OK

ColinPitrat commented 3 years ago

@Fistynuts I'm really interested by your emulator speed issue. You're the only one to report it.

Because timing is controlled by audio when active, one thing you can try is disable sound (enabled=0 in the [sound] section of cap32.cfg or uncheck Enable Sound Emulation in options in the UI) and check if the speed is right in this case.

I also added some logs, so if building from git branch, after updating the code you can rebuild with make debug and try the following ./cap32 -v | grep "$Audio\|Timing$" which should display some logs that may help identify where the problem comes from.

patxoca commented 3 years ago

Done some testing on an AMD FX(tm)-8300, GeForce GT 730, dual display, Ubuntu 20.04.2. I've added a sheet to the spreadsheet with the details.

Some problems with the keyboard layout, specifically with [, { and }:

[ -> cpitrat: pressed: (1073741824: )
{ -> cpitrat: pressed: (1073741824: )
} -> cpitrat: pressed: ç(231: )

On software bicubic it runs at 19 FPS but I assume that's because the CPU is old.

NigeNigeNige commented 3 years ago

@Fistynuts you mention bicubic fails on SDL1. Do you have more details on the failure? Is it just speed? This video plugin is known to be very slow even on modern hardware, I'm considering removing it completely ...

Hey, sorry for the delay. On my system, selecting software bicubic on SDL1 causes black emulator output then a crash. This is the same in windowed and fullscreen modes. It works fine on SDL2 in both modes other than the aspect ratio issue already raised. It's not a mode I would ever use in reality, as you said it's unusably slow. My vote would be to remove the option.

NigeNigeNige commented 3 years ago

@Fistynuts I'm really interested by your emulator speed issue. You're the only one to report it.

Because timing is controlled by audio when active, one thing you can try is disable sound (enabled=0 in the [sound] section of cap32.cfg or uncheck Enable Sound Emulation in options in the UI) and check if the speed is right in this case.

I also added some logs, so if building from git branch, after updating the code you can rebuild with make debug and try the following ./cap32 -v | grep "$Audio\|Timing$" which should display some logs that may help identify where the problem comes from.

I initially thought this was related to screen refresh rates, as I run the Windows desktop at 144Hz. However I switched to 60Hz and retested - the results were the same.

With sound emulation unchecked in the UI, I get odd results. Before, at 50% emulation speed selected in the UI, I would get 100% actual emulation speed. Now with the same setting but with sound disabled, I get 42-44%. With the emulator set at 100% speed, I get 62-66%.

I don't have the ability right now to build from source - could you possibly release a new win64 zip with the additional logging?

Edit: I just realised I went through the test plan somewhat academically. I never tested if sound was working. I apologise. Sound is completely silent. I assume it's selecting the wrong output - I have several, some are not connected. Sound on SDL1 is OK.

ColinPitrat commented 3 years ago

My vote would be to remove the option.

Yes, definitely something I would have already done if there wasn't the issue of config backward compatibility. There are solutions around that, but they require a little more thinking & work. I'll dig into it now though, as I have to also remove the HW scaling options.

Sound is completely silent.

I'll add more debugging info around sound initialization then, and trigger a new build for the SDL2 version.

ColinPitrat commented 3 years ago

@Fistynuts May I interest you in https://github.com/ColinPitrat/caprice32/releases/tag/sdl2_v2

It has more logs around timing and audio at INFO level which should allow you to see them. You just have to start caprice from the command line.

This can be useful to other windows testers too, to ensure they test with the latest changes.

ColinPitrat commented 3 years ago

@sielenk Your upper case Z and X when pressing some lower case keys sounds a lot like joystick emulation. You can activate / deactivate it with F7. This can be confirmed by pressing arrow keys. If they display arrows, joystick emulation is on. If they move the cursor it's off.

Joystick emulation allows to play to games requiring a joystick without having one, using arrow keys and Z & X keys.

NigeNigeNige commented 3 years ago

@Fistynuts May I interest you in https://github.com/ColinPitrat/caprice32/releases/tag/sdl2_v2

It has more logs around timing and audio at INFO level which should allow you to see them. You just have to start caprice from the command line.

This can be useful to other windows testers too, to ensure they test with the latest changes.

Here is the output - the emulator now automatically closes after the last log is output.

PS D:\games\Emulator\cap32-win64-v2> .\cap32.exe
No valid configuration file found, using empty config.
INFO    src/cap32.cpp:1305 - Audio: device 0: Digital Audio (S/PDIF) (High Definition Audio Device)
INFO    src/cap32.cpp:1305 - Audio: device 1: Speakers (Bigscreen Audio Stream 1.2)
INFO    src/cap32.cpp:1305 - Audio: device 2: S2419HGF (NVIDIA High Definition Audio)
INFO    src/cap32.cpp:1305 - Audio: device 3: Digital Audio (S/PDIF) (High Definition Audio Device) (2)
INFO    src/cap32.cpp:1305 - Audio: device 4: Speakers (Steam Streaming Microphone)
INFO    src/cap32.cpp:1305 - Audio: device 5: Speakers (High Definition Audio Device)
INFO    src/cap32.cpp:1305 - Audio: device 6: Speakers (Steam Streaming Speakers)
INFO    src/cap32.cpp:1315 - Audio: Device ID: 2
INFO    src/cap32.cpp:1316 - Audio: Desired: Freq: 44100, Format: 32784, Channels: ☻, Samples: 1024
INFO    src/cap32.cpp:1317 - Audio: Obtained: Freq: 44100, Format: 33056, Channels: ☻, Samples: 441
INFO    src/psg.cpp:730 - Timing: CPC.speed: 4 - freq: 44100
INFO    src/psg.cpp:731 - Timing: z80 cycles per sample: 90.7029 - LoopCountInit: 2.83447
No joystick found.
joysticks_init() failed. Joysticks won't work.
INFO    src/cap32.cpp:1639 - Timing: First frame at 251 - next frame in 20 ( 20/(4/4) ) at 271

The audio device it should select (which is Windows default) is 5.

ColinPitrat commented 3 years ago

I have to look more into it, but I just found out that the device id returned by SDL_OpenAudioDevice is not actually the index of the device as can be used to query their names, so when it says 'Device ID: 2' it doesn't mean that it selected the S2419HGF device. However some similar code could allow to provide a way to select the output device from options from the UI.

From the logs, the format obtained is not the same as the one requested. Unless I'm mistaken:

32 bits instead of 16 bits
float instead of int
smaller buffer size (441 instead of 1024, the former not being a power of 2 contrary to what the doc says ...)

The code currently only supports 8 or 16 bits integers in mono or stereo and, unless I'm mistaken, doesn't adapt to the spec obtained. I'm not sure whether this could be what impacts the timing though. AFAICT, this shouldn't as long as the frequency obtained is the one desired.

ColinPitrat commented 3 years ago

From the doc, it seems I can just pass 0 as a flag for allowed changes and SDL will do the necessary conversion, so let's try that.

If that flag was ''not* set, SDL will prepare to convert your callback's float32 audio to int16 before feeding it to the hardware and will keep the originally requested format in the obtained structure.

If your application can only handle one specific data format, pass a zero for allowed_changes and let SDL transparently handle any differences.

https://wiki.libsdl.org/SDL_OpenAudioDevice

ColinPitrat commented 3 years ago

The SDL2 version is now on the main branch. I consider it has fully done. Please open an issue if you encounter any problem. I think I covered everything except potentially keyboard issues under windows.

ColinPitrat / caprice32

SDL2 -- sdl1 is dead #185

map file selection should be automatic

map files feel like they replicate known information

Fullscreen:

Keyboard (layout=keymap_es_linux.map)