gles: Define a TextureShaderElement

[ids1024]

GlesFrame has support for using a custom shader, but there doesn't seem to be a particularly convenient way to use this. I'm not sure exactly how override_default_tex_program would best be used, and it's probably not a good way to use a shader for one specific element.

TextureShaderElement wraps a TextureRenderElement with a shader and uniforms. So doesn't have to duplicate the implementation of different constructors TextureRenderElement provides.

Leaving as a draft until I test this (and add some documentation).

[ids1024]

This seems to be working. I think implementing Element like this to mostly pass through to TextureRenderElement is correct. (underlying_storage() isn't implemented since that would bypass the shader.)

[Drakulix]

This looks useful and mostly is in line of what I thought a higher-level abstraction could look like, when I designed the lower-level renderer apis.

I would be intrigued though to here, if this would be actually useful or would be too limited for real life use-cases.
@ids1024 Where are you using this? What is your motivation?
@YaLTeR I think niri is the only compositor supporting custom shaders. Would you use something like this? If not why? Anything specific that is missing?

[ids1024]

My code for testing this in cosmic-comp is rather hacky at the moment but my use-case is to apply a post-process shader to the whole screen. Which we discussed wanting for a couple features and for testing purposes. (Invert colors accessibility feature, color blindness simulation shader, etc.)

Something like this TextureShaderElement seems like a reasonable way to handle this.

[YaLTeR]

I guess in niri I could use this as the backbone for the ClippedSurfaceRenderElement instead of manual override_default_tex_program(). ClippedSurface does clipping to toplevel geometry and rounded corners, and it falls into this use case of pixel-wise shading that more or less preserves damage.

It works on a WaylandSurfaceRenderElement though, not a raw texture. I will need some separate handling for the solid color buffer once I add that, not sure how annoying that would be.

I am also planning to have post-process shaders, and when ClippedSurface is in use I guess I might just append the code into its shader, but when ClipperSurface is not in use, this PR's TextureShaderElement should work. Unfortunately, it doesn't really help with any of the unobvious math required to pass the toplevel geometry into the shader and relate it to the texture geometry (this gets complex when i.e. drawing a subsurface or with viewporter, etc.).

For my animation shaders, I need some more functionality like bigger shader area or passing multiple textures (think resize crossfade between two buffers), which is a different use case from this PR.

[ids1024]

override_default_tex_program() seems like a kind of awkward API since it isn't composable. clear_tex_program_override() won't restore if there's already an override. I guess it can only be used at the lowest level to call code you know won't call override_default_tex_program(), and always clearing the override afterward.

I guess it works if you follow certain rules, but TextureShaderElement feels cleaner to me.

I will need some separate handling for the solid color buffer once I add that, not sure how annoying that would be.

That's true with either override_default_tex_program() or TextureShaderElement, right? Since draw_solid uses solid_program and isn't using the tex program.

Though how best to handle that is a good question, and may be a common use case for something like this. I guess WaylandSurfaceTexture::SolidColor could be mapped to a PixelShaderElement, but wouldn't use the same shader as the texture shader?

Not sure the best way to handle that.

[Drakulix]

Okay so we have two quite different use-cases here:

• Essentially two-pass rendering, where first compositing happens offscreen and then a post-processing filter is applied to the whole texture rendering it again
• Shading a window buffer, that is being drawn.

It seems for the first one this is more than enough, but for the second it seems possibly too simplistic and secondly not helpful for the additional complexity, that is WaylandSurfaceRenderElement with different buffer types, e.g. solid-color buffers.

I would like to solve the second in smithay eventually, as every compositor implements it's own clipping atm, rounded corners were also a highly requested feature and it might make sense to standardize some uniforms/attributes for user-provided window-shaders to make sharing easier.

But for the first use-case, maybe we could just add an optional shader-argument to the constructor of the TextureRenderElement instead of wrapping it again?

[ids1024]

I couldn't think of a good way to add this as a feature to TextureRenderElement, since that is generic over renderers, while shaders are GLES specific. And it doesn't call the render_texture_from_to function that takes a shader, but the one from the Frame trait that doesn't.

[Drakulix]

I couldn't think of a good way to add this as a feature to TextureRenderElement, since that is generic over renderers, while shaders are GLES specific. And it doesn't call the render_texture_from_to function that takes a shader, but the one from the Frame trait that doesn't.

Ah, so this is essentially a specialization for a specific renderer. Maybe we should call it GlesTextureRenderElement then and mimic the constructors from TextureRenderElement?

We could internally still wrap it and/or provide constructors/From-implementations from the TextureRenderElement, I am just thinking about how to make this more ergonomic.

[ids1024]

It could work that way. I guess that would be slightly more ergonomic, though it's not really that hard to call TextureShaderElement::new to wrap with a shader.

I guess where it matters more is which works better for generic code that wants to also have a pixman renderer, but use the shader feature only with gles (either using nothing, or something different with pixman).

But I don't know exactly how we'd want to handle that, or what API would be most useful for it.

[Drakulix]

But I don't know exactly how we'd want to handle that, or what API would be most useful for it.

Good point. I guess changing the constructor later isn't so bad, we have had worse api breakage for sure.

[ids1024]

Yeah, and it's likely we'll want some API changes in Smithay to better handle dynamic renderer switching like that regardless.