Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] Optionally support SamplerAddressMode::kDecal on the OpenGLES backend #46650

Merged
auto-submit merged 7 commits into from
Oct 9, 2023
Merged

[Impeller] Optionally support SamplerAddressMode::kDecal on the OpenGLES backend #46650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
dd5b80a
[Impeller] Optionally support SamplerAddressMode::kDecal on the OpenG…
ColdPaleLight Oct 7, 2023
e5fd823
Clean code
ColdPaleLight Oct 8, 2023
5367092
Fix issue that essl can't use uint
ColdPaleLight Oct 8, 2023
1852799
Fix compile error
ColdPaleLight Oct 8, 2023
ea78ff1
Update malioc.json
ColdPaleLight Oct 8, 2023
990d7d2
Add 'ifdef' back
ColdPaleLight Oct 9, 2023
1de6e16
Update malioc.json
ColdPaleLight Oct 9, 2023
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
2 changes: 2 additions & 0 deletions impeller/entity/contents/atlas_contents.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,8 @@ bool AtlasContents::Render(const ContentContext& renderer,
dst_sampler_descriptor.width_address_mode = SamplerAddressMode::kDecal;
dst_sampler_descriptor.height_address_mode = SamplerAddressMode::kDecal;
}
frag_info.supports_decal_sampler_address_mode =
renderer.GetDeviceCapabilities().SupportsDecalSamplerAddressMode();
auto dst_sampler = renderer.GetContext()->GetSamplerLibrary()->GetSampler(
dst_sampler_descriptor);
FS::BindTextureSamplerDst(cmd, texture_, dst_sampler);
Expand Down
6 changes: 6 additions & 0 deletions impeller/entity/contents/filters/blend_filter_contents.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,8 @@ static std::optional<Entity> AdvancedBlend(
dst_sampler_descriptor.width_address_mode = SamplerAddressMode::kDecal;
dst_sampler_descriptor.height_address_mode = SamplerAddressMode::kDecal;
}
blend_info.supports_decal_sampler_address_mode =
renderer.GetDeviceCapabilities().SupportsDecalSamplerAddressMode();
auto dst_sampler = renderer.GetContext()->GetSamplerLibrary()->GetSampler(
dst_sampler_descriptor);
FS::BindTextureSamplerDst(cmd, dst_snapshot->texture, dst_sampler);
Expand Down Expand Up @@ -355,6 +357,8 @@ std::optional<Entity> BlendFilterContents::CreateForegroundAdvancedBlend(
dst_sampler_descriptor.width_address_mode = SamplerAddressMode::kDecal;
dst_sampler_descriptor.height_address_mode = SamplerAddressMode::kDecal;
}
blend_info.supports_decal_sampler_address_mode =
renderer.GetDeviceCapabilities().SupportsDecalSamplerAddressMode();
auto dst_sampler = renderer.GetContext()->GetSamplerLibrary()->GetSampler(
dst_sampler_descriptor);
FS::BindTextureSamplerDst(cmd, dst_snapshot->texture, dst_sampler);
Expand Down Expand Up @@ -479,6 +483,8 @@ std::optional<Entity> BlendFilterContents::CreateForegroundPorterDuffBlend(
dst_sampler_descriptor.width_address_mode = SamplerAddressMode::kDecal;
dst_sampler_descriptor.height_address_mode = SamplerAddressMode::kDecal;
}
frag_info.supports_decal_sampler_address_mode =
renderer.GetDeviceCapabilities().SupportsDecalSamplerAddressMode();
auto dst_sampler = renderer.GetContext()->GetSamplerLibrary()->GetSampler(
dst_sampler_descriptor);
FS::BindTextureSamplerDst(cmd, dst_snapshot->texture, dst_sampler);
Expand Down
2 changes: 2 additions & 0 deletions impeller/entity/contents/filters/morphology_filter_contents.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ std::optional<Entity> DirectionalMorphologyFilterContents::RenderFilter(
sampler_descriptor.width_address_mode = SamplerAddressMode::kDecal;
sampler_descriptor.height_address_mode = SamplerAddressMode::kDecal;
}
frag_info.supports_decal_sampler_address_mode =
renderer.GetDeviceCapabilities().SupportsDecalSamplerAddressMode();

FS::BindTextureSampler(
cmd, input_snapshot->texture,
Expand Down
8 changes: 6 additions & 2 deletions impeller/entity/shaders/blending/advanced_blend.glsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ uniform BlendInfo {
float16_t src_input_alpha;
float16_t color_factor;
f16vec4 color; // This color input is expected to be unpremultiplied.
float supports_decal_sampler_address_mode;
}
blend_info;

Expand All @@ -24,9 +25,12 @@ in vec2 v_src_texture_coords;
out f16vec4 frag_color;

f16vec4 Sample(f16sampler2D texture_sampler, vec2 texture_coords) {
// gles 2.0 is the only backend without native decal support.
#ifdef IMPELLER_TARGET_OPENGLES
return IPSampleDecal(texture_sampler, texture_coords);
if (blend_info.supports_decal_sampler_address_mode > 0.0) {
Copy link
Contributor

@jonahwilliams jonahwilliams Oct 9, 2023

Choose a reason for hiding this comment

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

These can still technically be ifdefs since vulkan/metal always support this.

Copy link
Member Author

@ColdPaleLight ColdPaleLight Oct 9, 2023

Choose a reason for hiding this comment

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

Got it, added them back.

return texture(texture_sampler, texture_coords);
} else {
return IPHalfSampleDecal(texture_sampler, texture_coords);
}
#else
return texture(texture_sampler, texture_coords);
#endif
Expand Down
8 changes: 6 additions & 2 deletions impeller/entity/shaders/blending/porter_duff_blend.frag
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ uniform FragInfo {
float16_t dst_coeff_src_color;
float16_t input_alpha;
float16_t output_alpha;
float supports_decal_sampler_address_mode;
}
frag_info;

Expand All @@ -28,9 +29,12 @@ in f16vec4 v_color;
out f16vec4 frag_color;

f16vec4 Sample(f16sampler2D texture_sampler, vec2 texture_coords) {
// gles 2.0 is the only backend without native decal support.
#ifdef IMPELLER_TARGET_OPENGLES
return IPSampleDecal(texture_sampler, texture_coords);
if (frag_info.supports_decal_sampler_address_mode > 0.0) {
return texture(texture_sampler, texture_coords);
} else {
return IPHalfSampleDecal(texture_sampler, texture_coords);
}
#else
return texture(texture_sampler, texture_coords);
#endif
Expand Down
11 changes: 8 additions & 3 deletions impeller/entity/shaders/morphology_filter.frag
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ uniform FragInfo {
f16vec2 uv_offset;
float16_t radius;
float16_t morph_type;
float supports_decal_sampler_address_mode;
}
frag_info;

Expand All @@ -32,11 +33,15 @@ void main() {
for (float16_t i = -frag_info.radius; i <= frag_info.radius; i++) {
vec2 texture_coords = v_texture_coords + frag_info.uv_offset * i;

// gles 2.0 is the only backend without native decal support.
f16vec4 color;
#ifdef IMPELLER_TARGET_OPENGLES
f16vec4 color = IPHalfSampleDecal(texture_sampler, texture_coords);
if (frag_info.supports_decal_sampler_address_mode > 0.0) {
color = texture(texture_sampler, texture_coords);
} else {
color = IPHalfSampleDecal(texture_sampler, texture_coords);
}
#else
f16vec4 color = texture(texture_sampler, texture_coords);
color = texture(texture_sampler, texture_coords);
#endif

if (frag_info.morph_type == kMorphTypeDilate) {
Expand Down
8 changes: 7 additions & 1 deletion impeller/renderer/backend/gles/capabilities_gles.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,12 @@ CapabilitiesGLES::CapabilitiesGLES(const ProcTableGLES& gl) {
gl.GetIntegerv(GL_NUM_SHADER_BINARY_FORMATS, &value);
num_shader_binary_formats = value;
}

if (gl.GetDescription()->HasExtension("GL_EXT_texture_border_clamp") ||
gl.GetDescription()->HasExtension("GL_NV_texture_border_clamp") ||
gl.GetDescription()->HasExtension("GL_OES_texture_border_clamp")) {
supports_decal_sampler_address_mode_ = true;
}
}

size_t CapabilitiesGLES::GetMaxTextureUnits(ShaderStage stage) const {
Expand Down Expand Up @@ -140,7 +146,7 @@ bool CapabilitiesGLES::SupportsReadFromResolve() const {
}

bool CapabilitiesGLES::SupportsDecalSamplerAddressMode() const {
return false;
return supports_decal_sampler_address_mode_;
}

bool CapabilitiesGLES::SupportsDeviceTransientTextures() const {
Expand Down
3 changes: 3 additions & 0 deletions impeller/renderer/backend/gles/capabilities_gles.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,9 @@ class CapabilitiesGLES final

// |Capabilities|
PixelFormat GetDefaultDepthStencilFormat() const override;

private:
bool supports_decal_sampler_address_mode_ = false;
};

} // namespace impeller
6 changes: 4 additions & 2 deletions impeller/renderer/backend/gles/context_gles.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,15 @@ ContextGLES::ContextGLES(std::unique_ptr<ProcTableGLES> gl,
}
}

device_capabilities_ = reactor_->GetProcTable().GetCapabilities();

// Create the sampler library.
{
sampler_library_ =
std::shared_ptr<SamplerLibraryGLES>(new SamplerLibraryGLES());
std::shared_ptr<SamplerLibraryGLES>(new SamplerLibraryGLES(
device_capabilities_->SupportsDecalSamplerAddressMode()));
}

device_capabilities_ = reactor_->GetProcTable().GetCapabilities();
is_valid_ = true;
}

Expand Down
1 change: 1 addition & 0 deletions impeller/renderer/backend/gles/gles.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

// IWYU pragma: begin_exports
#include "GLES3/gl3.h"
#define GL_CLAMP_TO_BORDER 0x812D
#define GL_GLEXT_PROTOTYPES
#include "GLES2/gl2ext.h"
// IWYU pragma: end_exports
20 changes: 14 additions & 6 deletions impeller/renderer/backend/gles/sampler_gles.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ static GLint ToParam(MinMagFilter minmag_filter,
FML_UNREACHABLE();
}

static GLint ToAddressMode(SamplerAddressMode mode) {
static GLint ToAddressMode(SamplerAddressMode mode,
Copy link
Contributor

@jonahwilliams jonahwilliams Oct 7, 2023

Choose a reason for hiding this comment

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

nit: I might just pass in a bool for whether or not decal is supported.

Copy link
Contributor

@chinmaygarde chinmaygarde Oct 7, 2023

Choose a reason for hiding this comment

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

Just a drive by comment; Is it just me or does anyone else find "decal" mode to be a non-standard and/or overly confusing term? How about just ClampToBorderColor? Thats what the various APIs call it (GL_CLAMP_TO_BORDER_EXT, MTLSamplerAddressModeClampToBorderColor, VK_SAMPLER_ADDRESS_MODE_CLAMP_TO_BORDER).

Perhaps later, we can rename this enum value for consistency.

Copy link
Contributor

@jonahwilliams jonahwilliams Oct 7, 2023

Choose a reason for hiding this comment

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

There is a cost for non-consistency with the underlying graphics API, but there is also a value in matching the framework level enumeration which uses decal as a terminology. I don't feel as strongly about decal as I do about "FilterQuality", which is entirely opaque - at least decal has some semantic meaning

Copy link
Member Author

@ColdPaleLight ColdPaleLight Oct 8, 2023

Choose a reason for hiding this comment

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

nit: I might just pass in a bool for whether or not decal is supported.

Done

bool supports_decal_sampler_address_mode) {
switch (mode) {
case SamplerAddressMode::kClampToEdge:
return GL_CLAMP_TO_EDGE;
Expand All @@ -62,7 +63,10 @@ static GLint ToAddressMode(SamplerAddressMode mode) {
case SamplerAddressMode::kMirror:
return GL_MIRRORED_REPEAT;
case SamplerAddressMode::kDecal:
break; // Unsupported.
if (supports_decal_sampler_address_mode) {
return GL_CLAMP_TO_BORDER;
}
break;
}
FML_UNREACHABLE();
}
Expand Down Expand Up @@ -96,10 +100,14 @@ bool SamplerGLES::ConfigureBoundTexture(const TextureGLES& texture,
ToParam(desc.min_filter, mip_filter));
gl.TexParameteri(target.value(), GL_TEXTURE_MAG_FILTER,
ToParam(desc.mag_filter));
gl.TexParameteri(target.value(), GL_TEXTURE_WRAP_S,
ToAddressMode(desc.width_address_mode));
gl.TexParameteri(target.value(), GL_TEXTURE_WRAP_T,
ToAddressMode(desc.height_address_mode));
gl.TexParameteri(
Copy link
Contributor

@chinmaygarde chinmaygarde Oct 7, 2023

Choose a reason for hiding this comment

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

How do we set the border color? TEXTURE_BORDER_COLOR_EXT

Copy link
Contributor

@jonahwilliams jonahwilliams Oct 7, 2023

Choose a reason for hiding this comment

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

Maybe this defaults to transparent black so we don't need to worry about it?

Copy link
Contributor

@jonahwilliams jonahwilliams Oct 7, 2023

Choose a reason for hiding this comment

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

according to https://registry.khronos.org/OpenGL-Refpages/es3/html/glTexParameter.xhtml this value defaults to transparent black so we don't need to set anything.

Copy link
Contributor

@chinmaygarde chinmaygarde Oct 7, 2023

Choose a reason for hiding this comment

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

I guess. I took a look at Vulkan and setting the border color is an extension. But setting it to VK_SAMPLER_ADDRESS_MODE_CLAMP_TO_BORDER is not. I guess this is fine. I don't think we set the color on Metal either. Just really odd.

Copy link
Contributor

@jonahwilliams jonahwilliams Oct 7, 2023

Choose a reason for hiding this comment

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

Flutter does not expose an API for setting the border color.

Copy link
Contributor

@chinmaygarde chinmaygarde Oct 7, 2023

Choose a reason for hiding this comment

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

Nevermind. Its an extension that is core in 1.1. And we do set it to transparent black. Let's be explicit and do the same here as well please?

Copy link
Contributor

@chinmaygarde chinmaygarde Oct 7, 2023

Choose a reason for hiding this comment

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

Its is also dealers choice on Metal at the moment. Let's be explicit and set borderColor here too. In fact, a default border color specification in formats.h would be good for consistency.

Copy link
Contributor

@chinmaygarde chinmaygarde Oct 7, 2023

Choose a reason for hiding this comment

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

Flutter does not expose an API for setting the border color.

Yeah, just didn't want to read the spec to see if the default was transparent black. Just being explicit seemed better. There isn't any bug though.

Copy link
Member Author

@ColdPaleLight ColdPaleLight Oct 8, 2023

Choose a reason for hiding this comment

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

Issue filed: flutter/flutter#136153

target.value(), GL_TEXTURE_WRAP_S,
ToAddressMode(desc.width_address_mode,
gl.GetCapabilities()->SupportsDecalSamplerAddressMode()));
gl.TexParameteri(
target.value(), GL_TEXTURE_WRAP_T,
ToAddressMode(desc.height_address_mode,
gl.GetCapabilities()->SupportsDecalSamplerAddressMode()));
return true;
}

Expand Down
16 changes: 8 additions & 8 deletions impeller/renderer/backend/gles/sampler_library_gles.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,22 @@

namespace impeller {

SamplerLibraryGLES::SamplerLibraryGLES() = default;
SamplerLibraryGLES::SamplerLibraryGLES(bool supports_decal_sampler_address_mode)
: supports_decal_sampler_address_mode_(
supports_decal_sampler_address_mode) {}

// |SamplerLibrary|
SamplerLibraryGLES::~SamplerLibraryGLES() = default;

// |SamplerLibrary|
std::shared_ptr<const Sampler> SamplerLibraryGLES::GetSampler(
SamplerDescriptor descriptor) {
// TODO(bdero): Change this validation once optional support for kDecal is
// added to the OpenGLES backend:
// https://github.com/flutter/flutter/issues/129358
if (descriptor.width_address_mode == SamplerAddressMode::kDecal ||
descriptor.height_address_mode == SamplerAddressMode::kDecal ||
descriptor.depth_address_mode == SamplerAddressMode::kDecal) {
if (!supports_decal_sampler_address_mode_ &&
(descriptor.width_address_mode == SamplerAddressMode::kDecal ||
descriptor.height_address_mode == SamplerAddressMode::kDecal ||
descriptor.depth_address_mode == SamplerAddressMode::kDecal)) {
VALIDATION_LOG << "SamplerAddressMode::kDecal is not supported by the "
"OpenGLES backend.";
"current OpenGLES backend.";
return nullptr;
}

Expand Down
3 changes: 3 additions & 0 deletions impeller/renderer/backend/gles/sampler_library_gles.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ namespace impeller {

class SamplerLibraryGLES final : public SamplerLibrary {
public:
explicit SamplerLibraryGLES(bool supports_decal_sampler_address_mode);
// |SamplerLibrary|
~SamplerLibraryGLES() override;

Expand All @@ -26,6 +27,8 @@ class SamplerLibraryGLES final : public SamplerLibrary {
std::shared_ptr<const Sampler> GetSampler(
SamplerDescriptor descriptor) override;

bool supports_decal_sampler_address_mode_ = false;

FML_DISALLOW_COPY_AND_ASSIGN(SamplerLibraryGLES);
};

Expand Down
10 changes: 10 additions & 0 deletions impeller/renderer/backend/gles/test/capabilities_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,15 @@ TEST(CapabilitiesGLES, CanInitializeWithDefaults) {
PixelFormat::kD24UnormS8Uint);
}

TEST(CapabilitiesGLES, SupportsDecalSamplerAddressMode) {
auto const extensions = std::vector<const unsigned char*>{
reinterpret_cast<const unsigned char*>("GL_KHR_debug"), //
reinterpret_cast<const unsigned char*>("GL_EXT_texture_border_clamp"), //
};
auto mock_gles = MockGLES::Init(extensions);
auto capabilities = mock_gles->GetProcTable().GetCapabilities();
EXPECT_TRUE(capabilities->SupportsDecalSamplerAddressMode());
}

} // namespace testing
} // namespace impeller
12 changes: 8 additions & 4 deletions impeller/renderer/backend/gles/test/mock_gles.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ static std::mutex g_test_lock;

static std::weak_ptr<MockGLES> g_mock_gles;

static std::vector<const unsigned char*> g_extensions;

// Has friend visibility into MockGLES to record calls.
void RecordGLCall(const char* name) {
if (auto mock_gles = g_mock_gles.lock()) {
Expand All @@ -37,7 +39,7 @@ void doNothing() {}

auto const kMockVendor = (unsigned char*)"MockGLES";
auto const kMockVersion = (unsigned char*)"3.0";
auto const kExtensions = std::vector<unsigned char*>{
auto const kExtensions = std::vector<const unsigned char*>{
(unsigned char*)"GL_KHR_debug" //
};

Expand All @@ -60,7 +62,7 @@ static_assert(CheckSameSignature<decltype(mockGetString), //
const unsigned char* mockGetStringi(GLenum name, GLuint index) {
switch (name) {
case GL_EXTENSIONS:
return kExtensions[index];
return g_extensions[index];
default:
return (unsigned char*)"";
}
Expand All @@ -72,7 +74,7 @@ static_assert(CheckSameSignature<decltype(mockGetStringi), //
void mockGetIntegerv(GLenum name, int* value) {
switch (name) {
case GL_NUM_EXTENSIONS: {
*value = kExtensions.size();
*value = g_extensions.size();
} break;
case GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS:
*value = 8;
Expand Down Expand Up @@ -110,10 +112,12 @@ void mockPushDebugGroupKHR(GLenum source,
static_assert(CheckSameSignature<decltype(mockPushDebugGroupKHR), //
decltype(glPushDebugGroupKHR)>::value);

std::shared_ptr<MockGLES> MockGLES::Init() {
std::shared_ptr<MockGLES> MockGLES::Init(
const std::optional<std::vector<const unsigned char*>>& extensions) {
// If we cannot obtain a lock, MockGLES is already being used elsewhere.
FML_CHECK(g_test_lock.try_lock())
<< "MockGLES is already being used by another test.";
g_extensions = extensions.value_or(kExtensions);
auto mock_gles = std::shared_ptr<MockGLES>(new MockGLES());
g_mock_gles = mock_gles;
return mock_gles;
Expand Down
5 changes: 4 additions & 1 deletion impeller/renderer/backend/gles/test/mock_gles.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// found in the LICENSE file.

#include <memory>
#include <optional>
#include "fml/macros.h"
#include "impeller/renderer/backend/gles/proc_table_gles.h"

Expand All @@ -24,7 +25,9 @@ class MockGLES final {
/// This method overwrites mocked global GLES function pointers to record
/// invocations on this instance of |MockGLES|. As such, it should only be
/// called once per test.
static std::shared_ptr<MockGLES> Init();
static std::shared_ptr<MockGLES> Init(
const std::optional<std::vector<const unsigned char*>>& extensions =
std::nullopt);

/// @brief Returns a configured |ProcTableGLES| instance.
const ProcTableGLES& GetProcTable() const { return proc_table_; }
Expand Down
Loading