libretro / RetroArch

Cross-platform, sophisticated frontend for the libretro API. Licensed GPLv3.
http://www.libretro.com
GNU General Public License v3.0
10.08k stars 1.81k forks source link

Graceful switching between video drivers [$165] #4804

Closed hizzlekizzle closed 4 years ago

hizzlekizzle commented 7 years ago

First and foremost consider this:

Description

RetroArch currently behaves unpredictably and unstably when switching to cores that want a context other than what is currently active. This can happen because of video_driver settings being different in a core config override or because a core's core options are telling it to use a different renderer than is active (e.g., GL vs Vulkan)

Expected behavior

One would expect RetroArch to switch to the appropriate video_driver for the core, which the core can request, even if that means tearing the context down completely and rebuilding.

Actual behavior

GL->Vulkan with soft rendered core: Works, crashes when toggling the menu.

#0  gl_raster_font_render_line (font=0xeb5a1f0, msg=<optimized out>,
    msg_len=<optimized out>, scale=1, color=0x45cebc0, pos_x=inf,
    pos_y=-nan(0x400000), text_align=0)
    at gfx/drivers_font/gl_raster_font.c:339
#1  0x0000000000000000 in ?? ()
Backtrace stopped: previous frame inner to this frame (corrupt stack?)

GL->Vulkan with HW mednafen psx: Appears to work, loads correctly, the core loads and game loads too but I think it's just falling back to GL. Slang shaders don't work but CG shaders do

GL->Vulkan with HW parallel: Crashes

[Switching to thread 1 (Thread 5280.0xf4c)]
#0  RDP::begin_frame () at mupen64plus-video-paraLLEl/rdp.cpp:131
131     in mupen64plus-video-paraLLEl/rdp.cpp

Vulkan->GL anything Crashes right away

Thread 1 received signal SIGSEGV, Segmentation fault.
[Switching to Thread 7256.0x32e8]
vulkan_create_texture (vk=0x6c1d020, old=old@entry=0x0, width=544,
    height=544, format=format@entry=VK_FORMAT_R8_UNORM, initial=0x1346baa0,
    swizzle=swizzle@entry=0x0, type=type@entry=VULKAN_TEXTURE_STATIC)
    at gfx/common/vulkan_common.c:293
293        VkDevice device                      = vk->context->device;
(gdb) bt
#0  vulkan_create_texture (vk=0x6c1d020, old=old@entry=0x0, width=544,
    height=544, format=format@entry=VK_FORMAT_R8_UNORM, initial=0x1346baa0,
    swizzle=swizzle@entry=0x0, type=type@entry=VULKAN_TEXTURE_STATIC)
    at gfx/common/vulkan_common.c:293
#1  0x000000000050534d in vulkan_raster_font_init_font (data=0x6c1d020,
    font_path=0x45c8740 "userdata\\fonts\\font.ttf", font_size=32)
    at gfx/drivers_font/vulkan_raster_font.c:69
#2  0x000000000044e002 in gl_font_init_first (font_size=<optimized out>,
    font_path=<optimized out>, video_data=<optimized out>,
    font_handle=<optimized out>, font_driver=<optimized out>)
    at gfx/font_driver.c:137
#3  font_init_first (api=FONT_DRIVER_RENDER_VULKAN_API, font_size=32,
    font_path=0x45c8740 "userdata\\fonts\\font.ttf", video_data=0x6c1d020,
    font_handle=0x45c8688, font_driver=0x45c8680) at gfx/font_driver.c:282
#4  font_driver_init_first (video_data=0x6c1d020,
    font_path=font_path@entry=0x45c8740 "userdata\\fonts\\font.ttf",
    font_size=font_size@entry=32, threading_hint=threading_hint@entry=true,
    api=api@entry=FONT_DRIVER_RENDER_VULKAN_API) at gfx/font_driver.c:381
#5  0x000000000061edf4 in menu_display_vk_font_init_first (
    font_handle=font_handle@entry=0x45c8738, video_data=<optimized out>,
    font_path=font_path@entry=0x45c8740 "userdata\\fonts\\font.ttf",
    font_size=font_size@entry=32)
    at menu/drivers_display/menu_display_vulkan.c:258
#6  0x00000000004bb904 in menu_display_font_main_init (
    font=<synthetic pointer>) at menu/menu_display.c:225
#7  menu_display_font (
    type=type@entry=APPLICATION_SPECIAL_DIRECTORY_ASSETS_XMB_FONT,
    font_size=32) at menu/menu_display.c:215
#8  0x0000000000487c19 in xmb_context_reset (data=0x12c005a0)
    at menu/drivers/xmb.c:3308
#9  0x000000000048d09b in menu_driver_ctl (
    state=state@entry=RARCH_MENU_CTL_CONTEXT_RESET, data=data@entry=0x0)
    at menu/menu_driver.c:981
#10 0x000000000042d4c8 in init_drivers (flags=<optimized out>) at driver.c:351
#11 driver_ctl (state=<optimized out>, data=<optimized out>) at driver.c:466
#12 0x000000000042d3f7 in driver_ctl (
    state=state@entry=RARCH_DRIVER_CTL_INIT_ALL, data=data@entry=0x0)
    at driver.c:472
#13 0x0000000000405105 in retroarch_main_init (argc=0, argv=0x0,
    argv@entry=0x45cc280) at retroarch.c:1083
#14 0x00000000004152f2 in content_load (info=0x45cd370)
    at tasks/task_content.c:280
#15 task_load_content (content_info=content_info@entry=0x45cd3f0,
    launched_from_menu=launched_from_menu@entry=false,
    mode=CONTENT_MODE_LOAD_CONTENT_FROM_PLAYLIST_FROM_MENU)
    at tasks/task_content.c:902
#16 0x00000000004161b9 in command_event_cmd_exec (
    mode=CONTENT_MODE_LOAD_CONTENT_FROM_PLAYLIST_FROM_MENU,
    data=0x12e8dad0 "D:\\PortableData\\GameData\\EmulatorData\\Games\\Console\\Super Nintendo Entertainment System\\Virtual Bart (USA).zip")
    at tasks/task_content.c:1016
#17 task_push_content_load_default (
    core_path=core_path@entry=0x45cd500 "D:\\PortableData\\GameData\\Emulators\\RetroArch\\libretro\\bsnes_balanced_libretro.dll",
    fullpath=0x12e8dad0 "D:\\PortableData\\GameData\\EmulatorData\\Games\\Console\\Super Nintendo Entertainment System\\Virtual Bart (USA).zip",
    content_info=content_info@entry=0x45ce500,
    type=type@entry=CORE_TYPE_PLAIN,
    mode=mode@entry=CONTENT_MODE_LOAD_CONTENT_FROM_PLAYLIST_FROM_MENU,
    cb=cb@entry=0x0, user_data=user_data@entry=0x0)
    at tasks/task_content.c:1154
#18 0x00000000004a67b8 in generic_action_ok_file_load (
    content_enum_idx=CONTENT_MODE_LOAD_CONTENT_FROM_PLAYLIST_FROM_MENU,
    action_type=CORE_TYPE_PLAIN, fullpath=<optimized out>,
    corepath=0x45cd500 "D:\\PortableData\\GameData\\Emulators\\RetroArch\\libretro\\bsnes_balanced_libretro.dll") at menu/cbs/menu_cbs_ok.c:867
#19 action_ok_playlist_entry_collection (path=<optimized out>,
    label=<optimized out>, type=<optimized out>, idx=<optimized out>,
    entry_idx=0) at menu/cbs/menu_cbs_ok.c:1095
#20 0x00000000004a072a in menu_entry_action (entry=entry@entry=0x45cf620,
    i=<optimized out>, action=action@entry=MENU_ACTION_OK)
    at menu/widgets/menu_entry.c:523
#21 0x00000000004c7d4b in generic_menu_iterate (data=0x12bfc190,
    userdata=<optimized out>, action=MENU_ACTION_OK)
    at menu/drivers/menu_generic.c:233
#22 0x000000000048de39 in menu_driver_ctl (
    state=state@entry=RARCH_MENU_CTL_ITERATE, data=data@entry=0x45cfc60)
    at menu/menu_driver.c:888
#23 0x0000000000412779 in runloop_check_state (
    settings=settings@entry=0x71fb040, current_input=current_input@entry=1,
    old_input=old_input@entry=0, sleep_ms=0x45cfde0) at runloop.c:888
#24 0x0000000000412fa9 in runloop_iterate (sleep_ms=sleep_ms@entry=0x45cfde0)
    at runloop.c:1203
#25 0x00000000004016c0 in rarch_main (argc=<optimized out>,
    argv=<optimized out>, data=0x0) at frontend/frontend.c:130
#26 0x00000000006691f8 in main_getcmdline ()
#27 0x00000000004013f8 in __tmainCRTStartup ()
    at C:/repo/mingw-w64-crt-git/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:334
#28 0x00000000004014eb in WinMainCRTStartup ()
    at C:/repo/mingw-w64-crt-git/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:184

Steps to reproduce the bug

  1. Set video driver to "gl"
  2. Load and run a core that has no core override
  3. Enter menu
  4. Change "video driver" setting to "vulkan"
  5. Save core override from quick menu
  6. Close RA
  7. Run RA and check menu to verify "video driver" is set to "gl". If not, set to "gl" and close RA then run RA again
  8. Load and run the core you saved the core override for earlier
  9. Try to enter menu

Bisect Results

always

Version/Commit

You can find this information under Information/System Information

all versions

Environment information

Any

This issue combines the complaints and desired functionality from both: https://github.com/libretro/RetroArch/issues/3297 and https://github.com/libretro/RetroArch/issues/4588

--- There is a **[$25 open bounty](https://www.bountysource.com/issues/43877115-graceful-switching-between-video-drivers?utm_campaign=plugin&utm_content=tracker%2F296058&utm_medium=issues&utm_source=github)** on this issue. Add to the bounty at [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F296058&utm_medium=issues&utm_source=github).
orbea commented 4 years ago

are you on RetroArch discord server?

Never. You can contact me on irc or we can discuss it here.

I tried the diff and now RA + dolphin aborts when closing content. The crash when loading from a playlist seems unchanged.

corrupted size vs. prev_size while consolidating
Aborted

log: ra_dolphin_x.log

Rinnegatamante commented 4 years ago

I definitely need to setup a linux partition to test this. I've added some logging to see what's going wrong in x context reinitialization (since that seems to be the issue). Can you send log with this diff? x.zip

orbea commented 4 years ago

log: ra_dolphin_x-2.log

Rinnegatamante commented 4 years ago

Can you remove the previous diff? (the one adding that destroy call.)

orbea commented 4 years ago

log: ra_dolphin_x-3.log

klepp0906 commented 4 years ago

was this pushed into the nightlies? Im having the weirdest issue ever crop up where psx hw is now forcing my video driver to vulkan for no good reason ;p

happens with no other cores, theres no override set, yet i load with open GL, back out to the menu, and my video driver is set to vulkan /boggle

when i close the game/content, its back to ogl again.

Rinnegatamante commented 4 years ago

@klepp0906 that's the video driver force code in place. I discussed with themaister and twinaphex a solution for it and we came out to a replacement to fix this. However will probably be a follow up to this issue since not completely related to this.

@orbea https://github.com/libretro/RetroArch/pull/9591 I've updated my PR. This solved the crashing issues on my Ubuntu partition. Can you check it works for you too?

klepp0906 commented 4 years ago

ah, thank you for clarifying. It was largely a cause for concern due to timing. I play 99% of my retroarch remotely via steam streaming and anything vulkan used to result in a black screen. Fortunately I found a workaround for that (just yesterday), however I didnt intend to use vulkan in case the issue cropped back up, or if i had to for some other reason.

Thats when I noticed i couldnt get it switched off vulkan now even if i wanted to.

At least now i know its nothing on my end. Appreciate your work on this :)

orbea commented 4 years ago

However will probably be a follow up to this issue since not completely related to this.

This is 100% related to this issue and must be fixed before this issue is closed. The solution is to just remove the relevant code like I told you. Frankly it would be better to revert the code that fixed that misfeature than it would be to leave it in place.

Your PR causes an build issue.

retroarch.c:7836:9: error: use of undeclared identifier 'RETRO_ENVIRONMENT_GET_PREFERRED_HW_RENDER'
          case RETRO_ENVIRONMENT_GET_PREFERRED_HW_RENDER:
               ^
1 error generated.
make: *** [Makefile:213: obj-unix/release/retroarch.o] Error 1
make: *** Waiting for unfinished jobs....
inactive123 commented 4 years ago

@orbea Rome wasn't built in a day. This is a necessary intermediary step we have to take in order to move forward.

I think @Rinnegatamante can address that compilation error though, but other than that we have to start somewhere when it comes to this issue.

Rinnegatamante commented 4 years ago

You can either comment out that switch case or update libretro-common with the PR i made. As for the force driver feature, it won't be removed, admins wanted to go this way and you're not in charge to take designs choices here since this is not your project @orbea . If you want to keep having this passive aggressive attitude towards me i'm just going to ignore your messages straight away since i've no time to waste in meaningless discussions.

inactive123 commented 4 years ago

Guys, let's not have this lead to a major altercation here. We have to deal with a codebase full of legacy code and certain drastic decisions have to be made in order to move forward. Please let's not have this lead to even more arguments, it's already hard enough rewriting this code to begin with.

Let's not have every minor disagreement lead to some standoff.

orbea commented 4 years ago

@Rinnegatamante I'm not sure what you are getting at, but this is not a passive aggressive attitude, its just being blunt that this is a serious issue that needs to be fixed and is directly related to this issue and your PR. Not being able to choose the desired video driver even makes it impossible for developers to test their code.

@twinaphex I agree, it can be addressed in multiple PRs, but it really should be addressed in only one issue (This one).

inactive123 commented 4 years ago

@orbea OK, then let's just focus on this issue that needs to be addressed before things go further out of hand. I don't want any more blowups. Just state what should be fixed and how and we can put this situation to bed.

orbea commented 4 years ago

OK, then let's just focus on this issue that needs to be addressed before things go further out of hand. I don't want any more blowups

100% agreed, this can be taken one step at a time. I'll be here to test as long as someone more capable is willing to spend time on this.

Rinnegatamante commented 4 years ago

The issue has already been fixed, that "case" you get in your building process error is related to the solution getting in place. You can disable that portion for now to actually test the linux fix.

The solution will require work on cores side and this is the solution we decided to adopt accordingly with whom is in charge to design RetroArch general way of working.

orbea commented 4 years ago

The issue has already been fixed, that "case" you get in your building process error is related to the solution getting in place. You can disable that portion for now to actually test the linux fix.

I have no idea what you mean here?

The solution will require work on cores side and this is the solution we decided to adopt accordingly with whom is in charge to design RetroArch general way of working.

This is a frontend issue, not a core issue. To be entirely clear not being able to select the desired video driver in the frontend is a blocker for core development and just a very bad idea.

If you want you can add an option to automatically switch video drivers based on the core under the video settings, but it should not be on by default and is not a requirement for this issue. Its much more likely people will be asking why they can't use opengl in beetle-psx than how to get the core to select the desired video driver automatically. And core/content overrides should always take precedence.

Rinnegatamante commented 4 years ago

@orbea https://github.com/libretro/beetle-psx-libretro/pull/546#event-2714500394 is this good enough for you to understand it requires work on cores side or are you going for long time still to rebut on everything someone tells you?

As for the build error you were having, what i meant is that you could've disabled what the last commit added cause it was related to implementing what's required for RetroArch to handle the driver negotiation with cores. Btw i've pushed another commit that will solve the error straight away.

orbea commented 4 years ago

@Rinnegatamante No its not, new frontends should not break old cores, this is an API change. The bottom line is that this should not force the video driver for any core except maybe as an optional feature or when the video driver is outright not supported (Using vulkan on an opengl only core). Again I stress this is a deal breaker for core development and for at least some users.

I would suggest to squash your PRs so that they only contain working commits, it really matters when it comes to bisecting.

I tested your PR and now it crashes when closing content with dolphin-emu when loading content from the playlist or command-line. I can't start the core in gdb anymore either where it crashes with a garbage backtrace.

Rinnegatamante commented 4 years ago

Retroarch logs for the crashes? Drivers set before loading content and in dolphin?

undeadindustries commented 4 years ago

I agree that the functionality should be: Only switch driver if core does not support current loaded driver.

Rinnegatamante commented 4 years ago

@undeadindustries That's what the API change we did does. Btw i don't want to keep talking about this anymore. If you have any further bitching on the subject, join Discord server and contact themaister or twinaphex and bitch with them.

jvook commented 4 years ago

Why you yelling at that random guy?

On Tue, Oct 15, 2019, 4:51 PM Rinnegatamante notifications@github.com wrote:

@undeadindustries https://github.com/undeadindustries That's what the API change we did does. Btw i don't want to keep talking about this anymore. If you have any further bitching on the subject, join Discord server and contact themaister or twinaphex and bitch with them.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/libretro/RetroArch/issues/4804?email_source=notifications&email_token=AEDNLU27MJPSJ6EAU5H5BODQOYULNA5CNFSM4DGYXJXKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBKFT2A#issuecomment-542398952, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEDNLU2W336HYT54NUFWZY3QOYULNANCNFSM4DGYXJXA .

undeadindustries commented 4 years ago

@Rinnegatamante Ooook, I guess I didn't understand that. I wasn't bitching.

@jvook Also, not a random guy. I put in a lot of the bounty ;-)

inactive123 commented 4 years ago

I'm locking this thread for now since obviously people can't behave once again and this kind of behavior steers away promising new contributors.

I will reopen it again in a few days after people have had a cooldown period.

Guys, we all need to play ball here if good things are to happen. If we say to let a situation rest and that it will be addressed, we ask that you please do so. Continuous filibustering is not a good idea especially when it's so easy for things to go from 0 to 11 here. Please dont' make my job as a moderator here unnecessarily harder and let's have this be a hospitable place for all devs.

inactive123 commented 4 years ago

I believe @Rinnegatamante's solution is quite satisfactory now and the remaining big issues have been sorted out with it. Closing this issue so that the bounty can be collected.