-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
WIP: Nintendo Switch (libnx) Port - Reference PR #12323
base: master
Are you sure you want to change the base?
Conversation
Pushed my rebased branch, still in testing but so far no regressions, will push it to my other branches later on. |
Just a nitpick/annoyance, I saw comments that some of that code was copied from elsewhere, but could you fix the indent? We use tabs in this project. Also place curly braces on the end of the line instead of a new line please. It's pretty ugly to read stuff when parts of the same code are using different style. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like there are multiple cores updating the libretro headers. Maybe it'd be best to update them once in a separate pull?
-[Unknown]
UI/NativeApp.cpp
Outdated
g_Config.memStickDirectory = config + "/ppsspp/"; | ||
g_Config.flash0Directory = File::GetExeDirectory() + "/assets/flash0/"; | ||
g_Config.memStickDirectory = g_Config.internalDataDirectory + config + "/ppsspp/"; | ||
g_Config.flash0Directory = g_Config.internalDataDirectory + "assets/flash0/"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes shouldn't be needed (or the above), since it should be using the #elif PPSSPP_PLATFORM(SWITCH)
path on switch.
-[Unknown]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I can double check this.
Common/CommonFuncs.h
Outdated
@@ -31,6 +31,8 @@ | |||
|
|||
#if defined(_M_IX86) || defined(_M_X86) | |||
#define Crash() {asm ("int $3");} | |||
#elif defined(HAVE_LIBNX) | |||
#define Crash() { while(1); }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, this seems decidedly not a crash... can we just use like *(int *)0 = 0x1337;
?
-[Unknown]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can also install a full exception handler if needed + do a context restore, got it all ready in another project.
But yes, we can even throw a fatal, but that would cause a full reboot.
Yea it caused me some hassle already, I still have some stupid changes in there I think that are just redundant, need to take a look on that. Not sure which of these are even required anymore. |
@unknownbrackets just fyi pushed my rebase, I left extra commits in for now so we can see some relevant changes, will squash it the next time, ignore the gitlab file for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looked for a reason for vertexjit crashes, didn't see one.
Are ALL_CALLEE_SAVED / ALL_CALLEE_SAVED_FP correct on Switch?
-[Unknown]
SDL/SDLMain.cpp
Outdated
@@ -555,7 +564,7 @@ int main(int argc, char *argv[]) { | |||
if (mode & SDL_WINDOW_FULLSCREEN_DESKTOP) { | |||
pixel_xres = g_DesktopWidth; | |||
pixel_yres = g_DesktopHeight; | |||
g_Config.bFullScreen = true; | |||
g_Config.bFullScreen = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any specific reason to force this false?
-[Unknown]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cant remember, mightve been smth with docked maybe
@@ -7,7 +7,7 @@ | |||
|
|||
#ifdef __ANDROID__ | |||
#include "Common/GL/GLInterface/EGLAndroid.h" | |||
#elif PPSSPP_PLATFORM(SWITCH) | |||
#elif defined(HAVE_LIBNX) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intentional? Any specific reason not to use PPSSPP_PLATFORM(SWITCH)
?
-[Unknown]
UI/GameSettingsScreen.cpp
Outdated
@@ -1164,8 +1165,10 @@ void GameSettingsScreen::onFinish(DialogResult result) { | |||
KeyMap::UpdateNativeMenuKeys(); | |||
|
|||
// Wipe some caches after potentially changing settings. | |||
#ifndef HAVE_LIBNX |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was there a problem this caused?
This will prevent some features from working, like post-processing shaders and certain settings changes.
-[Unknown]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIrc some stuff triggered a context reset(?) and that caused a crash
#include <stdio.h> | ||
#include <cstdio> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So using cstudio
doesn't work? I guess we should just only do stdio.h
then.
-[Unknown]
Pushed WIP rebase, not much cleanup done yet, only the most important ones. Most critical issue on this one is we need to deal with the detach in ThreadManager::EnqueueTask |
292913a
to
785bdc7
Compare
[Sqash me] Fixes after rebase [Squash me] Revert reporting disable for standalone [Squash me] Change GLInterface macro define for libretro purpose Rework JIT Integration/Masking -Needs more changes/reverts later on, WIP WIP fixes after rebase, confirmed partially working at least More safe buffer sizes Make it link after rebase gl3stub not required for switch Note: glBindFragDataLocationEXT, glGetProgramResourceLocationIndexEXT, glGetFragDataIndexEXT don't get currently populated Dirty fix libnx libretro core Compile and link after rebase for libretro Compile and link after rebase for standalone Fixes after rebase [CI/CD] Libretro CI only for Switch [CI/CD] Makefile path fix Fixes after rebase Split off libretro-common for Switch [Libnx] Fixes after rebase [Libnx] Remove leftover Rebase fixes Critical mitigation, cant call detach Fixes after first batch of PR's No longer required
Review commit: m4xw@871d5de |
After talking with @hrydgard, we decided I should open the PR already (also with 1.9.0 around the corner).
There are still quite a few Issues with the code that I didn't clean up yet.
I am aware of almost all issues.
This is still in a WIP state since I kinda got sidetracked with the standalone port when I started the cleanup.
A minimal libretro common is added, it can be cut even further, in the end I just needed a few headers but not the full statemachine (glsm). Ignore that for now.
Also the gl_common stuff was generated with some notepad++ macros.
I made a branch with the squashed history of this PR and libretro-common ripped out for easy review here: m4xw@7ff8d94
The current rebased changes (today) seem to run fine but further testing is needed from my part.
Total relevant changes ~800loc.
Don't merge it anytime soon, also I might swap the branch later to "switch", which currently still has non-squashed changes)