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.
Updated all shaders to check for WebGL 2 context on the canvas element before compiling. If a WebGL 2 context is found, Open GL 3.0 syntax is inserted at the beginning of each shader. Code is also mildly style updated to utilize template literals.
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
Did you test your solution?
Problem Description
As noted in #38, shaders have been broken since THREE.js r118. I believe it is because these newer THREE.js versions are expecting OpenGL ES 3.0 syntax in their shaders.
Solution Description
I added a check in each shader to look for WebGL 2 rendering context on the canvas. If no WebGL rendering context is found, the shaders remain more or less untouched (see below). Otherwise, if WebGL 2 context is found, the shaders use template literals to insert OpenGL ES 3.0 syntax at the head of each shader.
Template literals are also now used throughout the shaders for ease of use and readability.
The only other slight change is found in the vertex shaders. They now import position as a vec3 attribute, inserting the attribute into a vec4 inside the main function. I believe this change is inconsequential, merely adopting the format of the standard THREE.js shaders.
Side Effects, Risks, Impact
I found no side effects when testing this in my own projects as well as internally testing the npm scrips in the package.json.
Aditional comments:
I believe that the tests in this repo are actually using a much later version of THREE.js than the dependency found in the package.json. I was playing around with updating a lot of these dependencies before committing to these specific shader changes, and I couldn't figure out why updating THREE.js to the latest version wasn't demanding OpenGL ES 3.0 on the shaders (which is actually what led me to think of adding a WebGL 2 check on the canvas before compiling each shader). I traced it to the three-orbit-viewer, and from there, three-orbit-controls. Updating the dependencies on those didn't seem to break anything, but it also didn't generate a WebGL 2 canvas either, so I ended up leaving that dependency rabbit hole for the time being in the interest of doing something more productive.
Hopefully this update will allow people using later versions of THREE.js to use the repo out of the box, without needing to dig into the shaders unless they want to, while also allowing people using currently using this repo with older THREE.js versions to continue updating it without breaking any functionality.
PS the first commit also included changes to index.js and load.js, which I rolled back and am including in a separate PR, as they aren't related to the shader changes at all.