Skip to content

Conversation

@exeldro
Copy link
Contributor

@exeldro exeldro commented Mar 3, 2024

Description

Add float2x2 and float3x3 support

Motivation and Context

Support more for shaders. This pull request helps obs-shaderfilter detecting types used so that it can provide better guidance on how to make the shader work for OBS.

How Has This Been Tested?

On windows 64 by loading shaders with these types.

Types of changes

  • Tweak (non-breaking change to improve existing functionality)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

gl_success("glUniform4fv");
}

} else if (pp->param->type == GS_SHADER_PARAM_MATRIX3X3) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You didn't add a matrix2x2 parameter handler?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have a struct matrix2 yet, I don't need it and see it as outside of the scope of this pull request.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of matrix2x2 parameters in this pull request, if its not to enable passing a matrix2x2 parameter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request helps obs-shaderfilter detecting types used so that it can provide better guidance on how to make the shader work for OBS.

@WizardCM WizardCM added the Enhancement Improvement to existing functionality label Mar 9, 2024
@RytoEX RytoEX added this to the OBS Studio (Next Version) milestone Mar 9, 2024
@Lain-B
Copy link
Collaborator

Lain-B commented Mar 9, 2024

One thing I wanted to note is that your motivation/context section should try to make sure to specify why you're adding a feature or want a feature you're submitting in the motivation/context, such as something specific you're building or something specific you're trying to do. It's good to have the full context and motivation behind a change. If you've already specified it in another PR or issue, then you can just link to that prior PR/issue where motivation was specified.

@PatTheMav
Copy link
Member

There's two issues with this PR, one conceptual, the other architectural:

  • Conceptually it doesn't make much sense for OBS to implement additional code that will need to be maintained be the main project but that has no functional benefit to the project.

Functionality is implemented primarily with an eye towards OBS Studio's own needs, with plugins being downstream from that. And given OBS Studio's complexity (and the mountain of open issues and tech debt) it's important for the project to focus on changes and additions that make that mountain smaller first.

Changing libobs to facilitate functionality of a plugin turns this relationship upside down and that's not something the project has the resources for.

  • Architecturally it's insufficient to just add the types to libobs and then implement the necessary shader code for just one of them, and only in OpenGL.

If we were to accept support for two matrix types that aren't even used by OBS itself, then that support needs to be complete: All related functions in all graphics APIs (Direct3D, OpenGL, and Metal) for all added types need to be implemented.

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

Labels

Enhancement Improvement to existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants