Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ci/licenses_golden/excluded_files
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@
../../../flutter/impeller/docs
../../../flutter/impeller/entity/contents/checkerboard_contents_unittests.cc
../../../flutter/impeller/entity/contents/filters/inputs/filter_input_unittests.cc
../../../flutter/impeller/entity/entity_pass_target_unittests.cc
../../../flutter/impeller/entity/entity_unittests.cc
../../../flutter/impeller/entity/geometry/geometry_unittests.cc
../../../flutter/impeller/entity/render_target_cache_unittests.cc
Expand Down
1 change: 0 additions & 1 deletion impeller/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ impeller_component("impeller_unittests") {
"aiks:aiks_unittests",
"display_list:display_list_unittests",
"entity:entity_unittests",
"entity:render_target_cache_unittests",
"fixtures",
"geometry:geometry_unittests",
"image:image_unittests",
Expand Down
1 change: 0 additions & 1 deletion impeller/core/texture.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

#include <string_view>

#include "flutter/fml/macros.h"
#include "flutter/fml/mapping.h"
#include "impeller/core/formats.h"
#include "impeller/core/texture_descriptor.h"
Expand Down
13 changes: 2 additions & 11 deletions impeller/entity/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -270,10 +270,12 @@ impeller_component("entity_unittests") {
sources = [
"contents/checkerboard_contents_unittests.cc",
"contents/filters/inputs/filter_input_unittests.cc",
"entity_pass_target_unittests.cc",
"entity_playground.cc",
"entity_playground.h",
"entity_unittests.cc",
"geometry/geometry_unittests.cc",
"render_target_cache_unittests.cc",
]

deps = [
Expand All @@ -283,14 +285,3 @@ impeller_component("entity_unittests") {
"//flutter/impeller/typographer/backends/skia:typographer_skia_backend",
]
}

impeller_component("render_target_cache_unittests") {
testonly = true

sources = [ "render_target_cache_unittests.cc" ]

deps = [
":entity",
"//flutter/testing:testing_lib",
]
}
5 changes: 4 additions & 1 deletion impeller/entity/entity_pass.cc
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,10 @@ bool EntityPass::Render(ContentContext& renderer,
entity.SetContents(contents);
entity.SetBlendMode(BlendMode::kSource);

entity.Render(renderer, *render_pass);
if (!entity.Render(renderer, *render_pass)) {
VALIDATION_LOG << "Failed to render EntityPass root blit.";
return false;
}
}

if (!render_pass->EncodeCommands()) {
Expand Down
11 changes: 10 additions & 1 deletion impeller/entity/entity_pass_target.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,16 @@ std::shared_ptr<Texture> EntityPassTarget::Flip(Allocator& allocator) {
}
}

std::swap(color0.resolve_texture, secondary_color_texture_);
// If the color0 resolve texture is the same as the texture, then we're
// running on the GLES backend with implicit resolve.
if (color0.resolve_texture == color0.texture) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aside: I wish we could do a capabilities check instead, but this is better than nothing I suppose

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want me to I can pass that in? I'll just do that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to you. I can't imagine another scenario but at least we wouldn't accidentally capture a bug versus an intent. Thanks!

auto new_secondary = color0.resolve_texture;
color0.resolve_texture = secondary_color_texture_;
color0.texture = secondary_color_texture_;
secondary_color_texture_ = new_secondary;
} else {
std::swap(color0.resolve_texture, secondary_color_texture_);
}

target_.SetColorAttachment(color0, 0);

Expand Down
88 changes: 88 additions & 0 deletions impeller/entity/entity_pass_target_unittests.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include <memory>

#include "flutter/testing/testing.h"
#include "gtest/gtest.h"
#include "impeller/entity/entity_pass_target.h"
#include "impeller/entity/entity_playground.h"

namespace impeller {
namespace testing {

using EntityPassTargetTest = EntityPlayground;
INSTANTIATE_PLAYGROUND_SUITE(EntityPassTargetTest);

TEST_P(EntityPassTargetTest, SwapWithMSAATexture) {
if (GetContentContext()
->GetDeviceCapabilities()
.SupportsImplicitResolvingMSAA()) {
GTEST_SKIP() << "Implicit MSAA is used on this device.";
}
auto content_context = GetContentContext();
auto buffer = content_context->GetContext()->CreateCommandBuffer();
auto render_target = RenderTarget::CreateOffscreenMSAA(
*content_context->GetContext(),
*GetContentContext()->GetRenderTargetCache(), {100, 100});

auto entity_pass_target = EntityPassTarget(render_target, false);

auto color0 = entity_pass_target.GetRenderTarget()
.GetColorAttachments()
.find(0u)
->second;
auto msaa_tex = color0.texture;
auto resolve_tex = color0.resolve_texture;

entity_pass_target.Flip(
*content_context->GetContext()->GetResourceAllocator());

color0 = entity_pass_target.GetRenderTarget()
.GetColorAttachments()
.find(0u)
->second;

ASSERT_EQ(msaa_tex, color0.texture);
ASSERT_NE(resolve_tex, color0.resolve_texture);
}

TEST_P(EntityPassTargetTest, SwapWithMSAAImplicitResolve) {
if (!GetContentContext()
->GetDeviceCapabilities()
.SupportsImplicitResolvingMSAA()) {
GTEST_SKIP() << "Implicit MSAA is not used on this device.";
}
auto content_context = GetContentContext();
auto buffer = content_context->GetContext()->CreateCommandBuffer();
auto render_target = RenderTarget::CreateOffscreenMSAA(
*content_context->GetContext(),
*GetContentContext()->GetRenderTargetCache(), {100, 100});

auto entity_pass_target = EntityPassTarget(render_target, false);

auto color0 = entity_pass_target.GetRenderTarget()
.GetColorAttachments()
.find(0u)
->second;
auto msaa_tex = color0.texture;
auto resolve_tex = color0.resolve_texture;

ASSERT_EQ(msaa_tex, resolve_tex);

entity_pass_target.Flip(
*content_context->GetContext()->GetResourceAllocator());

color0 = entity_pass_target.GetRenderTarget()
.GetColorAttachments()
.find(0u)
->second;

ASSERT_EQ(msaa_tex, color0.texture);
ASSERT_NE(resolve_tex, color0.resolve_texture);
ASSERT_TRUE(false); // see if this runs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing if we have a CI config that has implicit MSAA

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not! I'll need to emulate this so it can be covered.

}

} // namespace testing
} // namespace impeller