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

libretro: Add the initial libretro implementation. #10407

Closed
wants to merge 6 commits into from

Conversation

orbea
Copy link
Contributor

@orbea orbea commented Dec 13, 2017

Hi,

I was told that @hrydgard is willing to consider supporting the libretro implementation in upstream ppsspp if a PR could be made. So here is the first step, with this ppsspp mostly works in linux (All I can test...) with a libretro frontend such as RetroArch. It is important to remember this is not so much about supporting RetroArch, but supporting the libretro ecosystem which has the benefit of supporting libretro frontends which could also include gnome-games or kodi (I'm not sure how far along they are in supporting libretro yet?). In this context libretro should be viewed as just another platform to port ppsspp to.

Also to use the libretro port the contents of the assets/ directory in the ppsspp source tree is required to be placed in the libertro assets/bios directory as PPSSPP. By default I think this is the content directory, but I have changed it in RetroArch to ~/games/bios/ here.

The main reasons to include this in upstream are:

  1. It helps foster community cooperation which can only mean more people working on a single code base.
  2. It greatly reduces the difficulty and burden of maintaining the libretro implementation which will not have to play nearly as much catch up with upstream.
  3. It offers the potential to improve the usefulness of ppsspp to its users who now have another way to use and enjoy ppsspp.

I made this PR in several steps to help the review process. The commits are as follows.

  1. Adds the glew_libretro.c which is only used for the libretro port.
  2. Adds the initial libretro implementation which includes the static Makefile preferred for building libretro cores.
  3. Adds a build path for libretro to CMakeLists.txt which will be easier to maintain and might be preferred by some people. However it should be noted that a few people using older systems have informed me they can not use this due the cmake version requirements and the static Makefile is there for them. The cmake build can be invoked with: mkdir build ; cd build; cmake -DLIBRETRO=ON .. && make
  4. Adds linux builds for the libretro port to travis to help reduce regressions. See this PR. https://github.com/libretro/ppsspp/pull/23
  5. Adds a potential fix for an issue triggered by recent changes to upstream code that caused any content with an existing texture cache to just exit. While it does work for me, I am not sure this fix is ideal and I really could use help improving it if need be.

Disclaimer: I am not a developer by training and while I am more than willing to help where I can, this is mostly reduced to trivial issues and debugging. As a result there are still several blocking issues which really should be resolved, but which I have not been able to do so. It would really make my day if someone could step in and help finish this PR.

  1. Context reset is broken which means changing from full screen to windowed or vica versa will only display garbage until the content is closed and started again, See https://github.com/libretro/ppsspp/issues/27.
  2. Tekken 6 does seems to freeze during the boot process and never boots.
  3. Vulkan is probably not hooked up. As I do not have any vulkan capable devices I can not help here.
  4. Several potentially useful ppsspp options might not be hooked up, but unfortunately those who have worked on the ppsspp core are not familiar with what options should be included...

Other issues which should be fixed, but are not as severe.

  1. Crashes if fastmem is turned off, this crash seems to be entirely in upstream code and may not be something wrong in the libretro implementation? See https://github.com/libretro/ppsspp/issues/32
  2. Crashes if RetroArch has ASAN/UBSAN enabled. See https://github.com/libretro/ppsspp/issues/12
  3. Crashes when opening a directory accidentally. See https://github.com/libretro/ppsspp/issues/33

There are other issues, but they can be addressed in the following discussion.

Also pinging @twinaphex so he can help help with the discussion and troubleshooting if possible.

@hrydgard
Copy link
Owner

hrydgard commented Dec 13, 2017

What's the deal with glew_libretro, why is it required?

I wonder if we could avoid having yet another build system - since you are adding cmake stuff, could we skip libretro/Makefile.common ?

The commit message on the last commit is a bit odd.

As for the PSP_Init stuff, you need to first call PSP_Init, then call IsInited/PSP_InitUpdate in a loop (or once per frame so we can show progress) until it PSP_InitUpdate returns true. If this isn't done and there's a shader cache to load, it won't init properly.

@orbea
Copy link
Contributor Author

orbea commented Dec 13, 2017

glew_libretro.c is explained here by @twinaphex.
https://github.com/libretro/ppsspp/pull/25#issuecomment-345566737

I have a strong preference for the static makefile and if possible I would like to be able to use it in the future. Additionally I think some libretro build environments may not have access to cmake at all? I suspect it will end up maintained either way so the choice is yours.

The last commit may not be ideal and is separate for that reason. With my at best limited understanding of c/c++ I have not been able to come with a better solution despite a lot of fruitless effort... I would really appreciate if someone could show how it could be better. :)

@orbea
Copy link
Contributor Author

orbea commented Dec 14, 2017

I rebased this against master to include fix for the crash with fast memory disabled thanks to @hrydgard. That is one issue down. :)

@Orphis
Copy link
Collaborator

Orphis commented Dec 14, 2017

There's no such thing as an environment that CMake can't target. You aren't building on the target system most of the time but crosscompiling, so it should be fine, they just need to make some proper CMake toolchain files for each system.

@hrydgard
Copy link
Owner

I'm working on hooking up Vulkan on SDL/X11 (by grabbing the X11 window handle and initializing Vulkan into it), how does retroarch do it or does it leave it up to the cores?

@inactive123
Copy link
Contributor

inactive123 commented Dec 14, 2017

Hi @hrydgard,

regarding your Vulkan question, I can refer you to our Vulkan libretro samples -

https://github.com/libretro/libretro-samples/tree/master/video/vulkan

There are two examples here - one using async compute from within a (self-contained) libretro core, and the other doing plain rendering of a triangle. Both of these examples can easily be compiled as a standalone libretro core, and then run inside RetroArch. They are both small examples, there is only one relevant source file, the other file is just a symbol wrapper which you can probably leave identical.

Let me know any questions you have about this and I will try to forward these to @Themaister (the one who implemented the Vulkan backend) so he can hopefully provide you with some answers to your questions.

@Themaister
Copy link

Themaister commented Dec 14, 2017

Hi, stumbled upon this. Well, so the basic gist is that the core can create the device on its own if you need multiple queues, etc (async compute is the prime case here). You also have the opportunity to enable extensions.

The frontend owns the swapchain and the instance. There is an interface for a core to send completed frames to the frontend, using semaphores for managing dependencies (or just pipeline barriers if single-queue is used). It is fairly easy to abstract a Vulkan swapchain on top of this system. It is up to the core to create these images. For each frame, the core can ask for the current swapchain index to pump sync like you would with VkSwapchainKHR.

The images you send to frontend will either be scaled to screen, go through shader subsystem, etc. The core does not deal directly with a swapchain at the very least.

Here's another example of how it can be integrated in a more complex renderer: https://github.com/Themaister/Granite/blob/master/application/platforms/application_libretro_utils.cpp
https://github.com/Themaister/Granite/blob/master/application/platforms/application_libretro.cpp

@orbea
Copy link
Contributor Author

orbea commented Dec 24, 2017

Sorry for the silence, several things came up here that are time consuming and distracting, but I do want to see this PR through to the end. :)

First I rebased against master and encountered no new issues.

Second, as I have very limited C/C++ experience and knowledge some of these issues I simply don't know how to handle. It would be very nice if I could get some hands on help here. :)

@Orphis I am not here to debate with you. :) I added cmake because someone went through effort to implement it and I didn't want their work to be lost, but I still think for libretro purposes the static makefile just fits better. However if you want to spend time getting cmake to work with various consoles I won't complain. :)

@orbea
Copy link
Contributor Author

orbea commented Jan 4, 2018

@hrydgard

Do you think this is a better way to do the PSP_Init? It "works", but I still don't really know what I'm doing. :)

diff --git a/libretro/libretro.cpp b/libretro/libretro.cpp
index 320d4c79c..b78fda3b7 100644
--- a/libretro/libretro.cpp
+++ b/libretro/libretro.cpp
@@ -1184,11 +1184,22 @@ void retro_run(void)
 	      bool success = libretro_draw->CreatePresets();
 	      assert(success);
 
-	      if(!PSP_Init(coreParam, &error_string))
+	      bool bootPending_ = !PSP_Init(coreParam, &error_string);
+
+	      if(PSP_IsIniting())
 	      {
-		      if (log_cb)
-			      log_cb(RETRO_LOG_ERROR, "PSP_Init() failed: %s.\n", error_string.c_str());
-		      environ_cb(RETRO_ENVIRONMENT_SHUTDOWN, nullptr);
+		     bootPending_ = !PSP_InitUpdate(&error_string);
+
+		     if(!bootPending_)
+		     {
+			_initialized = !PSP_IsInited();
+			if (_initialized)
+			{
+				if (log_cb)
+					log_cb(RETRO_LOG_ERROR, "PSP_Init() failed: %s.\n", error_string.c_str());
+				environ_cb(RETRO_ENVIRONMENT_SHUTDOWN, nullptr);
+			}
+		     }
 	      }
 
 	      host->BootDone();

@orbea
Copy link
Contributor Author

orbea commented Jan 23, 2018

I rebased it again, it still "works" here.

@orbea
Copy link
Contributor Author

orbea commented Jan 23, 2018

I added one more commit which implemented the above diff (Mostly) which I'm told looks alright in #retroarch. I'd like to hear that confirmed from ppsspp devs too though? It still works here...

@webgeek1234
Copy link
Contributor

I'll just throw this out for reference. It hasn't been rebased in a while, but I've got a less invasive implementation that only uses the cmake build system. Tested against Linux and Android. Don't have Windows or OSX to test them, however. The last two commits here would need folded in, however. I put in some effort to make sure cmake works with the libretro buildbot and switched builds for the above platforms over to using it. So if someone could test the remaining platforms, for purely release purposes, only the cmake scripts would be needed.

https://github.com/webgeek1234/ppsspp/commits/master

Also, correct that vulkan is not hooked up. When I wrote the CMakeLists for the libretro port, I couldn't figure out how to hook it up. But I did make sure to compile all the upstream files, though I did have to make a change to get the vulkan files to compile into a static lib.

@orbea
Copy link
Contributor Author

orbea commented Jan 25, 2018

@webgeek1234 Is cmake the preferred build system for android? After my previous rebase it became clear that android was creating broken builds with cmake. I then noticed that PPSSPP is using their JNI Android.mk and not cmake for their own builds and says cmake may not be supported for android in the github wiki. For now we got it working with the old jni android makefiles, but its not ideal. Perhaps libretro support should be added to the upstream jni makefiles? Can you help with this issue?

Additionally osx and windows no longer build with the static makefile, but I do not have those platforms either.

I will agree that the Makefile needs work beyond what I can provide, but its still the highly preferred for libretro devs to support it. Is it really so bad to have both?

@webgeek1234
Copy link
Contributor

Cmake worked fine for the android target when I added in libretro support to the scripts. And when I switched the libretro buildbot over. I used a few builds off the bot and they were working just fine. Put several hours into Star Ocean 2, no problems. That's changed?

My thing about multiple build systems is extra effort. Upstream here has cmake, why try to force them to maintain something else, or carry something that will break every time they make a build system change?

@orbea
Copy link
Contributor Author

orbea commented Jan 25, 2018

Yes, cmake does not work with android for ppsspp libretro anymore. Upstream ppsspp is not using cmake for android either (See the build instructions on the wiki, travis.sh and android/ab.sh) and using cmake does not seem to be well maintained/supported (?). Should we add support to the upstream jni makefile, use libretroto's jni makefile or am I missing something here?

With cmake it will build successfully, but the cores do not start due to undefined references.

@Orphis
Copy link
Collaborator

Orphis commented Jan 25, 2018

CMake is the supported Android way to build. If it doesn't work for you and JUST you, file a bug, give more information so it can be fixed.
The other build systems are not really there to stay.

@orbea
Copy link
Contributor Author

orbea commented Jan 25, 2018

@Orphis That doesn't seem to match what is actually being used. Can you clear up these contradictions?

  • Why is travis.sh using android/ab.sh which is a jni build and not cmake?
  • Why does the wiki say this about building android with cmake?
This is not recommended and may not be maintained. Please use the method mentioned earlier.

https://github.com/hrydgard/ppsspp/wiki/Build-instructions

I don't have an android build environment, this is an issue with the libretro buildbot.

@webgeek1234
Copy link
Contributor

webgeek1234 commented Jan 25, 2018

I'm poking at getting cmake working again for libretro android. First thing is it looks like ndk r15c isn't supported anymore. I pulled r16b and got a lot closer, but got some pointer cast errors in the x64emitter atm.

Edit: Trying to build arm64-v8a, for reference. There are some void* cast to u32 that the compiler doesn't like.

@webgeek1234
Copy link
Contributor

armeabi-v7a upstream compiles, but fails linking as it trys to link in X11. My libretro source needs updated it appears, as gl_lost no longer exists. But it seems to compile other than that. arm64-v8a appears to be a complete mess and fails all over the place, though. I get a ton of various errors even trying to compile without libretro.

@orbea
Copy link
Contributor Author

orbea commented Jan 26, 2018

@webgeek1234 Thanks, its much appreciated. I don't have much of an opinion which build system android uses as long as upstream is willing to support it. That said I'm still not sure that cmake is well supported given the above contradictions...

@webgeek1234
Copy link
Contributor

Fixed and tested armv7. Needed an extra cmake flag to force neon on. Specifically -DCMAKE_ANDROID_ARM_NEON=ON . Looking to PR that back to the libretro buildbot in some form or fashion.
I'm not certain what's up with armv8. Getting loads and loads of errors. And that's the arch I actually use. ><

@Orphis
Copy link
Collaborator

Orphis commented Jan 26, 2018

@orbea It hasn't been updated. But official builds have been built with CMake and Gradle for a while now.

@hrydgard
Copy link
Owner

Yeah. Travis is not official builds, it's our buildbot to catch errors. Official builds use CMake. Could argue that we should use the same buildsystem everywhere, but for now it's still easy to maintain the old build system so to keep that running we've left Travis on it.

@orbea
Copy link
Contributor Author

orbea commented Jan 26, 2018

Alright, thanks for clearing that up. Would you mind updating the wiki to prevent further confusion? :)

@Orphis
Copy link
Collaborator

Orphis commented Jan 26, 2018

I think Google's NDK team fixed the issues downloading CMake a while ago, so we could update Travis probably.

@hrydgard
Copy link
Owner

The merge of the new GL threading support will likely have broken this and @webgeek1234 's branch mentioned above . Once I've managed to fix the other issues I'll help with fixing that up.

@orbea
Copy link
Contributor Author

orbea commented Feb 13, 2018

@hrydgard Thanks, that would be very much appreciated. I really like the idea of a libretro core, but unfortunately lot of the issues with it are beyond my current ability. I can fix minor issues sometimes, but always with significant effort...

@hrydgard
Copy link
Owner

This got replaced with #10780 .

@hrydgard hrydgard closed this Mar 24, 2018
@orbea orbea deleted the libretro branch March 24, 2018 16:50
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.

6 participants