Skip to content

Conversation

DeltaEvo
Copy link
Contributor

@DeltaEvo DeltaEvo commented Aug 8, 2025

Description

The counter is decremented for raw and intermediate shaders but only incremented for raw shaders, this lead to a negative counter value and did not represent the number of allocated shaders.

Testing

Verified that the counter was not negative anymore and had an expected value

Screenshot 2025-08-08 at 14 22 35

Squash or Rebase?

Rebase

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.

@DeltaEvo DeltaEvo requested a review from a team as a code owner August 8, 2025 12:23
@cwfitzgerald cwfitzgerald self-assigned this Aug 13, 2025
Comment on lines 1957 to 1958

self.counters.shader_modules.sub(1);
Copy link
Member

Choose a reason for hiding this comment

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

I think this is the wrong fix to the problem, we should always be incrementing the counter, no matter what kind of shader module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍, I've modified the code to do that, I took the liberty of refactoring it a bit to remove the early return so we only increment the counter in one place

@DeltaEvo DeltaEvo force-pushed the david/vulkan_shader_count branch from 847afc7 to 9930b84 Compare August 25, 2025 14:46
@DeltaEvo DeltaEvo changed the title hal/vulkan: Only decrement counter for raw shaders hal/vulkan: Increment counter for intermediate shaders Aug 25, 2025
@DeltaEvo DeltaEvo requested a review from cwfitzgerald August 25, 2025 14:47
@DeltaEvo DeltaEvo force-pushed the david/vulkan_shader_count branch 2 times, most recently from 39381a0 to d4dbe49 Compare August 25, 2025 14:50
The counter is decremented for raw and intermediate shaders but
only incremented for raw shaders, this lead to a negative counter
value and did not represent the number of allocated shaders.
@DeltaEvo DeltaEvo force-pushed the david/vulkan_shader_count branch from d4dbe49 to e8d7429 Compare August 26, 2025 12:27
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Nice!

@cwfitzgerald cwfitzgerald merged commit 780bdfc into gfx-rs:trunk Aug 27, 2025
40 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.

2 participants