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

Add a way to specify an additional uniform name along with a sampler param for the associated transform matrix #8490

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ajmalk
Copy link

@ajmalk ajmalk commented Mar 5, 2025

Followup to #8489

From: https://developer.android.com/reference/android/graphics/SurfaceTexture

When sampling from the texture one should first transform the texture coordinates using the matrix queried via getTransformMatrix(float[]). The transform matrix may change each time updateTexImage() is called, so it should be re-queried each time the texture image is updated.

Adding a way to specify the transform name in the material is the second step to being able to apply it to the uniform in the shader.

Copy link
Collaborator

@pixelflinger pixelflinger left a comment

Choose a reason for hiding this comment

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

This will break materials so you need to add an entry in NEW_RELEASE_NOTES.md (look how it's done in RELEASE_NOTES.md, it's obvious).

The only thing I don't like is that it ads this new associated field to all fields of UBOs, even if that's an extremely rare case. At the moment I can't think of a better way though.

maybe we could add a new associate method to BufferInterfaceBlock::Builder? It would take a uniform name + sampler binding?

@ajmalk
Copy link
Author

ajmalk commented Mar 5, 2025

Maybe we could add a new associate method to BufferInterfaceBlock::Builder? It would take a uniform name + sampler binding?

I can make this change but this would still add the field to all the BufferInterfaceBlocks after they are built iiuc.

@ajmalk
Copy link
Author

ajmalk commented Mar 5, 2025

Alternatively I could add a constructor with default values to the InterfaceBlockEntry that I think would be better since you can still pass in list like before.

@ajmalk
Copy link
Author

ajmalk commented Mar 5, 2025

Done

Copy link
Collaborator

@pixelflinger pixelflinger left a comment

Choose a reason for hiding this comment

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

Does it make sense for the transformName to be set for non external samplers?

Also maybe update docs/Materials.md.html around line 1025 to document the new parameter.

@ajmalk
Copy link
Author

ajmalk commented Mar 5, 2025

Probably not. I'll add a check.

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

Successfully merging this pull request may close these issues.

2 participants