Skip to content
This repository has been archived by the owner on Apr 5, 2023. It is now read-only.

RFC: Change behavior of how vertex shader layers are modifying positions #38

Open
hmans opened this issue Jun 18, 2022 · 2 comments · May be fixed by #29
Open

RFC: Change behavior of how vertex shader layers are modifying positions #38

hmans opened this issue Jun 18, 2022 · 2 comments · May be fixed by #29
Labels
1.2.0 enhancement New feature or request

Comments

@hmans
Copy link

hmans commented Jun 18, 2022

It's great that Lamina layers can also provide vertex shaders that modify the vertex positions, but unless I've severely misunderstood something (and please tell me if I did), the way that vertex shaders are currently set up makes it kinda iffy to have multiple vertex-shading layers stacked in sequence.

As far as I understand, from the perspective of a layer's vertex shader, the following happens:

  • A position variable is provided, containing the original vertex position from the geometry buffer.
  • The vertex shader is expected to implement a void main that returns a new, modified position.
  • If you have multiple Lamina layers that do this, each new layer will start with position being set to the original vertex position, essentially overwriting what the layers before already did to mutate it.

This makes it very hard to implement stacks of animation shaders (where one, for example, would animate the vertices based on velocity applied over time, and another apply a rotation) without completely bypassing this setup and writing all changes into a shared global variable, only to then have the final layer write into the actual position.

I don't know if this has implications (beyond potentially breaking existing code), so consider this an RFC: I propose that the system is changed to expect layer vertex shaders to mutate an existing position variable, eg.:

// wobble position vertexShader
void main() {
  position.x += sin(u_time);
}

// wobble scale vertexShader
void main() {
  position *= 2.0 + cos(u_time);
}

// etc.

Blend modes and ratios do not make sense here, so plain old mutation like this should be fine, but I'd be happy to hear what kind of stuff this will break.

@FarazzShaikh
Copy link
Member

It is a good idea but goes againt the idea that each layer is an isolated function. It wouldn't break much in the core lib but would in if users defined custom layers. I like to keep breaking changes to minor releases.

I will add this to 1.2.0. Been really procrastinating it for some reason, mostly because i cant get my brain into coding mode on weekends after insanely busy weeks. But it will come out soon

@FarazzShaikh FarazzShaikh added enhancement New feature or request 1.2.0 labels Jun 19, 2022
@FarazzShaikh FarazzShaikh linked a pull request Jun 19, 2022 that will close this issue
10 tasks
@hmans
Copy link
Author

hmans commented Jun 19, 2022

No need to immediately schedule this for addition -- especially if you're worried about this going against the library's design principles. Maybe we can iterate on the idea and find something better? Maybe there is a way to set up a "vertex modification layer" system similar to the fragment shading layers, where every vertex shader is passed the current position as its input and is then returning the modified position.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1.2.0 enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants