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

RFC: Retroarch preset inject #16279

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

kokoko3k
Copy link

When the frontend preprocessor finds "#pragma inject_preset_code" it will search for a file named, and in the same path, as the loaded preset, but with the extension ".inc", and, if found, will "include" in the shader its content.
The use case is to allow presets to change "static" shader features.
This commit does not change the default behaviour of the existing shaders or presets; if a shader wants to take advantage of the feature it has to use "#pragma inject_preset_code" directive.

@hizzlekizzle @HyperspaceMadness

When the frontend preprocessor finds "#pragma inject_preset_code" it will search for a file named, and in the same path, as the loaded preset, but with the extension ".inc", and, if found, will "include" in the shader its content.
The use case is to allow presets to change "static" shader features.
This commit does not change the default behaviour of the existing shaders or presets; if a shader wants to take advantage of the feature it has to use "#pragma inject_preset_code" directive.
@LibretroAdmin
Copy link
Contributor

Hi @HyperspaceMadness , when you have some time, could you give this a quick look? Thanks.

@kokoko3k kokoko3k changed the title Retroarch preset inject RFC: Retroarch preset inject Feb 25, 2024
@HyperspaceMadness
Copy link
Contributor

I think this will cause problems with the simple presets.

If the user saved a new simple preset based on a preset like this, when this simple preset is loaded it would no longer do this include the .inc file it was before , because the new preset saved would have a different name and location than the original.

So after saving the new simple preset and reloading it you would not get the same result as you had before you saved the new preset

@kokoko3k
Copy link
Author

kokoko3k commented Feb 26, 2024

There were some dicsussion on the programming-shader discord channel.
I'll try to summarize key points; please expand/comment:

Injection Syntax:
	1 raw code to inject
		Allow for anything to be injected in the shader, not just defines
		The simpler way to do it
		Relies on external file
		Prevents Simple presets Inheritance
		
	2 $kParameter=value
		Can live in external file/and or right in the shader
		Cleaner for users

Simple presets Inheritance:
	1 Yes (locked parameter in a previous preset results in a locked parameter in subsequent chain links)
		User workflow is similar to the previous one
		Tracking why a parameter is locked is harder
				need a way to unset the lock?
						Add code and user experience complexity
		
	2 No (only last preset define what is locked)
		Simpler workflow
		Workflow would differ from what users are used to
		
Injections source location:
	1 External file
		One can lock/unlock presets by just renaming/deleting the injection file
		One can use a common injection file with multiple presets by renaming/copying the whole file
		
	2 In the preset
		Users workflow is similar to the previous one
		
	3 Both
		Decide priority
		
Injections destination:
	1 Shader choice (similar to include statements)
		Let the shader coder handles where or even if he wants define to be injected via a #pragma directive
		It is the shader coder choice to support injections or not
		shaders need to be updated to use injections

	2 Fixed
		All shaders get the injections without code modifications
		

By now I think we all agree that "Injection Syntax:2" is better

@kokoko3k
Copy link
Author

With the new commit, the injection format is *KEY=VALUE (no more wild code injection) and it is read from the preset and from the .inc file if present.

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

Successfully merging this pull request may close these issues.

None yet

3 participants