libretro / dolphin

Dolphin is a GameCube / Wii emulator, allowing you to play games for these two platforms on PC with improvements.
https://dolphin-emu.org/
GNU General Public License v2.0
86 stars 68 forks source link

Update to upstream #319

Open zorn-v opened 11 months ago

zorn-v commented 11 months ago

Seems there some significant changes (removed bEMUThread option which is "false" in libretro core but "considered true" in current master) and we stuck in RunGpuLoop even with "Null" video driver

srk15372 commented 11 months ago

hi, thanks for this massive work. hope you could achieve this regards

1985a commented 11 months ago

could you please tell me what is the line command to compile this? I tried last night and there were many errors at my end.

zorn-v commented 11 months ago

I used such flags for build

mkdir build && cd build
cmake -DLIBRETRO=ON -DENABLE_X11=OFF -DENABLE_NOGUI=OFF -DENABLE_QT=OFF -DENABLE_ALSA=OFF -DENABLE_PULSEAUDIO=OFF \
    -DENABLE_TESTS=OFF -DUSE_DISCORD_PRESENCE=OFF -DENABLE_AUTOUPDATE=OFF -DENABLE_SDL=OFF -DENABLE_EVDEV=OFF -DENABLE_CLI_TOOL=OFF \
    ..
make -j$(nproc)
1985a commented 11 months ago

Thanks, I successfully compiled the core. Now I'm facing issues loading any content.

If I changed the video driver to vulkan, retroarch crash. If I changed to GL driver, it trows this error

My system is running Archlinux. The processor is a Ryzen 5600X The video card is a nVidia RTX 3700 Ultra

I reset the core config to avoid any bad behavior from the before one.

[INFO] [Config]: Looking for config in: "/home/1985a/.config/retroarch/retroarch.cfg".
GPU: OGL ERROR: Does your video card support OpenGL 2.0?
GPU: OGL ERROR: Does your video card support OpenGL 2.0?
GPU: OGL ERROR: Does your video card support OpenGL 2.0?
GPU: OGL ERROR: Does your video card support OpenGL 2.0?
Failed to initialize video backend!
Failed to create shared context for shader compiling.
Failed to create shared context for shader compiling.
Failed to create shared context for shader compiling.
Failed to create shared context for shader compiling.

With vulkan driver

[INFO] [Config]: Looking for config in: "/home/1985a/.config/retroarch/retroarch.cfg".
Failed to create Vulkan surface.
Failed to initialize video backend!
[1]    2857058 segmentation fault (core dumped)  retroarch --verbose
zorn-v commented 11 months ago

Idk, maybe wrong video driver ? Is standard dolppin emu run ok ?

1985a commented 11 months ago

Yeah, I was testing the standard emu, and it ran ok.

I don't know why it didn't on Retroarch.

RobLoach commented 10 months ago

Thanks for picking this up. Happy to test things out. Do you have an idea of what's needed to move it forwards? What's missing?

1985a commented 10 months ago

I don't know much about programming but this commit appears to be the culprit because the error I saw above https://github.com/zorn-v/dolphin-libretro/commit/89413e4172acefe795a565ec3816ef4d94c57af4#diff-de13bad525bb52a6234910293730c6e2c3e88caaa855d7a0c478d769a3b9034cR153

zorn-v commented 10 months ago

Thanks for picking this up. Happy to test things out. Do you have an idea of what's needed to move it forwards? What's missing?

Probably find all bEMUThread references in current core master branch and figure out how to resolve it without setting it to false (it is considered true now and cannot be changed). Maybe some little (or not) insert somewhere under #ifdef __LIBRETRO__ preprocessor condition. The ugly way - just return all if where it was (and return option to config) and see what happens. But it brings problems for future updates.

but this commit appears to be the culprit because the error I saw above

No, its not. It just fix linker errors about "undefined reference to" and uncomment some "deinit" in video context destroy (which is not called in your case). Does current libretro dolphine core runs ok in retroarch on your machine ? BTW, did you download "Dolphin.zip" in "Online updates -> Download core system files" ?

JesseTG commented 10 months ago

@zorn-v I see that you're in the Discord. I pinged you because you might want to hear what I said, but I'll reproduce it here:

JesseTG — Yesterday at 9:50 AM Let's see where it goes. He's taking a very different approach than I would; I'd embed Dolphin as a dependency instead of forking it, just like I'm doing with melonDS DS.

Updating the Dolphin version needs to be easy since it's rapidly evolving, and I'm not convinced that forking the repo would enable that. I tried. It became overwhelming, and did not solve any problems in the long run.

Contrast with melonDS DS; merge conflicts do not happen with upstream (since they're two separate codebases), and updating the melonDS version is as easy as changing a line in CMake. (Or even defining a variable on the command line, if you want to use another branch or fork.)

BiscuitCakes — Yesterday at 10:12 AM I like this approach a lot better too. The dolphin folks iterate too quickly to have a fork keep up with it without it being a full-time job image That said kudos to them for tackling it. It's sorely needed and if it works then I have no complaints lol

JesseTG — Yesterday at 10:56 AM Problem is that even if it works, it does not prevent the need for mega-PRs like this every six months.

@zorn_v My primary focus right now is on melonDS DS, but I'll tell you what; if you decide to refactor (or even remake) the Dolphin libretro core to use the upstream emulator as a dependency (as opposed to a fork), I will help you and contribute to maintenance.

That offer is 100% serious. If you're willing to work on something that won't need as much maintenance in the long run (at the cost of some front-heavy preparation and the need for upstream collaboration), then I can help you get it started.

zorn-v commented 10 months ago

I doubt that it is always possible. Some methods are private, some classes are final etc. Also some refactoring may performed (like this https://github.com/citra-emu/citra/commit/2bb7f89c3021e3d19b446ba6e816a2769dff3574) and you unable to do some things like you do it before (in that commit there is no global VideoCore::g_renderer anymore for example). Or like in dolphin, there is no bEMUThread config value anymore. I just suppose that it is the true culprit, but seems like that.

JesseTG commented 10 months ago

I doubt that it is always possible. Some methods are private, some classes are final etc.

Hence the need to send patches upstream. I've sent a ton to melonDS that were mutually beneficial (including to other non-libretro frontends).

Also some refactoring may performed (like this citra-emu/citra@2bb7f89) and you unable to do some things like you do it before (in that commit there is no global VideoCore::g_renderer anymore for example). Or like in dolphin, there is no bEMUThread config value anymore. I just suppose that it is the true culprit, but seems like that.

Better than dealing with massive merge conflicts, in my experience.

zorn-v commented 10 months ago

Hence the need to send patches upstream

And wait until they merged... or not.

Better than dealing with massive merge conflicts, in my experience.

Merge conflicts not so scary as behavior changes. And they helps to understand codebase by the way. In my experience :wink:

Just do minimum editing as possible of original code (and comment/do talking naming to parts that related to libretro) and all should be all right in future I suppose. In ideal world :smile:

1985a commented 10 months ago

BTW, did you download "Dolphin.zip" in "Online updates -> Download core system files" ?

Yeah, I downloaded that file from retroarch.

JesseTG commented 10 months ago

And wait until they merged... or not.

Then we annoy the hell out of work closely with the team to find a mutually-agreeable solution. It wouldn't be the first time; I'm stubborn as all hell, I'll make it happen.

Merge conflicts not so scary as behavior changes.

Those behavior changes would happen with or without merge conflicts. I'd argue that they're worse when dealing with merge conflicts, as an improper merge can cause behavior changes. If dealing with Dolphin as a dependency, you can at least be assured that behavior-changing updates don't come from the core's codebase.

Also, here's the kicker with embedding it as a dependency; you don't have to update to the latest upstream commit immediately. You can move it forward a few dozen commits at a time as needed.

And they helps to understand codebase by the way. In my experience 😉

So does embedding upstream as a dependency. I think we might have slightly different goals; my goal with melonDS DS was not to understand the upstream emulator. It just so happened to be required for what I really wanted to do (make a kickass, highly-polished libretro core).

Just do minimum editing as possible of original code (and comment/do talking naming to parts that related to libretro) and all should be all right in future I suppose. In ideal world 😄

This world is not ideal, and nothing about that million-and-a-half-line diff is minimal. That approach is what brought us a Dolphin core that breaks a large portion of the Wii/WiiWare library and needs an overhaul every year. It is not sustainable.

zorn-v commented 10 months ago

This world is not ideal

Yep

I really wanted to do (make a kickass, highly-polished libretro core).

I sincerely wish to make this world a little better :wink:

JesseTG commented 10 months ago

I sincerely wish to make this world a little better 😉

Me too. Let's do it in a way that lasts longer than a year and doesn't have to be redone by some other poor bastard. What do you say?

zorn-v commented 10 months ago

Unfortunately I does not have much free time now. Sometime work need to be worked )

j8r commented 10 months ago

Also, if a change is pending upstream or is refused for some reason, a patch can be added in the repository in order to modify the upstream project before compilation, without having to fork it. That's a common practice in the packaging world.

Of course, as already said above, better to have this patches upstreamed. Howerver, that's not always possible.

zorn-v commented 10 months ago

a patch can be added in the repository in order to modify the upstream project before compilation, without having to fork it

Hm, I forgot that it can be done like this. Well, agreed that using original project as dependency is better than fork for libretro core. It is like inheritance vs composition in OOP :smile:

But most worse is hard fork (like LRPS2) - you did not even know what commit it was be in original project to compare changes.

JesseTG commented 10 months ago

Unfortunately I does not have much free time now. Sometime work need to be worked )

All right, not a problem. I do have a question, though; if I were to start up a new Dolphin core after I finish my current one, would you be interested in contributing to it? There's another core I'm considering as well, but I can only devote my time to one major project.

JesseTG commented 10 months ago

Also, if a change is pending upstream or is refused for some reason, a patch can be added in the repository in order to modify the upstream project before compilation, without having to fork it. That's a common practice in the packaging world.

Even if you do ultimately have to fork upstream, this kind of separation makes it very easy to update your core. Just change a git hash in the submodules file, or CMakeLists.txt, or whatever. I don't just use my melonDS fork for contributions. When I want to push out a melonDS DS release while waiting for a PR or two to be reviewed, I temporarily change it to depend on the branches I need. Works like a charm!

Of course, as already said above, better to have this patches upstreamed. Howerver, that's not always possible.

In this case I tend to propose some kind of extension point or generalization, like breaking a class into smaller pieces. That way I can define the desired behavior on the "frontend" (the core). This is something you'd want to do even in a standalone frontend unrelated to libretro, anyway.

zorn-v commented 10 months ago

if I were to start up a new Dolphin core after I finish my current one, would you be interested in contributing to it?

Why not, if I can help. But for me it will be better to make Cemu core just because there is no such one. I know that code is pretty tied there (to gui, threads etc.) but all possible I think.

melonDS DS is seems like high-quality core (compared to original melonDS core where even no option to hide cursor and no upscale). But miss some pretty options like texture scale etc. I know that there is no such functions in standalone melonDS yet, but I stay with DeSmuME core for now, even it is significant slower.