From f9eeee310c4dfb199628f310cded07ec3d976ab5 Mon Sep 17 00:00:00 2001 From: zljj0818 Date: Thu, 18 Jun 2020 02:14:05 +0800 Subject: [PATCH] Poor video scaling quality #53080 (#18814) Use bilinear instead of nearest filter to draw surface texture Related Issues: https://github.com/flutter/flutter/issues/53080 Tests: - TextureLayerTest --- flow/layers/texture_layer.cc | 11 ++++-- flow/layers/texture_layer.h | 5 ++- flow/layers/texture_layer_unittests.cc | 36 ++++++++++++++++--- flow/testing/mock_texture.cc | 11 +++--- flow/testing/mock_texture.h | 4 ++- flow/testing/mock_texture_unittests.cc | 26 +++++++++++--- flow/texture.h | 3 +- lib/ui/compositing.dart | 17 ++++----- lib/ui/compositing/scene_builder.cc | 6 ++-- lib/ui/compositing/scene_builder.h | 3 +- .../compositor/layer_scene_builder.dart | 1 + .../lib/src/engine/surface/scene_builder.dart | 5 +-- lib/web_ui/lib/src/ui/compositing.dart | 1 + shell/common/shell_unittests.cc | 3 +- .../android/android_external_texture_gl.cc | 7 ++-- .../android/android_external_texture_gl.h | 3 +- .../darwin/ios/ios_external_texture_gl.h | 6 +++- .../darwin/ios/ios_external_texture_gl.mm | 7 ++-- .../darwin/ios/ios_external_texture_metal.h | 6 +++- .../darwin/ios/ios_external_texture_metal.mm | 7 ++-- .../embedder/embedder_external_texture_gl.cc | 9 +++-- .../embedder/embedder_external_texture_gl.h | 3 +- 22 files changed, 134 insertions(+), 46 deletions(-) diff --git a/flow/layers/texture_layer.cc b/flow/layers/texture_layer.cc index 00d976eeed7..b81224deaa8 100644 --- a/flow/layers/texture_layer.cc +++ b/flow/layers/texture_layer.cc @@ -11,8 +11,13 @@ namespace flutter { TextureLayer::TextureLayer(const SkPoint& offset, const SkSize& size, int64_t texture_id, - bool freeze) - : offset_(offset), size_(size), texture_id_(texture_id), freeze_(freeze) {} + bool freeze, + SkFilterQuality filter_quality) + : offset_(offset), + size_(size), + texture_id_(texture_id), + freeze_(freeze), + filter_quality_(filter_quality) {} void TextureLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) { TRACE_EVENT0("flutter", "TextureLayer::Preroll"); @@ -35,7 +40,7 @@ void TextureLayer::Paint(PaintContext& context) const { return; } texture->Paint(*context.leaf_nodes_canvas, paint_bounds(), freeze_, - context.gr_context); + context.gr_context, filter_quality_); } } // namespace flutter diff --git a/flow/layers/texture_layer.h b/flow/layers/texture_layer.h index 20f6c709d61..250cefd10ee 100644 --- a/flow/layers/texture_layer.h +++ b/flow/layers/texture_layer.h @@ -6,6 +6,7 @@ #define FLUTTER_FLOW_LAYERS_TEXTURE_LAYER_H_ #include "flutter/flow/layers/layer.h" +#include "third_party/skia/include/core/SkFilterQuality.h" #include "third_party/skia/include/core/SkPoint.h" #include "third_party/skia/include/core/SkSize.h" @@ -16,7 +17,8 @@ class TextureLayer : public Layer { TextureLayer(const SkPoint& offset, const SkSize& size, int64_t texture_id, - bool freeze); + bool freeze, + SkFilterQuality filter_quality); void Preroll(PrerollContext* context, const SkMatrix& matrix) override; void Paint(PaintContext& context) const override; @@ -26,6 +28,7 @@ class TextureLayer : public Layer { SkSize size_; int64_t texture_id_; bool freeze_; + SkFilterQuality filter_quality_; FML_DISALLOW_COPY_AND_ASSIGN(TextureLayer); }; diff --git a/flow/layers/texture_layer_unittests.cc b/flow/layers/texture_layer_unittests.cc index 9adf273e611..525239a8e0f 100644 --- a/flow/layers/texture_layer_unittests.cc +++ b/flow/layers/texture_layer_unittests.cc @@ -18,8 +18,8 @@ using TextureLayerTest = LayerTest; TEST_F(TextureLayerTest, InvalidTexture) { const SkPoint layer_offset = SkPoint::Make(0.0f, 0.0f); const SkSize layer_size = SkSize::Make(8.0f, 8.0f); - auto layer = - std::make_shared(layer_offset, layer_size, 0, false); + auto layer = std::make_shared(layer_offset, layer_size, 0, + false, kNone_SkFilterQuality); layer->Preroll(preroll_context(), SkMatrix()); EXPECT_EQ(layer->paint_bounds(), @@ -36,8 +36,8 @@ TEST_F(TextureLayerTest, PaintingEmptyLayerDies) { const SkSize layer_size = SkSize::Make(0.0f, 0.0f); const int64_t texture_id = 0; auto mock_texture = std::make_shared(texture_id); - auto layer = std::make_shared(layer_offset, layer_size, - texture_id, false); + auto layer = std::make_shared( + layer_offset, layer_size, texture_id, false, kNone_SkFilterQuality); // Ensure the texture is located by the Layer. preroll_context()->texture_registry.RegisterTexture(mock_texture); @@ -49,7 +49,33 @@ TEST_F(TextureLayerTest, PaintingEmptyLayerDies) { layer->Paint(paint_context()); EXPECT_EQ(mock_texture->paint_calls(), std::vector({MockTexture::PaintCall{ - mock_canvas(), layer->paint_bounds(), false, nullptr}})); + mock_canvas(), layer->paint_bounds(), false, nullptr, + kNone_SkFilterQuality}})); + EXPECT_EQ(mock_canvas().draw_calls(), std::vector()); +} + +TEST_F(TextureLayerTest, PaintingWithLowFilterQuality) { + const SkPoint layer_offset = SkPoint::Make(0.0f, 0.0f); + const SkSize layer_size = SkSize::Make(8.0f, 8.0f); + const int64_t texture_id = 0; + auto mock_texture = std::make_shared(texture_id); + auto layer = std::make_shared( + layer_offset, layer_size, texture_id, false, kLow_SkFilterQuality); + + // Ensure the texture is located by the Layer. + preroll_context()->texture_registry.RegisterTexture(mock_texture); + + layer->Preroll(preroll_context(), SkMatrix()); + EXPECT_EQ(layer->paint_bounds(), + (SkRect::MakeSize(layer_size) + .makeOffset(layer_offset.fX, layer_offset.fY))); + EXPECT_TRUE(layer->needs_painting()); + + layer->Paint(paint_context()); + EXPECT_EQ(mock_texture->paint_calls(), + std::vector({MockTexture::PaintCall{ + mock_canvas(), layer->paint_bounds(), false, nullptr, + kLow_SkFilterQuality}})); EXPECT_EQ(mock_canvas().draw_calls(), std::vector()); } diff --git a/flow/testing/mock_texture.cc b/flow/testing/mock_texture.cc index 26e49b764cd..ee032355d2f 100644 --- a/flow/testing/mock_texture.cc +++ b/flow/testing/mock_texture.cc @@ -12,19 +12,22 @@ MockTexture::MockTexture(int64_t textureId) : Texture(textureId) {} void MockTexture::Paint(SkCanvas& canvas, const SkRect& bounds, bool freeze, - GrContext* context) { - paint_calls_.emplace_back(PaintCall{canvas, bounds, freeze, context}); + GrContext* context, + SkFilterQuality filter_quality) { + paint_calls_.emplace_back( + PaintCall{canvas, bounds, freeze, context, filter_quality}); } bool operator==(const MockTexture::PaintCall& a, const MockTexture::PaintCall& b) { return &a.canvas == &b.canvas && a.bounds == b.bounds && - a.context == b.context && a.freeze == b.freeze; + a.context == b.context && a.freeze == b.freeze && + a.filter_quality == b.filter_quality; } std::ostream& operator<<(std::ostream& os, const MockTexture::PaintCall& data) { return os << &data.canvas << " " << data.bounds << " " << data.context << " " - << data.freeze; + << data.freeze << " " << data.filter_quality; } } // namespace testing diff --git a/flow/testing/mock_texture.h b/flow/testing/mock_texture.h index 5de0346f216..d03907525fe 100644 --- a/flow/testing/mock_texture.h +++ b/flow/testing/mock_texture.h @@ -21,6 +21,7 @@ class MockTexture : public Texture { SkRect bounds; bool freeze; GrContext* context; + SkFilterQuality filter_quality; }; explicit MockTexture(int64_t textureId); @@ -29,7 +30,8 @@ class MockTexture : public Texture { void Paint(SkCanvas& canvas, const SkRect& bounds, bool freeze, - GrContext* context) override; + GrContext* context, + SkFilterQuality filter_quality) override; void OnGrContextCreated() override { gr_context_created_ = true; } void OnGrContextDestroyed() override { gr_context_destroyed_ = true; } diff --git a/flow/testing/mock_texture_unittests.cc b/flow/testing/mock_texture_unittests.cc index 107eb763079..9b53892aafe 100644 --- a/flow/testing/mock_texture_unittests.cc +++ b/flow/testing/mock_texture_unittests.cc @@ -30,12 +30,30 @@ TEST(MockTextureTest, PaintCalls) { const SkRect paint_bounds1 = SkRect::MakeWH(1.0f, 1.0f); const SkRect paint_bounds2 = SkRect::MakeWH(2.0f, 2.0f); const auto expected_paint_calls = - std::vector{MockTexture::PaintCall{canvas, paint_bounds1, false, nullptr}, - MockTexture::PaintCall{canvas, paint_bounds2, true, nullptr}}; + std::vector{MockTexture::PaintCall{canvas, paint_bounds1, false, nullptr, + kNone_SkFilterQuality}, + MockTexture::PaintCall{canvas, paint_bounds2, true, nullptr, + kNone_SkFilterQuality}}; auto texture = std::make_shared(0); - texture->Paint(canvas, paint_bounds1, false, nullptr); - texture->Paint(canvas, paint_bounds2, true, nullptr); + texture->Paint(canvas, paint_bounds1, false, nullptr, kNone_SkFilterQuality); + texture->Paint(canvas, paint_bounds2, true, nullptr, kNone_SkFilterQuality); + EXPECT_EQ(texture->paint_calls(), expected_paint_calls); +} + +TEST(MockTextureTest, PaintCallsWithLowFilterQuality) { + SkCanvas canvas; + const SkRect paint_bounds1 = SkRect::MakeWH(1.0f, 1.0f); + const SkRect paint_bounds2 = SkRect::MakeWH(2.0f, 2.0f); + const auto expected_paint_calls = + std::vector{MockTexture::PaintCall{canvas, paint_bounds1, false, nullptr, + kLow_SkFilterQuality}, + MockTexture::PaintCall{canvas, paint_bounds2, true, nullptr, + kLow_SkFilterQuality}}; + auto texture = std::make_shared(0); + + texture->Paint(canvas, paint_bounds1, false, nullptr, kLow_SkFilterQuality); + texture->Paint(canvas, paint_bounds2, true, nullptr, kLow_SkFilterQuality); EXPECT_EQ(texture->paint_calls(), expected_paint_calls); } diff --git a/flow/texture.h b/flow/texture.h index ad473479871..ef963bf70f7 100644 --- a/flow/texture.h +++ b/flow/texture.h @@ -22,7 +22,8 @@ class Texture { virtual void Paint(SkCanvas& canvas, const SkRect& bounds, bool freeze, - GrContext* context) = 0; + GrContext* context, + SkFilterQuality quality) = 0; // Called from raster thread. virtual void OnGrContextCreated() = 0; diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index 63efe4920ad..ea3c0b71446 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -699,18 +699,19 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// texture just before resizing the Android view and un-freezes it when it is /// certain that a frame with the new size is ready. void addTexture( - int textureId, { - Offset offset = Offset.zero, - double width = 0.0, - double height = 0.0, - bool freeze = false, + int/*!*/ textureId, { + Offset/*!*/ offset = Offset.zero, + double/*!*/ width = 0.0, + double/*!*/ height = 0.0, + bool/*!*/ freeze = false, + FilterQuality/*!*/ filterQuality = FilterQuality.low, }) { assert(offset != null, 'Offset argument was null'); // ignore: unnecessary_null_comparison - _addTexture(offset.dx, offset.dy, width, height, textureId, freeze); + _addTexture(offset.dx, offset.dy, width, height, textureId, freeze, filterQuality.index); } - void _addTexture(double dx, double dy, double width, double height, int textureId, bool freeze) - native 'SceneBuilder_addTexture'; + void _addTexture(double dx, double dy, double width, double height, int textureId, bool freeze, + int filterQuality) native 'SceneBuilder_addTexture'; /// Adds a platform view (e.g an iOS UIView) to the scene. /// diff --git a/lib/ui/compositing/scene_builder.cc b/lib/ui/compositing/scene_builder.cc index 4ae901428f4..46645b456d7 100644 --- a/lib/ui/compositing/scene_builder.cc +++ b/lib/ui/compositing/scene_builder.cc @@ -229,9 +229,11 @@ void SceneBuilder::addTexture(double dx, double width, double height, int64_t textureId, - bool freeze) { + bool freeze, + int filterQuality) { auto layer = std::make_unique( - SkPoint::Make(dx, dy), SkSize::Make(width, height), textureId, freeze); + SkPoint::Make(dx, dy), SkSize::Make(width, height), textureId, freeze, + static_cast(filterQuality)); AddLayer(std::move(layer)); } diff --git a/lib/ui/compositing/scene_builder.h b/lib/ui/compositing/scene_builder.h index 11c04951226..b89afadd8fc 100644 --- a/lib/ui/compositing/scene_builder.h +++ b/lib/ui/compositing/scene_builder.h @@ -92,7 +92,8 @@ class SceneBuilder : public RefCountedDartWrappable { double width, double height, int64_t textureId, - bool freeze); + bool freeze, + int filterQuality); void addPlatformView(double dx, double dy, diff --git a/lib/web_ui/lib/src/engine/compositor/layer_scene_builder.dart b/lib/web_ui/lib/src/engine/compositor/layer_scene_builder.dart index b2302e92c00..64c0ac8802e 100644 --- a/lib/web_ui/lib/src/engine/compositor/layer_scene_builder.dart +++ b/lib/web_ui/lib/src/engine/compositor/layer_scene_builder.dart @@ -67,6 +67,7 @@ class LayerSceneBuilder implements ui.SceneBuilder { double width = 0.0, double height = 0.0, bool freeze = false, + ui.FilterQuality filterQuality = ui.FilterQuality.low, }) { // TODO(b/128315641): implement addTexture. } diff --git a/lib/web_ui/lib/src/engine/surface/scene_builder.dart b/lib/web_ui/lib/src/engine/surface/scene_builder.dart index 12a9e7cdc21..4a6ebaced0f 100644 --- a/lib/web_ui/lib/src/engine/surface/scene_builder.dart +++ b/lib/web_ui/lib/src/engine/surface/scene_builder.dart @@ -377,13 +377,14 @@ class SurfaceSceneBuilder implements ui.SceneBuilder { double width = 0.0, double height = 0.0, bool freeze = false, + ui.FilterQuality filterQuality = ui.FilterQuality.low, }) { assert(offset != null, 'Offset argument was null'); - _addTexture(offset.dx, offset.dy, width, height, textureId); + _addTexture(offset.dx, offset.dy, width, height, textureId, filterQuality.index); } void _addTexture( - double dx, double dy, double width, double height, int textureId) { + double dx, double dy, double width, double height, int textureId, int filterQuality) { // In test mode, allow this to be a no-op. if (!ui.debugEmulateFlutterTesterEnvironment) { throw UnimplementedError('Textures are not supported in Flutter Web'); diff --git a/lib/web_ui/lib/src/ui/compositing.dart b/lib/web_ui/lib/src/ui/compositing.dart index df903d52995..635dd726158 100644 --- a/lib/web_ui/lib/src/ui/compositing.dart +++ b/lib/web_ui/lib/src/ui/compositing.dart @@ -327,6 +327,7 @@ abstract class SceneBuilder { double width = 0.0, double height = 0.0, bool freeze = false, + FilterQuality filterQuality = FilterQuality.low, }); /// Adds a platform view (e.g an iOS UIView) to the scene. diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 83a3dd5df95..083393a4f62 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -937,7 +937,8 @@ class MockTexture : public Texture { void Paint(SkCanvas& canvas, const SkRect& bounds, bool freeze, - GrContext* context) override {} + GrContext* context, + SkFilterQuality filter_quality) override {} void OnGrContextCreated() override {} diff --git a/shell/platform/android/android_external_texture_gl.cc b/shell/platform/android/android_external_texture_gl.cc index 01822b2329b..00befbad243 100644 --- a/shell/platform/android/android_external_texture_gl.cc +++ b/shell/platform/android/android_external_texture_gl.cc @@ -36,7 +36,8 @@ void AndroidExternalTextureGL::MarkNewFrameAvailable() { void AndroidExternalTextureGL::Paint(SkCanvas& canvas, const SkRect& bounds, bool freeze, - GrContext* context) { + GrContext* context, + SkFilterQuality filter_quality) { if (state_ == AttachmentState::detached) { return; } @@ -67,7 +68,9 @@ void AndroidExternalTextureGL::Paint(SkCanvas& canvas, transformAroundCenter.postTranslate(0.5, 0.5); canvas.concat(transformAroundCenter); } - canvas.drawImage(image, 0, 0); + SkPaint paint; + paint.setFilterQuality(filter_quality); + canvas.drawImage(image, 0, 0, &paint); } } diff --git a/shell/platform/android/android_external_texture_gl.h b/shell/platform/android/android_external_texture_gl.h index c8268be8a46..72b9cf64ded 100644 --- a/shell/platform/android/android_external_texture_gl.h +++ b/shell/platform/android/android_external_texture_gl.h @@ -24,7 +24,8 @@ class AndroidExternalTextureGL : public flutter::Texture { void Paint(SkCanvas& canvas, const SkRect& bounds, bool freeze, - GrContext* context) override; + GrContext* context, + SkFilterQuality filter_quality) override; void OnGrContextCreated() override; diff --git a/shell/platform/darwin/ios/ios_external_texture_gl.h b/shell/platform/darwin/ios/ios_external_texture_gl.h index 46033ecb7ac..e8f8fa3763d 100644 --- a/shell/platform/darwin/ios/ios_external_texture_gl.h +++ b/shell/platform/darwin/ios/ios_external_texture_gl.h @@ -27,7 +27,11 @@ class IOSExternalTextureGL final : public Texture { fml::CFRef buffer_ref_; // |Texture| - void Paint(SkCanvas& canvas, const SkRect& bounds, bool freeze, GrContext* context) override; + void Paint(SkCanvas& canvas, + const SkRect& bounds, + bool freeze, + GrContext* context, + SkFilterQuality filter_quality) override; // |Texture| void OnGrContextCreated() override; diff --git a/shell/platform/darwin/ios/ios_external_texture_gl.mm b/shell/platform/darwin/ios/ios_external_texture_gl.mm index 697992d1b7c..25b3b2be5e1 100644 --- a/shell/platform/darwin/ios/ios_external_texture_gl.mm +++ b/shell/platform/darwin/ios/ios_external_texture_gl.mm @@ -63,7 +63,8 @@ bool IOSExternalTextureGL::NeedUpdateTexture(bool freeze) { void IOSExternalTextureGL::Paint(SkCanvas& canvas, const SkRect& bounds, bool freeze, - GrContext* context) { + GrContext* context, + SkFilterQuality filter_quality) { EnsureTextureCacheExists(); if (NeedUpdateTexture(freeze)) { auto pixelBuffer = [external_texture_.get() copyPixelBuffer]; @@ -84,7 +85,9 @@ void IOSExternalTextureGL::Paint(SkCanvas& canvas, kRGBA_8888_SkColorType, kPremul_SkAlphaType, nullptr); FML_DCHECK(image) << "Failed to create SkImage from Texture."; if (image) { - canvas.drawImage(image, bounds.x(), bounds.y()); + SkPaint paint; + paint.setFilterQuality(filter_quality); + canvas.drawImage(image, bounds.x(), bounds.y(), &paint); } } diff --git a/shell/platform/darwin/ios/ios_external_texture_metal.h b/shell/platform/darwin/ios/ios_external_texture_metal.h index 9bcce844d39..a3f916151fb 100644 --- a/shell/platform/darwin/ios/ios_external_texture_metal.h +++ b/shell/platform/darwin/ios/ios_external_texture_metal.h @@ -35,7 +35,11 @@ class IOSExternalTextureMetal final : public Texture { sk_sp external_image_; // |Texture| - void Paint(SkCanvas& canvas, const SkRect& bounds, bool freeze, GrContext* context) override; + void Paint(SkCanvas& canvas, + const SkRect& bounds, + bool freeze, + GrContext* context, + SkFilterQuality filter_quality) override; // |Texture| void OnGrContextCreated() override; diff --git a/shell/platform/darwin/ios/ios_external_texture_metal.mm b/shell/platform/darwin/ios/ios_external_texture_metal.mm index a5ea79fb99b..b5aefca8178 100644 --- a/shell/platform/darwin/ios/ios_external_texture_metal.mm +++ b/shell/platform/darwin/ios/ios_external_texture_metal.mm @@ -26,7 +26,8 @@ IOSExternalTextureMetal::~IOSExternalTextureMetal() = default; void IOSExternalTextureMetal::Paint(SkCanvas& canvas, const SkRect& bounds, bool freeze, - GrContext* context) { + GrContext* context, + SkFilterQuality filter_quality) { const bool needs_updated_texture = (!freeze && texture_frame_available_) || !external_image_; if (needs_updated_texture) { @@ -45,10 +46,12 @@ void IOSExternalTextureMetal::Paint(SkCanvas& canvas, } if (external_image_) { + SkPaint paint; + paint.setFilterQuality(filter_quality); canvas.drawImageRect(external_image_, // image external_image_->bounds(), // source rect bounds, // destination rect - nullptr, // paint + &paint, // paint SkCanvas::SrcRectConstraint::kFast_SrcRectConstraint // constraint ); } diff --git a/shell/platform/embedder/embedder_external_texture_gl.cc b/shell/platform/embedder/embedder_external_texture_gl.cc index 83b26be729a..47d25721d16 100644 --- a/shell/platform/embedder/embedder_external_texture_gl.cc +++ b/shell/platform/embedder/embedder_external_texture_gl.cc @@ -21,7 +21,8 @@ EmbedderExternalTextureGL::~EmbedderExternalTextureGL() = default; void EmbedderExternalTextureGL::Paint(SkCanvas& canvas, const SkRect& bounds, bool freeze, - GrContext* context) { + GrContext* context, + SkFilterQuality filter_quality) { if (auto image = external_texture_callback_( Id(), // canvas.getGrContext(), // @@ -31,10 +32,12 @@ void EmbedderExternalTextureGL::Paint(SkCanvas& canvas, } if (last_image_) { + SkPaint paint; + paint.setFilterQuality(filter_quality); if (bounds != SkRect::Make(last_image_->bounds())) { - canvas.drawImageRect(last_image_, bounds, nullptr); + canvas.drawImageRect(last_image_, bounds, &paint); } else { - canvas.drawImage(last_image_, bounds.x(), bounds.y()); + canvas.drawImage(last_image_, bounds.x(), bounds.y(), &paint); } } } diff --git a/shell/platform/embedder/embedder_external_texture_gl.h b/shell/platform/embedder/embedder_external_texture_gl.h index df2927e0f6e..30bb63238dc 100644 --- a/shell/platform/embedder/embedder_external_texture_gl.h +++ b/shell/platform/embedder/embedder_external_texture_gl.h @@ -30,7 +30,8 @@ class EmbedderExternalTextureGL : public flutter::Texture { void Paint(SkCanvas& canvas, const SkRect& bounds, bool freeze, - GrContext* context) override; + GrContext* context, + SkFilterQuality filter_quality) override; // |flutter::Texture| void OnGrContextCreated() override;