From affd675ad175c6ef9ee8991ae41c4f8d0395cb49 Mon Sep 17 00:00:00 2001 From: Zachary Anderson Date: Tue, 23 Jan 2024 10:42:10 -0800 Subject: [PATCH 1/3] Exclude prebuilts/Library from Mac builder_cache (#49971) Related https://github.com/flutter/flutter/issues/141688 --- .ci.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.ci.yaml b/.ci.yaml index 3f80069bb6bfd..487a0a6add754 100644 --- a/.ci.yaml +++ b/.ci.yaml @@ -130,7 +130,8 @@ targets: ] ignore_cache_paths: >- [ - "builder/src/flutter/prebuilts/SDKs" + "builder/src/flutter/prebuilts/SDKs", + "builder/src/flutter/prebuilts/Library" ] gclient_variables: >- { From afb29dac2bbc790ba3ca26443e56011659df6fb9 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Tue, 23 Jan 2024 10:57:52 -0800 Subject: [PATCH 2/3] [Impeller] Fix runtime effect pipeline depth/stencil. (#49953) - Setup depth and stencil attachments for RuntimeEffect. - Add validation check in `EntityPass` to force backends to set things up the root pass correctly. - Add checks in `ContentContextOptions::ApplyToPipelineDescriptor` to validate that callers are holding it right. --- impeller/entity/contents/content_context.cc | 14 ++++++ .../contents/runtime_effect_contents.cc | 9 ++-- impeller/entity/entity_pass.cc | 6 +++ impeller/entity/entity_unittests.cc | 47 +++++++++++++++++++ 4 files changed, 72 insertions(+), 4 deletions(-) diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc index f3baf7437f517..f4c186199a817 100644 --- a/impeller/entity/contents/content_context.cc +++ b/impeller/entity/contents/content_context.cc @@ -142,12 +142,26 @@ void ContentContextOptions::ApplyToPipelineDescriptor( } auto maybe_stencil = desc.GetFrontStencilAttachmentDescriptor(); + auto maybe_depth = desc.GetDepthStencilAttachmentDescriptor(); + FML_DCHECK(has_depth_stencil_attachments == maybe_depth.has_value()) + << "Depth attachment doesn't match expected pipeline state. " + "has_depth_stencil_attachments=" + << has_depth_stencil_attachments; + FML_DCHECK(has_depth_stencil_attachments == maybe_stencil.has_value()) + << "Stencil attachment doesn't match expected pipeline state. " + "has_depth_stencil_attachments=" + << has_depth_stencil_attachments; if (maybe_stencil.has_value()) { StencilAttachmentDescriptor stencil = maybe_stencil.value(); stencil.stencil_compare = stencil_compare; stencil.depth_stencil_pass = stencil_operation; desc.SetStencilAttachmentDescriptors(stencil); } + if (maybe_depth.has_value()) { + DepthAttachmentDescriptor depth = maybe_depth.value(); + depth.depth_compare = CompareFunction::kAlways; + depth.depth_write_enabled = false; + } desc.SetPrimitiveType(primitive_type); diff --git a/impeller/entity/contents/runtime_effect_contents.cc b/impeller/entity/contents/runtime_effect_contents.cc index 7da74e4a56b0b..896731622eb7e 100644 --- a/impeller/entity/contents/runtime_effect_contents.cc +++ b/impeller/entity/contents/runtime_effect_contents.cc @@ -141,7 +141,7 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer, const std::shared_ptr& caps = context->GetCapabilities(); const auto color_attachment_format = caps->GetDefaultColorFormat(); - const auto stencil_attachment_format = caps->GetDefaultStencilFormat(); + const auto stencil_attachment_format = caps->GetDefaultDepthStencilFormat(); using VS = RuntimeEffectVertexShader; @@ -303,11 +303,12 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer, desc.SetColorAttachmentDescriptor( 0u, {.format = color_attachment_format, .blending_enabled = true}); - StencilAttachmentDescriptor stencil0; - stencil0.stencil_compare = CompareFunction::kEqual; - desc.SetStencilAttachmentDescriptors(stencil0); + desc.SetStencilAttachmentDescriptors(StencilAttachmentDescriptor{}); desc.SetStencilPixelFormat(stencil_attachment_format); + desc.SetDepthStencilAttachmentDescriptor(DepthAttachmentDescriptor{}); + desc.SetDepthPixelFormat(stencil_attachment_format); + options.ApplyToPipelineDescriptor(desc); auto pipeline = context->GetPipelineLibrary()->GetPipeline(desc).Get(); if (!pipeline) { diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index e56e4bb7f9744..701910f5e893a 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -328,6 +328,12 @@ bool EntityPass::Render(ContentContext& renderer, VALIDATION_LOG << "The root RenderTarget must have a color attachment."; return false; } + if (root_render_target.GetDepthAttachment().has_value() != + root_render_target.GetStencilAttachment().has_value()) { + VALIDATION_LOG << "The root RenderTarget should have a stencil attachment " + "iff it has a depth attachment."; + return false; + } capture.AddRect("Coverage", Rect::MakeSize(root_render_target.GetRenderTargetSize()), diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index fc14e47e4cf12..a6a21d9bdeb02 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -2200,6 +2200,53 @@ TEST_P(EntityTest, RuntimeEffect) { ASSERT_TRUE(OpenPlaygroundHere(callback)); } +TEST_P(EntityTest, RuntimeEffectCanSuccessfullyRender) { + auto runtime_stages = + OpenAssetAsRuntimeStage("runtime_stage_example.frag.iplr"); + auto runtime_stage = + runtime_stages[PlaygroundBackendToRuntimeStageBackend(GetBackend())]; + ASSERT_TRUE(runtime_stage); + ASSERT_TRUE(runtime_stage->IsDirty()); + + auto contents = std::make_shared(); + contents->SetGeometry(Geometry::MakeCover()); + + contents->SetRuntimeStage(runtime_stage); + + struct FragUniforms { + Vector2 iResolution; + Scalar iTime; + } frag_uniforms = { + .iResolution = Vector2(GetWindowSize().width, GetWindowSize().height), + .iTime = static_cast(GetSecondsElapsed()), + }; + auto uniform_data = std::make_shared>(); + uniform_data->resize(sizeof(FragUniforms)); + memcpy(uniform_data->data(), &frag_uniforms, sizeof(FragUniforms)); + contents->SetUniformData(uniform_data); + + Entity entity; + entity.SetContents(contents); + + // Create a render target with a depth-stencil, similar to how EntityPass + // does. + RenderTarget target = RenderTarget::CreateOffscreenMSAA( + *GetContext(), *GetContentContext()->GetRenderTargetCache(), + {GetWindowSize().width, GetWindowSize().height}, 1, + "RuntimeEffect Texture"); + testing::MockRenderPass pass(GetContext(), target); + + ASSERT_TRUE(contents->Render(*GetContentContext(), entity, pass)); + ASSERT_EQ(pass.GetCommands().size(), 1u); + const auto& command = pass.GetCommands()[0]; + ASSERT_TRUE(command.pipeline->GetDescriptor() + .GetDepthStencilAttachmentDescriptor() + .has_value()); + ASSERT_TRUE(command.pipeline->GetDescriptor() + .GetFrontStencilAttachmentDescriptor() + .has_value()); +} + TEST_P(EntityTest, RuntimeEffectSetsRightSizeWhenUniformIsStruct) { if (GetBackend() != PlaygroundBackend::kVulkan) { GTEST_SKIP() << "Test only applies to Vulkan"; From 57d6b518f9205027256391e3c71529ee510598cf Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 23 Jan 2024 11:17:05 -0800 Subject: [PATCH 3/3] [Impeller] fix validation error for playground texture upload. (#49957) Buffer to image is missing barriers. We don't really need to use this for playgrounds though, as set contents does the right thing more or less. Fixes https://github.com/flutter/flutter/issues/142024 --- .../golden_playground_test_mac.cc | 1 + impeller/playground/playground.cc | 74 +++++-------------- 2 files changed, 21 insertions(+), 54 deletions(-) diff --git a/impeller/golden_tests/golden_playground_test_mac.cc b/impeller/golden_tests/golden_playground_test_mac.cc index c8bbe289d60e0..2077e7a2baea8 100644 --- a/impeller/golden_tests/golden_playground_test_mac.cc +++ b/impeller/golden_tests/golden_playground_test_mac.cc @@ -63,6 +63,7 @@ static const std::vector kSkipTests = { /// TODO(https://github.com/flutter/flutter/issues/142017): Turn on validation /// for all vulkan tests. static const std::vector kVulkanValidationTests = { + "impeller_Play_AiksTest_CanRenderImageRect_Vulkan", "impeller_Play_AiksTest_CanRenderTextFrame_Vulkan", }; diff --git a/impeller/playground/playground.cc b/impeller/playground/playground.cc index c4f4f1338c391..d018885377fb8 100644 --- a/impeller/playground/playground.cc +++ b/impeller/playground/playground.cc @@ -385,29 +385,26 @@ static std::shared_ptr CreateTextureForDecompressedImage( const std::shared_ptr& context, DecompressedImage& decompressed_image, bool enable_mipmapping) { - // TODO(https://github.com/flutter/flutter/issues/123468): copying buffers to - // textures is not implemented for GLES. - if (context->GetCapabilities()->SupportsBufferToTextureBlits()) { - impeller::TextureDescriptor texture_descriptor; - texture_descriptor.storage_mode = impeller::StorageMode::kDevicePrivate; - texture_descriptor.format = PixelFormat::kR8G8B8A8UNormInt; - texture_descriptor.size = decompressed_image.GetSize(); - texture_descriptor.mip_count = - enable_mipmapping ? decompressed_image.GetSize().MipCount() : 1u; - - auto dest_texture = - context->GetResourceAllocator()->CreateTexture(texture_descriptor); - if (!dest_texture) { - FML_DLOG(ERROR) << "Could not create Impeller texture."; - return nullptr; - } - - auto buffer = context->GetResourceAllocator()->CreateBufferWithCopy( - *decompressed_image.GetAllocation().get()); + auto texture_descriptor = TextureDescriptor{}; + texture_descriptor.storage_mode = StorageMode::kHostVisible; + texture_descriptor.format = PixelFormat::kR8G8B8A8UNormInt; + texture_descriptor.size = decompressed_image.GetSize(); + texture_descriptor.mip_count = + enable_mipmapping ? decompressed_image.GetSize().MipCount() : 1u; - dest_texture->SetLabel( - impeller::SPrintF("ui.Image(%p)", dest_texture.get()).c_str()); + auto texture = + context->GetResourceAllocator()->CreateTexture(texture_descriptor); + if (!texture) { + VALIDATION_LOG << "Could not allocate texture for fixture."; + return nullptr; + } + auto uploaded = texture->SetContents(decompressed_image.GetAllocation()); + if (!uploaded) { + VALIDATION_LOG << "Could not upload texture to device memory for fixture."; + return nullptr; + } + if (enable_mipmapping) { auto command_buffer = context->CreateCommandBuffer(); if (!command_buffer) { FML_DLOG(ERROR) @@ -415,47 +412,16 @@ static std::shared_ptr CreateTextureForDecompressedImage( return nullptr; } command_buffer->SetLabel("Mipmap Command Buffer"); - auto blit_pass = command_buffer->CreateBlitPass(); - if (!blit_pass) { - FML_DLOG(ERROR) << "Could not create blit pass for mipmap generation."; - return nullptr; - } blit_pass->SetLabel("Mipmap Blit Pass"); - blit_pass->AddCopy(DeviceBuffer::AsBufferView(buffer), dest_texture); - if (enable_mipmapping) { - blit_pass->GenerateMipmap(dest_texture); - } - + blit_pass->GenerateMipmap(texture); blit_pass->EncodeCommands(context->GetResourceAllocator()); if (!command_buffer->SubmitCommands()) { FML_DLOG(ERROR) << "Failed to submit blit pass command buffer."; return nullptr; } - return dest_texture; - } else { // Doesn't support buffer-to-texture blits. - auto texture_descriptor = TextureDescriptor{}; - texture_descriptor.storage_mode = StorageMode::kHostVisible; - texture_descriptor.format = PixelFormat::kR8G8B8A8UNormInt; - texture_descriptor.size = decompressed_image.GetSize(); - texture_descriptor.mip_count = - enable_mipmapping ? decompressed_image.GetSize().MipCount() : 1u; - - auto texture = - context->GetResourceAllocator()->CreateTexture(texture_descriptor); - if (!texture) { - VALIDATION_LOG << "Could not allocate texture for fixture."; - return nullptr; - } - - auto uploaded = texture->SetContents(decompressed_image.GetAllocation()); - if (!uploaded) { - VALIDATION_LOG - << "Could not upload texture to device memory for fixture."; - return nullptr; - } - return texture; } + return texture; } std::shared_ptr Playground::CreateTextureForMapping(