-
Notifications
You must be signed in to change notification settings - Fork 3
Fix GLSL shader compilation with WebGL standards-compliant MyGLSLPatcher version handling #361
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Copilot
wants to merge
15
commits into
main
Choose a base branch
from
copilot/fix-2cca6fa8-0420-42a0-8f0b-7ce9ec624702
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+800
−70
Open
Changes from 10 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
6049f8c
Initial plan
Copilot b54d9c8
Fix GLSL shader compilation by injecting missing #version directive
Copilot 9db99f4
Refine GLSL version detection and add comprehensive test coverage
Copilot caf493c
Make GLSL version handling WebGL standards-compliant
Copilot e7ca3f7
Implement ANGLE-style WebGL 1.0 to desktop OpenGL translation
Copilot 84dae54
Fix gl_FragData array access translation for desktop OpenGL compatibi…
Copilot f97f471
Move version patch logic from Rust to C++ as requested
Copilot 77bba30
Implement client-side WebGL version handling in MyGLSLPatcher
Copilot a347c64
Move WebGL version handling to client-side WebGLContext as requested
Copilot a223e7c
Move WebGL version handling to MyGLSLPatcher in webgl.rs as requested
Copilot 944794c
Revert style_rule.rs file to original state as requested
Copilot 9b998fd
Revert css/mod.rs file to original state as requested
Copilot 1c263db
Refactor MyGLSLPatcher to enhance fragment output handling and shader…
EndlessJour9527 31fa5e8
Refactor test-shader-version-fix.html for improved readability and st…
EndlessJour9527 377515c
Enhance fragment output handling in MyGLSLPatcher by ensuring proper …
EndlessJour9527 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| pub(crate) mod properties; | ||
| pub(crate) mod selectors; | ||
| pub(crate) mod values; | ||
| pub(crate) mod stylesheets; | ||
| pub(crate) mod values; |
EndlessJour9527 marked this conversation as resolved.
Show resolved
Hide resolved
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
EndlessJour9527 marked this conversation as resolved.
Show resolved
Hide resolved
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,21 +62,72 @@ impl VisitorMut for MyGLSLPatcher { | |
| } | ||
| } | ||
|
|
||
| /// Detect if shader source uses WebGL 2.0 syntax | ||
| /// WebGL 2.0 requires #version 300 es per specification, WebGL 1.0 doesn't require version | ||
| fn detect_webgl2_syntax(s: &str) -> bool { | ||
EndlessJour9527 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| // WebGL 2.0 strong indicators (these only exist in WebGL 2.0) | ||
| if s.contains("out vec4") | ||
| || s.contains("out mediump") | ||
| || s.contains("out lowp") | ||
| || s.contains("out highp") | ||
| || s.contains("layout(location") | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| // Check for WebGL 2.0 built-ins - texture() without texture2D() | ||
| if s.contains("texture(") && !s.contains("texture2D(") { | ||
| return true; | ||
| } | ||
|
|
||
| // WebGL 1.0 strong indicators (these are deprecated/removed in WebGL 2.0) | ||
| if s.contains("gl_FragColor") | ||
| || s.contains("gl_FragData") | ||
| || s.contains("attribute ") | ||
| || s.contains("varying ") | ||
| || s.contains("texture2D(") | ||
| || s.contains("textureCube(") | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| // Check for 'in ' keyword (could be WebGL 2.0, but be more specific) | ||
| if s.contains("in vec") | ||
| || s.contains("in mediump") | ||
| || s.contains("in lowp") | ||
| || s.contains("in highp") | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| // Default to WebGL 1.0 for safety | ||
| false | ||
EndlessJour9527 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| fn patch_glsl_source_from_str(s: &str) -> String { | ||
| use glsl_lang::{ | ||
| ast::TranslationUnit, lexer::full::fs::PreprocessorExt, parse::IntoParseBuilderExt, | ||
| }; | ||
|
|
||
| // Apply WebGL standards-compliant version handling first | ||
| let source_to_parse = if !s.contains("#version") && detect_webgl2_syntax(s) { | ||
| // WebGL 2.0 requires #version 300 es per specification | ||
| format!("#version 300 es\n{}", s) | ||
| } else { | ||
| // WebGL 1.0 shaders work without version directives per WebGL standard | ||
| s.to_string() | ||
EndlessJour9527 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| }; | ||
|
|
||
| let mut processor = glsl_lang_pp::processor::fs::StdProcessor::new(); | ||
| let mut tu: TranslationUnit = processor | ||
| .open_source(s, Path::new(".")) | ||
| .open_source(&source_to_parse, Path::new(".")) | ||
| .builder() | ||
| .parse() | ||
| .map(|(mut tu, _, iter)| { | ||
| iter.into_directives().inject(&mut tu); | ||
| tu | ||
| }) | ||
| .expect(format!("Failed to parse GLSL source: \n{}\n", s).as_str()); | ||
| .expect(format!("Failed to parse GLSL source: \n{}\n", source_to_parse).as_str()); | ||
|
|
||
| let mut my_glsl_patcher = MyGLSLPatcher {}; | ||
| tu.visit_mut(&mut my_glsl_patcher); | ||
|
|
@@ -235,4 +286,117 @@ vec3 test() { | |
| "# | ||
| ) | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_patch_glsl_source_missing_version_webgl1_fragment() { | ||
| let source_str = r#" | ||
| precision mediump float; | ||
| void main() { | ||
| gl_FragColor = vec4(0., 1., 0., 1.); | ||
| } | ||
| "#; | ||
| let patched_source_str = patch_glsl_source_from_str(source_str); | ||
| // WebGL 1.0 shaders should remain unchanged (no version injection) | ||
|
||
| assert_eq!( | ||
| patched_source_str, | ||
| r#"precision mediump float; | ||
| void main() { | ||
| gl_FragColor = vec4(0., 1., 0., 1.); | ||
| } | ||
| "# | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_patch_glsl_source_missing_version_webgl2_fragment() { | ||
| let source_str = r#" | ||
| precision mediump float; | ||
| out vec4 fragColor; | ||
| void main() { | ||
| fragColor = vec4(1.0, 0.0, 0.0, 1.0); | ||
| } | ||
| "#; | ||
| let patched_source_str = patch_glsl_source_from_str(source_str); | ||
| // WebGL 2.0 shaders should get #version 300 es injected | ||
| assert_eq!( | ||
| patched_source_str, | ||
| r#"#version 300 es | ||
| precision mediump float; | ||
| out vec4 fragColor; | ||
| void main() { | ||
| fragColor = vec4(1., 0., 0., 1.); | ||
| } | ||
| "# | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_patch_glsl_source_missing_version_webgl1_vertex() { | ||
| let source_str = r#" | ||
| attribute vec4 a_position; | ||
| varying vec2 v_texcoord; | ||
| void main() { | ||
| gl_Position = a_position; | ||
| v_texcoord = a_position.xy; | ||
| } | ||
| "#; | ||
| let patched_source_str = patch_glsl_source_from_str(source_str); | ||
| // WebGL 1.0 vertex shader should remain unchanged | ||
| assert_eq!( | ||
| patched_source_str, | ||
| r#"attribute vec4 a_position; | ||
| varying vec2 v_texcoord; | ||
| void main() { | ||
| gl_Position = a_position; | ||
| v_texcoord = a_position.xy; | ||
| } | ||
| "# | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_patch_glsl_source_missing_version_webgl2_vertex() { | ||
| let source_str = r#" | ||
| layout(location = 0) in vec4 a_position; | ||
| out vec2 v_texcoord; | ||
| void main() { | ||
| gl_Position = a_position; | ||
| v_texcoord = a_position.xy; | ||
| } | ||
| "#; | ||
| let patched_source_str = patch_glsl_source_from_str(source_str); | ||
| // WebGL 2.0 vertex shader should get #version 300 es injected | ||
| assert_eq!( | ||
| patched_source_str, | ||
| r#"#version 300 es | ||
| layout(location = 0) in vec4 a_position; | ||
| out vec2 v_texcoord; | ||
| void main() { | ||
| gl_Position = a_position; | ||
| v_texcoord = a_position.xy; | ||
| } | ||
| "# | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_patch_glsl_source_existing_version_unchanged() { | ||
| let source_str = r#"#version 100 | ||
| precision mediump float; | ||
| void main() { | ||
| gl_FragColor = vec4(0., 1., 0., 1.); | ||
| } | ||
| "#; | ||
| let patched_source_str = patch_glsl_source_from_str(source_str); | ||
| // Existing version should remain unchanged | ||
| assert_eq!( | ||
| patched_source_str, | ||
| r#"#version 100 | ||
| precision mediump float; | ||
| void main() { | ||
| gl_FragColor = vec4(0., 1., 0., 1.); | ||
| } | ||
| "# | ||
| ); | ||
| } | ||
| } | ||
162 changes: 162 additions & 0 deletions
162
fixtures/html/webgl-conformance/test-shader-version-fix.html
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,162 @@ | ||
| <!DOCTYPE html> | ||
EndlessJour9527 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="UTF-8"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
| <title>WebGL Standards-Compliant GLSL Version Handling Test</title> | ||
| <style> | ||
| body { font-family: Arial, sans-serif; margin: 20px; } | ||
| .test-result { margin: 10px 0; padding: 8px; border-radius: 4px; } | ||
| .pass { background: #d4edda; border: 1px solid #c3e6cb; color: #155724; } | ||
| .fail { background: #f8d7da; border: 1px solid #f5c6cb; color: #721c24; } | ||
| .test-section { margin: 20px 0; border: 1px solid #ddd; padding: 15px; border-radius: 4px; } | ||
| pre { background: #f8f9fa; padding: 10px; border-radius: 4px; overflow-x: auto; } | ||
| </style> | ||
| </head> | ||
| <body> | ||
| <h1>WebGL Standards-Compliant GLSL Version Handling Test</h1> | ||
| <p>This test validates WebGL standards-compliant GLSL version handling:</p> | ||
| <ul> | ||
| <li><strong>WebGL 1.0</strong>: Version directives are optional (should default to GLSL ES 1.00)</li> | ||
| <li><strong>WebGL 2.0</strong>: Version directives are required (#version 300 es)</li> | ||
| </ul> | ||
|
|
||
| <div id="results"></div> | ||
|
|
||
| <script> | ||
| const results = document.getElementById('results'); | ||
|
|
||
| function addResult(message, isPass, section = null) { | ||
| const div = document.createElement('div'); | ||
| div.className = `test-result ${isPass ? 'pass' : 'fail'}`; | ||
| div.innerHTML = `${isPass ? '✅' : '❌'} ${message}`; | ||
|
|
||
| if (section) { | ||
| section.appendChild(div); | ||
| } else { | ||
| results.appendChild(div); | ||
| } | ||
| } | ||
|
|
||
| function addSection(title) { | ||
| const section = document.createElement('div'); | ||
| section.className = 'test-section'; | ||
| const header = document.createElement('h2'); | ||
| header.textContent = title; | ||
| section.appendChild(header); | ||
| results.appendChild(section); | ||
| return section; | ||
| } | ||
|
|
||
| function createShader(gl, type, source) { | ||
| const shader = gl.createShader(type); | ||
| gl.shaderSource(shader, source); | ||
| gl.compileShader(shader); | ||
|
|
||
| if (!gl.getShaderParameter(shader, gl.COMPILE_STATUS)) { | ||
| console.error('Shader compilation error:', gl.getShaderInfoLog(shader)); | ||
| return null; | ||
| } | ||
| return shader; | ||
| } | ||
|
|
||
| function testShaderCompilation(gl, shaderType, source, testName, section) { | ||
| try { | ||
| const shader = createShader(gl, shaderType, source); | ||
| if (shader) { | ||
| addResult(`${testName}: Shader compiled successfully`, true, section); | ||
| gl.deleteShader(shader); | ||
| return true; | ||
| } else { | ||
| const error = gl.getShaderInfoLog(createShader(gl, shaderType, source)); | ||
| addResult(`${testName}: Compilation failed - ${error}`, false, section); | ||
| return false; | ||
| } | ||
| } catch (e) { | ||
| addResult(`${testName}: Exception - ${e.message}`, false, section); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| function runTests() { | ||
| const gl = navigator.gl; | ||
| if (!gl) { | ||
EndlessJour9527 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| addResult('WebGL not supported - test cannot run', false); | ||
| return; | ||
| } | ||
|
|
||
| addResult(`WebGL context available: ${gl.constructor.name}`, true); | ||
| addResult(`WebGL version: ${gl.getParameter(gl.VERSION)}`, true); | ||
|
|
||
| // Test 1: WebGL 1.0 shaders without version directive | ||
| // Per WebGL spec, these should work WITHOUT version directives | ||
| const webgl1Section = addSection('WebGL 1.0 Shaders (should work without #version per WebGL spec)'); | ||
|
|
||
| // The exact failing case from the issue | ||
| const fragmentShaderNoVersion = ` | ||
| void main() { | ||
| gl_FragColor = vec4(0., 1., 0., 1.); | ||
| }`; | ||
|
|
||
| const vertexShaderNoVersion = ` | ||
| attribute vec4 position; | ||
| void main() { | ||
| gl_Position = position; | ||
| }`; | ||
|
|
||
| testShaderCompilation(gl, gl.VERTEX_SHADER, vertexShaderNoVersion, | ||
| 'Vertex shader without #version (using attribute)', webgl1Section); | ||
| testShaderCompilation(gl, gl.FRAGMENT_SHADER, fragmentShaderNoVersion, | ||
| 'Fragment shader without #version (using gl_FragColor)', webgl1Section); | ||
|
|
||
| // Test 2: WebGL 2.0 style shaders without version directive | ||
| // Per WebGL spec, these REQUIRE #version 300 es | ||
| if (gl.constructor.name.includes('WebGL2')) { | ||
| const webgl2Section = addSection('WebGL 2.0 Shaders (MUST have #version 300 es per WebGL spec)'); | ||
|
|
||
| const vertexShaderWebGL2 = ` | ||
| in vec4 position; | ||
| void main() { | ||
| gl_Position = position; | ||
| }`; | ||
|
|
||
| const fragmentShaderWebGL2 = ` | ||
| precision mediump float; | ||
| out vec4 fragColor; | ||
| void main() { | ||
| fragColor = vec4(0., 1., 0., 1.); | ||
| }`; | ||
|
|
||
| testShaderCompilation(gl, gl.VERTEX_SHADER, vertexShaderWebGL2, | ||
| 'Vertex shader without #version (using in)', webgl2Section); | ||
| testShaderCompilation(gl, gl.FRAGMENT_SHADER, fragmentShaderWebGL2, | ||
| 'Fragment shader without #version (using out)', webgl2Section); | ||
| } | ||
|
|
||
| // Test 3: Existing shaders with version directives (should work as before) | ||
| const existingSection = addSection('Existing Shaders (should remain unchanged)'); | ||
|
|
||
| const vertexWithVersion = ` | ||
| #version 100 | ||
| attribute vec4 position; | ||
| void main() { | ||
| gl_Position = position; | ||
| }`; | ||
|
|
||
| const fragmentWithVersion = ` | ||
| #version 100 | ||
| void main() { | ||
| gl_FragColor = vec4(1.0, 0.0, 0.0, 1.0); | ||
| }`; | ||
|
|
||
| testShaderCompilation(gl, gl.VERTEX_SHADER, vertexWithVersion, | ||
| 'Vertex shader with existing #version 100', existingSection); | ||
| testShaderCompilation(gl, gl.FRAGMENT_SHADER, fragmentWithVersion, | ||
| 'Fragment shader with existing #version 100', existingSection); | ||
| } | ||
|
|
||
| // Run tests when page loads | ||
| document.addEventListener('DOMContentLoaded', runTests); | ||
| </script> | ||
| </body> | ||
| </html> | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.