diff --git a/src/dawn_native/CopyTextureForBrowserHelper.cpp b/src/dawn_native/CopyTextureForBrowserHelper.cpp index 46da2c8cec..d64870c428 100644 --- a/src/dawn_native/CopyTextureForBrowserHelper.cpp +++ b/src/dawn_native/CopyTextureForBrowserHelper.cpp @@ -33,17 +33,14 @@ namespace dawn_native { namespace { -// TODO(crbug.com/1221110): Remove this header macro by merging vertex and -// fragment shaders into one shader source. Now it blocks by -// crbug.com/dawn/947 and crbug.com/tint/915 -#define HEADER \ - " [[block]] struct Uniforms {\n" \ - " u_scale: vec2;\n" \ - " u_offset: vec2;\n" \ - " u_alphaOp: u32;\n" \ - " };\n" - static const char sCopyTextureForBrowserVertex[] = HEADER R"( + static const char sCopyTextureForBrowserShader[] = R"( + [[block]] struct Uniforms { + u_scale: vec2; + u_offset: vec2; + u_alphaOp: u32; + }; + [[binding(0), group(0)]] var uniforms : Uniforms; struct VertexOutputs { @@ -51,7 +48,7 @@ namespace dawn_native { [[builtin(position)]] position : vec4; }; - [[stage(vertex)]] fn main( + [[stage(vertex)]] fn vs_main( [[builtin(vertex_index)]] VertexIndex : u32 ) -> VertexOutputs { var texcoord = array, 3>( @@ -84,14 +81,11 @@ namespace dawn_native { return output; } - )"; - static const char sCopyTextureForBrowserFragment[] = HEADER R"( - [[binding(0), group(0)]] var uniforms : Uniforms; [[binding(1), group(0)]] var mySampler: sampler; [[binding(2), group(0)]] var myTexture: texture_2d; - [[stage(fragment)]] fn main( + [[stage(fragment)]] fn fs_main( [[location(0)]] texcoord : vec2 ) -> [[location(0)]] vec4 { // Clamp the texcoord and discard the out-of-bound pixels. @@ -226,39 +220,27 @@ namespace dawn_native { if (GetCachedPipeline(store, dstFormat) == nullptr) { // Create vertex shader module if not cached before. - if (store->copyTextureForBrowserVS == nullptr) { + if (store->copyTextureForBrowser == nullptr) { ShaderModuleDescriptor descriptor; ShaderModuleWGSLDescriptor wgslDesc; - wgslDesc.source = sCopyTextureForBrowserVertex; + wgslDesc.source = sCopyTextureForBrowserShader; descriptor.nextInChain = reinterpret_cast(&wgslDesc); - DAWN_TRY_ASSIGN(store->copyTextureForBrowserVS, + DAWN_TRY_ASSIGN(store->copyTextureForBrowser, device->CreateShaderModule(&descriptor)); } - ShaderModuleBase* vertexModule = store->copyTextureForBrowserVS.Get(); - - // Create fragment shader module if not cached before. - if (store->copyTextureForBrowserFS == nullptr) { - ShaderModuleDescriptor descriptor; - ShaderModuleWGSLDescriptor wgslDesc; - wgslDesc.source = sCopyTextureForBrowserFragment; - descriptor.nextInChain = reinterpret_cast(&wgslDesc); - DAWN_TRY_ASSIGN(store->copyTextureForBrowserFS, - device->CreateShaderModule(&descriptor)); - } - - ShaderModuleBase* fragmentModule = store->copyTextureForBrowserFS.Get(); + ShaderModuleBase* shaderModule = store->copyTextureForBrowser.Get(); // Prepare vertex stage. VertexState vertex = {}; - vertex.module = vertexModule; - vertex.entryPoint = "main"; + vertex.module = shaderModule; + vertex.entryPoint = "vs_main"; // Prepare frgament stage. FragmentState fragment = {}; - fragment.module = fragmentModule; - fragment.entryPoint = "main"; + fragment.module = shaderModule; + fragment.entryPoint = "fs_main"; // Prepare color state. ColorTargetState target = {}; diff --git a/src/dawn_native/InternalPipelineStore.h b/src/dawn_native/InternalPipelineStore.h index 1d901595de..3066a9a940 100644 --- a/src/dawn_native/InternalPipelineStore.h +++ b/src/dawn_native/InternalPipelineStore.h @@ -28,8 +28,7 @@ namespace dawn_native { std::unordered_map> copyTextureForBrowserPipelines; - Ref copyTextureForBrowserVS; - Ref copyTextureForBrowserFS; + Ref copyTextureForBrowser; Ref timestampComputePipeline; Ref timestampCS; diff --git a/src/dawn_native/opengl/ComputePipelineGL.cpp b/src/dawn_native/opengl/ComputePipelineGL.cpp index 837b928464..7680ceebb0 100644 --- a/src/dawn_native/opengl/ComputePipelineGL.cpp +++ b/src/dawn_native/opengl/ComputePipelineGL.cpp @@ -18,12 +18,19 @@ namespace dawn_native { namespace opengl { - ComputePipeline::ComputePipeline(Device* device, const ComputePipelineDescriptor* descriptor) - : ComputePipelineBase(device, descriptor) { - PerStage modules(nullptr); - modules[SingleShaderStage::Compute] = ToBackend(descriptor->compute.module); + // static + ResultOrError> ComputePipeline::Create( + Device* device, + const ComputePipelineDescriptor* descriptor) { + Ref pipeline = AcquireRef(new ComputePipeline(device, descriptor)); + DAWN_TRY(pipeline->Initialize(descriptor)); + return pipeline; + } - PipelineGL::Initialize(device->gl, ToBackend(descriptor->layout), GetAllStages()); + MaybeError ComputePipeline::Initialize(const ComputePipelineDescriptor*) { + DAWN_TRY( + InitializeBase(ToBackend(GetDevice())->gl, ToBackend(GetLayout()), GetAllStages())); + return {}; } void ComputePipeline::ApplyNow() { diff --git a/src/dawn_native/opengl/ComputePipelineGL.h b/src/dawn_native/opengl/ComputePipelineGL.h index 95f6db9131..e84e366676 100644 --- a/src/dawn_native/opengl/ComputePipelineGL.h +++ b/src/dawn_native/opengl/ComputePipelineGL.h @@ -27,12 +27,16 @@ namespace dawn_native { namespace opengl { class ComputePipeline final : public ComputePipelineBase, public PipelineGL { public: - ComputePipeline(Device* device, const ComputePipelineDescriptor* descriptor); + static ResultOrError> Create( + Device* device, + const ComputePipelineDescriptor* descriptor); void ApplyNow(); private: + using ComputePipelineBase::ComputePipelineBase; ~ComputePipeline() override = default; + MaybeError Initialize(const ComputePipelineDescriptor* descriptor) override; }; }} // namespace dawn_native::opengl diff --git a/src/dawn_native/opengl/DeviceGL.cpp b/src/dawn_native/opengl/DeviceGL.cpp index dd38faa0fe..fb7b4ea7b6 100644 --- a/src/dawn_native/opengl/DeviceGL.cpp +++ b/src/dawn_native/opengl/DeviceGL.cpp @@ -129,7 +129,7 @@ namespace dawn_native { namespace opengl { } ResultOrError> Device::CreateComputePipelineImpl( const ComputePipelineDescriptor* descriptor) { - return AcquireRef(new ComputePipeline(this, descriptor)); + return ComputePipeline::Create(this, descriptor); } ResultOrError> Device::CreatePipelineLayoutImpl( const PipelineLayoutDescriptor* descriptor) { @@ -141,7 +141,7 @@ namespace dawn_native { namespace opengl { } ResultOrError> Device::CreateRenderPipelineImpl( const RenderPipelineDescriptor* descriptor) { - return AcquireRef(new RenderPipeline(this, descriptor)); + return RenderPipeline::Create(this, descriptor); } ResultOrError> Device::CreateSamplerImpl(const SamplerDescriptor* descriptor) { return AcquireRef(new Sampler(this, descriptor)); diff --git a/src/dawn_native/opengl/PipelineGL.cpp b/src/dawn_native/opengl/PipelineGL.cpp index 01100254b4..4541e651f7 100644 --- a/src/dawn_native/opengl/PipelineGL.cpp +++ b/src/dawn_native/opengl/PipelineGL.cpp @@ -15,7 +15,6 @@ #include "dawn_native/opengl/PipelineGL.h" #include "common/BitSetIterator.h" -#include "common/Log.h" #include "dawn_native/BindGroupLayout.h" #include "dawn_native/Device.h" #include "dawn_native/Pipeline.h" @@ -26,6 +25,7 @@ #include "dawn_native/opengl/ShaderModuleGL.h" #include +#include namespace dawn_native { namespace opengl { @@ -47,11 +47,11 @@ namespace dawn_native { namespace opengl { PipelineGL::PipelineGL() = default; PipelineGL::~PipelineGL() = default; - void PipelineGL::Initialize(const OpenGLFunctions& gl, - const PipelineLayout* layout, - const PerStage& stages) { + MaybeError PipelineGL::InitializeBase(const OpenGLFunctions& gl, + const PipelineLayout* layout, + const PerStage& stages) { auto CreateShader = [](const OpenGLFunctions& gl, GLenum type, - const char* source) -> GLuint { + const char* source) -> ResultOrError { GLuint shader = gl.CreateShader(type); gl.ShaderSource(shader, 1, &source, nullptr); gl.CompileShader(shader); @@ -65,8 +65,9 @@ namespace dawn_native { namespace opengl { if (infoLogLength > 1) { std::vector buffer(infoLogLength); gl.GetShaderInfoLog(shader, infoLogLength, nullptr, &buffer[0]); - dawn::ErrorLog() << source << "\nProgram compilation failed:\n" - << buffer.data(); + std::stringstream ss; + ss << source << "\nProgram compilation failed:\n" << buffer.data(); + return DAWN_VALIDATION_ERROR(ss.str().c_str()); } } return shader; @@ -87,10 +88,12 @@ namespace dawn_native { namespace opengl { bool needsDummySampler = false; for (SingleShaderStage stage : IterateStages(activeStages)) { const ShaderModule* module = ToBackend(stages[stage].module.Get()); - std::string glsl = - module->TranslateToGLSL(stages[stage].entryPoint.c_str(), stage, - &combinedSamplers[stage], layout, &needsDummySampler); - GLuint shader = CreateShader(gl, GLShaderType(stage), glsl.c_str()); + std::string glsl; + DAWN_TRY_ASSIGN(glsl, module->TranslateToGLSL(stages[stage].entryPoint.c_str(), stage, + &combinedSamplers[stage], layout, + &needsDummySampler)); + GLuint shader; + DAWN_TRY_ASSIGN(shader, CreateShader(gl, GLShaderType(stage), glsl.c_str())); gl.AttachShader(mProgram, shader); } @@ -115,7 +118,9 @@ namespace dawn_native { namespace opengl { if (infoLogLength > 1) { std::vector buffer(infoLogLength); gl.GetProgramInfoLog(mProgram, infoLogLength, nullptr, &buffer[0]); - dawn::ErrorLog() << "Program link failed:\n" << buffer.data(); + std::stringstream ss; + ss << "Program link failed:\n" << buffer.data(); + return DAWN_VALIDATION_ERROR(ss.str().c_str()); } } @@ -172,6 +177,7 @@ namespace dawn_native { namespace opengl { textureUnit++; } + return {}; } const std::vector& PipelineGL::GetTextureUnitsForSampler( diff --git a/src/dawn_native/opengl/PipelineGL.h b/src/dawn_native/opengl/PipelineGL.h index 33e53410de..e21060696a 100644 --- a/src/dawn_native/opengl/PipelineGL.h +++ b/src/dawn_native/opengl/PipelineGL.h @@ -37,10 +37,6 @@ namespace dawn_native { namespace opengl { PipelineGL(); ~PipelineGL(); - void Initialize(const OpenGLFunctions& gl, - const PipelineLayout* layout, - const PerStage& stages); - // For each unit a sampler is bound to we need to know if we should use filtering or not // because int and uint texture are only complete without filtering. struct SamplerUnit { @@ -53,6 +49,11 @@ namespace dawn_native { namespace opengl { void ApplyNow(const OpenGLFunctions& gl); + protected: + MaybeError InitializeBase(const OpenGLFunctions& gl, + const PipelineLayout* layout, + const PerStage& stages); + private: GLuint mProgram; std::vector> mUnitsForSamplers; diff --git a/src/dawn_native/opengl/RenderPipelineGL.cpp b/src/dawn_native/opengl/RenderPipelineGL.cpp index aed8c019e4..5bc596f053 100644 --- a/src/dawn_native/opengl/RenderPipelineGL.cpp +++ b/src/dawn_native/opengl/RenderPipelineGL.cpp @@ -219,16 +219,26 @@ namespace dawn_native { namespace opengl { } // anonymous namespace + // static + ResultOrError> RenderPipeline::Create( + Device* device, + const RenderPipelineDescriptor* descriptor) { + Ref pipeline = AcquireRef(new RenderPipeline(device, descriptor)); + DAWN_TRY(pipeline->Initialize()); + return pipeline; + } + RenderPipeline::RenderPipeline(Device* device, const RenderPipelineDescriptor* descriptor) : RenderPipelineBase(device, descriptor), mVertexArrayObject(0), mGlPrimitiveTopology(GLPrimitiveTopology(GetPrimitiveTopology())) { - PerStage modules(nullptr); - modules[SingleShaderStage::Vertex] = ToBackend(descriptor->vertex.module); - modules[SingleShaderStage::Fragment] = ToBackend(descriptor->fragment->module); + } - PipelineGL::Initialize(device->gl, ToBackend(GetLayout()), GetAllStages()); + MaybeError RenderPipeline::Initialize() { + DAWN_TRY( + InitializeBase(ToBackend(GetDevice())->gl, ToBackend(GetLayout()), GetAllStages())); CreateVAOForVertexState(); + return {}; } RenderPipeline::~RenderPipeline() { diff --git a/src/dawn_native/opengl/RenderPipelineGL.h b/src/dawn_native/opengl/RenderPipelineGL.h index 960b50b2ff..3c7a4d321a 100644 --- a/src/dawn_native/opengl/RenderPipelineGL.h +++ b/src/dawn_native/opengl/RenderPipelineGL.h @@ -29,7 +29,9 @@ namespace dawn_native { namespace opengl { class RenderPipeline final : public RenderPipelineBase, public PipelineGL { public: - RenderPipeline(Device* device, const RenderPipelineDescriptor* descriptor); + static ResultOrError> Create( + Device* device, + const RenderPipelineDescriptor* descriptor); GLenum GetGLPrimitiveTopology() const; ityp::bitset GetAttributesUsingVertexBuffer( @@ -38,7 +40,10 @@ namespace dawn_native { namespace opengl { void ApplyNow(PersistentPipelineState& persistentPipelineState); private: + RenderPipeline(Device* device, const RenderPipelineDescriptor* descriptor); ~RenderPipeline() override; + MaybeError Initialize(); + void CreateVAOForVertexState(); // TODO(yunchao.he@intel.com): vao need to be deduplicated between pipelines. diff --git a/src/dawn_native/opengl/ShaderModuleGL.cpp b/src/dawn_native/opengl/ShaderModuleGL.cpp index a5bbcb9aef..6aac35de75 100644 --- a/src/dawn_native/opengl/ShaderModuleGL.cpp +++ b/src/dawn_native/opengl/ShaderModuleGL.cpp @@ -94,18 +94,18 @@ namespace dawn_native { namespace opengl { return DAWN_VALIDATION_ERROR(errorStream.str().c_str()); } - mGLSpirv = std::move(result.spirv); - DAWN_TRY_ASSIGN(mGLEntryPoints, ReflectShaderUsingSPIRVCross(GetDevice(), mGLSpirv)); + DAWN_TRY_ASSIGN(mGLEntryPoints, + ReflectShaderUsingSPIRVCross(GetDevice(), result.spirv)); } return {}; } - std::string ShaderModule::TranslateToGLSL(const char* entryPointName, - SingleShaderStage stage, - CombinedSamplerInfo* combinedSamplers, - const PipelineLayout* layout, - bool* needsDummySampler) const { + ResultOrError ShaderModule::TranslateToGLSL(const char* entryPointName, + SingleShaderStage stage, + CombinedSamplerInfo* combinedSamplers, + const PipelineLayout* layout, + bool* needsDummySampler) const { // If these options are changed, the values in DawnSPIRVCrossGLSLFastFuzzer.cpp need to // be updated. spirv_cross::CompilerGLSL::Options options; @@ -125,8 +125,32 @@ namespace dawn_native { namespace opengl { options.es = version.IsES(); options.version = version.GetMajor() * 100 + version.GetMinor() * 10; - spirv_cross::CompilerGLSL compiler( - GetDevice()->IsToggleEnabled(Toggle::UseTintGenerator) ? mGLSpirv : GetSpirv()); + std::vector spirv; + if (GetDevice()->IsToggleEnabled(Toggle::UseTintGenerator)) { + tint::transform::SingleEntryPoint singleEntryPointTransform; + + tint::transform::DataMap transformInputs; + transformInputs.Add(entryPointName); + + tint::Program program; + DAWN_TRY_ASSIGN(program, RunTransforms(&singleEntryPointTransform, GetTintProgram(), + transformInputs, nullptr, nullptr)); + + tint::writer::spirv::Options options; + options.disable_workgroup_init = + GetDevice()->IsToggleEnabled(Toggle::DisableWorkgroupInit); + auto result = tint::writer::spirv::Generate(&program, options); + if (!result.success) { + std::ostringstream errorStream; + errorStream << "Generator: " << result.error << std::endl; + return DAWN_VALIDATION_ERROR(errorStream.str().c_str()); + } + + spirv = std::move(result.spirv); + } else { + spirv = GetSpirv(); + } + spirv_cross::CompilerGLSL compiler(std::move(spirv)); compiler.set_common_options(options); compiler.set_entry_point(entryPointName, ShaderStageToExecutionModel(stage)); diff --git a/src/dawn_native/opengl/ShaderModuleGL.h b/src/dawn_native/opengl/ShaderModuleGL.h index 7f598b9b70..d6d9f74028 100644 --- a/src/dawn_native/opengl/ShaderModuleGL.h +++ b/src/dawn_native/opengl/ShaderModuleGL.h @@ -50,18 +50,17 @@ namespace dawn_native { namespace opengl { const ShaderModuleDescriptor* descriptor, ShaderModuleParseResult* parseResult); - std::string TranslateToGLSL(const char* entryPointName, - SingleShaderStage stage, - CombinedSamplerInfo* combinedSamplers, - const PipelineLayout* layout, - bool* needsDummySampler) const; + ResultOrError TranslateToGLSL(const char* entryPointName, + SingleShaderStage stage, + CombinedSamplerInfo* combinedSamplers, + const PipelineLayout* layout, + bool* needsDummySampler) const; private: ShaderModule(Device* device, const ShaderModuleDescriptor* descriptor); ~ShaderModule() override = default; MaybeError Initialize(ShaderModuleParseResult* parseResult); - std::vector mGLSpirv; EntryPointMetadataTable mGLEntryPoints; }; diff --git a/src/dawn_native/vulkan/ShaderModuleVk.cpp b/src/dawn_native/vulkan/ShaderModuleVk.cpp index 525dbb1197..0144eefeff 100644 --- a/src/dawn_native/vulkan/ShaderModuleVk.cpp +++ b/src/dawn_native/vulkan/ShaderModuleVk.cpp @@ -104,7 +104,6 @@ namespace dawn_native { namespace vulkan { DAWN_TRY_ASSIGN(program, RunTransforms(&transformManager, parseResult->tintProgram.get(), transformInputs, nullptr, nullptr)); - // We will miss the messages generated in this RunTransforms. tint::writer::spirv::Options options; options.emit_vertex_point_size = true; @@ -203,11 +202,14 @@ namespace dawn_native { namespace vulkan { tint::transform::Manager transformManager; transformManager.append(std::make_unique()); + // Many Vulkan drivers can't handle multi-entrypoint shader modules. + transformManager.append(std::make_unique()); tint::transform::DataMap transformInputs; transformInputs.Add(std::move(bindingPoints), std::move(accessControls), /* mayCollide */ false); + transformInputs.Add(entryPointName); tint::Program program; DAWN_TRY_ASSIGN(program, RunTransforms(&transformManager, GetTintProgram(), transformInputs, diff --git a/src/tests/end2end/CopyTextureForBrowserTests.cpp b/src/tests/end2end/CopyTextureForBrowserTests.cpp index 1aca79d5c1..768e36a339 100644 --- a/src/tests/end2end/CopyTextureForBrowserTests.cpp +++ b/src/tests/end2end/CopyTextureForBrowserTests.cpp @@ -140,6 +140,10 @@ class CopyTextureForBrowserTests : public DawnTest { void SetUp() override { DawnTest::SetUp(); + // crbug.com/dawn/948: Tint required for multiple entrypoints in a module. + // CopyTextureForBrowser uses and internal pipeline with a multi-entrypoint + // shader module. + DAWN_TEST_UNSUPPORTED_IF(!HasToggleEnabled("use_tint_generator")); testPipeline = MakeTestPipeline(); diff --git a/src/tests/end2end/ShaderTests.cpp b/src/tests/end2end/ShaderTests.cpp index fa427aa8bf..c635048867 100644 --- a/src/tests/end2end/ShaderTests.cpp +++ b/src/tests/end2end/ShaderTests.cpp @@ -262,8 +262,8 @@ fn main(input : FragmentIn) -> [[location(0)]] vec4 { // Tests that shaders I/O structs can be shared between vertex and fragment shaders. TEST_P(ShaderTests, WGSLSharedStructIO) { - // TODO(tint:714): Not yet implemeneted in tint yet, but intended to work. - DAWN_SUPPRESS_TEST_IF(IsD3D12() || IsVulkan() || IsMetal() || IsOpenGL() || IsOpenGLES()); + // crbug.com/dawn/948: Tint required for multiple entrypoints in a module. + DAWN_TEST_UNSUPPORTED_IF(!HasToggleEnabled("use_tint_generator")); std::string shader = R"( struct VertexIn {