search

flyinghead / flycast

Flycast is a multiplatform Sega Dreamcast, Naomi, Naomi 2 and Atomiswave emulator
GNU General Public License v2.0
1.3k stars 163 forks source link

[Libretro] Check for per-pixel compatibility and hide the option if not supported #1458

Closed bslenul closed 4 months ago

bslenul commented 4 months ago

Currently if the Libretro core is built with per-pixel support then the option will always be available to the user, even when using a renderer that doesn't support it (e.g. GLES). It's not rare to see people confused because they have glitchy eyes in Sonic or Shenmue for example while they properly have "Per-Pixel" selected in the core options, because it silently falls back to per-triangle without any indication.

This PR will check if the current renderer supports per-pixel by using the hasPerPixel() function (I had to add it for VK since it didn't exist for the Libretro core), if it's not supported then the 2 values here will be nulled out: https://github.com/flyinghead/flycast/blob/056a02357d46886f42249407e83bf598b599ee4d/shell/libretro/libretro_core_options.h#L349 so it doesn't appear, and a "Current renderer does not support 'Per-Pixel' Alpha Sorting." message will be printed in logs.

As we discuss on Discord, the VK changes might be undeed because it's pretty rare that it's not compatible, I'll remove these changes if you want, please let me know. Other than that, does it look OK? I'm pretty limited with the devices I can test this, I only have a Windows PC and an old Android 32bit phone but AFAICT it works fine, the option is properly hidden on my phone that only supports GLES for example. I'd just like to make sure there's no way the option ends up being hidden when it's supported.

flyinghead commented 4 months ago

When per pixel is selected, it is not enabled at start up. The regular per triangle/per strip renderer is used instead.

This is because you put the renderer selection code in update_variables() to only run if !first_startup. This code is needed to select the correct renderer when loading a game. However the graphics context hasn't been created yet, so you can't check if per pixel is supported at this point. However you can assume that it's supported by the hardware if it was selected in a previous run.

bslenul commented 4 months ago

Ah yeah, I think while testing I was relying on update_variables(false) called in retro_set_controller_port_device(), when reaching this one the renderer is already set so I had no issue, but yeah I can see that not being reliable, especially since it's only called for DC games...

I'll try to think of something else. Thank you for the feedback!

bslenul commented 4 months ago

How about now? Reverted everything inside update_variables() and simply added check_per_pixel_opt() in each context_reset() function.

edit: Sorry for the 2 force-pushes, forgot to rebase :x

flyinghead commented 4 months ago

LGTM

flyinghead commented 4 months ago

oh boy, I thought this was on the master branch...

bslenul commented 4 months ago

Is that a problem? I can submit a PR on master too if you want.

flyinghead commented 4 months ago

No problem