Skip to content

Commit

Permalink
Merge pull request IrisShaders#1884 from douira/1.18.2-transform-patcher
Browse files Browse the repository at this point in the history
Array-type out declaration handling in compatibility transformer
  • Loading branch information
IMS212 authored Feb 13, 2023
2 parents 7371c47 + bc40395 commit 64f1b34
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 20 deletions.
3 changes: 2 additions & 1 deletion docs/usage/debugging.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ The following is a list of the compatibility patches that are applied to shader

- Empty external declarations, which are just a single semicolon at the global level `;`, are removed. This is a workaround for a bug where they are not recognized as legal external declarations by some drivers. A semicolon following a function definition like `void main() {};` is in fact not part of the function definition but its own external declaration, namely an empty one.
- The `const` keyword is removed from declaration (statements) that refer to `const` parameters in their initializer expression, the part that comes after the `=`. Parameters with the `const` qualifier are not constant but rather immutable, meaning they may not be assigned to. Declarations with `const` are, depending on the driver and GLSL version, treated as constant meaning they only accept expressions that can be evaluated at compile time. Immutable parameters don't fulfill this requirement and thus cause a compilation error when they are used in the initializer of a constant. This is done to ensure better compatibility drivers' varying behavior at different versions. See Section 4.3.2 on Constant Qualifiers and Section 4.3.3 on Constant Expressions in the GLSL 4.1 and 4.2 specifications for more information. Additionally, see https://wiki.shaderlabs.org/wiki/Compiler_Behavior_Notes for the varying behavior of drivers.
- When an input variable, declared with `in`, is declared and used in a geometry or fragment shader, but there is no corresponding output variable in the shader of the previous stage, some drivers will error. To mitigate this, the missing declaration is inserted and initialized with a default value at the top of the `main` function.
- When an input variable, declared with `in`, is declared and used in a geometry or fragment shader, but there is no corresponding output variable in the shader of the previous stage, some drivers will error. To mitigate this, the missing declaration is inserted and initialized with a default value at the top of the `main` function.
- If the out declaration does exist but is never assigned to, an initialization is created. If the out declaration has an array type, compatibility patching is skipped for it and a warning is produced.
- When the types of declared and used input and output variables don't match, some drivers will error. To mitigate this, the type of the declaration is changed to match the type in the later stage (the fragment shader). An internal variable with the original type is added so that the code can assign a value to it. At the end of the `main` function this value is either truncated or padded to patch the expected type.
- Unused functions are removed. Some drivers don't do certain semantic checks on unused functions which may result in errors when the code is then compiled with stricter drivers that do perform these checks before unused code removal. This heuristic is not perfect and may fail to remove unreachable functions that are used in another unreachable function.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ public String toString() {
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + (texture ? 1231 : 1237);
result = prime * result + (lightmap ? 1231 : 1237);
result = prime * result + (overlay ? 1231 : 1237);
result = prime * result + (texture ? 1231 : 1237);
return result;
}

Expand All @@ -68,12 +68,12 @@ public boolean equals(Object obj) {
if (getClass() != obj.getClass())
return false;
InputAvailability other = (InputAvailability) obj;
if (texture != other.texture)
return false;
if (lightmap != other.lightmap)
return false;
if (overlay != other.overlay)
return false;
if (texture != other.texture)
return false;
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,11 @@ public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + (color ? 1231 : 1237);
result = prime * result + (tex ? 1231 : 1237);
result = prime * result + (overlay ? 1231 : 1237);
result = prime * result + (light ? 1231 : 1237);
result = prime * result + (newLines ? 1231 : 1237);
result = prime * result + (normal ? 1231 : 1237);
result = prime * result + (overlay ? 1231 : 1237);
result = prime * result + (tex ? 1231 : 1237);
result = prime * result + (newLines ? 1231 : 1237);
return result;
}

Expand All @@ -101,15 +101,15 @@ public boolean equals(Object obj) {
ShaderAttributeInputs other = (ShaderAttributeInputs) obj;
if (color != other.color)
return false;
if (light != other.light)
if (tex != other.tex)
return false;
if (newLines != other.newLines)
if (overlay != other.overlay)
return false;
if (normal != other.normal)
if (light != other.light)
return false;
if (overlay != other.overlay)
if (normal != other.normal)
return false;
if (tex != other.tex)
if (newLines != other.newLines)
return false;
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@ public class SodiumParameters extends Parameters {
public final float positionScale;
public final float positionOffset;
public final float textureScale;
public AlphaTest alpha;
// WARNING: adding new fields requires updating hashCode and equals methods!

// DO NOT include this field in hashCode or equals, it's mutable!
// (See use of setAlphaFor in TransformPatcher)
public AlphaTest alpha;

public SodiumParameters(Patch patch,
Object2ObjectMap<Tri<String, TextureType, TextureStage>, String> textureMap,
AlphaTest cutoutAlpha,
Expand Down Expand Up @@ -68,7 +71,6 @@ public int hashCode() {
result = prime * result + Float.floatToIntBits(positionScale);
result = prime * result + Float.floatToIntBits(positionOffset);
result = prime * result + Float.floatToIntBits(textureScale);
result = prime * result + ((alpha == null) ? 0 : alpha.hashCode());
return result;
}

Expand Down Expand Up @@ -102,11 +104,6 @@ public boolean equals(Object obj) {
return false;
if (Float.floatToIntBits(textureScale) != Float.floatToIntBits(other.textureScale))
return false;
if (alpha == null) {
if (other.alpha != null)
return false;
} else if (!alpha.equals(other.alpha))
return false;
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.Deque;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -473,6 +472,20 @@ public static void transformGrouped(
Type inType = inTypeSpecifier.type;
Type outType = outTypeSpecifier.type;

// check if the out declaration is an array-type, if so, skip it.
// this only checks the out declaration because it's the one that when it's an
// array type means that both declarations are arrays and we're not just in the
// case of a geometry shader where the in declaration is an array and the out
// declaration is not
if (outTypeSpecifier.getArraySpecifier() != null) {
LOGGER.warn(
"The out declaration '" + name + "' in the " + prevPatchTypes.glShaderType.name()
+ " shader that has a missing corresponding in declaration in the next stage "
+ type.name()
+ " has an array type and could not be compatibility-patched. See debugging.md for more information.");
continue;
}

// skip if the type matches, nothing has to be done
if (inType == outType) {
// if the types match but it's never assigned a value,
Expand All @@ -484,6 +497,12 @@ public static void transformGrouped(
// add an initialization statement for this declaration
prevTree.prependMainFunctionBody(getInitializer(prevRoot, name, inType));
outDeclarations.put(name, null);

LOGGER.warn(
"The in declaration '" + name + "' in the " + currentType.glShaderType.name()
+ " shader that is never assigned to in the previous stage "
+ prevType.name()
+ " has been compatibility-patched by adding an initialization for it. See debugging.md for more information.");
continue;
}

Expand Down

0 comments on commit 64f1b34

Please sign in to comment.