Skip to content

Commit

Permalink
Errors galore
Browse files Browse the repository at this point in the history
- Make everything in the compiler chain's results not null
- Throw errors immediately when encountered
- Log error messages when falling back
- Do not eagerly grab utility programs in IndirectDrawManager so we can
  actually catch errors and fall back
- Remove CompilerStats
  • Loading branch information
Jozufozu committed Sep 28, 2024
1 parent 11ce4ac commit ef05f7d
Show file tree
Hide file tree
Showing 16 changed files with 103 additions and 216 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import org.slf4j.LoggerFactory;

import dev.engine_room.flywheel.api.Flywheel;
import dev.engine_room.flywheel.backend.compile.core.CompilerStats;
import dev.engine_room.flywheel.backend.glsl.ShaderSources;
import dev.engine_room.flywheel.backend.glsl.SourceComponent;
import net.minecraft.resources.ResourceLocation;
Expand All @@ -29,17 +28,9 @@ static void reload(ResourceManager resourceManager) {

var sources = new ShaderSources(resourceManager);
SOURCES = sources;
var stats = new CompilerStats("ubershaders");

var fragmentComponentsHeader = sources.get(COMPONENTS_HEADER_FRAG);

// TODO: separate compilation for cutout OFF, but keep the rest uber'd?
if (stats.errored() || fragmentComponentsHeader == null) {
// Probably means the shader sources are missing.
stats.emitErrorLog();
return;
}

List<SourceComponent> vertexComponents = List.of();
List<SourceComponent> fragmentComponents = List.of(fragmentComponentsHeader);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ static void reload(ShaderSources sources, List<SourceComponent> vertexComponents
return;
}


var pipelineCompiler = PipelineCompiler.create(sources, Pipelines.INSTANCING, vertexComponents, fragmentComponents, EXTENSIONS);
InstancingPrograms newInstance = new InstancingPrograms(pipelineCompiler);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
import java.util.HashMap;
import java.util.Map;

import org.jetbrains.annotations.Nullable;

import dev.engine_room.flywheel.backend.gl.GlObject;
import dev.engine_room.flywheel.backend.gl.shader.GlProgram;
import dev.engine_room.flywheel.backend.glsl.ShaderSources;
Expand All @@ -14,31 +12,22 @@ public class CompilationHarness<K> {
private final KeyCompiler<K> compiler;
private final ShaderCache shaderCache;
private final ProgramLinker programLinker;
private final CompilerStats stats;

private final Map<K, GlProgram> programs = new HashMap<>();

public CompilationHarness(String marker, ShaderSources sources, KeyCompiler<K> compiler) {
this.sources = sources;
this.compiler = compiler;
stats = new CompilerStats(marker);
shaderCache = new ShaderCache(stats);
programLinker = new ProgramLinker(stats);
shaderCache = new ShaderCache();
programLinker = new ProgramLinker();
}

public GlProgram get(K key) {
return programs.computeIfAbsent(key, this::compile);
}

private GlProgram compile(K key) {
var out = compiler.compile(key, sources, shaderCache, programLinker);

if (out == null) {
// TODO: populate exception with error details
throw new ShaderException();
}

return out;
return compiler.compile(key, sources, shaderCache, programLinker);
}

public void delete() {
Expand All @@ -51,6 +40,6 @@ public void delete() {
}

public interface KeyCompiler<K> {
@Nullable GlProgram compile(K key, ShaderSources loader, ShaderCache shaderCache, ProgramLinker programLinker);
GlProgram compile(K key, ShaderSources loader, ShaderCache shaderCache, ProgramLinker programLinker);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
import java.util.function.Consumer;
import java.util.function.Function;

import org.jetbrains.annotations.Nullable;

import dev.engine_room.flywheel.backend.compile.FlwPrograms;
import dev.engine_room.flywheel.backend.gl.shader.GlProgram;
import dev.engine_room.flywheel.backend.gl.shader.GlShader;
Expand Down Expand Up @@ -44,7 +42,7 @@ public ProgramStitcher<K> program() {
public static class ShaderCompiler<K> {
private final GlslVersion glslVersion;
private final ShaderType shaderType;
private final List<BiFunction<K, ShaderSources, @Nullable SourceComponent>> fetchers = new ArrayList<>();
private final List<BiFunction<K, ShaderSources, SourceComponent>> fetchers = new ArrayList<>();
private BiConsumer<K, Compilation> compilationCallbacks = ($, $$) -> {
};
private Function<K, String> nameMapper = Object::toString;
Expand All @@ -59,21 +57,21 @@ public ShaderCompiler<K> nameMapper(Function<K, String> nameMapper) {
return this;
}

public ShaderCompiler<K> with(BiFunction<K, ShaderSources, @Nullable SourceComponent> fetch) {
public ShaderCompiler<K> with(BiFunction<K, ShaderSources, SourceComponent> fetch) {
fetchers.add(fetch);
return this;
}

public ShaderCompiler<K> withComponents(Collection<@Nullable SourceComponent> components) {
public ShaderCompiler<K> withComponents(Collection<SourceComponent> components) {
components.forEach(this::withComponent);
return this;
}

public ShaderCompiler<K> withComponent(@Nullable SourceComponent component) {
public ShaderCompiler<K> withComponent(SourceComponent component) {
return withComponent($ -> component);
}

public ShaderCompiler<K> withComponent(Function<K, @Nullable SourceComponent> sourceFetcher) {
public ShaderCompiler<K> withComponent(Function<K, SourceComponent> sourceFetcher) {
return with((key, $) -> sourceFetcher.apply(key));
}

Expand Down Expand Up @@ -122,22 +120,12 @@ public ShaderCompiler<K> requireExtensions(Collection<String> extensions) {
});
}

@Nullable
private GlShader compile(K key, ShaderCache compiler, ShaderSources loader) {
long start = System.nanoTime();

var components = new ArrayList<SourceComponent>();
boolean ok = true;
for (var fetcher : fetchers) {
SourceComponent apply = fetcher.apply(key, loader);
if (apply == null) {
ok = false;
}
components.add(apply);
}

if (!ok) {
return null;
components.add(fetcher.apply(key, loader));
}

Consumer<Compilation> cb = ctx -> compilationCallbacks.accept(key, ctx);
Expand Down Expand Up @@ -182,7 +170,6 @@ public ProgramStitcher<K> preLink(BiConsumer<K, GlProgram> preLink) {
}

@Override
@Nullable
public GlProgram compile(K key, ShaderSources loader, ShaderCache shaderCache, ProgramLinker programLinker) {
if (compilers.isEmpty()) {
throw new IllegalStateException("No shader compilers were added!");
Expand All @@ -192,24 +179,13 @@ public GlProgram compile(K key, ShaderSources loader, ShaderCache shaderCache, P

List<GlShader> shaders = new ArrayList<>();

boolean ok = true;
for (ShaderCompiler<K> compiler : compilers.values()) {
var shader = compiler.compile(key, shaderCache, loader);
if (shader == null) {
ok = false;
}
shaders.add(shader);
}

if (!ok) {
return null;
shaders.add(compiler.compile(key, shaderCache, loader));
}

var out = programLinker.link(shaders, p -> preLink.accept(key, p));

if (out != null) {
postLink.accept(key, out);
}
postLink.accept(key, out);

long end = System.nanoTime();

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
package dev.engine_room.flywheel.backend.compile.core;

import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import dev.engine_room.flywheel.backend.gl.shader.GlProgram;

public sealed interface LinkResult {
@Nullable
default GlProgram unwrap() {
return null;
}
GlProgram unwrap();

record Success(GlProgram program, String log) implements LinkResult {
@Override
Expand All @@ -20,6 +16,10 @@ public GlProgram unwrap() {
}

record Failure(String failure) implements LinkResult {
@Override
public GlProgram unwrap() {
throw new ShaderException.Link(failure);
}
}

static LinkResult success(GlProgram program, String log) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,17 @@
import java.util.List;
import java.util.function.Consumer;

import org.jetbrains.annotations.Nullable;

import dev.engine_room.flywheel.backend.gl.shader.GlProgram;
import dev.engine_room.flywheel.backend.gl.shader.GlShader;

public class ProgramLinker {
private final CompilerStats stats;

public ProgramLinker(CompilerStats stats) {
this.stats = stats;
public ProgramLinker() {
}

@Nullable
public GlProgram link(List<GlShader> shaders, Consumer<GlProgram> preLink) {
// this probably doesn't need caching
var linkResult = linkInternal(shaders, preLink);
stats.linkResult(linkResult);
return linkResult.unwrap();
return linkInternal(shaders, preLink).unwrap();
}

private LinkResult linkInternal(List<GlShader> shaders, Consumer<GlProgram> preLink) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,20 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.Consumer;

import org.jetbrains.annotations.Nullable;

import dev.engine_room.flywheel.backend.gl.shader.GlShader;
import dev.engine_room.flywheel.backend.gl.shader.ShaderType;
import dev.engine_room.flywheel.backend.glsl.GlslVersion;
import dev.engine_room.flywheel.backend.glsl.SourceComponent;

public class ShaderCache {
private final Map<ShaderKey, ShaderResult> inner = new HashMap<>();
private final CompilerStats stats;

public ShaderCache(CompilerStats stats) {
this.stats = stats;
public ShaderCache() {
}

@Nullable
public GlShader compile(GlslVersion glslVersion, ShaderType shaderType, String name, Consumer<Compilation> callback, List<SourceComponent> sourceComponents) {
var key = new ShaderKey(glslVersion, shaderType, name);
var cached = inner.get(key);
Expand All @@ -41,15 +35,14 @@ public GlShader compile(GlslVersion glslVersion, ShaderType shaderType, String n

ShaderResult out = ctx.compile(shaderType, name);
inner.put(key, out);
stats.shaderResult(out);
return out.unwrap();
}

public void delete() {
inner.values()
.stream()
.filter(r -> r instanceof ShaderResult.Success)
.map(ShaderResult::unwrap)
.filter(Objects::nonNull)
.forEach(GlShader::delete);
inner.clear();
}
Expand Down
Loading

0 comments on commit ef05f7d

Please sign in to comment.