From d27929c307f9a1bf30e886fe43f9e49575c99309 Mon Sep 17 00:00:00 2001 From: Jozufozu Date: Sat, 13 May 2023 17:02:18 -0700 Subject: [PATCH] Shader sanity - Switch #use to #include - Stop ignoring errors from within shader compilers - Track load errors in CompilerStats - Move LoadError to separate sealed interface, LoadResult.Failure wraps a LoadError - Move SourceFile member parsing methods to respective classes - Add tests for SourceFile loading - Start work on tests for error messages --- .../backend/compile/AbstractCompiler.java | 10 ++ .../backend/compile/CullingCompiler.java | 21 +-- .../backend/compile/PipelineCompiler.java | 58 +++++-- .../backend/compile/core/CompilerStats.java | 40 ++++- .../compile/core/FailedCompilation.java | 2 +- .../backend/compile/core/ShaderCompiler.java | 23 +-- .../com/jozufozu/flywheel/glsl/LoadError.java | 58 +++++++ .../jozufozu/flywheel/glsl/LoadResult.java | 25 +-- .../flywheel/glsl/ShaderLoadingException.java | 12 -- .../jozufozu/flywheel/glsl/ShaderSources.java | 52 +++--- .../jozufozu/flywheel/glsl/SourceFile.java | 162 +++--------------- .../flywheel/glsl/error/ErrorBuilder.java | 66 +++++-- .../flywheel/glsl/error/ErrorLevel.java | 18 +- .../flywheel/glsl/error/ErrorReporter.java | 115 ------------- .../flywheel/glsl/error/lines/NestedLine.java | 8 + .../jozufozu/flywheel/glsl/package-info.java | 6 + .../jozufozu/flywheel/glsl/parse/Import.java | 24 ++- .../flywheel/glsl/parse/ShaderField.java | 23 +++ .../flywheel/glsl/parse/ShaderFunction.java | 62 +++++++ .../flywheel/glsl/parse/ShaderStruct.java | 22 +++ .../flywheel/flywheel/context/common.vert | 4 +- .../flywheel/flywheel/context/crumbling.frag | 2 +- .../flywheel/flywheel/context/crumbling.vert | 2 +- .../flywheel/flywheel/context/world.frag | 2 +- .../flywheel/flywheel/context/world.vert | 2 +- .../flywheel/flywheel/instance/oriented.vert | 6 +- .../flywheel/instance/transformed.vert | 2 +- .../flywheel/flywheel/internal/draw.frag | 2 +- .../flywheel/internal/indirect_cull.glsl | 4 +- .../flywheel/internal/indirect_draw.vert | 4 +- .../internal/indirect_draw_command.glsl | 2 +- .../internal/instanced_arrays_draw.vert | 2 +- .../flywheel/flywheel/layout/block.vert | 2 +- .../flywheel/layout/pos_tex_normal.vert | 2 +- .../flywheel/flywheel/material/cutout.frag | 4 +- .../flywheel/flywheel/material/default.frag | 4 +- .../flywheel/flywheel/material/default.vert | 2 +- .../flywheel/flywheel/material/shaded.vert | 4 +- .../flywheel/glsl/MockShaderSources.java | 38 ++++ .../com/jozufozu/flywheel/glsl/TestBase.java | 67 ++++++++ .../flywheel/glsl/TestErrorMessages.java | 33 ++++ .../glsl/TestShaderSourceLoading.java | 146 ++++++++++++++++ 42 files changed, 731 insertions(+), 412 deletions(-) create mode 100644 src/main/java/com/jozufozu/flywheel/glsl/LoadError.java delete mode 100644 src/main/java/com/jozufozu/flywheel/glsl/ShaderLoadingException.java delete mode 100644 src/main/java/com/jozufozu/flywheel/glsl/error/ErrorReporter.java create mode 100644 src/main/java/com/jozufozu/flywheel/glsl/error/lines/NestedLine.java create mode 100644 src/main/java/com/jozufozu/flywheel/glsl/package-info.java create mode 100644 src/test/java/com/jozufozu/flywheel/glsl/MockShaderSources.java create mode 100644 src/test/java/com/jozufozu/flywheel/glsl/TestBase.java create mode 100644 src/test/java/com/jozufozu/flywheel/glsl/TestErrorMessages.java create mode 100644 src/test/java/com/jozufozu/flywheel/glsl/TestShaderSourceLoading.java diff --git a/src/main/java/com/jozufozu/flywheel/backend/compile/AbstractCompiler.java b/src/main/java/com/jozufozu/flywheel/backend/compile/AbstractCompiler.java index 922921fa4..cfa1aead9 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/compile/AbstractCompiler.java +++ b/src/main/java/com/jozufozu/flywheel/backend/compile/AbstractCompiler.java @@ -12,6 +12,9 @@ import com.jozufozu.flywheel.backend.compile.core.ProgramLinker; import com.jozufozu.flywheel.backend.compile.core.ShaderCompiler; import com.jozufozu.flywheel.gl.shader.GlProgram; import com.jozufozu.flywheel.glsl.ShaderSources; +import com.jozufozu.flywheel.glsl.SourceFile; + +import net.minecraft.resources.ResourceLocation; public abstract class AbstractCompiler { protected final ShaderSources sources; @@ -31,6 +34,13 @@ public abstract class AbstractCompiler { @Nullable protected abstract GlProgram compile(K key); + @Nullable + protected SourceFile findOrReport(ResourceLocation rl) { + var out = sources.find(rl); + stats.loadResult(out); + return out.unwrap(); + } + @Nullable public Map compileAndReportErrors() { stats.start(); diff --git a/src/main/java/com/jozufozu/flywheel/backend/compile/CullingCompiler.java b/src/main/java/com/jozufozu/flywheel/backend/compile/CullingCompiler.java index 8af57d948..919476a04 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/compile/CullingCompiler.java +++ b/src/main/java/com/jozufozu/flywheel/backend/compile/CullingCompiler.java @@ -1,7 +1,5 @@ package com.jozufozu.flywheel.backend.compile; -import java.util.List; - import org.jetbrains.annotations.Nullable; import com.google.common.collect.ImmutableList; @@ -13,7 +11,6 @@ import com.jozufozu.flywheel.gl.shader.GlProgram; import com.jozufozu.flywheel.gl.shader.ShaderType; import com.jozufozu.flywheel.glsl.GLSLVersion; import com.jozufozu.flywheel.glsl.ShaderSources; -import com.jozufozu.flywheel.glsl.SourceComponent; import com.jozufozu.flywheel.glsl.SourceFile; import net.minecraft.resources.ResourceLocation; @@ -33,7 +30,14 @@ public class CullingCompiler extends AbstractCompiler> { @Nullable @Override protected GlProgram compile(InstanceType key) { - var computeComponents = getComputeComponents(key); + var instanceAssembly = new IndirectComponent(sources, key); + var instance = findOrReport(key.instanceShader()); + + if (instance == null) { + return null; + } + + var computeComponents = ImmutableList.of(uniformComponent, instanceAssembly, instance, pipelineCompute); var compute = shaderCompiler.compile(GLSLVersion.V460, ShaderType.COMPUTE, computeComponents); if (compute == null) { @@ -43,15 +47,6 @@ public class CullingCompiler extends AbstractCompiler> { return programLinker.link(compute); } - private List getComputeComponents(InstanceType instanceType) { - var instanceAssembly = new IndirectComponent(sources, instanceType); - ResourceLocation key = instanceType.instanceShader(); - var instance = sources.find(key) - .unwrap(); - - return ImmutableList.of(uniformComponent, instanceAssembly, instance, pipelineCompute); - } - private static final class Files { public static final ResourceLocation INDIRECT_CULL = Flywheel.rl("internal/indirect_cull.glsl"); } diff --git a/src/main/java/com/jozufozu/flywheel/backend/compile/PipelineCompiler.java b/src/main/java/com/jozufozu/flywheel/backend/compile/PipelineCompiler.java index f03f399ad..5d59e6176 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/compile/PipelineCompiler.java +++ b/src/main/java/com/jozufozu/flywheel/backend/compile/PipelineCompiler.java @@ -8,6 +8,7 @@ import com.google.common.collect.ImmutableList; import com.jozufozu.flywheel.backend.compile.component.MaterialAdapterComponent; import com.jozufozu.flywheel.backend.compile.component.UniformComponent; import com.jozufozu.flywheel.gl.shader.GlProgram; +import com.jozufozu.flywheel.gl.shader.GlShader; import com.jozufozu.flywheel.gl.shader.ShaderType; import com.jozufozu.flywheel.glsl.ShaderSources; import com.jozufozu.flywheel.glsl.SourceComponent; @@ -37,10 +38,8 @@ public class PipelineCompiler extends AbstractCompiler { @Nullable @Override protected GlProgram compile(PipelineProgramKey key) { - var glslVersion = pipeline.glslVersion(); - - var vertex = shaderCompiler.compile(glslVersion, ShaderType.VERTEX, getVertexComponents(key)); - var fragment = shaderCompiler.compile(glslVersion, ShaderType.FRAGMENT, getFragmentComponents(key)); + GlShader vertex = compileVertex(key); + GlShader fragment = compileFragment(key); if (vertex == null || fragment == null) { return null; @@ -52,27 +51,54 @@ public class PipelineCompiler extends AbstractCompiler { return glProgram; } + @Nullable + private GlShader compileVertex(PipelineProgramKey key) { + var vertexComponents = getVertexComponents(key); + if (vertexComponents == null) { + return null; + } + + return shaderCompiler.compile(pipeline.glslVersion(), ShaderType.VERTEX, vertexComponents); + } + + @Nullable + private GlShader compileFragment(PipelineProgramKey key) { + var fragmentComponents = getFragmentComponents(key); + if (fragmentComponents == null) { + return null; + } + + return shaderCompiler.compile(pipeline.glslVersion(), ShaderType.FRAGMENT, fragmentComponents); + } + + @Nullable private List getVertexComponents(PipelineProgramKey key) { var instanceAssembly = pipeline.assembler() .assemble(new Pipeline.InstanceAssemblerContext(sources, key.vertexType(), key.instanceType())); - var layout = sources.find(key.vertexType() - .layoutShader()) - .unwrap(); - var instance = sources.find(key.instanceType() - .instanceShader()) - .unwrap(); - var context = sources.find(key.contextShader() - .vertexShader()) - .unwrap(); + var layout = findOrReport(key.vertexType() + .layoutShader()); + var instance = findOrReport(key.instanceType() + .instanceShader()); + var context = findOrReport(key.contextShader() + .vertexShader()); + + if (instanceAssembly == null || layout == null || instance == null || context == null) { + return null; + } return ImmutableList.of(uniformComponent, vertexMaterialComponent, instanceAssembly, layout, instance, context, pipelineVertex); } + @Nullable private List getFragmentComponents(PipelineProgramKey key) { - var context = sources.find(key.contextShader() - .fragmentShader()) - .unwrap(); + var context = findOrReport(key.contextShader() + .fragmentShader()); + + if (context == null) { + return null; + } + return ImmutableList.of(uniformComponent, fragmentMaterialComponent, context, pipelineFragment); } } diff --git a/src/main/java/com/jozufozu/flywheel/backend/compile/core/CompilerStats.java b/src/main/java/com/jozufozu/flywheel/backend/compile/core/CompilerStats.java index 168c4d6ce..4ce56e402 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/compile/core/CompilerStats.java +++ b/src/main/java/com/jozufozu/flywheel/backend/compile/core/CompilerStats.java @@ -1,10 +1,17 @@ package com.jozufozu.flywheel.backend.compile.core; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; +import org.jetbrains.annotations.NotNull; + import com.jozufozu.flywheel.Flywheel; +import com.jozufozu.flywheel.glsl.LoadError; +import com.jozufozu.flywheel.glsl.LoadResult; +import com.jozufozu.flywheel.glsl.error.ErrorBuilder; import com.jozufozu.flywheel.util.StringUtil; public class CompilerStats { @@ -12,6 +19,8 @@ public class CompilerStats { private final List shaderErrors = new ArrayList<>(); private final List programErrors = new ArrayList<>(); + private final Set loadErrors = new HashSet<>(); + private boolean errored = false; private int shaderCount = 0; private int programCount = 0; @@ -32,8 +41,28 @@ public class CompilerStats { } public String generateErrorLog() { - return String.join("\n", programErrors) + '\n' + shaderErrors.stream() - .map(FailedCompilation::getMessage) + return """ + %s + %s + %s + """.formatted(loadErrors(), compileErrors(), linkErrors()); + } + + private String compileErrors() { + return shaderErrors.stream() + .map(FailedCompilation::generateMessage) + .collect(Collectors.joining("\n")); + } + + @NotNull + private String linkErrors() { + return String.join("\n", programErrors); + } + + private String loadErrors() { + return loadErrors.stream() + .map(LoadError::generateMessage) + .map(ErrorBuilder::build) .collect(Collectors.joining("\n")); } @@ -52,4 +81,11 @@ public class CompilerStats { } programCount++; } + + public void loadResult(LoadResult loadResult) { + if (loadResult instanceof LoadResult.Failure f) { + loadErrors.add(f.error()); + errored = true; + } + } } diff --git a/src/main/java/com/jozufozu/flywheel/backend/compile/core/FailedCompilation.java b/src/main/java/com/jozufozu/flywheel/backend/compile/core/FailedCompilation.java index 0ada67369..55fbde354 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/compile/core/FailedCompilation.java +++ b/src/main/java/com/jozufozu/flywheel/backend/compile/core/FailedCompilation.java @@ -33,7 +33,7 @@ public class FailedCompilation { this.errorLog = errorLog; } - public String getMessage() { + public String generateMessage() { return ConsoleColors.RED_BOLD_BRIGHT + "Failed to compile " + shaderName + ":\n" + errorString(); } diff --git a/src/main/java/com/jozufozu/flywheel/backend/compile/core/ShaderCompiler.java b/src/main/java/com/jozufozu/flywheel/backend/compile/core/ShaderCompiler.java index 9cb6d8b45..8fec7bec8 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/compile/core/ShaderCompiler.java +++ b/src/main/java/com/jozufozu/flywheel/backend/compile/core/ShaderCompiler.java @@ -8,7 +8,6 @@ import java.util.Objects; import java.util.Set; import java.util.function.Consumer; -import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import com.jozufozu.flywheel.gl.shader.GlShader; @@ -24,10 +23,6 @@ public class ShaderCompiler { this.stats = stats; } - public int shaderCount() { - return shaderCache.size(); - } - @Nullable public GlShader compile(GLSLVersion glslVersion, ShaderType shaderType, List sourceComponents) { var key = new ShaderKey(glslVersion, shaderType, sourceComponents); @@ -36,7 +31,13 @@ public class ShaderCompiler { return cached.unwrap(); } - ShaderResult out = compileUncached(new Compilation(glslVersion, shaderType), sourceComponents); + Compilation ctx = new Compilation(glslVersion, shaderType); + ctx.enableExtension("GL_ARB_explicit_attrib_location"); + ctx.enableExtension("GL_ARB_conservative_depth"); + + expand(sourceComponents, ctx::appendComponent); + + ShaderResult out = ctx.compile(); shaderCache.put(key, out); stats.shaderResult(out); return out.unwrap(); @@ -50,16 +51,6 @@ public class ShaderCompiler { .forEach(GlShader::delete); } - @NotNull - private ShaderResult compileUncached(Compilation ctx, List sourceComponents) { - ctx.enableExtension("GL_ARB_explicit_attrib_location"); - ctx.enableExtension("GL_ARB_conservative_depth"); - - expand(sourceComponents, ctx::appendComponent); - - return ctx.compile(); - } - private static void expand(List rootSources, Consumer out) { var included = new LinkedHashSet(); // use hash set to deduplicate. linked to preserve order for (var component : rootSources) { diff --git a/src/main/java/com/jozufozu/flywheel/glsl/LoadError.java b/src/main/java/com/jozufozu/flywheel/glsl/LoadError.java new file mode 100644 index 000000000..a22764d9b --- /dev/null +++ b/src/main/java/com/jozufozu/flywheel/glsl/LoadError.java @@ -0,0 +1,58 @@ +package com.jozufozu.flywheel.glsl; + +import java.io.IOException; +import java.util.List; +import java.util.stream.Collectors; + +import com.jozufozu.flywheel.glsl.error.ErrorBuilder; +import com.jozufozu.flywheel.glsl.span.Span; +import com.jozufozu.flywheel.util.Pair; + +import net.minecraft.resources.ResourceLocation; + +sealed public interface LoadError { + ErrorBuilder generateMessage(); + + record CircularDependency(ResourceLocation offender, List stack) implements LoadError { + public String format() { + return stack.stream() + .dropWhile(l -> !l.equals(offender)) + .map(ResourceLocation::toString) + .collect(Collectors.joining(" -> ")); + } + + @Override + public ErrorBuilder generateMessage() { + return ErrorBuilder.create() + .error("files are circularly dependent") + .note(format()); + } + } + + record IncludeError(ResourceLocation location, List> innerErrors) implements LoadError { + @Override + public ErrorBuilder generateMessage() { + var out = ErrorBuilder.create() + .error("could not load shader due to errors in included files") + .pointAtFile(location); + + for (var innerError : innerErrors) { + var err = innerError.second() + .generateMessage(); + out.pointAt(innerError.first()) + .nested(err); + } + + return out; + } + } + + record IOError(ResourceLocation location, IOException exception) implements LoadError { + @Override + public ErrorBuilder generateMessage() { + return ErrorBuilder.create() + .error("could not load \"" + location + "\" due to an IO error") + .note(exception.getMessage()); + } + } +} diff --git a/src/main/java/com/jozufozu/flywheel/glsl/LoadResult.java b/src/main/java/com/jozufozu/flywheel/glsl/LoadResult.java index 58a5ae9b3..bd32a0940 100644 --- a/src/main/java/com/jozufozu/flywheel/glsl/LoadResult.java +++ b/src/main/java/com/jozufozu/flywheel/glsl/LoadResult.java @@ -1,20 +1,14 @@ package com.jozufozu.flywheel.glsl; -import java.io.IOException; -import java.util.List; - import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import net.minecraft.resources.ResourceLocation; - public sealed interface LoadResult { - static LoadResult success(SourceFile sourceFile) { - return new Success(sourceFile); + @Nullable + default SourceFile unwrap() { + return null; } - @Nullable SourceFile unwrap(); - record Success(SourceFile source) implements LoadResult { @Override @NotNull @@ -23,17 +17,6 @@ public sealed interface LoadResult { } } - record IOError(ResourceLocation location, IOException exception) implements LoadResult { - @Override - public SourceFile unwrap() { - return null; - } - } - - record IncludeError(ResourceLocation location, List innerFailures) implements LoadResult { - @Override - public SourceFile unwrap() { - return null; - } + record Failure(LoadError error) implements LoadResult { } } diff --git a/src/main/java/com/jozufozu/flywheel/glsl/ShaderLoadingException.java b/src/main/java/com/jozufozu/flywheel/glsl/ShaderLoadingException.java deleted file mode 100644 index 8ecd90769..000000000 --- a/src/main/java/com/jozufozu/flywheel/glsl/ShaderLoadingException.java +++ /dev/null @@ -1,12 +0,0 @@ -package com.jozufozu.flywheel.glsl; - -public class ShaderLoadingException extends RuntimeException { - - public ShaderLoadingException(String message) { - super(message); - } - - public ShaderLoadingException(String message, Throwable cause) { - super(message, cause); - } -} diff --git a/src/main/java/com/jozufozu/flywheel/glsl/ShaderSources.java b/src/main/java/com/jozufozu/flywheel/glsl/ShaderSources.java index 88273d7d4..7e9d351df 100644 --- a/src/main/java/com/jozufozu/flywheel/glsl/ShaderSources.java +++ b/src/main/java/com/jozufozu/flywheel/glsl/ShaderSources.java @@ -4,11 +4,14 @@ import java.io.IOException; import java.util.ArrayDeque; import java.util.Deque; import java.util.HashMap; +import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import javax.annotation.Nonnull; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.VisibleForTesting; + import com.jozufozu.flywheel.util.ResourceUtil; import com.jozufozu.flywheel.util.StringUtil; @@ -23,7 +26,8 @@ public class ShaderSources { private final ResourceManager manager; - private final Map cache = new HashMap<>(); + @VisibleForTesting + protected final Map cache = new HashMap<>(); /** * Tracks where we are in the mutual recursion to detect circular imports. @@ -36,19 +40,34 @@ public class ShaderSources { @Nonnull public LoadResult find(ResourceLocation location) { - pushFindStack(location); + if (findStack.contains(location)) { + // Make a copy of the find stack with the offending location added on top to show the full path. + findStack.addLast(location); + var copy = List.copyOf(findStack); + findStack.removeLast(); + return new LoadResult.Failure(new LoadError.CircularDependency(location, copy)); + } + findStack.addLast(location); + + LoadResult out = _find(location); + + findStack.removeLast(); + return out; + } + + @NotNull + private LoadResult _find(ResourceLocation location) { // Can't use computeIfAbsent because mutual recursion causes ConcurrentModificationExceptions var out = cache.get(location); if (out == null) { out = load(location); cache.put(location, out); } - popFindStack(); return out; } @Nonnull - private LoadResult load(ResourceLocation loc) { + protected LoadResult load(ResourceLocation loc) { try { var resource = manager.getResource(ResourceUtil.prefixed(SHADER_DIR, loc)); @@ -56,28 +75,7 @@ public class ShaderSources { return SourceFile.parse(this, loc, sourceString); } catch (IOException e) { - return new LoadResult.IOError(loc, e); + return new LoadResult.Failure(new LoadError.IOError(loc, e)); } } - - private void generateRecursiveImportException(ResourceLocation location) { - findStack.add(location); - String path = findStack.stream() - .dropWhile(l -> !l.equals(location)) - .map(ResourceLocation::toString) - .collect(Collectors.joining(" -> ")); - findStack.clear(); - throw new ShaderLoadingException("recursive import: " + path); - } - - private void pushFindStack(ResourceLocation location) { - if (findStack.contains(location)) { - generateRecursiveImportException(location); - } - findStack.addLast(location); - } - - private void popFindStack() { - findStack.removeLast(); - } } diff --git a/src/main/java/com/jozufozu/flywheel/glsl/SourceFile.java b/src/main/java/com/jozufozu/flywheel/glsl/SourceFile.java index f3a7de09c..50d07bb00 100644 --- a/src/main/java/com/jozufozu/flywheel/glsl/SourceFile.java +++ b/src/main/java/com/jozufozu/flywheel/glsl/SourceFile.java @@ -2,13 +2,10 @@ package com.jozufozu.flywheel.glsl; import java.util.ArrayList; import java.util.Collection; -import java.util.HashMap; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.regex.Matcher; import org.jetbrains.annotations.NotNull; @@ -18,9 +15,9 @@ import com.jozufozu.flywheel.glsl.parse.Import; import com.jozufozu.flywheel.glsl.parse.ShaderField; import com.jozufozu.flywheel.glsl.parse.ShaderFunction; import com.jozufozu.flywheel.glsl.parse.ShaderStruct; -import com.jozufozu.flywheel.glsl.span.ErrorSpan; import com.jozufozu.flywheel.glsl.span.Span; import com.jozufozu.flywheel.glsl.span.StringSpan; +import com.jozufozu.flywheel.util.Pair; import net.minecraft.resources.ResourceLocation; @@ -70,10 +67,10 @@ public class SourceFile implements SourceComponent { public static LoadResult parse(ShaderSources sourceFinder, ResourceLocation name, String stringSource) { var source = new SourceLines(name, stringSource); - var imports = parseImports(source); + var imports = Import.parseImports(source); List included = new ArrayList<>(); - List failures = new ArrayList<>(); + List> failures = new ArrayList<>(); Set seen = new HashSet<>(); for (Import i : imports) { @@ -85,20 +82,20 @@ public class SourceFile implements SourceComponent { var result = sourceFinder.find(new ResourceLocation(string)); if (result instanceof LoadResult.Success s) { included.add(s.unwrap()); - } else { - failures.add(result); + } else if (result instanceof LoadResult.Failure e) { + failures.add(Pair.of(fileSpan, e.error())); } } if (!failures.isEmpty()) { - return new LoadResult.IncludeError(name, failures); + return new LoadResult.Failure(new LoadError.IncludeError(name, failures)); } - var functions = parseFunctions(source); - var structs = parseStructs(source); - var fields = parseFields(source); + var functions = ShaderFunction.parseFunctions(source); + var structs = ShaderStruct.parseStructs(source); + var fields = ShaderField.parseFields(source); var finalSource = generateFinalSource(imports, source); - return LoadResult.success(new SourceFile(name, source, functions, structs, imports, fields, included, finalSource)); + return new LoadResult.Success(new SourceFile(name, source, functions, structs, imports, fields, included, finalSource)); } @Override @@ -111,25 +108,6 @@ public class SourceFile implements SourceComponent { return finalSource; } - @NotNull - private static String generateFinalSource(ImmutableList imports, SourceLines source) { - var out = new StringBuilder(); - - int lastEnd = 0; - - for (var include : imports) { - var loc = include.self(); - - out.append(source, lastEnd, loc.startIndex()); - - lastEnd = loc.endIndex(); - } - - out.append(source, lastEnd, source.length()); - - return out.toString(); - } - @Override public ResourceLocation name() { return name; @@ -216,119 +194,23 @@ public class SourceFile implements SourceComponent { return System.identityHashCode(this); } - /** - * Scan the source for {@code #use "..."} directives. - * Records the contents of the directive into an {@link Import} object, and marks the directive for elision. - */ - private static ImmutableList parseImports(SourceLines source) { - Matcher uses = Import.PATTERN.matcher(source); + @NotNull + private static String generateFinalSource(ImmutableList imports, SourceLines source) { + var out = new StringBuilder(); - var imports = ImmutableList.builder(); + int lastEnd = 0; - while (uses.find()) { - Span use = Span.fromMatcher(source, uses); - Span file = Span.fromMatcher(source, uses, 1); + for (var include : imports) { + var loc = include.self(); - imports.add(new Import(use, file)); + out.append(source, lastEnd, loc.startIndex()); + + lastEnd = loc.endIndex(); } - return imports.build(); + out.append(source, lastEnd, source.length()); + + return out.toString(); } - /** - * Scan the source for function definitions and "parse" them into objects that contain properties of the function. - */ - private static ImmutableMap parseFunctions(SourceLines source) { - Matcher matcher = ShaderFunction.PATTERN.matcher(source); - - Map functions = new HashMap<>(); - - while (matcher.find()) { - Span type = Span.fromMatcher(source, matcher, 1); - Span name = Span.fromMatcher(source, matcher, 2); - Span args = Span.fromMatcher(source, matcher, 3); - - int blockStart = matcher.end(); - int blockEnd = findEndOfBlock(source, blockStart); - - Span self; - Span body; - if (blockEnd > blockStart) { - self = new StringSpan(source, matcher.start(), blockEnd + 1); - body = new StringSpan(source, blockStart, blockEnd); - } else { - self = new ErrorSpan(source, matcher.start(), matcher.end()); - body = new ErrorSpan(source, blockStart); - } - - ShaderFunction function = new ShaderFunction(self, type, name, args, body); - - functions.put(name.get(), function); - } - - return ImmutableMap.copyOf(functions); - } - - /** - * Scan the source for function definitions and "parse" them into objects that contain properties of the function. - */ - private static ImmutableMap parseStructs(SourceLines source) { - Matcher matcher = ShaderStruct.PATTERN.matcher(source); - - ImmutableMap.Builder structs = ImmutableMap.builder(); - while (matcher.find()) { - Span self = Span.fromMatcher(source, matcher); - Span name = Span.fromMatcher(source, matcher, 1); - Span body = Span.fromMatcher(source, matcher, 2); - Span variableName = Span.fromMatcher(source, matcher, 3); - - ShaderStruct shaderStruct = new ShaderStruct(self, name, body, variableName); - - structs.put(name.get(), shaderStruct); - } - - return structs.build(); - } - - /** - * Scan the source for function definitions and "parse" them into objects that contain properties of the function. - */ - private static ImmutableMap parseFields(SourceLines source) { - Matcher matcher = ShaderField.PATTERN.matcher(source); - - ImmutableMap.Builder fields = ImmutableMap.builder(); - while (matcher.find()) { - Span self = Span.fromMatcher(source, matcher); - Span location = Span.fromMatcher(source, matcher, 1); - Span decoration = Span.fromMatcher(source, matcher, 2); - Span type = Span.fromMatcher(source, matcher, 3); - Span name = Span.fromMatcher(source, matcher, 4); - - fields.put(location.get(), new ShaderField(self, location, decoration, type, name)); - } - - return fields.build(); - } - - /** - * Given the position of an opening brace, scans through the source for a paired closing brace. - */ - private static int findEndOfBlock(CharSequence source, int start) { - int blockDepth = 0; - for (int i = start + 1; i < source.length(); i++) { - char ch = source.charAt(i); - - if (ch == '{') { - blockDepth++; - } else if (ch == '}') { - blockDepth--; - } - - if (blockDepth < 0) { - return i; - } - } - - return -1; - } } diff --git a/src/main/java/com/jozufozu/flywheel/glsl/error/ErrorBuilder.java b/src/main/java/com/jozufozu/flywheel/glsl/error/ErrorBuilder.java index af609671b..c74591f9e 100644 --- a/src/main/java/com/jozufozu/flywheel/glsl/error/ErrorBuilder.java +++ b/src/main/java/com/jozufozu/flywheel/glsl/error/ErrorBuilder.java @@ -2,14 +2,19 @@ package com.jozufozu.flywheel.glsl.error; import java.util.ArrayList; import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.VisibleForTesting; import com.jozufozu.flywheel.glsl.SourceFile; import com.jozufozu.flywheel.glsl.SourceLines; import com.jozufozu.flywheel.glsl.error.lines.ErrorLine; import com.jozufozu.flywheel.glsl.error.lines.FileLine; import com.jozufozu.flywheel.glsl.error.lines.HeaderLine; +import com.jozufozu.flywheel.glsl.error.lines.NestedLine; import com.jozufozu.flywheel.glsl.error.lines.SourceLine; import com.jozufozu.flywheel.glsl.error.lines.SpanHighlightLine; import com.jozufozu.flywheel.glsl.error.lines.TextLine; @@ -17,7 +22,12 @@ import com.jozufozu.flywheel.glsl.span.Span; import com.jozufozu.flywheel.util.ConsoleColors; import com.jozufozu.flywheel.util.StringUtil; +import net.minecraft.resources.ResourceLocation; + public class ErrorBuilder { + // set to false for testing + @VisibleForTesting + public static boolean CONSOLE_COLORS = true; private final List lines = new ArrayList<>(); @@ -56,11 +66,15 @@ public class ErrorBuilder { } public ErrorBuilder pointAtFile(SourceFile file) { - return pointAtFile(file.name.toString()); + return pointAtFile(file.name); } public ErrorBuilder pointAtFile(SourceLines source) { - return pointAtFile(source.name.toString()); + return pointAtFile(source.name); + } + + public ErrorBuilder pointAtFile(ResourceLocation file) { + return pointAtFile(file.toString()); } public ErrorBuilder pointAtFile(String file) { @@ -126,6 +140,34 @@ public class ErrorBuilder { } public String build() { + Stream lineStream = getLineStream(); + + if (CONSOLE_COLORS) { + lineStream = lineStream.map(line -> line + ConsoleColors.RESET); + } + + return lineStream.collect(Collectors.joining("\n")); + } + + @NotNull + private Stream getLineStream() { + int maxMargin = calculateMargin(); + + return lines.stream() + .map(line -> addPaddingToLine(maxMargin, line)); + } + + private static String addPaddingToLine(int maxMargin, ErrorLine errorLine) { + int neededMargin = errorLine.neededMargin(); + + if (neededMargin >= 0) { + return StringUtil.repeatChar(' ', maxMargin - neededMargin) + errorLine.build(); + } else { + return errorLine.build(); + } + } + + private int calculateMargin() { int maxMargin = -1; for (ErrorLine line : lines) { int neededMargin = line.neededMargin(); @@ -134,20 +176,12 @@ public class ErrorBuilder { maxMargin = neededMargin; } } + return maxMargin; + } - StringBuilder builder = new StringBuilder(); - for (ErrorLine line : lines) { - int neededMargin = line.neededMargin(); - - if (neededMargin >= 0) { - builder.append(StringUtil.repeatChar(' ', maxMargin - neededMargin)); - } - - builder.append(line.build()) - .append(ConsoleColors.RESET) - .append('\n'); - } - - return builder.toString(); + public void nested(ErrorBuilder err) { + err.getLineStream() + .map(NestedLine::new) + .forEach(lines::add); } } diff --git a/src/main/java/com/jozufozu/flywheel/glsl/error/ErrorLevel.java b/src/main/java/com/jozufozu/flywheel/glsl/error/ErrorLevel.java index 71d3fa1e6..1bbe60c68 100644 --- a/src/main/java/com/jozufozu/flywheel/glsl/error/ErrorLevel.java +++ b/src/main/java/com/jozufozu/flywheel/glsl/error/ErrorLevel.java @@ -3,20 +3,26 @@ package com.jozufozu.flywheel.glsl.error; import com.jozufozu.flywheel.util.ConsoleColors; public enum ErrorLevel { - WARN(ConsoleColors.YELLOW + "warn"), - ERROR(ConsoleColors.RED + "error"), - HINT(ConsoleColors.WHITE_BRIGHT + "hint"), - NOTE(ConsoleColors.WHITE_BRIGHT + "note"), + WARN(ConsoleColors.YELLOW, "warn"), + ERROR(ConsoleColors.RED, "error"), + HINT(ConsoleColors.WHITE_BRIGHT, "hint"), + NOTE(ConsoleColors.WHITE_BRIGHT, "note"), ; + private final String color; private final String error; - ErrorLevel(String error) { + ErrorLevel(String color, String error) { + this.color = color; this.error = error; } @Override public String toString() { - return error; + if (ErrorBuilder.CONSOLE_COLORS) { + return color + error; + } else { + return error; + } } } diff --git a/src/main/java/com/jozufozu/flywheel/glsl/error/ErrorReporter.java b/src/main/java/com/jozufozu/flywheel/glsl/error/ErrorReporter.java deleted file mode 100644 index a73265797..000000000 --- a/src/main/java/com/jozufozu/flywheel/glsl/error/ErrorReporter.java +++ /dev/null @@ -1,115 +0,0 @@ -package com.jozufozu.flywheel.glsl.error; - -import java.util.ArrayList; -import java.util.List; -import java.util.stream.Collectors; - -import org.slf4j.Logger; - -import com.jozufozu.flywheel.glsl.ShaderLoadingException; -import com.jozufozu.flywheel.glsl.SourceFile; -import com.jozufozu.flywheel.glsl.span.Span; -import com.jozufozu.flywheel.lib.math.MoreMath; -import com.jozufozu.flywheel.util.StringUtil; -import com.mojang.logging.LogUtils; - -public class ErrorReporter { - private static final Logger LOGGER = LogUtils.getLogger(); - - private final List reportedErrors = new ArrayList<>(); - - public void generateMissingStruct(SourceFile file, Span vertexName, CharSequence msg) { - generateMissingStruct(file, vertexName, msg, ""); - } - - public void generateMissingStruct(SourceFile file, Span vertexName, CharSequence msg, CharSequence hint) { - // Optional span = file.parent.index.getStructDefinitionsMatching(vertexName) - // .stream() - // .findFirst() - // .map(ShaderStruct::getName); - // - // this.error(msg) - // .pointAtFile(file) - // .pointAt(vertexName, 1) - // .hintIncludeFor(span.orElse(null), hint); - } - - public void generateMissingFunction(SourceFile file, CharSequence functionName, CharSequence msg) { - generateMissingFunction(file, functionName, msg, ""); - } - - public void generateMissingFunction(SourceFile file, CharSequence functionName, CharSequence msg, CharSequence hint) { - // Optional span = file.parent.index.getFunctionDefinitionsMatching(functionName) - // .stream() - // .findFirst() - // .map(ShaderFunction::getName); - // - // this.error(msg) - // .pointAtFile(file) - // .hintIncludeFor(span.orElse(null), hint); - } - - public ErrorBuilder generateFunctionArgumentCountError(String name, int requiredArguments, Span span) { - var msg = '"' + name + "\" function must "; - - if (requiredArguments == 0) { - msg += "not have any arguments"; - } else { - msg += "have exactly " + requiredArguments + " argument" + (requiredArguments == 1 ? "" : "s"); - } - - return generateSpanError(span, msg); - } - - public ErrorBuilder generateSpanError(Span span, String message) { - var file = span.source(); - - return error(message).pointAtFile(file) - .pointAt(span, 2); - } - - public ErrorBuilder generateFileError(SourceFile file, String message) { - return error(message).pointAtFile(file); - } - - public ErrorBuilder error(String msg) { - var out = ErrorBuilder.create() - .error(msg); - reportedErrors.add(out); - return out; - } - - public boolean hasErrored() { - return !reportedErrors.isEmpty(); - } - - public ShaderLoadingException dump() { - var allErrors = reportedErrors.stream() - .map(ErrorBuilder::build) - .collect(Collectors.joining()); - - return new ShaderLoadingException(allErrors); - } - - public static void printLines(String string) { - List lines = string.lines() - .toList(); - - int size = lines.size(); - - int maxWidth = MoreMath.numDigits(size) + 1; - - StringBuilder builder = new StringBuilder().append('\n'); - - for (int i = 0; i < size; i++) { - - builder.append(i) - .append(StringUtil.repeatChar(' ', maxWidth - MoreMath.numDigits(i))) - .append("| ") - .append(lines.get(i)) - .append('\n'); - } - - LOGGER.error(builder.toString()); - } -} diff --git a/src/main/java/com/jozufozu/flywheel/glsl/error/lines/NestedLine.java b/src/main/java/com/jozufozu/flywheel/glsl/error/lines/NestedLine.java new file mode 100644 index 000000000..3cd623d03 --- /dev/null +++ b/src/main/java/com/jozufozu/flywheel/glsl/error/lines/NestedLine.java @@ -0,0 +1,8 @@ +package com.jozufozu.flywheel.glsl.error.lines; + +public record NestedLine(String right) implements ErrorLine { + @Override + public String right() { + return right; + } +} diff --git a/src/main/java/com/jozufozu/flywheel/glsl/package-info.java b/src/main/java/com/jozufozu/flywheel/glsl/package-info.java new file mode 100644 index 000000000..388ebb866 --- /dev/null +++ b/src/main/java/com/jozufozu/flywheel/glsl/package-info.java @@ -0,0 +1,6 @@ +@MethodsReturnNonnullByDefault @ParametersAreNonnullByDefault +package com.jozufozu.flywheel.glsl; + +import javax.annotation.ParametersAreNonnullByDefault; + +import net.minecraft.MethodsReturnNonnullByDefault; diff --git a/src/main/java/com/jozufozu/flywheel/glsl/parse/Import.java b/src/main/java/com/jozufozu/flywheel/glsl/parse/Import.java index 4ef2033ba..664a6ec11 100644 --- a/src/main/java/com/jozufozu/flywheel/glsl/parse/Import.java +++ b/src/main/java/com/jozufozu/flywheel/glsl/parse/Import.java @@ -1,9 +1,31 @@ package com.jozufozu.flywheel.glsl.parse; +import java.util.regex.Matcher; import java.util.regex.Pattern; +import com.google.common.collect.ImmutableList; +import com.jozufozu.flywheel.glsl.SourceLines; import com.jozufozu.flywheel.glsl.span.Span; public record Import(Span self, Span file) { - public static final Pattern PATTERN = Pattern.compile("^\\s*#\\s*use\\s+\"(.*)\"", Pattern.MULTILINE); + public static final Pattern PATTERN = Pattern.compile("^\\s*#\\s*include\\s+\"(.*)\"", Pattern.MULTILINE); + + /** + * Scan the source for {@code #use "..."} directives. + * Records the contents of the directive into an {@link Import} object, and marks the directive for elision. + */ + public static ImmutableList parseImports(SourceLines source) { + Matcher uses = PATTERN.matcher(source); + + var imports = ImmutableList.builder(); + + while (uses.find()) { + Span use = Span.fromMatcher(source, uses); + Span file = Span.fromMatcher(source, uses, 1); + + imports.add(new Import(use, file)); + } + + return imports.build(); + } } diff --git a/src/main/java/com/jozufozu/flywheel/glsl/parse/ShaderField.java b/src/main/java/com/jozufozu/flywheel/glsl/parse/ShaderField.java index 8d451478e..5723f54cf 100644 --- a/src/main/java/com/jozufozu/flywheel/glsl/parse/ShaderField.java +++ b/src/main/java/com/jozufozu/flywheel/glsl/parse/ShaderField.java @@ -1,9 +1,12 @@ package com.jozufozu.flywheel.glsl.parse; +import java.util.regex.Matcher; import java.util.regex.Pattern; import org.jetbrains.annotations.Nullable; +import com.google.common.collect.ImmutableMap; +import com.jozufozu.flywheel.glsl.SourceLines; import com.jozufozu.flywheel.glsl.span.Span; public class ShaderField { @@ -24,6 +27,26 @@ public class ShaderField { this.name = name; } + /** + * Scan the source for function definitions and "parse" them into objects that contain properties of the function. + */ + public static ImmutableMap parseFields(SourceLines source) { + Matcher matcher = PATTERN.matcher(source); + + ImmutableMap.Builder fields = ImmutableMap.builder(); + while (matcher.find()) { + Span self = Span.fromMatcher(source, matcher); + Span location = Span.fromMatcher(source, matcher, 1); + Span decoration = Span.fromMatcher(source, matcher, 2); + Span type = Span.fromMatcher(source, matcher, 3); + Span name = Span.fromMatcher(source, matcher, 4); + + fields.put(location.get(), new ShaderField(self, location, decoration, type, name)); + } + + return fields.build(); + } + public enum Decoration { IN, OUT, diff --git a/src/main/java/com/jozufozu/flywheel/glsl/parse/ShaderFunction.java b/src/main/java/com/jozufozu/flywheel/glsl/parse/ShaderFunction.java index e9680ee41..45298cbd9 100644 --- a/src/main/java/com/jozufozu/flywheel/glsl/parse/ShaderFunction.java +++ b/src/main/java/com/jozufozu/flywheel/glsl/parse/ShaderFunction.java @@ -1,11 +1,17 @@ package com.jozufozu.flywheel.glsl.parse; +import java.util.HashMap; +import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.jozufozu.flywheel.glsl.SourceLines; +import com.jozufozu.flywheel.glsl.span.ErrorSpan; import com.jozufozu.flywheel.glsl.span.Span; +import com.jozufozu.flywheel.glsl.span.StringSpan; public class ShaderFunction { // https://regexr.com/60n3d @@ -32,6 +38,62 @@ public class ShaderFunction { this.parameters = parseArguments(); } + /** + * Scan the source for function definitions and "parse" them into objects that contain properties of the function. + */ + public static ImmutableMap parseFunctions(SourceLines source) { + Matcher matcher = PATTERN.matcher(source); + + Map functions = new HashMap<>(); + + while (matcher.find()) { + Span type = Span.fromMatcher(source, matcher, 1); + Span name = Span.fromMatcher(source, matcher, 2); + Span args = Span.fromMatcher(source, matcher, 3); + + int blockStart = matcher.end(); + int blockEnd = findEndOfBlock(source, blockStart); + + Span self; + Span body; + if (blockEnd > blockStart) { + self = new StringSpan(source, matcher.start(), blockEnd + 1); + body = new StringSpan(source, blockStart, blockEnd); + } else { + self = new ErrorSpan(source, matcher.start(), matcher.end()); + body = new ErrorSpan(source, blockStart); + } + + ShaderFunction function = new ShaderFunction(self, type, name, args, body); + + functions.put(name.get(), function); + } + + return ImmutableMap.copyOf(functions); + } + + /** + * Given the position of an opening brace, scans through the source for a paired closing brace. + */ + private static int findEndOfBlock(CharSequence source, int start) { + int blockDepth = 0; + for (int i = start + 1; i < source.length(); i++) { + char ch = source.charAt(i); + + if (ch == '{') { + blockDepth++; + } else if (ch == '}') { + blockDepth--; + } + + if (blockDepth < 0) { + return i; + } + } + + return -1; + } + public Span getType() { return type; } diff --git a/src/main/java/com/jozufozu/flywheel/glsl/parse/ShaderStruct.java b/src/main/java/com/jozufozu/flywheel/glsl/parse/ShaderStruct.java index cf4651bdb..aa139d792 100644 --- a/src/main/java/com/jozufozu/flywheel/glsl/parse/ShaderStruct.java +++ b/src/main/java/com/jozufozu/flywheel/glsl/parse/ShaderStruct.java @@ -5,6 +5,7 @@ import java.util.regex.Pattern; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.jozufozu.flywheel.glsl.SourceLines; import com.jozufozu.flywheel.glsl.span.Span; public class ShaderStruct { @@ -29,6 +30,27 @@ public class ShaderStruct { this.fields2Types = createTypeLookup(); } + /** + * Scan the source for function definitions and "parse" them into objects that contain properties of the function. + */ + public static ImmutableMap parseStructs(SourceLines source) { + Matcher matcher = PATTERN.matcher(source); + + ImmutableMap.Builder structs = ImmutableMap.builder(); + while (matcher.find()) { + Span self = Span.fromMatcher(source, matcher); + Span name = Span.fromMatcher(source, matcher, 1); + Span body = Span.fromMatcher(source, matcher, 2); + Span variableName = Span.fromMatcher(source, matcher, 3); + + ShaderStruct shaderStruct = new ShaderStruct(self, name, body, variableName); + + structs.put(name.get(), shaderStruct); + } + + return structs.build(); + } + public Span getName() { return name; } diff --git a/src/main/resources/assets/flywheel/flywheel/context/common.vert b/src/main/resources/assets/flywheel/flywheel/context/common.vert index 65e126de1..631af80e6 100644 --- a/src/main/resources/assets/flywheel/flywheel/context/common.vert +++ b/src/main/resources/assets/flywheel/flywheel/context/common.vert @@ -1,5 +1,5 @@ -#use "flywheel:api/vertex.glsl" -#use "flywheel:util/fog.glsl" +#include "flywheel:api/vertex.glsl" +#include "flywheel:util/fog.glsl" void flw_contextVertex() { flw_distance = fog_distance(flw_vertexPos.xyz, flywheel.cameraPos.xyz, flywheel.fogShape); diff --git a/src/main/resources/assets/flywheel/flywheel/context/crumbling.frag b/src/main/resources/assets/flywheel/flywheel/context/crumbling.frag index c2c32b9c1..a8647b8b7 100644 --- a/src/main/resources/assets/flywheel/flywheel/context/crumbling.frag +++ b/src/main/resources/assets/flywheel/flywheel/context/crumbling.frag @@ -1,4 +1,4 @@ -#use "flywheel:api/fragment.glsl" +#include "flywheel:api/fragment.glsl" uniform sampler2D flw_diffuseTex; diff --git a/src/main/resources/assets/flywheel/flywheel/context/crumbling.vert b/src/main/resources/assets/flywheel/flywheel/context/crumbling.vert index 6b2d25e44..02395303d 100644 --- a/src/main/resources/assets/flywheel/flywheel/context/crumbling.vert +++ b/src/main/resources/assets/flywheel/flywheel/context/crumbling.vert @@ -1 +1 @@ -#use "flywheel:context/common.vert" +#include "flywheel:context/common.vert" diff --git a/src/main/resources/assets/flywheel/flywheel/context/world.frag b/src/main/resources/assets/flywheel/flywheel/context/world.frag index 8b5ffdc24..245835d6d 100644 --- a/src/main/resources/assets/flywheel/flywheel/context/world.frag +++ b/src/main/resources/assets/flywheel/flywheel/context/world.frag @@ -1,4 +1,4 @@ -#use "flywheel:api/fragment.glsl" +#include "flywheel:api/fragment.glsl" // optimize discard usage #ifdef ALPHA_DISCARD diff --git a/src/main/resources/assets/flywheel/flywheel/context/world.vert b/src/main/resources/assets/flywheel/flywheel/context/world.vert index 6b2d25e44..02395303d 100644 --- a/src/main/resources/assets/flywheel/flywheel/context/world.vert +++ b/src/main/resources/assets/flywheel/flywheel/context/world.vert @@ -1 +1 @@ -#use "flywheel:context/common.vert" +#include "flywheel:context/common.vert" diff --git a/src/main/resources/assets/flywheel/flywheel/instance/oriented.vert b/src/main/resources/assets/flywheel/flywheel/instance/oriented.vert index c4091fb3c..ef844ec09 100644 --- a/src/main/resources/assets/flywheel/flywheel/instance/oriented.vert +++ b/src/main/resources/assets/flywheel/flywheel/instance/oriented.vert @@ -1,5 +1,5 @@ -#use "flywheel:api/vertex.glsl" -#use "flywheel:util/quaternion.glsl" +#include "flywheel:api/vertex.glsl" +#include "flywheel:util/quaternion.glsl" void flw_transformBoundingSphere(in FlwInstance i, inout vec3 center, inout float radius) { vec4 rotation = i.rotation; @@ -9,7 +9,7 @@ void flw_transformBoundingSphere(in FlwInstance i, inout vec3 center, inout floa center = rotateVertexByQuat(center - pivot, rotation) + pivot + pos; } - #ifdef VERTEX_SHADER +#ifdef VERTEX_SHADER void flw_instanceVertex(in FlwInstance i) { flw_vertexPos = vec4(rotateVertexByQuat(flw_vertexPos.xyz - i.pivot, i.rotation) + i.pivot + i.position, 1.0); flw_vertexNormal = rotateVertexByQuat(flw_vertexNormal, i.rotation); diff --git a/src/main/resources/assets/flywheel/flywheel/instance/transformed.vert b/src/main/resources/assets/flywheel/flywheel/instance/transformed.vert index 9eded92b2..fe3351e6a 100644 --- a/src/main/resources/assets/flywheel/flywheel/instance/transformed.vert +++ b/src/main/resources/assets/flywheel/flywheel/instance/transformed.vert @@ -1,4 +1,4 @@ -#use "flywheel:api/vertex.glsl" +#include "flywheel:api/vertex.glsl" void flw_transformBoundingSphere(in FlwInstance i, inout vec3 center, inout float radius) { mat4 pose = i.pose; diff --git a/src/main/resources/assets/flywheel/flywheel/internal/draw.frag b/src/main/resources/assets/flywheel/flywheel/internal/draw.frag index f57dd9a87..9d81dc3ea 100644 --- a/src/main/resources/assets/flywheel/flywheel/internal/draw.frag +++ b/src/main/resources/assets/flywheel/flywheel/internal/draw.frag @@ -1,4 +1,4 @@ -#use "flywheel:api/fragment.glsl" +#include "flywheel:api/fragment.glsl" void main() { flw_initFragment(); diff --git a/src/main/resources/assets/flywheel/flywheel/internal/indirect_cull.glsl b/src/main/resources/assets/flywheel/flywheel/internal/indirect_cull.glsl index a8d3df9df..30743f94e 100644 --- a/src/main/resources/assets/flywheel/flywheel/internal/indirect_cull.glsl +++ b/src/main/resources/assets/flywheel/flywheel/internal/indirect_cull.glsl @@ -1,8 +1,8 @@ #define FLW_SUBGROUP_SIZE 32 layout(local_size_x = FLW_SUBGROUP_SIZE) in; -#use "flywheel:util/types.glsl" -#use "flywheel:internal/indirect_draw_command.glsl" +#include "flywheel:util/types.glsl" +#include "flywheel:internal/indirect_draw_command.glsl" // populated by instancers layout(std430, binding = 0) restrict readonly buffer ObjectBuffer { diff --git a/src/main/resources/assets/flywheel/flywheel/internal/indirect_draw.vert b/src/main/resources/assets/flywheel/flywheel/internal/indirect_draw.vert index 8b7230209..5061ee9e0 100644 --- a/src/main/resources/assets/flywheel/flywheel/internal/indirect_draw.vert +++ b/src/main/resources/assets/flywheel/flywheel/internal/indirect_draw.vert @@ -1,5 +1,5 @@ -#use "flywheel:api/vertex.glsl" -#use "flywheel:internal/indirect_draw_command.glsl" +#include "flywheel:api/vertex.glsl" +#include "flywheel:internal/indirect_draw_command.glsl" layout(std430, binding = 0) restrict readonly buffer ObjectBuffer { FlwPackedInstance objects[]; diff --git a/src/main/resources/assets/flywheel/flywheel/internal/indirect_draw_command.glsl b/src/main/resources/assets/flywheel/flywheel/internal/indirect_draw_command.glsl index 6699bd126..cb3f71ebd 100644 --- a/src/main/resources/assets/flywheel/flywheel/internal/indirect_draw_command.glsl +++ b/src/main/resources/assets/flywheel/flywheel/internal/indirect_draw_command.glsl @@ -1,4 +1,4 @@ -#use "flywheel:util/types.glsl" +#include "flywheel:util/types.glsl" struct MeshDrawCommand { uint indexCount; diff --git a/src/main/resources/assets/flywheel/flywheel/internal/instanced_arrays_draw.vert b/src/main/resources/assets/flywheel/flywheel/internal/instanced_arrays_draw.vert index f8716ca1c..104aaeb4a 100644 --- a/src/main/resources/assets/flywheel/flywheel/internal/instanced_arrays_draw.vert +++ b/src/main/resources/assets/flywheel/flywheel/internal/instanced_arrays_draw.vert @@ -1,4 +1,4 @@ -#use "flywheel:api/vertex.glsl" +#include "flywheel:api/vertex.glsl" uniform uvec2 _flw_materialID_instancing; diff --git a/src/main/resources/assets/flywheel/flywheel/layout/block.vert b/src/main/resources/assets/flywheel/flywheel/layout/block.vert index a3af3b16d..cbf38fe42 100644 --- a/src/main/resources/assets/flywheel/flywheel/layout/block.vert +++ b/src/main/resources/assets/flywheel/flywheel/layout/block.vert @@ -1,4 +1,4 @@ -#use "flywheel:api/vertex.glsl" +#include "flywheel:api/vertex.glsl" layout(location = 0) in vec3 _flw_v_pos; layout(location = 1) in vec4 _flw_v_color; diff --git a/src/main/resources/assets/flywheel/flywheel/layout/pos_tex_normal.vert b/src/main/resources/assets/flywheel/flywheel/layout/pos_tex_normal.vert index 4ecfb2c3c..271cd2608 100644 --- a/src/main/resources/assets/flywheel/flywheel/layout/pos_tex_normal.vert +++ b/src/main/resources/assets/flywheel/flywheel/layout/pos_tex_normal.vert @@ -1,4 +1,4 @@ -#use "flywheel:api/vertex.glsl" +#include "flywheel:api/vertex.glsl" layout(location = 0) in vec3 _flw_v_pos; layout(location = 1) in vec2 _flw_v_texCoord; diff --git a/src/main/resources/assets/flywheel/flywheel/material/cutout.frag b/src/main/resources/assets/flywheel/flywheel/material/cutout.frag index c86c4202a..bee51d582 100644 --- a/src/main/resources/assets/flywheel/flywheel/material/cutout.frag +++ b/src/main/resources/assets/flywheel/flywheel/material/cutout.frag @@ -1,5 +1,5 @@ -#use "flywheel:api/fragment.glsl" -#use "flywheel:util/fog.glsl" +#include "flywheel:api/fragment.glsl" +#include "flywheel:util/fog.glsl" void flw_materialFragment() { } diff --git a/src/main/resources/assets/flywheel/flywheel/material/default.frag b/src/main/resources/assets/flywheel/flywheel/material/default.frag index e5ea00f6c..ba5d33d42 100644 --- a/src/main/resources/assets/flywheel/flywheel/material/default.frag +++ b/src/main/resources/assets/flywheel/flywheel/material/default.frag @@ -1,5 +1,5 @@ -#use "flywheel:api/fragment.glsl" -#use "flywheel:util/fog.glsl" +#include "flywheel:api/fragment.glsl" +#include "flywheel:util/fog.glsl" void flw_materialFragment() { } diff --git a/src/main/resources/assets/flywheel/flywheel/material/default.vert b/src/main/resources/assets/flywheel/flywheel/material/default.vert index 677d8bd7d..c067c086c 100644 --- a/src/main/resources/assets/flywheel/flywheel/material/default.vert +++ b/src/main/resources/assets/flywheel/flywheel/material/default.vert @@ -1,4 +1,4 @@ -#use "flywheel:api/vertex.glsl" +#include "flywheel:api/vertex.glsl" void flw_materialVertex() { } diff --git a/src/main/resources/assets/flywheel/flywheel/material/shaded.vert b/src/main/resources/assets/flywheel/flywheel/material/shaded.vert index a14f8f826..979e923ac 100644 --- a/src/main/resources/assets/flywheel/flywheel/material/shaded.vert +++ b/src/main/resources/assets/flywheel/flywheel/material/shaded.vert @@ -1,5 +1,5 @@ -#use "flywheel:api/vertex.glsl" -#use "flywheel:util/diffuse.glsl" +#include "flywheel:api/vertex.glsl" +#include "flywheel:util/diffuse.glsl" void flw_materialVertex() { flw_vertexNormal = normalize(flw_vertexNormal); diff --git a/src/test/java/com/jozufozu/flywheel/glsl/MockShaderSources.java b/src/test/java/com/jozufozu/flywheel/glsl/MockShaderSources.java new file mode 100644 index 000000000..811898a61 --- /dev/null +++ b/src/test/java/com/jozufozu/flywheel/glsl/MockShaderSources.java @@ -0,0 +1,38 @@ +package com.jozufozu.flywheel.glsl; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.Assertions; + +import net.minecraft.resources.ResourceLocation; +import net.minecraft.server.packs.resources.ResourceManager; + +public class MockShaderSources extends ShaderSources { + private final Map sources = new HashMap<>(); + + public MockShaderSources() { + super(ResourceManager.Empty.INSTANCE); + } + + public void add(ResourceLocation loc, String source) { + sources.put(loc, source); + } + + @NotNull + @Override + protected LoadResult load(ResourceLocation loc) { + var maybeFound = sources.get(loc); + if (maybeFound == null) { + return new LoadResult.Failure(new LoadError.IOError(loc, new IOException("Mock source not found"))); + } + return SourceFile.parse(this, loc, maybeFound); + } + + public LoadResult assertLoaded(ResourceLocation loc) { + Assertions.assertTrue(cache.containsKey(loc), "Expected " + loc + " to be cached"); + return cache.get(loc); + } +} diff --git a/src/test/java/com/jozufozu/flywheel/glsl/TestBase.java b/src/test/java/com/jozufozu/flywheel/glsl/TestBase.java new file mode 100644 index 000000000..c354bb93e --- /dev/null +++ b/src/test/java/com/jozufozu/flywheel/glsl/TestBase.java @@ -0,0 +1,67 @@ +package com.jozufozu.flywheel.glsl; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import java.util.List; + +import org.jetbrains.annotations.NotNull; + +import com.jozufozu.flywheel.Flywheel; +import com.jozufozu.flywheel.glsl.error.ErrorBuilder; + +import net.minecraft.resources.ResourceLocation; + +public class TestBase { + public static final ResourceLocation FLW_A = Flywheel.rl("a.glsl"); + public static final ResourceLocation FLW_B = Flywheel.rl("b.glsl"); + public static final ResourceLocation FLW_C = Flywheel.rl("c.glsl"); + + public static T assertSingletonList(List list) { + assertEquals(1, list.size()); + return list.get(0); + } + + @NotNull + public static E findAndAssertError(Class clazz, MockShaderSources sources, ResourceLocation loc) { + var result = sources.find(loc); + var failure = assertInstanceOf(LoadResult.Failure.class, result); + return assertInstanceOf(clazz, failure.error()); + } + + @NotNull + public static ErrorBuilder assertErrorAndGetMessage(MockShaderSources sources, ResourceLocation loc) { + var result = sources.find(loc); + var failure = assertInstanceOf(LoadResult.Failure.class, result); + return failure.error() + .generateMessage(); + } + + static E assertSimpleNestedErrorsToDepth(Class finalErrType, LoadError err, int depth) { + var includeError = assertInstanceOf(LoadError.IncludeError.class, err); + + var pair = assertSingletonList(includeError.innerErrors()); + for (int i = 1; i < depth; i++) { + includeError = assertInstanceOf(LoadError.IncludeError.class, pair.second()); + pair = assertSingletonList(includeError.innerErrors()); + } + return assertInstanceOf(finalErrType, pair.second()); + } + + @NotNull + public static SourceFile findAndAssertSuccess(MockShaderSources sources, ResourceLocation loc) { + var result = sources.find(loc); + return assertSuccessAndUnwrap(loc, result); + } + + @NotNull + public static SourceFile assertSuccessAndUnwrap(ResourceLocation expectedName, LoadResult result) { + assertInstanceOf(LoadResult.Success.class, result); + + var file = result.unwrap(); + assertNotNull(file); + assertEquals(expectedName, file.name); + return file; + } +} diff --git a/src/test/java/com/jozufozu/flywheel/glsl/TestErrorMessages.java b/src/test/java/com/jozufozu/flywheel/glsl/TestErrorMessages.java new file mode 100644 index 000000000..099ea80e3 --- /dev/null +++ b/src/test/java/com/jozufozu/flywheel/glsl/TestErrorMessages.java @@ -0,0 +1,33 @@ +package com.jozufozu.flywheel.glsl; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import com.jozufozu.flywheel.glsl.error.ErrorBuilder; + +public class TestErrorMessages extends TestBase { + @BeforeAll + static void disableConsoleColors() { + ErrorBuilder.CONSOLE_COLORS = false; + } + + @Test + void testMissingIncludeMsg() { + var sources = new MockShaderSources(); + sources.add(FLW_A, """ + #include "flywheel:b.glsl" + """); + + var aErr = assertErrorAndGetMessage(sources, FLW_A); + + assertEquals(""" + error: could not load shader due to errors in included files + --> flywheel:a.glsl + 1 | #include "flywheel:b.glsl" + | ^^^^^^^^^^^^^^^ + | error: could not load "flywheel:b.glsl" due to an IO error + | note: Mock source not found""", aErr.build()); + } +} diff --git a/src/test/java/com/jozufozu/flywheel/glsl/TestShaderSourceLoading.java b/src/test/java/com/jozufozu/flywheel/glsl/TestShaderSourceLoading.java new file mode 100644 index 000000000..a23c90d20 --- /dev/null +++ b/src/test/java/com/jozufozu/flywheel/glsl/TestShaderSourceLoading.java @@ -0,0 +1,146 @@ +package com.jozufozu.flywheel.glsl; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + +import org.junit.jupiter.api.Test; + +import com.google.common.collect.ImmutableList; +import com.jozufozu.flywheel.glsl.parse.Import; + +public class TestShaderSourceLoading extends TestBase { + @Test + void testSimpleFind() { + var sources = new MockShaderSources(); + sources.add(FLW_A, ""); + + SourceFile file = findAndAssertSuccess(sources, FLW_A); + + assertEquals("", file.finalSource); + } + + @Test + void testMissingFileAtRoot() { + var sources = new MockShaderSources(); + findAndAssertError(LoadError.IOError.class, sources, FLW_A); + } + + @Test + void testMissingInclude() { + var sources = new MockShaderSources(); + sources.add(FLW_A, """ + #include "flywheel:b.glsl" + """); + + var aErr = findAndAssertError(LoadError.IncludeError.class, sources, FLW_A); + + var ioErr = assertSimpleNestedErrorsToDepth(LoadError.IOError.class, aErr, 1); + assertEquals(FLW_B, ioErr.location()); + } + + @Test + void testBasicInclude() { + var sources = new MockShaderSources(); + sources.add(FLW_A, """ + #include "flywheel:b.glsl" + """); + sources.add(FLW_B, ""); + + SourceFile a = findAndAssertSuccess(sources, FLW_A); + sources.assertLoaded(FLW_B); + + var includeB = assertSingletonList(a.imports); + assertEquals(FLW_B.toString(), includeB.file() + .toString()); + + assertEquals(""" + + """, a.finalSource, "Include statements should be elided."); + } + + @Test + void testRedundantInclude() { + var sources = new MockShaderSources(); + sources.add(FLW_A, """ + #include "flywheel:b.glsl" + #include "flywheel:b.glsl" + """); + sources.add(FLW_B, ""); + + SourceFile a = findAndAssertSuccess(sources, FLW_A); + sources.assertLoaded(FLW_B); + + assertEquals(2, a.imports.size()); + for (Import include : a.imports) { + assertEquals(FLW_B.toString(), include.file() + .toString()); + } + + assertEquals(""" + + + """, a.finalSource, "Both include statements should be elided."); + + LoadResult bResult = sources.assertLoaded(FLW_B); + SourceFile b = assertSuccessAndUnwrap(FLW_B, bResult); + + assertEquals(ImmutableList.of(b), a.included); + } + + @Test + void testSelfInclude() { + var sources = new MockShaderSources(); + sources.add(FLW_A, """ + #include "flywheel:a.glsl" + """); + + var aErr = findAndAssertError(LoadError.IncludeError.class, sources, FLW_A); + + var shouldBeRecursiveIncludePair = assertSingletonList(aErr.innerErrors()); + + var circularDependency = assertInstanceOf(LoadError.CircularDependency.class, shouldBeRecursiveIncludePair.second()); + assertEquals(ImmutableList.of(FLW_A, FLW_A), circularDependency.stack()); + assertEquals(FLW_A, circularDependency.offender()); + + assertEquals(FLW_A.toString(), shouldBeRecursiveIncludePair.first() + .toString()); + } + + @Test + void test2LayerCircularDependency() { + var sources = new MockShaderSources(); + sources.add(FLW_A, """ + #include "flywheel:b.glsl" + """); + sources.add(FLW_B, """ + #include "flywheel:a.glsl" + """); + + var aErr = findAndAssertError(LoadError.IncludeError.class, sources, FLW_A); + sources.assertLoaded(FLW_B); + + var recursiveInclude = assertSimpleNestedErrorsToDepth(LoadError.CircularDependency.class, aErr, 2); + assertEquals(ImmutableList.of(FLW_A, FLW_B, FLW_A), recursiveInclude.stack()); + } + + @Test + void test3LayerCircularDependency() { + var sources = new MockShaderSources(); + sources.add(FLW_A, """ + #include "flywheel:b.glsl" + """); + sources.add(FLW_B, """ + #include "flywheel:c.glsl" + """); + sources.add(FLW_C, """ + #include "flywheel:a.glsl" + """); + + var aErr = findAndAssertError(LoadError.IncludeError.class, sources, FLW_A); + sources.assertLoaded(FLW_B); + sources.assertLoaded(FLW_C); + + var recursiveInclude = assertSimpleNestedErrorsToDepth(LoadError.CircularDependency.class, aErr, 3); + assertEquals(ImmutableList.of(FLW_A, FLW_B, FLW_C, FLW_A), recursiveInclude.stack()); + } +}