sdltrs: display of typed characters lags keyboard, sometimes permanently by one character

mitu

I guess we'll try to find the bug in the emulator. On Windows it doesn't seem to suffer from this issue and it doesn't seem to be a SDL2 input problem.

n2185x

@mitu My suspicion is that it's either a platform-specific display bug (do you already have a Raspberry Pi 3 set up? If so, what happens there?), or a platform-specific timer bug.

As I mentioned, I've seen something very similar to this with the hatari emulator (not lr-hatari, however), with respect to its configuration GUI, wherein the state of the displayed dialog box seems to lag behind by one action (e.g., you click on a checkbox and it doesn't seem to change state until you hit an arrow key or something that would cause it to change the display, or you hit a button and the corresponding sub-dialog doesn't come up until you hit an arrow key or something that would cause it to change the display). This is what makes me conclude that the issue isn't in the emulator itself, but rather is in SDL2 on the Pi 4 at least.

mitu

I've enabled input debugging in the emulator and I can see all the keys presses as registered without any issue, it's the emulator that doesn't seem to react to them. Since the input works fine if a disk image is loaded - or with a Model3/4 - I don't think it is a SDL2 issue.

n2185x

@mitu The input works "fine" if the cursor is blinking (see below). Try it with Model 1 emulation and TRSDOS version 2.3. That version does not blink the cursor and the end result is the same problem.

The issue has nothing to do with whether or not there's a disk image involved. It has everything to do with a blinking cursor, i.e. continuously changing output to the screen. This is why I believe we're up against an SDL issue -- because the same phenomenon is visible in multiple emulators that link against SDL2.

A note about it working "fine": in reality it's not really "fine" as such. There are delays in display of things to the screen that shouldn't be there at all. For instance, in Model 3 emulation (the md5 checksum of level3.rom is ecd0a5a83fca19ea860fe565fad4b061), in BASIC, it will often delay halfway in the display of "READY", e.g. it will display "REA", then delay a little, then display "DY". This isn't completely consistent but the overall behavior and feel of the emulator is such that it's apparent that there are internal delays that simply shouldn't be there, and probably aren't in the same emulator on other platforms.

I think it may be worth testing this on a Pi 3 to see just how much of a platform dependency there seems to be.

n2185x

@mitu It's possible that this isn't an SDL2 issue, but whatever it is, it has the following characteristics:

It is common across multiple emulators (hatari and sdltrs, at a minimum)
It is absent from the same emulator in libretro form (lr-hatari)

That means that whatever it is, it exists in code that is common between two completely different emulators and is simultaneously absent from a different variant of one of the emulators. The only thing I know of that is present in both hatari and sdltrs and is simultaneously absent from lr-hatari is SDL2. Maybe there are other relevant things that qualify, but I don't know what they'd be.

n2185x

@mitu I downloaded the Pi 3 version of RetroPie and put it onto a Pi 3 B+. I then performed a full update of it, then installed sdltrs via retropie_setup.sh (from binaries), then copied over all of the .rom files in the BIOS directory from my Pi 4 as well as the entirety of the trs-80 subdirectory of the "roms" directory. In short, everything on the Pi 3 is identical to how it is on the Pi 4.

The issue does not reproduce on the Pi 3, which means this is almost certainly an SDL2 issue or graphics driver issue (or something else that is specific to the Pi 4). And seeing how plenty of other emulators work just fine, I'm skeptical to the point of disbelief that it's a graphics driver issue.

mitu

My test was a bit simpler (haven't got to test on a Pi3) - I started the emulator from a desktop session and the issue is gone. The difference between the 2 cases is the different input and video driver - when starting from the desktop, it uses X11 input and video, without it relies on the KMS/DRM video and input drivers.

It's a bit difficult to pin it down to an particular (or obvious) SDL2 issue, since other emulators don't exhibit the same behavior when using the same video/input drivers (i.e. vice for C64, openmsx for MSX, scummvm, etc.)

EDIT: a similar issue/behavior was reported for 'draStic', the Nintendo DS emulator, but we couldn't find a proper solution since it's a closed source application. At least for sdl2trs the source is available, so it's a starting point in trying to find the root cause of this behavior.

n2185x

@mitu I wouldn't know where to begin with respect to tracking this down, as I have no familiarity with any of the code involved here. But I am happy to help however I can, and would really like to see this tracked down and fixed.

JenGun

Hi,

as the current maintainer of sdl2trs I've experienced nearly the same issue with Wayland. Please try the attached patch and test if this fixes it also on the Pi4 ...

diff --git a/src/trs_sdl_interface.c b/src/trs_sdl_interface.c
index b0dbfe4..a2c8734 100644
--- a/src/trs_sdl_interface.c
+++ b/src/trs_sdl_interface.c
@@ -1508,6 +1508,7 @@ void trs_sdl_flush(void)
   }
 
   SDL_UpdateTexture(texture, NULL, screen->pixels, screen->pitch);
+  SDL_RenderClear(render);
   SDL_RenderCopy(render, texture, NULL, NULL);
   SDL_RenderPresent(render);
 
@@ -1716,9 +1717,7 @@ void trs_get_event(int wait)
         break;
       case SDL_WINDOWEVENT:
         if (event.window.event & SDL_WINDOWEVENT_RESIZED) {
-          SDL_RenderClear(render);
-          SDL_RenderCopy(render, texture, NULL, NULL);
-          SDL_RenderPresent(render);
+          drawnRectCount = MAX_RECTS;
         }
         if (event.window.event & SDL_WINDOWEVENT_EXPOSED) {
           SDL_FlushEvent(SDL_KEYDOWN);

mitu

@JenGun thanks for stopping by !

The patch does fix the issue.

My temporary workaround that I found was to force trs_sdl_flush to redraw, by ignoring the drawnRectCount check (which seems to work also, but obviously not correct) and my next step was to investigate why the check fails - in some cases - with KMSDRM.

n2185x

@mitu I'll give that patch a shot as well.

I do have one question about such things in general: what's the best way to pull down the source, compile it, and put the resulting binary into play? For atari800, I simply hit ctrl-c while it was in the middle of compiling during an "install from source" operation within retropie_setup.sh, then moved the directory and was from that point able to make my changes, compile them up, and manually install the resulting binary over the one on the system.

But that seems a bit excessive. My hope is that you have some well-defined method for doing these things that isn't quite as involved.

mitu

@n2185x said in sdltrs: display of typed characters lags keyboard, sometimes permanently by one character:

But that seems a bit excessive. My hope is that you have some well-defined method for doing these things that isn't quite as involved.

You can use retropie_packages.sh to run each scriptmodule helper methods:

# download sources & apply local patches
sudo ./retropie_packages.sh sdltrs sources

# build
sudo ./retropie_packages.sh sdltrs build

# install 
sudo ./retropie_packages.sh sdltrs install

# configure 
sudo ./retropie_packages.sh sdltrs configure

Or you can just clone the upstream repository (the sdl2 branch) and build it manually, then copy sdl2trs to /opt/retropie/emulators/sdltrs.

n2185x

@mitu Fantastic, that's exactly what I was looking for.

I can confirm that the patch works brilliantly.

This forum is awesome.

JenGun

@mitu Thanks for testing and adding sdl2trs to RetroPie! :-)

My temporary workaround that I found was to force trs_sdl_flush to redraw, by ignoring the drawnRectCount check (which seems to work also, but obviously not correct) and my next step was to investigate why the check fails - in some cases - with KMSDRM.

That was also my first try, but the emulator screen was not redrawn after leaving the TextGUI. It will also burn some CPU cycles ... ;-)

Sometimes the screen is not updated in Wayland after resizing, but for now the patch is committed: https://gitlab.com/jengun/sdltrs/-/commit/a817f5e817db7166a4654fa735a4abb848534a45

n2185x

@JenGun Thanks very much for showing up with a fix. This is excellent.

It does raise a question, however: does the change actually qualify as a fix to a bug in sdl2trs, or is this a workaround to a bug that is elsewhere?

I think it's at least somewhat important. If you read my comments above, you'll see that a similar issue occurs with respect to the hatari gui. This suggests that the real bug is downstream of sdl2trs. It might be in SDL itself, or it might even be downstream of that. No idea. As long as the fix here is benign then it's all good, but if there's any way to use this to discover where the real bug is, it might be worth pursuing.

Thanks again for the fix, it's greatly appreciated!

JenGun

@n2185x

It does raise a question, however: does the change actually qualify as a fix to a bug in sdl2trs, or is this a workaround to a bug that is elsewhere?

Good question ... I would cautiously refer to it as a "workaround": the error only seems to occur on a few systems and why RenderClear fixes it is not entirely clear to me ... maybe this will flush the render command queue ...

Something similar happens here on another machine: the SDL window turns completely white after a few seconds. An old version (SDL 2.0.9) fixes the problem ... with newer versions a superfluous SDL_RenderDrawPoint is needed after SDL_UpdateTexture ...

In hatari the RenderClear command is also "missing", so the fix might also apply:

diff --git a/src/screen.c b/src/screen.c
index 32765a48..a609ca34 100644
--- a/src/screen.c
+++ b/src/screen.c
@@ -115,6 +115,7 @@ void SDL_UpdateRects(SDL_Surface *screen, int numrects, SDL_Rect *rects)
        if (bUseSdlRenderer)
        {
                SDL_UpdateTexture(sdlTexture, NULL, screen->pixels, screen->pitch);
+               SDL_RenderClear(sdlRenderer);
                SDL_RenderCopy(sdlRenderer, sdlTexture, NULL, NULL);
                SDL_RenderPresent(sdlRenderer);
        }

mitu

@JenGun this does seem to improve things, at least for the Hatari menu. For the record, RetroPie includes libSDL2 2.0.10.

JenGun

@mitu It makes the SDL GUI work in Wayland. Replacing SDL_RenderClear with SDL_RenderDrawPoint also seems to fix the issue ...

JenGun

The issue in hatari is also fixed upstream:
https://github.com/hatari/hatari/commit/191a35fe24b340be143e9a368698123cdf0280b7

mitu

@JenGun Thank you for submitting the patch. We may have to include it locally before a new Hatari release.