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

Support CUSTOM shader attributes in 2D #76276

Conversation

Giwayume
Copy link
Contributor

@Giwayume Giwayume commented Apr 20, 2023

Closes: godotengine/godot-proposals#6703

A general summary of changes:

  • The vertex shader attributes CUSTOM0, CUSTOM1, CUSTOM2, CUSTOM3 are now supported for canvas item shaders for the Forward+ and Mobile renderers
  • The vertex shader attributes CUSTOM0, CUSTOM1 are supported for canvas item shaders for the Compatibility renderer

The reason only CUSTOM0, CUSTOM1 is supported for OpenGL ES 3, is I don't perceive the complexity behind reorganizing these attributes in the code as worth the maintenance it would require for the change. The assumption here is the attribute layout slots 14, 15 should exist under the minimum GL specification, so all of the slots are shifted down to make use of that. You can read more about this in the proposal linked above.

Sample project:
CanvasShaderTest.zip

@Giwayume Giwayume requested a review from a team as a code owner April 20, 2023 03:05
@Giwayume Giwayume force-pushed the feature/canvas-item-shader-custom-data branch 2 times, most recently from 933c8f2 to 5b41902 Compare April 20, 2023 03:20
@akien-mga akien-mga added this to the 4.x milestone Apr 20, 2023
@clayjohn
Copy link
Member

I haven't been able to take a deep look, but I was reminded of this while looking through the spec for something else. The spec guarantees that at least 16 vertex attribute slots will be available. You seem to have mixed up vertex attributes and varyings (vertex output to the fragment shader).

This means you have one more slot to play with than before!

@Giwayume Giwayume force-pushed the feature/canvas-item-shader-custom-data branch 3 times, most recently from 86bb736 to 9ebde53 Compare April 22, 2023 04:51
@Giwayume
Copy link
Contributor Author

Giwayume commented Apr 22, 2023

Updated to move the gles3 attributes down 1 more slot, so CUSTOM0 and CUSTOM1 works in compatibility.

@Giwayume
Copy link
Contributor Author

@clayjohn @akien-mga
Is there any interest from the core team in supporting this? It's been a while. If not, I'll close.

@Calinou
Copy link
Member

Calinou commented Sep 10, 2023

This looks good to me on principle, but I'd like to have a testing project to make sure this works as expected. Also, this PR needs to be rebased.

@mr11stein
Copy link

Is there anything I can do to help this move forward? I need this for a couple of plugins I'm writing for advanced 2D features.

@AThousandShips
Copy link
Member

Is there anything I can do to help this move forward?

Yes #76276 (comment)

@Giwayume Giwayume force-pushed the feature/canvas-item-shader-custom-data branch from 9ebde53 to d8fb632 Compare December 27, 2023 20:04
@AThousandShips
Copy link
Member

You squashed in a way that took the wrong commit message, and changed the author, because you used merging instead of rebasing, please fix your commit message

@Giwayume
Copy link
Contributor Author

This looks good to me on principle, but I'd like to have a testing project to make sure this works as expected. Also, this PR needs to be rebased.

I added a sample project in the description, what specifically are you looking for in a testing project?

@Giwayume
Copy link
Contributor Author

Giwayume commented Dec 27, 2023

You squashed in a way that took the wrong commit message, and changed the author, because you used merging instead of rebasing, please fix your commit message

🙏 forgive me, have not worked in Godot world for a long time. I did use pull --rebase to update, though.

@AThousandShips
Copy link
Member

See the PR workflow documentation 🙂

@Giwayume Giwayume force-pushed the feature/canvas-item-shader-custom-data branch 3 times, most recently from 3f03043 to 507e2b0 Compare December 27, 2023 20:12
@AThousandShips
Copy link
Member

Please rebase onto master, your history is garbled 🙂, you include a merge commit indirectly

@Giwayume Giwayume closed this Dec 27, 2023
@Giwayume Giwayume force-pushed the feature/canvas-item-shader-custom-data branch from 507e2b0 to 13a0d6e Compare December 27, 2023 20:16
@Giwayume
Copy link
Contributor Author

Yeah, the rebase just made it equal to master. I'm just going to manually fix this.

@Giwayume
Copy link
Contributor Author

Giwayume commented Dec 27, 2023

Github doesn't recognize that this PR can be reopened due to branch changes. Continued here: https://github.com/godotengine/godot/pull/86564/files

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

Successfully merging this pull request may close these issues.

Allow passing custom vertex data to canvas_item shaders using a custom vertex array
6 participants