Skip to content

Conversation

@langyo
Copy link

@langyo langyo commented Dec 21, 2025

Connections
N/A (Fixes a panic observed in downstream Bevy 0.17 usage)

Description
This PR fixes a panic in the GLSL backend when encountering a BindingArray during the global variable writing phase.

  • Problem: Previously, write_global did not explicitly handle TypeInner::BindingArray. It would fall through to the default case, which attempts to call write_global recursively. Since the underlying type of a texture array is AddressSpace::Handle, this triggered the unreachable! panic in write_global for AddressSpace::Handle.
  • Solution: Added a specific match arm for TypeInner::BindingArray in write_global. It now correctly generates the GLSL uniform declaration for texture arrays (e.g., uniform texture2D name[size];) and ignores sampler arrays.

Testing
Tested locally with a Bevy 0.17 reproduction case on an Intel Core Ultra system with integrated Arc Graphics.

Hardware Context:

  • CPU: Intel(R) Core(TM) Ultra 5 225H (14 Cores, 14 Logical Processors)
  • GPU: Intel(R) Arc(TM) 130T GPU (Integrated)
  • OS: Windows 11

Results:

  • Before: The application panicked with internal error: entered unreachable code in naga::back::glsl when loading a scene with StandardMaterial.
  • After: The application runs and renders correctly without panicking.

Squash or Rebase?
Rebase onto trunk.

Checklist

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

Copilot AI review requested due to automatic review settings December 21, 2025 17:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a panic in the GLSL backend when processing BindingArray types (e.g., texture arrays) during global variable generation. The panic occurred because BindingArray was not explicitly handled and would fall through to a default case that would then panic on AddressSpace::Handle types.

Key Changes:

  • Added explicit handling for TypeInner::BindingArray in the global variable writing code
  • Generates GLSL uniform declarations for texture arrays with proper array syntax
  • Handles sampler arrays by skipping them (consistent with regular sampler handling)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 985 to 999
TypeInner::Image {
dim,
arrayed,
class,
} => {
write!(self.out, "uniform ")?;
self.write_image_type(dim, arrayed, class)?;
let global_name = self.get_global_name(handle, global);
write!(self.out, " {global_name}")?;
self.write_array_size(base, size)?;
writeln!(self.out, ";")?;
writeln!(self.out)?;

self.reflection_names_globals.insert(handle, global_name);
}
Copy link

Copilot AI Dec 21, 2025

Choose a reason for hiding this comment

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

The BindingArray Image handling is missing several critical features that are present in the regular Image handling (lines 916-956):

  1. Storage image support: Missing format and access specifiers for storage images (lines 917-922, 943-949, 954-956). Storage images in binding arrays (e.g., binding_array<texture_storage_2d<rgba8unorm, write>>) require layout qualifiers that specify the storage format and access mode.

  2. Layout binding qualifiers: Missing explicit binding location handling (lines 930-935, 938-951). When the GLSL version supports explicit locations, binding arrays should include layout(binding = N) qualifiers.

  3. OpenGL ES dimension conversion: Missing D1 to D2 conversion for ES targets (lines 925-927). OpenGL ES doesn't support 1D textures, so binding arrays of 1D images need to be converted to 2D.

Without these features, the generated GLSL for binding arrays will be incomplete or incorrect for storage images, missing required binding qualifiers, and will fail to compile for ES targets with 1D textures.

Copilot uses AI. Check for mistakes.

This comment was marked as outdated.

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.

1 participant