From 7f6960711e98a523b0bd578d159e3a1efbfe3138 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 20 Mar 2023 12:38:49 -0700 Subject: [PATCH 1/5] [Impeller] change default sampler descriptor to use nearest mip level --- impeller/renderer/sampler_descriptor.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/renderer/sampler_descriptor.h b/impeller/renderer/sampler_descriptor.h index 01d0f293b9054..60678dac122bc 100644 --- a/impeller/renderer/sampler_descriptor.h +++ b/impeller/renderer/sampler_descriptor.h @@ -18,7 +18,7 @@ class Context; struct SamplerDescriptor final : public Comparable { MinMagFilter min_filter = MinMagFilter::kNearest; MinMagFilter mag_filter = MinMagFilter::kNearest; - MipFilter mip_filter = MipFilter::kNone; + MipFilter mip_filter = MipFilter::kNearest; SamplerAddressMode width_address_mode = SamplerAddressMode::kClampToEdge; SamplerAddressMode height_address_mode = SamplerAddressMode::kClampToEdge; From bc8afdfc4eb11c5ef3444fe346bd3d5e8bd4832d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 20 Mar 2023 13:55:01 -0700 Subject: [PATCH 2/5] remove none miplevel --- impeller/aiks/aiks_unittests.cc | 3 +-- impeller/entity/contents/text_contents.cc | 2 +- impeller/renderer/backend/gles/sampler_gles.cc | 9 +-------- impeller/renderer/backend/metal/formats_mtl.h | 2 -- impeller/renderer/backend/vulkan/formats_vk.h | 2 -- impeller/renderer/formats.h | 2 -- impeller/renderer/renderer_unittests.cc | 3 +-- impeller/scene/geometry.cc | 2 +- 8 files changed, 5 insertions(+), 20 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index aab4fbd0cfebd..50d09c9464da1 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -128,8 +128,7 @@ TEST_P(AiksTest, CanRenderTiledTexture) { Entity::TileMode::kClamp, Entity::TileMode::kRepeat, Entity::TileMode::kMirror, Entity::TileMode::kDecal}; const char* mip_filter_names[] = {"None", "Nearest", "Linear"}; - const MipFilter mip_filters[] = {MipFilter::kNone, MipFilter::kNearest, - MipFilter::kLinear}; + const MipFilter mip_filters[] = {MipFilter::kNearest, MipFilter::kLinear}; const char* min_mag_filter_names[] = {"Nearest", "Linear"}; const MinMagFilter min_mag_filters[] = {MinMagFilter::kNearest, MinMagFilter::kLinear}; diff --git a/impeller/entity/contents/text_contents.cc b/impeller/entity/contents/text_contents.cc index ed6b4efb8206a..17f1b14c4710e 100644 --- a/impeller/entity/contents/text_contents.cc +++ b/impeller/entity/contents/text_contents.cc @@ -114,7 +114,7 @@ static bool CommonRender( sampler_desc.min_filter = MinMagFilter::kLinear; sampler_desc.mag_filter = MinMagFilter::kLinear; } - sampler_desc.mip_filter = MipFilter::kNone; + sampler_desc.mip_filter = MipFilter::kNearest; typename FS::FragInfo frag_info; frag_info.text_color = ToVector(color.Premultiply()); diff --git a/impeller/renderer/backend/gles/sampler_gles.cc b/impeller/renderer/backend/gles/sampler_gles.cc index 72fea36726f95..4d7e666b01c31 100644 --- a/impeller/renderer/backend/gles/sampler_gles.cc +++ b/impeller/renderer/backend/gles/sampler_gles.cc @@ -21,13 +21,6 @@ bool SamplerGLES::IsValid() const { static GLint ToParam(MinMagFilter minmag_filter, MipFilter mip_filter) { switch (mip_filter) { - case MipFilter::kNone: - switch (minmag_filter) { - case MinMagFilter::kNearest: - return GL_NEAREST; - case MinMagFilter::kLinear: - return GL_LINEAR; - } case MipFilter::kNearest: switch (minmag_filter) { case MinMagFilter::kNearest: @@ -74,7 +67,7 @@ bool SamplerGLES::ConfigureBoundTexture(const TextureGLES& texture, gl.TexParameteri(target.value(), GL_TEXTURE_MIN_FILTER, ToParam(desc.min_filter, desc.mip_filter)); gl.TexParameteri(target.value(), GL_TEXTURE_MAG_FILTER, - ToParam(desc.mag_filter, MipFilter::kNone)); + ToParam(desc.mag_filter, MipFilter::kNearest)); gl.TexParameteri(target.value(), GL_TEXTURE_WRAP_S, ToAddressMode(desc.width_address_mode)); gl.TexParameteri(target.value(), GL_TEXTURE_WRAP_T, diff --git a/impeller/renderer/backend/metal/formats_mtl.h b/impeller/renderer/backend/metal/formats_mtl.h index 85a942364c0a3..533011717c5ab 100644 --- a/impeller/renderer/backend/metal/formats_mtl.h +++ b/impeller/renderer/backend/metal/formats_mtl.h @@ -332,8 +332,6 @@ constexpr MTLSamplerMinMagFilter ToMTLSamplerMinMagFilter(MinMagFilter filter) { constexpr MTLSamplerMipFilter ToMTLSamplerMipFilter(MipFilter filter) { switch (filter) { - case MipFilter::kNone: - return MTLSamplerMipFilterNotMipmapped; case MipFilter::kNearest: return MTLSamplerMipFilterNearest; case MipFilter::kLinear: diff --git a/impeller/renderer/backend/vulkan/formats_vk.h b/impeller/renderer/backend/vulkan/formats_vk.h index 83ea4acc463b0..d2f532fa0687b 100644 --- a/impeller/renderer/backend/vulkan/formats_vk.h +++ b/impeller/renderer/backend/vulkan/formats_vk.h @@ -225,8 +225,6 @@ constexpr vk::SamplerMipmapMode ToVKSamplerMipmapMode(MipFilter filter) { return vk::SamplerMipmapMode::eNearest; case MipFilter::kLinear: return vk::SamplerMipmapMode::eLinear; - case MipFilter::kNone: - return vk::SamplerMipmapMode::eNearest; } FML_UNREACHABLE(); diff --git a/impeller/renderer/formats.h b/impeller/renderer/formats.h index 170a068da607f..817b065a006bf 100644 --- a/impeller/renderer/formats.h +++ b/impeller/renderer/formats.h @@ -247,8 +247,6 @@ enum class MinMagFilter { }; enum class MipFilter { - /// Always sample from mip level 0. Other mip levels are ignored. - kNone, /// Sample from the nearest mip level. kNearest, /// Sample from the two nearest mip levels and linearly interpolate between diff --git a/impeller/renderer/renderer_unittests.cc b/impeller/renderer/renderer_unittests.cc index fa2d82356d91c..62175a96716ff 100644 --- a/impeller/renderer/renderer_unittests.cc +++ b/impeller/renderer/renderer_unittests.cc @@ -725,8 +725,7 @@ TEST_P(RendererTest, CanGenerateMipmaps) { bool first_frame = true; Renderer::RenderCallback callback = [&](RenderTarget& render_target) { const char* mip_filter_names[] = {"None", "Nearest", "Linear"}; - const MipFilter mip_filters[] = {MipFilter::kNone, MipFilter::kNearest, - MipFilter::kLinear}; + const MipFilter mip_filters[] = {MipFilter::kNearest, MipFilter::kLinear}; const char* min_filter_names[] = {"Nearest", "Linear"}; const MinMagFilter min_filters[] = {MinMagFilter::kNearest, MinMagFilter::kLinear}; diff --git a/impeller/scene/geometry.cc b/impeller/scene/geometry.cc index 7abc74f406f2f..ed6eda6ab64e8 100644 --- a/impeller/scene/geometry.cc +++ b/impeller/scene/geometry.cc @@ -246,7 +246,7 @@ void SkinnedVertexBufferGeometry::BindToCommand( SamplerDescriptor sampler_desc; sampler_desc.min_filter = MinMagFilter::kNearest; sampler_desc.mag_filter = MinMagFilter::kNearest; - sampler_desc.mip_filter = MipFilter::kNone; + sampler_desc.mip_filter = MipFilter::kNearest; sampler_desc.width_address_mode = SamplerAddressMode::kRepeat; sampler_desc.label = "NN Repeat"; From f8fd7119578939c651428313644554353ed60dd6 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 20 Mar 2023 15:37:09 -0700 Subject: [PATCH 3/5] use optional --- impeller/renderer/backend/gles/sampler_gles.cc | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/impeller/renderer/backend/gles/sampler_gles.cc b/impeller/renderer/backend/gles/sampler_gles.cc index 4d7e666b01c31..a1f726477581a 100644 --- a/impeller/renderer/backend/gles/sampler_gles.cc +++ b/impeller/renderer/backend/gles/sampler_gles.cc @@ -19,8 +19,18 @@ bool SamplerGLES::IsValid() const { return true; } -static GLint ToParam(MinMagFilter minmag_filter, MipFilter mip_filter) { - switch (mip_filter) { +static GLint ToParam(MinMagFilter minmag_filter, + std::optional mip_filter = std::nullopt) { + if (!mip_filter.has_value()) { + switch (minmag_filter) { + case MinMagFilter::kNearest: + return GL_NEAREST; + case MinMagFilter::kLinear: + return GL_LINEAR; + } + } + + switch (mip_filter.value()) { case MipFilter::kNearest: switch (minmag_filter) { case MinMagFilter::kNearest: @@ -67,7 +77,7 @@ bool SamplerGLES::ConfigureBoundTexture(const TextureGLES& texture, gl.TexParameteri(target.value(), GL_TEXTURE_MIN_FILTER, ToParam(desc.min_filter, desc.mip_filter)); gl.TexParameteri(target.value(), GL_TEXTURE_MAG_FILTER, - ToParam(desc.mag_filter, MipFilter::kNearest)); + 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, From 0d4554d3846414001558c4c1b2817fc89f5f104f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 20 Mar 2023 17:57:09 -0700 Subject: [PATCH 4/5] check mips --- impeller/renderer/backend/gles/sampler_gles.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/impeller/renderer/backend/gles/sampler_gles.cc b/impeller/renderer/backend/gles/sampler_gles.cc index a1f726477581a..7a6c481144589 100644 --- a/impeller/renderer/backend/gles/sampler_gles.cc +++ b/impeller/renderer/backend/gles/sampler_gles.cc @@ -72,10 +72,12 @@ bool SamplerGLES::ConfigureBoundTexture(const TextureGLES& texture, if (!target.has_value()) { return false; } + bool has_mips = texture.GetTextureDescriptor().mip_count > 1; const auto& desc = GetDescriptor(); - gl.TexParameteri(target.value(), GL_TEXTURE_MIN_FILTER, - ToParam(desc.min_filter, desc.mip_filter)); + gl.TexParameteri( + target.value(), GL_TEXTURE_MIN_FILTER, + ToParam(desc.min_filter, has_mips ? desc.mip_filter : std::nullopt)); gl.TexParameteri(target.value(), GL_TEXTURE_MAG_FILTER, ToParam(desc.mag_filter)); gl.TexParameteri(target.value(), GL_TEXTURE_WRAP_S, From 7402ddace0c5b8349bf526e3671de25f7acbd1e4 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 20 Mar 2023 19:16:34 -0700 Subject: [PATCH 5/5] Fix renderer unittests --- impeller/renderer/backend/gles/sampler_gles.cc | 15 ++++++++++----- impeller/renderer/renderer_unittests.cc | 4 ++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/impeller/renderer/backend/gles/sampler_gles.cc b/impeller/renderer/backend/gles/sampler_gles.cc index 7a6c481144589..47f2ce56ddddb 100644 --- a/impeller/renderer/backend/gles/sampler_gles.cc +++ b/impeller/renderer/backend/gles/sampler_gles.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "impeller/renderer/backend/gles/sampler_gles.h" +#include #include "impeller/renderer/backend/gles/formats_gles.h" #include "impeller/renderer/backend/gles/proc_table_gles.h" @@ -28,6 +29,7 @@ static GLint ToParam(MinMagFilter minmag_filter, case MinMagFilter::kLinear: return GL_LINEAR; } + FML_UNREACHABLE(); } switch (mip_filter.value()) { @@ -72,12 +74,15 @@ bool SamplerGLES::ConfigureBoundTexture(const TextureGLES& texture, if (!target.has_value()) { return false; } - bool has_mips = texture.GetTextureDescriptor().mip_count > 1; - const auto& desc = GetDescriptor(); - gl.TexParameteri( - target.value(), GL_TEXTURE_MIN_FILTER, - ToParam(desc.min_filter, has_mips ? desc.mip_filter : std::nullopt)); + + std::optional mip_filter = std::nullopt; + if (texture.GetTextureDescriptor().mip_count > 1) { + mip_filter = desc.mip_filter; + } + + gl.TexParameteri(target.value(), GL_TEXTURE_MIN_FILTER, + 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, diff --git a/impeller/renderer/renderer_unittests.cc b/impeller/renderer/renderer_unittests.cc index 62175a96716ff..d8a44de6c7d62 100644 --- a/impeller/renderer/renderer_unittests.cc +++ b/impeller/renderer/renderer_unittests.cc @@ -724,14 +724,14 @@ TEST_P(RendererTest, CanGenerateMipmaps) { bool first_frame = true; Renderer::RenderCallback callback = [&](RenderTarget& render_target) { - const char* mip_filter_names[] = {"None", "Nearest", "Linear"}; + const char* mip_filter_names[] = {"Nearest", "Linear"}; const MipFilter mip_filters[] = {MipFilter::kNearest, MipFilter::kLinear}; const char* min_filter_names[] = {"Nearest", "Linear"}; const MinMagFilter min_filters[] = {MinMagFilter::kNearest, MinMagFilter::kLinear}; // UI state. - static int selected_mip_filter = 2; + static int selected_mip_filter = 1; static int selected_min_filter = 0; static float lod = 4.5;