No analog controller support, no 32bit cursor support, crashing on exit

[50J0uRNeR]

Bug report

Here is a clear and concise description of what the problem is: While testing a source build of Scummvm in Kodi 19 on a PI 4b, I notice that the controller analog sticks will not move cursor no matter how the controller is mapped. Also while testing Myst I noticed a blue block where the cursor should be (the cursor is critical for playing Myst). Lastly the emulator would crash (bringing down Kodi with it) when:

1. Quitting the game
2. Returning to launcher
3. Forced quit (start + select or stop)
4. Changing games

To Reproduce

Steps to reproduce the behavior:

1. Run Myst in Kodi 19 (with a properly mapped controller).
2. Try to use mapped analog sticks to move cursor.
3. Notice blue block where cursor should be.
4. Quit and emulator will crash (in the right environment).

My Environment

Used Operating system:

• [ ] Android
• [ ] iOS
• [x] Linux
• [ ] OSX
• [x] Raspberri-Pi
• [ ] Windows
• [ ] Windows UWP

Operating system version/name: Raspbian GNU/Linux 11 (bullseye) 32bit

Kodi version: Kodi 19 Matrix, debian package 2:19.3-1-bullseye

Hardware: Raspberry Pi 4b 8gb

Emulator: Compiled from source using commit 80cb7269a33b233dcea27d8d01df084b0d35c80a from archive https://github.com/libretro/scummvm/archive/80cb7269a33b233dcea27d8d01df084b0d35c80a.tar.gz. Started from addon https://github.com/kodi-game/game.libretro.scummvm.

My Solution

I decided to go ahead and resolve these issues myself, as they were fairly simple to fix. The following is a series of patches that resolved these issues for me...

To fix the controller issue I simply needed to add the analog sticks to the retro_input_descriptor struct. Here's the patch:

--- orig/backends/platform/libretro/libretro.cpp        2021-12-22 17:09:47.000000000 -0500
+++ source/backends/platform/libretro/libretro.cpp      2022-02-25 00:10:07.564495701 -0500
@@ -352,6 +352,10 @@
       { 0, RETRO_DEVICE_JOYPAD, 0, RETRO_DEVICE_ID_JOYPAD_SELECT, "F1" },
       { 0, RETRO_DEVICE_MOUSE,  0, RETRO_DEVICE_ID_MOUSE_LEFT,    "Left click" },
       { 0, RETRO_DEVICE_MOUSE,  0, RETRO_DEVICE_ID_MOUSE_RIGHT,   "Right click" },
+      { 0, RETRO_DEVICE_ANALOG, RETRO_DEVICE_INDEX_ANALOG_LEFT,   RETRO_DEVICE_ID_ANALOG_X, "Left Analog X" },
+      { 0, RETRO_DEVICE_ANALOG, RETRO_DEVICE_INDEX_ANALOG_LEFT,   RETRO_DEVICE_ID_ANALOG_Y, "Left Analog Y" },
+      { 0, RETRO_DEVICE_ANALOG, RETRO_DEVICE_INDEX_ANALOG_RIGHT,  RETRO_DEVICE_ID_ANALOG_X, "Right Analog X" },
+      { 0, RETRO_DEVICE_ANALOG, RETRO_DEVICE_INDEX_ANALOG_RIGHT,  RETRO_DEVICE_ID_ANALOG_Y, "Right Analog Y" },
       { 0 },
   };

To fix the cursor issue I added 32 to 16 bit cursor support. Here's the patch:

--- orig/backends/platform/libretro/libretro_os.cpp     2021-12-22 17:09:47.000000000 -0500                                                                       
+++ source/backends/platform/libretro/libretro_os.cpp   2022-02-25 00:16:23.700556471 -0500                                                                       
@@ -238,6 +238,33 @@                                                                                                                                              
    }                                                                                                                                                             
 }                                                                                                                                                                

+static void blit_uint32_uint16(Graphics::Surface& aOut, const Graphics::Surface& aIn, int aX, int aY, const RetroPalette& aColors, uint32 aKeyColor)             
+{                                                                                                                                                                
+   for(int i = 0; i < aIn.h; i ++)                                                                                                                               
+   {                                                                                                                                                             
+      if((i + aY) < 0 || (i + aY) >= aOut.h)                                                                                                                     
+         continue;                                                                                                                                               
+                                                                                                                                                                 
+      uint32_t* const in = (uint32_t*)aIn.pixels + (i * aIn.w);                                                                                                  
+      uint16_t* const out = (uint16_t*)aOut.pixels + ((i + aY) * aOut.w);                                                                                        
+                                                                                                                                                                 
+      for(int j = 0; j < aIn.w; j ++)                                                                                                                            
+      {                                                                                                                                                          
+         if((j + aX) < 0 || (j + aX) >= aOut.w)                                                                                                                  
+            continue;                                                                                                                                            
+                                                                                                                                                                 
+         uint8 r, g, b;                                                                                                                                          
+                                                                                                                                                                 
+         const uint32_t val = in[j];                                                                                                                             
+         if(val != aKeyColor)                                                                                                                                    
+         {                                                                                                                                                       
+            aIn.format.colorToRGB(in[j], r, g, b);                                                                                                               
+            out[j + aX] = aOut.format.RGBToColor(r, g, b);                                                                                                       
+         }                                                                                                                                                       
+      }                                                                                                                                                          
+   }                                                                                                                                                             
+}                                                                                                                                                                
+                                                                                                                                                                 
 static INLINE void copyRectToSurface(uint8_t *pixels, int out_pitch, const uint8_t *src, int pitch, int x, int y, int w, int h, int out_bpp)                     
 {                                                                                                                                                                
    uint8_t *dst = pixels + y * out_pitch + x * out_bpp;                                                                                                          
@@ -497,10 +524,19 @@                                                                                                                                             
             const int x = _mouseX - _mouseHotspotX;                                                                                                              
             const int y = _mouseY - _mouseHotspotY;                                                                                                              

-            if(_mouseImage.format.bytesPerPixel == 1)                                                                                                            
-               blit_uint8_uint16(_screen, _mouseImage, x, y, _mousePaletteEnabled ? _mousePalette : _gamePalette, _mouseKeyColor);                               
-            else                                                                                                                                                 
-               blit_uint16_uint16(_screen, _mouseImage, x, y, _mousePaletteEnabled ? _mousePalette : _gamePalette, _mouseKeyColor);                              
+            switch(_mouseImage.format.bytesPerPixel)                                                                                                             
+            {                                                                                                                                                    
+               case 1:                                                                                                                                           
+               case 3:                                                                                                                                           
+                  blit_uint8_uint16(_screen, _mouseImage, x, y, _mousePaletteEnabled ? _mousePalette : _gamePalette, _mouseKeyColor);                            
+                  break;                                                                                                                                         
+               case 2:
+                  blit_uint16_uint16(_screen, _mouseImage, x, y, _mousePaletteEnabled ? _mousePalette : _gamePalette, _mouseKeyColor);
+                  break;
+               case 4:
+                  blit_uint32_uint16(_screen, _mouseImage, x, y, _mousePaletteEnabled ? _mousePalette : _gamePalette, _mouseKeyColor);
+                  break;
+            }
          }
       }

The crashing and stability issues are all related to the compiler flags in the Makefile interacting with my particular environment and may not be a problem for anyone else? This patch may only apply to me:

--- orig/backends/platform/libretro/build/Makefile      2022-02-18 17:41:11.537295875 -0500
+++ source/backends/platform/libretro/build/Makefile    2022-02-18 17:40:49.647555542 -0500
@@ -91,10 +91,10 @@
 else ifeq ($(platform), rpi4_64)
    TARGET = $(TARGET_NAME)_libretro.so
    DEFINES += -fPIC
-   CFLAGS += -fPIC
+   CFLAGS += -DARM -marm -mcpu=cortex-a72 -mfpu=neon -fPIC
    LDFLAGS += -shared -Wl,--version-script=../link.T -fPIC
-   CFLAGS += -mcpu=cortex-a72 -mtune=cortex-a72
-   CFLAGS += -fomit-frame-pointer -ffast-math
+   CFLAGS += -march=armv8-a+crc -mtune=cortex-a72
+   CFLAGS += -mfpu=neon-fp-armv8 -mfloat-abi=hard
    CXXFLAGS = $(CFLAGS) -frtti
    BUILD_64BIT = 1

Hopefully these will help anyone else having these issues.

[i30817]

Thanks for the patches. If you compile the core yourself and enable all engines on ./configure, or use the diablodiab core (with https://github.com/diablodiab/libretro-scummvm-backend and copying over the files with make && ../aux-data/bundle_aux_data.bash && 7z x -y -o$HOME/.config/retroarch/system/ ../aux-data/scummvm.zip && cp scummvm_libretro.so ~/.config/retroarch/cores/ from the build dir) you can test the 7th guest and 11th hour games, which have the same cursor problem as myst (possibly they have one more, because 11th hour cursor seems 'zoomed in' besides being blue with transparency artifacts. It's animated).

I haven't built it yet.

BTW, the 11th hour has another issue where upstream is allowing fast forward of unskippable videos by pressing esc, but the diablodiab core disables all sound, while upstream just disables voices and keeps music playing at the normal rate).

[i30817]

Edit: i tried it, and 11th hour seems to have a transparency problem:

Note the font too. These problems don't exist upstream.

7th guest, myst and riven appear fine.

[i30817]

detection.cpp at https://github.com/scummvm/scummvm/blob/master/engines/groovie/detection.cpp has links to demos of 11th hour.

This one (139 mb) works to show the problems (except the video sound i think).

https://archive.org/download/11th_Hour_demo/11th.iso

[50J0uRNeR]

Sorry, it took me some time to go over the libretro backend and Scummvm source.

I am not having issues with the cursor alpha for 11th Hour, but I am testing this in Kodi on a Raspberry Pi. I don't see why my changes would work differently in other environments however. Are you sure the patch is applied properly and you are using the same source as me? Here is a link to an tarball of the version I'm working with https://github.com/libretro/scummvm/archive/80cb7269a33b233dcea27d8d01df084b0d35c80a.tar.gz. I should also note that the version of Scummvm in this libretro-scummvm repo is older than the official Scummvm repo, and even the official repo states 11th Hour is still in testing and may not be playable.

In any case I have found that the addon's blit functions in libretro_os.cpp is masking pixels with the value 0xFFFFFFFF. I have done a bunch or tests and looked through several other backends and can't see why the libretro addon would be doing this? If any of the developers know why please let me know! However disabling the masking doesn't seem to cause any problems for any games I've tested so far.

The following patch includes the original fix for the cursor as well as removes the white pixel masking in the blit functions:

--- orig/backends/platform/libretro/libretro_os.cpp 2022-03-02 23:12:32.235277340 -0500
+++ patched/backends/platform/libretro/libretro_os.cpp  2022-03-02 23:12:06.845760338 -0500
@@ -109,7 +109,7 @@
          uint8 r, g, b;

          const uint8_t val = in[j];
-         if(val != 0xFFFFFFFF)
+         //if(val != 0xFFFFFFFF)
          {
             if(aIn.format.bytesPerPixel == 1)
             {
@@ -144,8 +144,8 @@

          uint8 r, g, b;

-         const uint32_t val = in[j];
-         if(val != 0xFFFFFFFF)
+         //const uint32_t val = in[j];
+         //if(val != 0xFFFFFFFF)
          {
             aIn.format.colorToRGB(in[j], r, g, b);
             out[j] = aOut.format.RGBToColor(r, g, b);
@@ -171,8 +171,8 @@

          uint8 r, g, b;

-         const uint16_t val = in[j];
-         if(val != 0xFFFFFFFF)
+         //const uint16_t val = in[j];
+         //if(val != 0xFFFFFFFF)
          {
             aIn.format.colorToRGB(in[j], r, g, b);
             out[j] = aOut.format.RGBToColor(r, g, b);
@@ -238,6 +238,33 @@
    }
 }

+static void blit_uint32_uint16(Graphics::Surface& aOut, const Graphics::Surface& aIn, int aX, int aY, const RetroPalette& aColors, uint32 aKeyColor)
+{
+   for(int i = 0; i < aIn.h; i ++)
+   {
+      if((i + aY) < 0 || (i + aY) >= aOut.h)
+         continue;
+
+      uint32_t* const in = (uint32_t*)aIn.pixels + (i * aIn.w);
+      uint16_t* const out = (uint16_t*)aOut.pixels + ((i + aY) * aOut.w);
+
+      for(int j = 0; j < aIn.w; j ++)
+      {
+         if((j + aX) < 0 || (j + aX) >= aOut.w)
+            continue;
+
+         uint8 r, g, b;
+
+         const uint32_t val = in[j];
+         if(val != aKeyColor)
+         {
+            aIn.format.colorToRGB(in[j], r, g, b);
+            out[j + aX] = aOut.format.RGBToColor(r, g, b);
+         }
+      }
+   }
+}
+
 static INLINE void copyRectToSurface(uint8_t *pixels, int out_pitch, const uint8_t *src, int pitch, int x, int y, int w, int h, int out_bpp)
 {
    uint8_t *dst = pixels + y * out_pitch + x * out_bpp;
@@ -497,10 +524,19 @@
             const int x = _mouseX - _mouseHotspotX;
             const int y = _mouseY - _mouseHotspotY;

-            if(_mouseImage.format.bytesPerPixel == 1)
-               blit_uint8_uint16(_screen, _mouseImage, x, y, _mousePaletteEnabled ? _mousePalette : _gamePalette, _mouseKeyColor);
-            else
-               blit_uint16_uint16(_screen, _mouseImage, x, y, _mousePaletteEnabled ? _mousePalette : _gamePalette, _mouseKeyColor);
+            switch(_mouseImage.format.bytesPerPixel)
+            {
+               case 1:
+               case 3:
+                  blit_uint8_uint16(_screen, _mouseImage, x, y, _mousePaletteEnabled ? _mousePalette : _gamePalette, _mouseKeyColor);
+                  break;
+               case 2:
+                  blit_uint16_uint16(_screen, _mouseImage, x, y, _mousePaletteEnabled ? _mousePalette : _gamePalette, _mouseKeyColor);
+                  break;
+               case 4:
+                  blit_uint32_uint16(_screen, _mouseImage, x, y, _mousePaletteEnabled ? _mousePalette : _gamePalette, _mouseKeyColor);
+                  break;
+            }
          }
       }

[i30817]

Ah, 11th hour starts 'fine' upstream and i'm using your libretro-backend repro with the current scummvm upstream.

Your changes here fix the font though.

The animated cursor remains with the black box around it.

As for 11th hour being in testing, i want to warn you that to get it working, upstream recently changed the required parameters to start it, so it's best to re-add it in the scummvm gui interface and start from there otherwise it will complain it has the wrong data if you're using a older ini entry iirc. I recommend having two entries to the same data with different names switch around when testing.

[i30817]

I'll try your upstream version in a moment. It already has your version libretro backend up to date right?

edit: answer: no it is not.

[i30817]

Ok, the tar.gz you posted, with the modifications you also posted, after readding the demo with that version of scummvm so the parameters are right and it starts, doesn't have the cursor problem (it has others, from using old scummvm ofc).

So something in between that version of scummvm/the libretrobackend and the current version fo scummvm/the git of your backed changed into a regression for the cursor.

Note that i also built current scummvm standalone to check, and that doesn't have the cursor bug, so it's probably the backend that has the cursor bug.

[50J0uRNeR]

Since no cores are available yet for Raspberry Pi OS Bullseye and Kodi 19, I have an automated build system that builds and installs several cores and addons into Kodi from https://github.com/kodi-game. I am using the version of Scummvm in this project https://github.com/libretro/scummvm with commit 80cb726 (latest commit) because it is what the addon is pointing to here https://github.com/kodi-game/game.libretro.scummvm/blob/master/depends/common/scummvm/scummvm.txt. I am assuming this is the official project for libretro/scummvm since I did not see a libretro core in https://github.com/scummvm/scummvm/tree/master/backends/platform?

Is there another libretro/scummvm fork that has a newer version of Scummvm with a libretro backend? Otherwise is there a simple way to merge this backend into the official project https://github.com/scummvm?

[i30817]

Oh yes. i was confusing you with @diablodiab because i wasn't paying attention to the notifications. There is this project that replaces the current scummvm core backend with a version that works in upstream master:

https://github.com/diablodiab/libretro-scummvm-backend

That was what i was mentioning. It builds here:

http://build.bot.nu/nightly/

with this as the autobuilt assets link: http://build.bot.nu/assets/system/ScummVM.zip

Or if you want to build it yourself, clone that repository as the libretro directory (replace it), cd to the build dir inside the the cloned dir, and do something like: make && ../aux-data/bundle_aux_data.bash && 7z x -y -o$HOME/.config/retroarch/system/ ../aux-data/scummvm.zip && cp scummvm_libretro.so ~/.config/retroarch/cores/

In linux.

You can also use my tool to create a playlist for retroarch after mass adding the games in the core gui (this tool creates .scummvm files but does not delete existing ones in the game directory btw, since it's the only way currently to launch the core since there is not way to specify a engine id instead of a 'path' to a file for cores that have their own game management, unfortunately):

https://github.com/i30817/libretro-mkscumm

[50J0uRNeR]

Oh nice!!! When I get home this evening I'll try it out. It looks like libretro_os.cpp is similar so I should be able to post a working patch there also (if it has the same cursor and white masking problems). Thanks!

[i30817]

As i mentioned, your patch fixes both the corrupt cursor and the font problems in 11th hour (and i think other games of the same engine) but not that black rectangle around the cursor. I think this might happen because the 11th hour cursor is animated, which is unusual, might be worthwhile to try to find another animated cursor game in scummvm to see if they have the same problem.

edit: although, there is no problem with the monkey island 'flashing' cursor, so i guess it's not that easy to find another example of the same problem.

[i30817]

This was fixed on the fork.