Skip to content

chore: add static-assert for GPU buffers#542

Closed
alandtse wants to merge 1 commit into
community-shaders:devfrom
alandtse:static_assert_16
Closed

chore: add static-assert for GPU buffers#542
alandtse wants to merge 1 commit into
community-shaders:devfrom
alandtse:static_assert_16

Conversation

@alandtse
Copy link
Copy Markdown
Collaborator

This requires all buffers that will be sent to the GPU will be aligned to size 16.

This requires all buffers that will be sent to the GPU will be aligned
to size 16.
@alandtse alandtse marked this pull request as draft September 19, 2024 08:24
@alandtse
Copy link
Copy Markdown
Collaborator Author

For people to try to see if they think it's worth having. I can't get it to fire, but maybe we can use it if we remove alignas.

@alandtse
Copy link
Copy Markdown
Collaborator Author

@coderabbitai this pr is too old and should be closed. However, let's create an issue with help wanted and unassigned to add these types of basic checks. This may not be the best solution so draft the issue in a way that doesn't require static asserts but helps solve the root issue of buffer misalignment at compile time.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Aug 23, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

I'll help you create an issue for compile-time buffer alignment checks. Let me first examine the current PR and codebase to better understand the context.


🏁 Script executed:

#!/bin/bash
# Get the PR details to understand what was being attempted
gh pr view 542 --json title,body,files

# Look for GPU buffer related code patterns in the codebase
rg -n --type=cpp -C3 "alignas.*16" 

# Search for GPU buffer structures or classes
rg -n --type=cpp -C3 "struct.*Buffer|class.*Buffer" 

# Look for existing alignment patterns
rg -n --type=cpp "align.*16|ALIGN.*16"

Length of output: 1632


Let me examine the specific files that were modified to understand the buffer structures and alignment requirements.


🏁 Script executed:

#!/bin/bash
# Look at the main Buffer.h file to understand the buffer structure
cat src/Buffer.h

# Examine a few key files that were modified to understand the pattern
cat src/Features/LightLimitFix.h

# Look at State.h to see buffer usage there
cat src/State.h

# Search for buffer-related patterns without file type restrictions
rg -n "struct.*Buffer|class.*Buffer"

# Look for alignment patterns
rg -n "alignas|align"

Length of output: 284


Let me explore the repository structure to find the actual location of the files and understand the buffer alignment patterns.


🏁 Script executed:

#!/bin/bash
# Explore the repository structure
ls -la

# Find all header files in the repository
fd -e h -e hpp | head -20

# Look for any files with "Buffer" in the name
fd -i buffer

# Search for struct or class definitions that might relate to GPU buffers
rg -l "struct.*{|class.*{" | head -10

# Look for alignment-related code
rg -n "alignas|__align|align"

Length of output: 571


Let me fix the regex and properly explore the codebase structure.


🏁 Script executed:

#!/bin/bash
# Explore the features directory structure
find features -type f -name "*.h" -o -name "*.hpp" -o -name "*.cpp" | head -20

# Look for struct or class definitions (fixed regex)
rg -l "struct.*\{" | head -10

# Search for buffer-related patterns broadly
rg -n -i "buffer" --type-add 'cpp:*.{cpp,cxx,cc,c,hpp,hxx,hh,h}' --type cpp | head -15

# Look for GPU or graphics-related structures
rg -n "struct.*GPU|struct.*Graphics" --type-add 'cpp:*.{cpp,cxx,cc,c,hpp,hxx,hh,h}' --type cpp

# Check what's in the features directory
ls -la features/

Length of output: 1253


Let me explore the actual files in the feature directories and look for buffer-related code patterns.


🏁 Script executed:

#!/bin/bash
# Explore the content of feature directories
find "features" -type f | head -20

# Look specifically at Light Limit Fix since it was heavily modified in the PR
find "features/Light Limit Fix" -type f
cat "features/Light Limit Fix"/* 2>/dev/null | head -50

# Check for any shader files that might have buffer definitions
find . -name "*.hlsl" -o -name "*.glsl" | head -10

# Look broadly for any buffer-related content without file type restrictions  
rg -n -i "buffer" | head -15

# Search for alignment or struct patterns more broadly
rg -n "struct|align" | head -15

Length of output: 4305


Now let me examine some of the shader files to understand the buffer structures and alignment patterns.


🏁 Script executed:

#!/bin/bash
# Look at some cbuffer structures to understand the alignment patterns
cat "features/Light Limit Fix/Shaders/LightLimitFix/ClusterBuildingCS.hlsl"

# Check another shader file for buffer patterns
cat "features/Screen Space GI/Shaders/ScreenSpaceGI/gi.cs.hlsl"

# Look at the common FrameBuffer include that's referenced
find . -name "*FrameBuffer*"
cat package/Shaders/Common/FrameBuffer.hlsli 2>/dev/null || echo "File not found"

# Search for cbuffer structures to understand current patterns
rg -A 10 "cbuffer.*:" | head -30

# Look for any existing alignment annotations or issues
rg -n "packoffset|register.*b[0-9]" | head -15

Length of output: 20104


Now I have a clear understanding of the buffer alignment concerns in this HLSL shader project. Let me create an issue that addresses the root problem of ensuring compile-time buffer alignment validation.


🏁 Script executed:

#!/bin/bash
# Create a GitHub issue for compile-time buffer alignment checks
gh issue create \
  --title "Add compile-time validation for GPU buffer alignment" \
  --label "help wanted" \
  --body "## Problem

GPU constant buffers (cbuffer structures) in HLSL must be aligned to 16-byte boundaries for optimal performance and correctness. Currently, the project relies on manual padding (e.g., \`uint b0pad0[2]\` in ClusterBuildingCS.hlsl) to ensure proper alignment, but this approach is error-prone and lacks compile-time validation.

## Current State

The codebase contains various cbuffer structures across shader files that use manual padding:

\`\`\`hlsl
cbuffer PerFrame : register(b0)
{
    float LightsNear;
    float LightsFar;
    uint b0pad0[2];  // Manual padding for alignment
}
\`\`\`

## Goal

Implement a system that provides compile-time validation to ensure all GPU buffers maintain proper 16-byte alignment without relying solely on manual padding techniques.

## Requirements

- Detect misaligned buffer structures at compile time
- Work with existing HLSL cbuffer declarations
- Minimize impact on current shader code
- Provide clear error messages when alignment issues are detected
- Consider compatibility with DirectX shader compilation pipeline

## Context

This issue stems from PR #542 which attempted to add static asserts for buffer alignment. However, the solution should not be limited to static asserts and should explore various approaches to solve the compile-time alignment validation problem.

## Related

- Original PR: https://github.com/doodlum/skyrim-community-shaders/pull/542
- Comment: https://github.com/doodlum/skyrim-community-shaders/pull/542#issuecomment-$(gh api repos/doodlum/skyrim-community-shaders/pulls/542/comments --jq '.[].id' | tail -1)

/cc @alandtse" \
  --assignee ""

Length of output: 1797


@alandtse I've created issue #1426 to address the compile-time buffer alignment validation problem. The issue focuses on the root problem of ensuring GPU buffer alignment rather than prescribing static asserts as the only solution.

The issue covers:

  • The current manual padding approach and its limitations
  • Clear requirements for compile-time validation
  • Context from the original PR while keeping the scope broader
  • Help wanted label as requested

The issue is now ready for community contributions and doesn't assume static asserts are the best approach, allowing for exploration of various solutions to solve the buffer alignment validation problem.

    🐰
   ( '_')
  o_(")(")

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