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

Skip uniform names that contain .@data. #3155

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jay3d
Copy link
Contributor

@jay3d jay3d commented Aug 4, 2023

In some instances where one would have a structured buffer such as:

struct MyStructure
{
    mat4 Matrix;
};

...

BUFFER_RO( u_MyBuffer, MyStructure, * );

Then the uniform name generated would be:

BGFX Creating uniform (handle *) `[email protected]`, num 1

This would fail the uniform name validation check, because of the .@data. in the name.

@jay3d jay3d requested a review from bkaradzic as a code owner August 4, 2023 09:14
Copy link
Owner

@bkaradzic bkaradzic left a comment

Choose a reason for hiding this comment

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

This should be fixed on shaderc side to now allow .@data. to be treated as uniform.

@jay3d jay3d requested a review from bkaradzic August 6, 2023 07:03
@jay3d jay3d changed the title A fix for certain cases where the unifom name could contain a . Skip uniform names that contain .@data. Aug 6, 2023
@jay3d
Copy link
Contributor Author

jay3d commented Aug 8, 2023

This should be fixed on shaderc side to now allow .@data. to be treated as uniform.

Done.

@bkaradzic
Copy link
Owner

Is this consistent behavior with D3D11?

@jay3d
Copy link
Contributor Author

jay3d commented Aug 9, 2023

Is this consistent behavior with D3D11?

I have no idea, I found no such check there to begin with.

@bkaradzic
Copy link
Owner

I have no idea, I found no such check there to begin with.

I mean if you compile with D3D compiler you get exactly the same uniform name that you get with SPIRV after trimming .@data.?

@jay3d jay3d force-pushed the uniform-name-fix branch 3 times, most recently from c248aaf to f16ad4a Compare May 5, 2024 00:52
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