From f9447719982608f042e2c1203bff2650c2011423 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Fri, 6 Oct 2017 18:57:48 +0100 Subject: [PATCH 1/4] gapis/shadertools: Add Preamble & Relaxed options. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Do a little refactoring while I’m there. --- cmd/shadertool/main.go | 2 +- gapis/api/gles/compat.go | 4 +- gapis/api/gles/find_issues.go | 2 +- gapis/shadertools/cc/libmanager.cpp | 12 +++--- gapis/shadertools/cc/libmanager.h | 2 + gapis/shadertools/shadertools.go | 57 ++++++++++++++++++----------- 6 files changed, 49 insertions(+), 30 deletions(-) diff --git a/cmd/shadertool/main.go b/cmd/shadertool/main.go index 18daa03035..d0776ee10f 100644 --- a/cmd/shadertool/main.go +++ b/cmd/shadertool/main.go @@ -88,7 +88,7 @@ func run(ctx context.Context) error { } func convert(source, shaderType string) (result string, err error) { - opts := shadertools.Option{} + opts := shadertools.Options{} switch shaderType { case ".vert": opts.ShaderType = shadertools.TypeVertex diff --git a/gapis/api/gles/compat.go b/gapis/api/gles/compat.go index c5008fbc2d..996ef853c9 100644 --- a/gapis/api/gles/compat.go +++ b/gapis/api/gles/compat.go @@ -454,7 +454,9 @@ func compat(ctx context.Context, device *device.Instance) (transform.Transformer if err != nil { log.W(ctx, "%v compat: %v", cmd, err) } - opts := shadertools.Option{ShaderType: st} + opts := shadertools.Options{ + ShaderType: st, + } // Trim any prefix whitespace / newlines. // This isn't legal if it comes before the #version, but this diff --git a/gapis/api/gles/find_issues.go b/gapis/api/gles/find_issues.go index f754b82f67..4722f0a7fd 100644 --- a/gapis/api/gles/find_issues.go +++ b/gapis/api/gles/find_issues.go @@ -168,7 +168,7 @@ func (t *findIssues) Transform(ctx context.Context, id api.CmdID, cmd api.Cmd, o t.onIssue(cmd, id, service.Severity_ErrorLevel, err) return } - opts := shadertools.Option{ + opts := shadertools.Options{ ShaderType: st, CheckAfterChanges: true, Disassemble: true, diff --git a/gapis/shadertools/cc/libmanager.cpp b/gapis/shadertools/cc/libmanager.cpp index fe24f064d6..ea1fb0671e 100644 --- a/gapis/shadertools/cc/libmanager.cpp +++ b/gapis/shadertools/cc/libmanager.cpp @@ -130,11 +130,11 @@ void set_error_msg(code_with_debug_info_t* x, std::string msg) { strcpy(x->message, msg.c_str()); } -std::vector parseGlslang(const char* code, std::string* err_msg, - shader_type type, bool es_profile) { +std::vector parseGlslang(const char* code, const char* preamble, + std::string* err_msg, shader_type type, bool es_profile, bool relaxed_errs) { std::vector spirv; - EShMessages messages = EShMsgDefault; + EShMessages messages = relaxed_errs ? EShMsgRelaxedErrors : EShMsgDefault; EShLanguage lang = EShLangVertex; switch (type) { case VERTEX: { lang = EShLangVertex; break; } @@ -147,6 +147,7 @@ std::vector parseGlslang(const char* code, std::string* err_msg, glslang::InitializeProcess(); glslang::TShader shader(lang); + shader.setPreamble(preamble); shader.setStrings(&code, 1); // use 100 for ES environment, 330 for desktop int default_version = es_profile ? 100 : 330; @@ -186,7 +187,8 @@ code_with_debug_info_t* convertGlsl(const char* input, size_t length, const opti code_with_debug_info_t* result = new code_with_debug_info_t{}; std::string err_msg; - std::vector spirv = parseGlslang(input, &err_msg, options->shader_type, true); + std::vector spirv = parseGlslang( + input, options->preamble, &err_msg, options->shader_type, true, options->relaxed); if (!err_msg.empty()) { set_error_msg(result, "Failed to parse original source code:\n" + err_msg); @@ -239,7 +241,7 @@ code_with_debug_info_t* convertGlsl(const char* input, size_t length, const opti // check if changed source code compiles again if (options->check_after_changes) { - parseGlslang(result->source_code, &err_msg, options->shader_type, false); + parseGlslang(result->source_code, nullptr, &err_msg, options->shader_type, false, false); } if (!err_msg.empty()) { diff --git a/gapis/shadertools/cc/libmanager.h b/gapis/shadertools/cc/libmanager.h index df55c2b963..19aca6e1c9 100644 --- a/gapis/shadertools/cc/libmanager.h +++ b/gapis/shadertools/cc/libmanager.h @@ -60,6 +60,7 @@ typedef enum shader_type_t { **/ typedef struct options_t { shader_type shader_type; + const char* preamble; /* optional */ bool prefix_names; const char* names_prefix; /* optional */ bool add_outputs_for_inputs; @@ -67,6 +68,7 @@ typedef struct options_t { bool make_debuggable; bool check_after_changes; bool disassemble; + bool relaxed; } options_t; typedef struct spirv_binary_t { diff --git a/gapis/shadertools/shadertools.go b/gapis/shadertools/shadertools.go index 27b3f22a1c..197c119ca9 100644 --- a/gapis/shadertools/shadertools.go +++ b/gapis/shadertools/shadertools.go @@ -103,10 +103,12 @@ func (t ShaderType) String() string { } } -// Option controls how ConvertGlsl converts its passed-in GLSL source code. -type Option struct { +// Options controls how ConvertGlsl converts its passed-in GLSL source code. +type Options struct { // The type of shader. ShaderType ShaderType + // Shader source preamble. + Preamble string // Whether to add prefix to all non-builtin symbols. PrefixNames bool // The name prefix to be added to all non-builtin symbols. @@ -121,33 +123,44 @@ type Option struct { CheckAfterChanges bool // Whether to disassemble the generated GLSL code. Disassemble bool + // If true, let some minor invalid statements compile. + Relaxed bool } // ConvertGlsl modifies the given GLSL according to the options specified via -// option and returns the modification status and result. Possible -// modifications includes creating output variables for input variables, -// prefixing all non-builtin symbols with a given prefix, etc. -func ConvertGlsl(source string, option *Option) (CodeWithDebugInfo, error) { +// o and returns the modification status and result. Possible modifications +// includes creating output variables for input variables, prefixing all +// non-builtin symbols with a given prefix, etc. +func ConvertGlsl(source string, o *Options) (CodeWithDebugInfo, error) { + toFree := []unsafe.Pointer{} + defer func() { + for _, ptr := range toFree { + C.free(ptr) + } + }() + mutex.Lock() defer mutex.Unlock() - np := C.CString(option.NamesPrefix) - op := C.CString(option.OutputPrefix) + cstr := func(s string) *C.char { + out := C.CString(s) + toFree = append(toFree, unsafe.Pointer(out)) + return out + } + opts := C.struct_options_t{ - shader_type: C.shader_type(option.ShaderType), - prefix_names: C.bool(option.PrefixNames), - names_prefix: np, - add_outputs_for_inputs: C.bool(option.AddOutputsForInputs), - output_prefix: op, - make_debuggable: C.bool(option.MakeDebuggable), - check_after_changes: C.bool(option.CheckAfterChanges), - disassemble: C.bool(option.Disassemble), + shader_type: C.shader_type(o.ShaderType), + preamble: cstr(o.Preamble), + prefix_names: C.bool(o.PrefixNames), + names_prefix: cstr(o.NamesPrefix), + add_outputs_for_inputs: C.bool(o.AddOutputsForInputs), + output_prefix: cstr(o.OutputPrefix), + make_debuggable: C.bool(o.MakeDebuggable), + check_after_changes: C.bool(o.CheckAfterChanges), + disassemble: C.bool(o.Disassemble), + relaxed: C.bool(o.Relaxed), } - csource := C.CString(source) - result := C.convertGlsl(csource, C.size_t(len(source)), &opts) - C.free(unsafe.Pointer(np)) - C.free(unsafe.Pointer(op)) - C.free(unsafe.Pointer(csource)) + result := C.convertGlsl(cstr(source), C.size_t(len(source)), &opts) defer C.deleteGlslCodeWithDebug(result) ret := CodeWithDebugInfo{ @@ -175,7 +188,7 @@ func ConvertGlsl(source string, option *Option) (CodeWithDebugInfo, error) { if !result.ok { msg := []string{ - fmt.Sprintf("Failed to convert %v shader.", option.ShaderType), + fmt.Sprintf("Failed to convert %v shader.", o.ShaderType), } if m := C.GoString(result.message); len(m) > 0 { msg = append(msg, m) From 7d66c661d55d13ffc93d1c7bac14a18b67626d99 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Fri, 6 Oct 2017 18:59:16 +0100 Subject: [PATCH 2/4] gles/compat: Use relaxed compilation for cross-compilation. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Errors are still reported with full severity in the report view, but this: a) Let’s you see something in the views (much like most Android devices) b) Doesn’t mean compilation errors snowball into other report issues. --- gapis/api/gles/compat.go | 1 + 1 file changed, 1 insertion(+) diff --git a/gapis/api/gles/compat.go b/gapis/api/gles/compat.go index 996ef853c9..6afd3b5741 100644 --- a/gapis/api/gles/compat.go +++ b/gapis/api/gles/compat.go @@ -456,6 +456,7 @@ func compat(ctx context.Context, device *device.Instance) (transform.Transformer } opts := shadertools.Options{ ShaderType: st, + Relaxed: true, // find_issues will still report bad GLSL. } // Trim any prefix whitespace / newlines. From 99ab27fd3eab0c04cb202a0ec18567d309558456 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Fri, 6 Oct 2017 19:00:43 +0100 Subject: [PATCH 3/4] gles/compat: Handle missing glTexStorage2DMultisample(). MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It doesn’t exist on mac (needs 4.3, got 4.1) Use glTexImage2DMultisample instead. --- gapis/api/gles/api/gl.api | 3 +++ gapis/api/gles/compat.go | 11 +++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/gapis/api/gles/api/gl.api b/gapis/api/gles/api/gl.api index 75f1d9d7b3..807c673cda 100644 --- a/gapis/api/gles/api/gl.api +++ b/gapis/api/gles/api/gl.api @@ -45,3 +45,6 @@ cmd void glTexStorage1D(GLenum target, GLsizei levels, GLenum internalformat, GL cmd void glBindFragDataLocation(ProgramId program, GLuint color, string name) { } + +cmd void glTexImage2DMultisample(GLenum target, GLsizei samples, GLenum internalformat, GLsizei width, GLsizei height, GLboolean fixedsamplelocations) { +} \ No newline at end of file diff --git a/gapis/api/gles/compat.go b/gapis/api/gles/compat.go index 6afd3b5741..c3ce44ff9c 100644 --- a/gapis/api/gles/compat.go +++ b/gapis/api/gles/compat.go @@ -601,12 +601,19 @@ func compat(ctx context.Context, device *device.Instance) (transform.Transformer return } case *GlTexStorage2DMultisample: - { + if version.IsES || version.AtLeastGL(4, 3) { + // glTexStorage2DMultisample is supported by replay device. cmd := *cmd textureCompat.convertFormat(ctx, cmd.Target, &cmd.Internalformat, nil, nil, out, id, &cmd) out.MutateAndWrite(ctx, id, &cmd) - return + } else { + // glTexStorage2DMultisample is not supported by replay device. + // Use glTexImage2DMultisample instead. + cmd := cb.GlTexImage2DMultisample(cmd.Target, cmd.Samples, cmd.Internalformat, cmd.Width, cmd.Height, cmd.Fixedsamplelocations) + textureCompat.convertFormat(ctx, cmd.Target, &cmd.Internalformat, nil, nil, out, id, cmd) + out.MutateAndWrite(ctx, id, cmd) } + return case *GlTexStorage3D: { cmd := *cmd From 83f2be2b7d7286953e85aa1342ff8ce3b89a3328 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Fri, 6 Oct 2017 19:01:26 +0100 Subject: [PATCH 4/4] gles/compat: Ignore glInvalidateSubFramebuffer. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Don’t remove glInvalidateFramebuffer if it is supported. --- gapis/api/gles/compat.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/gapis/api/gles/compat.go b/gapis/api/gles/compat.go index c3ce44ff9c..6ec5351da8 100644 --- a/gapis/api/gles/compat.go +++ b/gapis/api/gles/compat.go @@ -915,13 +915,17 @@ func compat(ctx context.Context, device *device.Instance) (transform.Transformer // It has no effect on rendering so just drop it. return - case *GlInvalidateFramebuffer, - *GlDiscardFramebufferEXT: // GL_EXT_discard_framebuffer + case *GlDiscardFramebufferEXT: // GL_EXT_discard_framebuffer // It may not be implemented by the replay driver. // It is only a hint so we can just drop it. // TODO: It has performance impact so we should not ignore it when profiling. return + case *GlInvalidateFramebuffer, *GlInvalidateSubFramebuffer: + if !version.AtLeastES(3, 0) || !version.AtLeastGL(4, 3) { + return // Not supported. Only a hint. Drop it. + } + case *GlMapBufferOES: if !version.IsES { // Remove extension suffix on desktop. cmd := cb.GlMapBuffer(cmd.Target, cmd.Access, memory.Pointer(cmd.Result))