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

Fix interpolant ES error #3335

Merged
merged 1 commit into from
Nov 11, 2023
Merged

Conversation

ncesario-lunarg
Copy link
Collaborator

The restriction of no swizzling and no struct fields as an interpolant were not being checked when using the ES profile.

Fixes #3277.

Comment on lines 2546 to 2548
TIntermediate::traverseLValueBase(
arg0, swizzleOkay, false,
[&isValid, &isIn, &interpolantErrorMsg, esProfile, &structAccessOp](const TIntermNode& n) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@arcady-lunarg @jeremy-lunarg I went back and forth with creating a dedicated function to fix this or a more generic function to traverse. Let me know what you prefer style-wise.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's better to keep it inline but I also would prefer a brief comment explaining what this is doing.

Copy link
Contributor

@arcady-lunarg arcady-lunarg left a comment

Choose a reason for hiding this comment

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

A couple comments on some details but the overall approach is good. Thanks for adding some pretty comprehensive tests.

Comment on lines 2546 to 2548
TIntermediate::traverseLValueBase(
arg0, swizzleOkay, false,
[&isValid, &isIn, &interpolantErrorMsg, esProfile, &structAccessOp](const TIntermNode& n) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's better to keep it inline but I also would prefer a brief comment explaining what this is doing.

Copy link
Contributor

@arcady-lunarg arcady-lunarg left a comment

Choose a reason for hiding this comment

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

Thanks, I think it's cleaner like this, one small nit left to address and this can be merged.

@@ -573,6 +574,8 @@ class TIntermediate {

// Tree ops
static const TIntermTyped* findLValueBase(const TIntermTyped*, bool swizzleOkay , bool BufferReferenceOk = false);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need to delete this because the function doesn't exist.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, yep, thanks for catching that!

The restriction of no swizzling and no struct fields as an interpolant
were not being checked when using the ES profile.

Fixes KhronosGroup#3277.
@arcady-lunarg arcady-lunarg merged commit 52c59ec into KhronosGroup:main Nov 11, 2023
22 checks passed
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.

interpolate struct member
2 participants