-
-
Notifications
You must be signed in to change notification settings - Fork 20
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
Converted select examples from obs-shaderfilter to obs-shaderfilterplus #13
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello, thank you for the PR!
I have looked over the shaders and tried them on Windows, and I seem to be having some issues. Some of them do not compile, and I have a feeling that one of them (shake.hlsl) is not doing what it is supposed to.
This is the result I get with shake.hlsl: https://streamable.com/nn57ek
With the following settings:
I have noticed you successfully reverse-engineered the primitive templating system that I made so that people would not have to specify the vertex shader. I now realize that it may have been a mistake to design the plugin this way, and that it would be preferable if it was possible for users to specify the vertex shader, as well. I am not sure yet how I would expose that functionality without breaking existing shaders, but I am willing to accept the workaround that you came up with, until a proper solution is found.
Edit: This issue with the shake.hlsl shader is probably caused by shadowing the original implementation of the builtin_shader_vertex
implementation and omitting this part.
Fair enough, I don't mind rewriting the vertex shaders if you do find a workaround. I'm pretty glad the current implementation works and helps on demonstrating how vertex shaders are useful as well. Having vertex shaders available helps on getting a lot of shaders easily compatible with shaders made on the original obs-shaderfilter.
Looking over, it is indeed intended. I did fix this a bit by centering it however there is one mandatory variable that isn't available. If you can expose a random variable that we can use, it'd be great. |
A random uniform variable could be added, though the number of such variables is questionable. Some effects will not make use of them at all, others may want to use more than one. Because of that, I think I prefer the pseudorandom generators like those you used. |
Well, I did end up using a 1D noise function. Although, I just ended up re-writing the entire thing to be more intuitive and makes more sense in a way that options are separated in-case no one wants the rotation effect. I also ended up separating wobble and rotation too. With that, I think everything should be fine now. If things are in the clear, should be ready to merge the pull request! |
Oops, accidental close. |
Sorry for the delay, I had a busy month. I tested the shaders and seem to be getting the following errors on Windows. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its pretty great work !!
I know it's been three years for a fix and funnily enough, a simple text substitution but I made sure to validate each of the shaders and ensure they work on Windows this time. Looks like everything works as intended now! |
I figure I'd help out on #7 by converting some of the filters I'm using from obs-shaderfilter.
Made sure these worked well in Linux and the GLSL conversion. I can't test this in Windows but I'm sure it should work.
It contains both basic and advanced shaders. However, it has some vertex shaders too referring to the example in src.
I would recommend testing this first on a Windows PC before merging just incase since I can't.
Especially since it's doing the GLSL conversion from HLSL and I had to correct the syntax to make it work.
I'll be porting the rest that I can from obs-shaderfilter if this works fine and if it's compatible. (Since I know some require another texture2d and is not currently supported.)