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

Modular GL shaders #1904

Open
emersion opened this issue Nov 7, 2019 · 7 comments
Open

Modular GL shaders #1904

emersion opened this issue Nov 7, 2019 · 7 comments

Comments

@emersion
Copy link
Member

@emersion emersion commented Nov 7, 2019

#1901 has introduced a way for compositors to use GLES2 wlr_texture with their custom shader programs. With multi-planar formats and color management, it'll become more and more difficult for a compositor to have its own shaders.

Internally, we'll need to combine different fragment shader code blocks to get the expected result. Weston has a MR to do this by concatenating strings together: https://gitlab.freedesktop.org/wayland/weston/merge_requests/132 (see the "Requirement based shader generation" commit).

However it seems like we could take a different approach: multiple fragment shaders can be linked together (as long as only one has a main function). One shader could define a function used in another shader.

We could expose this to compositors with a wlr_texture_get(x, y) GLSL function they can call in their custom fragment shader (getting all the texture format handling for free).

We'd also need to expose something like wlr_gles2_renderer_attach_texture_shader (which would call glAttachShader with the appropriate fragment shaders) and wlr_gles2_renderer_bind_texture (which would call glActiveTexture, glBindTexture and friends).

Originally posted in #1901 (comment)

@emersion

This comment has been minimized.

Copy link
Member Author

@emersion emersion commented Nov 10, 2019

I've tried implementing this, unfortunately with GLES it's not possible to attach multiple shaders of the same type to a single program (it's only supported on desktop GL). The only way seems to pass multiple strings to glShaderSource, which is a little bit annoying...

@ammen99

This comment has been minimized.

Copy link
Member

@ammen99 ammen99 commented Nov 10, 2019

@emersion I would consider this a good enough solution, of course if there aren't technical problems which I don't see ...

@emersion

This comment has been minimized.

Copy link
Member Author

@emersion emersion commented Nov 11, 2019

The "GLSL concatenation" solution would probably be implemented this way:

  • Add const char *wlr_gles2_texture_get_sampler_fragment_shader(struct wlr_texture *tex). This will return a chunk of GLSL shader implementing vec4 wlr_texture_sample(vec2 coord). The compositor is responsible for compiling and linking the shader program with this chunk. The returned const char * can change depending on the texture, so the compositor needs to have a "shader program pool" (check if the shader with this particular chunk has been compiled, compile it if not).
  • Add void wlr_gles2_texture_bind(struct wlr_texture *tex) which would call glGet{Attrib,Uniform}Location for wlr_* attribs/uniforms and set these.

One annoying thing is that #require GLSL directives must be at the top of the shader (so the shader returned by wlr_gles2_texture_get_sampler_fragment_shader cannot be inserted anywhere). One other annoying thing is that we'd call glGet{Attrib,Uniform}Location each frame.

I'm not very happy with this solution.

@ammen99

This comment has been minimized.

Copy link
Member

@ammen99 ammen99 commented Nov 11, 2019

@emersion I see. I have also been thinking about uniforms and then it becomes a bit difficult to have uniforms that the compositor needs, if the code is inside wlroots.

All in all it looks like such shaders are a better fit for the compositor, although I hoped we can share the code somehow.

@emersion

This comment has been minimized.

Copy link
Member Author

@emersion emersion commented Nov 11, 2019

it becomes a bit difficult to have uniforms that the compositor needs, if the code is inside wlroots.

That's probably fine, we can prefix all wlroots uniforms with a wlr_ prefix for instance, and then use glGetUniformLocation.

All in all it looks like such shaders are a better fit for the compositor, although I hoped we can share the code somehow.

Yes, I'm afraid compositors will need to copy-paste shaders from wlroots. GLES doesn't give us enough flexibility. Maybe Vulkan is better in this regard.

@emersion

This comment has been minimized.

Copy link
Member Author

@emersion emersion commented Nov 11, 2019

Another thing maybe worth investigating is GL_PROGRAM_SEPARABLE and shader pipelines. It's only available in GLES23 though.

@ddevault

This comment has been minimized.

Copy link
Member

@ddevault ddevault commented Nov 11, 2019

Concatenating shaders sounds awful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.