Skip to content

Fix type resolution issue in GL push constants#9116

Open
cwfitzgerald wants to merge 2 commits intogfx-rs:trunkfrom
cwfitzgerald:cw/regression-9115
Open

Fix type resolution issue in GL push constants#9116
cwfitzgerald wants to merge 2 commits intogfx-rs:trunkfrom
cwfitzgerald:cw/regression-9115

Conversation

@cwfitzgerald
Copy link
Copy Markdown
Member

@cwfitzgerald cwfitzgerald commented Feb 26, 2026 •
edited
Loading

Fixes #9115

@cwfitzgerald cwfitzgerald changed the title Add regression test for issue_9115 Fix type resolution issue in GL push constants Feb 26, 2026
@inner-daemons inner-daemons self-assigned this Feb 26, 2026
@inner-daemons inner-daemons self-requested a review February 26, 2026 17:01
inner-daemons
inner-daemons suggested changes Feb 26, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@inner-daemons inner-daemons left a comment

Choose a reason for hiding this comment

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

Generally looks good, 2 nits.

Also, the tests go in regression but is this actually a regression, or just an issue we've had since forever?

@cwfitzgerald
Copy link
Copy Markdown
Member Author

cwfitzgerald commented Feb 27, 2026

Regression tests help prevent re-regressions. Does this fully make sense, dunno. But I've used the regression folder for "misc tests to prevent a given issue from coming back"

@inner-daemons inner-daemons removed their assignment Feb 27, 2026
inner-daemons
inner-daemons approved these changes Mar 4, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@inner-daemons inner-daemons left a comment

Choose a reason for hiding this comment

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

I'm gonna approve this assuming you haven't changed anything majorly since last time.

jimblandy
jimblandy requested changes Mar 4, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@jimblandy jimblandy left a comment

Choose a reason for hiding this comment

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

Would it be better to simply change naga::back::glsl::ImmediateItem to hold a TypeInner instead of a handle? That's a trivial change to Naga, and then wgpu-hal would be simpler, not more complicated.

@cwfitzgerald
Copy link
Copy Markdown
Member Author

cwfitzgerald commented Mar 4, 2026

Yeah probably, let me see if that works.

@inner-daemons inner-daemons mentioned this pull request Mar 10, 2026
Open
6 tasks
@inner-daemons
Copy link
Copy Markdown
Collaborator

inner-daemons commented Mar 10, 2026

Care should be taken when merging in #9064 to this PR.

@inner-daemons
Copy link
Copy Markdown
Collaborator

inner-daemons commented Mar 14, 2026

Any updates? Should we try to get this in for the next release?

@cwfitzgerald cwfitzgerald requested a review from jimblandy March 14, 2026 04:45
@noituri noituri mentioned this pull request Mar 27, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jimblandy jimblandy Awaiting requested review from jimblandy

2 more reviewers

@atlv24 atlv24 atlv24 approved these changes

@inner-daemons inner-daemons inner-daemons approved these changes

Reviewers whose approvals may not affect merge requirements

Requested changes must be addressed to merge this pull request.

Assignees

@jimblandy jimblandy

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Using immediates on GL panics in certain condition

4 participants

@cwfitzgerald @inner-daemons @jimblandy @atlv24