Skip to content

Conversation

SupaMaggie70Incorporated
Copy link
Contributor

@SupaMaggie70Incorporated SupaMaggie70Incorporated commented Aug 24, 2025

Connections
Related to #8139

Description
Previously, the load_shader function on metal backend would panic with panic!("load_shader required a naga shader"); if you tried to create a non-compute pipeline with a passthrough shader. This is a quick fix to address that

Testing
Already exists

Squash or Rebase?

Only commit is named "only commit hopefully" so probably squash

Checklist

  • Run cargo fmt.
  • Run taplo format.
  • Run cargo clippy --tests. If applicable, add:
    • --target wasm32-unknown-unknown
  • Run cargo xtask test to run tests.
  • If this contains user-facing changes, add a CHANGELOG.md entry.

@SupaMaggie70Incorporated SupaMaggie70Incorporated changed the title Add more complete MSL passthroguh Add more complete MSL passthrough Aug 24, 2025
@SupaMaggie70Incorporated SupaMaggie70Incorporated marked this pull request as ready for review August 24, 2025 08:01
@SupaMaggie70Incorporated SupaMaggie70Incorporated requested a review from a team as a code owner August 24, 2025 08:01
@cwfitzgerald
Copy link
Member

Would be nice to get some tests in for this eventually.

@cwfitzgerald cwfitzgerald merged commit 24a31c4 into gfx-rs:trunk Aug 24, 2025
40 checks passed
@SupaMaggie70Incorporated
Copy link
Contributor Author

Would be nice to get some tests in for this eventually.

Probably covered by #8128

@jimblandy
Copy link
Member

@SupaMaggie70Incorporated @cwfitzgerald The commit message on 24a31c4 ("Only commit hopefully") is really not helpful. It's good that it links to the PR, but if we're going to try to help other developers, we need to give them just a brief summary of what the change is about.

@cwfitzgerald
Copy link
Member

cwfitzgerald commented Aug 25, 2025

@jimblandy oh that's weird, I expected it to have the squashed commit name be the PR title, which is why I wasn't worried about. This is the first PR I merged on my phone in a while, I wonder if the mobile app does something weird....

@SupaMaggie70Incorporated
Copy link
Contributor Author

@SupaMaggie70Incorporated @cwfitzgerald The commit message on 24a31c4 ("Only commit hopefully") is really not helpful. It's good that it links to the PR, but if we're going to try to help other developers, we need to give them just a brief summary of what the change is about.

The PR says this

Only commit is named "only commit hopefully" so probably squash

In the future I'll try to actually assign meaningful names to commits though

@jimblandy
Copy link
Member

Okay, I see. Yes, even just the PR summary would be better. We now have two commits in the recent history with this same message.

@SupaMaggie70Incorporated
Copy link
Contributor Author

Okay, I see. Yes, even just the PR summary would be better. We now have two commits in the recent history with this same message.

The previous commit having that name is why I added this to the summary XD

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.

3 participants