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

desktop-ui/presentation: Add an alternative path for loading shaders on Linux and BSD #1738

Merged
merged 1 commit into from
Jan 1, 2025

Conversation

Mastergatto
Copy link
Contributor

On some Linux and BSD distribution, Ares from the repositories may not come with shaders preinstalled. Instead, these one may come from another package, for example on Archlinux the package libretro-shaders-slang installs the shaders in the following path: /usr/share/libretro/shaders/shaders_slang/ .

This will make Ares fail to load any shaders on these distro. So to fix it, this PR adds the aforementioned location as another path to search for, as a fallback.

I know, the two dots are a bit ugly, given that locate() normally doesn't search for files or directories outside of those of Ares...Ideally, locate() should be expanded to search the entire file system, but I'm afraid the required changes may be a bit more far reaching than expected.

@jcm93
Copy link
Contributor

jcm93 commented Jan 1, 2025

I think this is fine so that we can introduce the baseline functionality of loading shaders from the slang-shaders package on Linux and BSD, which is important.

However, as discussed a bit out-of-band, I think we do need to reorganize the whole way shaders are loaded.

Currently, from my understanding, ares only considers shaders in one directory; either the first one locate finds called Shaders, or, under this PR, if none is found, the first one locate finds called libretro/shaders/shaders_slang.

This doesn't offer any clean way to override particular shaders, for example in the user folder. Even under this PR, shaders are "all or nothing" and coming from one sole directory.

I think ideally, the loadShaders function would iterate over a list of directories, and add shaders in sequence from them, and if the relative paths of two shaders are identical, have the first one found take precedence.

Based on my initial look at the problem, this is a non-trivial amount of work since the recursive trawler in loadShaders is very much only built to consider one directory at the moment.

As far as the use/abuse of locate here, I also think it's OK for now. I think ideally, we would introduce a locateShaders function in desktop-ui, which would deal specifically with shaders and their probable locations; on macOS and Windows, this could just fall through to locate, but on Linux/BSD could have more appropriate handling.

@LukeUsher LukeUsher merged commit 69795d5 into ares-emulator:master Jan 1, 2025
@bsdcode
Copy link
Contributor

bsdcode commented Jan 9, 2025

Unfortunatly this alternate shaders path is not correct for all use cases. The shaders from a libretro-shaders-slang package has to be treated as an external dependency. As such, for the FreeBSD port, there is a distinction between the location of ares and its dependencies which are provided by the system/package manager. This is reflected by two build environment variables:

  • PREFIX: to where ares is to be installed
  • LOCALBASE: where the system dependencies live

By default, and this is how the prebuild binary packages on FreeBSD are build, these two variables are identical and set to /usr/local. But users could decide to build it differently, e.g. PREFIX=/opt and LOCALBASE=/usr/local. In this case the shaders are still in /usr/local/share/libretro/shaders/slang_shaders, but ares whould look for them in /opt/share/libretro/shaders/slang_shaders and won't find them.

As per port policy and guideline I have to patch this so ares searches the shaders in ${LOCALBASE}/share/libretro/shaders/slang_shaders. This is no big problem and is also what the FreeBSD port already does. This is just intended as information for future work on this topic as pointed out by @jcm93. Some Linux distros might have similar policies.

With the new cmake build system we already have the slang_shaders_LOCATION variable when it finds the shaders package at configuration time. I think this should be used as the one source of truth when ares looks for the externally installed shaders.

@jcm93
Copy link
Contributor

jcm93 commented Jan 9, 2025

Currently, per desktop-ui's application logic, the following paths get searched when looking for shaders by the locate() call in this PR.

  1. <the binary location>/../libretro/shaders/shaders_slang/

  2. If XDG_DATA_HOME is defined as an environment variable, <XDG_DATA_HOME>/ares/../libretro/shaders/shaders_slang,
    otherwise,
    ~/.local/share/ares/../libretro/shaders/shaders_slang/
    Which are normalized to:
    <XDG_DATA_HOME>/libretro/shaders/shaders_slang and
    ~/.local/share/libretro/shaders/shaders_slang/
    respectively.

  3. The "prefix" path. This is evaluated as:
    <the program path>/../share/ares/../libretro/shaders/shaders_slang/
    Which we can reduce to:
    <the program path>/../share/libretro/shaders/shaders_slang/

  4. The "local shared data" path. This is evaluated as:
    /usr/local/share/ares/../libretro/shaders/shaders_slang/
    Which is normalized to:
    /usr/local/share/libretro/shaders/shaders_slang/

  5. The "shared data" path. This is evaluated as:
    /usr/share/ares/../libretro/shaders/shaders_slang/
    Which we can normalize to:
    /usr/share/libretro/shaders/shaders_slang/

While the logical flow here is not "ideal", especially removing the ares path components added by locate by using .. as done by this PR, and the other concerns noted above, this covers a lot of bases in terms of where shaders might be located.

The only case I think it does not cover is if you use a particular package manager to install slang-shaders to a custom prefix somewhere on the system, AND if that is not the same package manager you used to install ares.

In the event we decided that more custom application logic is necessary to find shaders, I think that using configure-time logic to define search paths within the build itself is to be avoided if at all possible.

The install step should be entirely independent of the build and the configure steps. Once an application is built, it should be able to be installed anywhere. In fact, verifying that a path chosen at install in CMake is the "correct" path we defined at configuration is not possible; hence we would be introducing the possibility for the user to create a fundamentally broken configuration without any way of telling them that information (configure with one prefix, install with another).

It seems to me that if further program logic is necessary, rather than patching the build to define any particular path, it should just be made part of the actual UI. "Locate Shaders..." in the menu which opens a directory selection modal, and that directory is stored along with the rest of the user's settings.

@bsdcode
Copy link
Contributor

bsdcode commented Jan 9, 2025

Thank you for your detailed response. Indeed in 99.9% of use cases the current behavior will just work. It's just this 0.01% chance of someone doing a non-standard build via the ports framework with different settings for PREFIX and LOCALBASE I have to think about. But that's more a downstream-maintainer-problem than something the ares project has to solve ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants