From 8673d5e31d1d758fdde31d72e19a5d56835a02be Mon Sep 17 00:00:00 2001 From: Ojan Vafai Date: Fri, 12 Dec 2014 16:45:49 -0800 Subject: [PATCH] Make reftests work for sky. -Add a --testing flag to sky_viewer and cause it to paint into an SkBitmap instead of a ganesh surface so we can get the pixels out. -Add GetPixelsForTesting to layer.cc to actually grab out the pixels. -Add a reftest and a mismatch reftest. They need a setTimeout after the load event. Unclear why or what the right fix is. Maybe we should give internals some way to force the paint? If we don't have the setTimeout, we paint a white page (so we do a paint, but with no content). -Add a DisplayDelegate to Layer so that Viewer can decide whether to use the real ganesh backend or the SkBitmap one without littering the whole code-base with is_testing bools and logic. R=esprehn@chromium.org Review URL: https://codereview.chromium.org/797063002 --- engine/src/flutter/compositor/BUILD.gn | 6 +++ .../flutter/compositor/display_delegate.cc | 24 +++++++++++ .../src/flutter/compositor/display_delegate.h | 43 +++++++++++++++++++ .../compositor/display_delegate_bitmap.cc | 39 +++++++++++++++++ .../compositor/display_delegate_bitmap.h | 33 ++++++++++++++ .../compositor/display_delegate_ganesh.cc | 27 ++++++++++++ .../compositor/display_delegate_ganesh.h | 31 +++++++++++++ engine/src/flutter/compositor/layer.cc | 19 ++++---- engine/src/flutter/compositor/layer.h | 7 +-- engine/src/flutter/compositor/layer_host.cc | 4 ++ engine/src/flutter/compositor/layer_host.h | 2 + .../tests/harness/reftest-expected.sky | 12 ++++++ .../reftest-mismatch-expected-mismatch.sky | 12 ++++++ .../tests/harness/reftest-mismatch.sky | 12 ++++++ engine/src/flutter/tests/harness/reftest.sky | 12 ++++++ .../flutter/tools/tester/test_harness_impl.cc | 5 ++- .../flutter/tools/tester/test_harness_impl.h | 3 +- .../src/flutter/tools/tester/test_runner.cc | 19 ++++++-- engine/src/flutter/tools/tester/test_runner.h | 6 ++- engine/src/flutter/tools/tester/tester.cc | 40 ++++++++++++++--- 20 files changed, 331 insertions(+), 25 deletions(-) create mode 100644 engine/src/flutter/compositor/display_delegate.cc create mode 100644 engine/src/flutter/compositor/display_delegate.h create mode 100644 engine/src/flutter/compositor/display_delegate_bitmap.cc create mode 100644 engine/src/flutter/compositor/display_delegate_bitmap.h create mode 100644 engine/src/flutter/compositor/display_delegate_ganesh.cc create mode 100644 engine/src/flutter/compositor/display_delegate_ganesh.h create mode 100644 engine/src/flutter/tests/harness/reftest-expected.sky create mode 100644 engine/src/flutter/tests/harness/reftest-mismatch-expected-mismatch.sky create mode 100644 engine/src/flutter/tests/harness/reftest-mismatch.sky create mode 100644 engine/src/flutter/tests/harness/reftest.sky diff --git a/engine/src/flutter/compositor/BUILD.gn b/engine/src/flutter/compositor/BUILD.gn index 2e03d45c3a3..71a48883591 100644 --- a/engine/src/flutter/compositor/BUILD.gn +++ b/engine/src/flutter/compositor/BUILD.gn @@ -4,6 +4,12 @@ source_set("compositor") { sources = [ + "display_delegate.cc", + "display_delegate.h", + "display_delegate_bitmap.cc", + "display_delegate_bitmap.h", + "display_delegate_ganesh.cc", + "display_delegate_ganesh.h", "layer.cc", "layer.h", "layer_client.cc", diff --git a/engine/src/flutter/compositor/display_delegate.cc b/engine/src/flutter/compositor/display_delegate.cc new file mode 100644 index 00000000000..c868558424a --- /dev/null +++ b/engine/src/flutter/compositor/display_delegate.cc @@ -0,0 +1,24 @@ +// Copyright 2014 The Chromium 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 "sky/compositor/display_delegate.h" + +namespace sky { + +static CreateDisplayDelegate createDisplayDelegateFunction = 0; + +void DisplayDelegate::setDisplayDelegateCreateFunction(CreateDisplayDelegate createFunction) +{ + DCHECK(createFunction); + DCHECK(!createDisplayDelegateFunction); + createDisplayDelegateFunction = createFunction; +} + +DisplayDelegate* DisplayDelegate::create(LayerClient* client) +{ + DCHECK(createDisplayDelegateFunction); + return createDisplayDelegateFunction(client); +} + +} // namespace sky diff --git a/engine/src/flutter/compositor/display_delegate.h b/engine/src/flutter/compositor/display_delegate.h new file mode 100644 index 00000000000..800e4c75625 --- /dev/null +++ b/engine/src/flutter/compositor/display_delegate.h @@ -0,0 +1,43 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef SKY_COMPOSITOR_DISPLAY_DELEGATE_H_ +#define SKY_COMPOSITOR_DISPLAY_DELEGATE_H_ + +#include + +#include "mojo/skia/ganesh_surface.h" + +class SkCanvas; + +namespace gfx { +class Rect; +} + +namespace sky { + +class DisplayDelegate; +class LayerClient; + +typedef DisplayDelegate* (*CreateDisplayDelegate)(LayerClient*); + +class DisplayDelegate { + public: + DisplayDelegate() {} + virtual ~DisplayDelegate() {} + + static DisplayDelegate* create(LayerClient*); + static void setDisplayDelegateCreateFunction(CreateDisplayDelegate); + + virtual void GetPixelsForTesting(std::vector* pixels) = 0; + virtual void Paint(mojo::GaneshSurface& surface, const gfx::Rect& size) = 0; + + DISALLOW_COPY_AND_ASSIGN(DisplayDelegate); +}; + + + +} // namespace sky + +#endif // SKY_COMPOSITOR_DISPLAY_DELEGATE_H_ diff --git a/engine/src/flutter/compositor/display_delegate_bitmap.cc b/engine/src/flutter/compositor/display_delegate_bitmap.cc new file mode 100644 index 00000000000..571eda1f536 --- /dev/null +++ b/engine/src/flutter/compositor/display_delegate_bitmap.cc @@ -0,0 +1,39 @@ +// Copyright 2014 The Chromium 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 "sky/compositor/display_delegate_bitmap.h" + +#include "sky/compositor/layer_client.h" +#include "third_party/skia/include/core/SkBitmapDevice.h" +#include "third_party/skia/include/core/SkCanvas.h" +#include "ui/gfx/codec/png_codec.h" +#include "ui/gfx/geometry/rect.h" + +namespace sky { + +DisplayDelegateBitmap::DisplayDelegateBitmap(LayerClient* client) + : client_(client) {} + +DisplayDelegateBitmap::~DisplayDelegateBitmap() {} + +DisplayDelegate* DisplayDelegateBitmap::create(LayerClient* client) { + return new DisplayDelegateBitmap(client); +} + +void DisplayDelegateBitmap::GetPixelsForTesting(std::vector* pixels) { + gfx::PNGCodec::EncodeBGRASkBitmap(bitmap_, false, pixels); +} + +void DisplayDelegateBitmap::Paint(mojo::GaneshSurface& surface, const gfx::Rect& size) { + bitmap_.allocN32Pixels(size.width(), size.height()); + SkBitmapDevice device(bitmap_); + SkCanvas canvas(&device); + // Draw red so we can see when we fail to paint. + canvas.drawColor(SK_ColorRED); + + client_->PaintContents(&canvas, size); + canvas.flush(); +} + +} // namespace sky diff --git a/engine/src/flutter/compositor/display_delegate_bitmap.h b/engine/src/flutter/compositor/display_delegate_bitmap.h new file mode 100644 index 00000000000..4df1cf90a5a --- /dev/null +++ b/engine/src/flutter/compositor/display_delegate_bitmap.h @@ -0,0 +1,33 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef SKY_COMPOSITOR_DISPLAY_DELEGATE_BITMAP_H_ +#define SKY_COMPOSITOR_DISPLAY_DELEGATE_BITMAP_H_ + +#include "sky/compositor/display_delegate.h" + +namespace sky { + +class LayerClient; + +class DisplayDelegateBitmap final : public DisplayDelegate { + public: + explicit DisplayDelegateBitmap(LayerClient* client); + ~DisplayDelegateBitmap() override; + + static DisplayDelegate* create(LayerClient* client); + + void GetPixelsForTesting(std::vector* pixels) override; + void Paint(mojo::GaneshSurface& surface, const gfx::Rect& size) override; + + private: + SkBitmap bitmap_; + LayerClient* client_; + + DISALLOW_COPY_AND_ASSIGN(DisplayDelegateBitmap); +}; + +} // namespace sky + +#endif // SKY_COMPOSITOR_DISPLAY_DELEGATE_BITMAP_H_ diff --git a/engine/src/flutter/compositor/display_delegate_ganesh.cc b/engine/src/flutter/compositor/display_delegate_ganesh.cc new file mode 100644 index 00000000000..0d93c968b2e --- /dev/null +++ b/engine/src/flutter/compositor/display_delegate_ganesh.cc @@ -0,0 +1,27 @@ +// Copyright 2014 The Chromium 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 "sky/compositor/display_delegate_ganesh.h" + +#include "sky/compositor/layer_client.h" +#include "third_party/skia/include/core/SkCanvas.h" + +namespace sky { + +DisplayDelegate* DisplayDelegateGanesh::create(LayerClient* client) { + return new DisplayDelegateGanesh(client); +} + +void DisplayDelegateGanesh::GetPixelsForTesting(std::vector* pixels) { + // TODO(ojan): When we change notifyTestComplete to only GetPixelsForTesting + // in pixel/ref tests, add a NOTREACHED() here. +} + +void DisplayDelegateGanesh::Paint(mojo::GaneshSurface& surface, const gfx::Rect& size) { + SkCanvas* canvas = surface.canvas(); + client_->PaintContents(canvas, size); + canvas->flush(); +} + +} // namespace sky diff --git a/engine/src/flutter/compositor/display_delegate_ganesh.h b/engine/src/flutter/compositor/display_delegate_ganesh.h new file mode 100644 index 00000000000..1b2deb58e51 --- /dev/null +++ b/engine/src/flutter/compositor/display_delegate_ganesh.h @@ -0,0 +1,31 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef SKY_COMPOSITOR_DISPLAY_DELEGATE_GANESH_H_ +#define SKY_COMPOSITOR_DISPLAY_DELEGATE_GANESH_H_ + +#include "sky/compositor/display_delegate.h" + +namespace sky { + +class LayerClient; + +class DisplayDelegateGanesh final : public DisplayDelegate { + public: + explicit DisplayDelegateGanesh(LayerClient* client) : client_(client) {} + ~DisplayDelegateGanesh() override {} + + static DisplayDelegate* create(LayerClient* client); + + void GetPixelsForTesting(std::vector* pixels) override; + void Paint(mojo::GaneshSurface& surface, const gfx::Rect& size) override; + + private: + LayerClient* client_; + DISALLOW_COPY_AND_ASSIGN(DisplayDelegateGanesh); +}; + +} // namespace sky + +#endif // SKY_COMPOSITOR_DISPLAY_DELEGATE_GANESH_H_ diff --git a/engine/src/flutter/compositor/layer.cc b/engine/src/flutter/compositor/layer.cc index 8d2ec8bc811..1142093f0c1 100644 --- a/engine/src/flutter/compositor/layer.cc +++ b/engine/src/flutter/compositor/layer.cc @@ -6,25 +6,29 @@ #include "base/debug/trace_event.h" #include "mojo/skia/ganesh_surface.h" +#include "sky/compositor/display_delegate.h" #include "sky/compositor/layer_host.h" #include "third_party/skia/include/core/SkCanvas.h" namespace sky { -Layer::Layer(LayerClient* client) : client_(client), host_(nullptr) { +Layer::Layer(LayerClient* client) + : client_(client), + host_(nullptr) { + delegate_.reset(DisplayDelegate::create(client)); } Layer::~Layer() { } -void Layer::ClearClient() { - client_ = nullptr; -} - void Layer::SetSize(const gfx::Size& size) { size_ = size; } +void Layer::GetPixelsForTesting(std::vector* pixels) { + delegate_->GetPixelsForTesting(pixels); +} + void Layer::Display() { TRACE_EVENT0("sky", "Layer::Display"); @@ -33,11 +37,8 @@ void Layer::Display() { mojo::GaneshSurface surface(host_->ganesh_context(), host_->resource_manager()->CreateTexture(size_)); - SkCanvas* canvas = surface.canvas(); - gfx::Rect rect(size_); - client_->PaintContents(canvas, rect); - canvas->flush(); + delegate_->Paint(surface, rect); texture_ = surface.TakeTexture(); } diff --git a/engine/src/flutter/compositor/layer.h b/engine/src/flutter/compositor/layer.h index 94925278652..0f478bb9d12 100644 --- a/engine/src/flutter/compositor/layer.h +++ b/engine/src/flutter/compositor/layer.h @@ -11,15 +11,16 @@ #include "ui/gfx/geometry/rect.h" namespace sky { + +class DisplayDelegate; class LayerHost; class Layer : public base::RefCounted { public: explicit Layer(LayerClient* client); - void ClearClient(); - void SetSize(const gfx::Size& size); + void GetPixelsForTesting(std::vector* pixels); void Display(); scoped_ptr GetTexture(); @@ -35,8 +36,8 @@ class Layer : public base::RefCounted { LayerClient* client_; LayerHost* host_; gfx::Size size_; - scoped_ptr texture_; + scoped_ptr delegate_; DISALLOW_COPY_AND_ASSIGN(Layer); }; diff --git a/engine/src/flutter/compositor/layer_host.cc b/engine/src/flutter/compositor/layer_host.cc index 36b3ec26e5c..1739c78e41b 100644 --- a/engine/src/flutter/compositor/layer_host.cc +++ b/engine/src/flutter/compositor/layer_host.cc @@ -42,6 +42,10 @@ void LayerHost::SetRootLayer(scoped_refptr layer) { root_layer_->set_host(this); } +void LayerHost::GetPixelsForTesting(std::vector* pixels) { + return root_layer_->GetPixelsForTesting(pixels); +} + void LayerHost::OnSurfaceConnectionCreated() { DCHECK_EQ(state_, kWaitingForSurfaceService); state_ = kReadyForFrame; diff --git a/engine/src/flutter/compositor/layer_host.h b/engine/src/flutter/compositor/layer_host.h index 0b9616c7ce4..390bc843c1d 100644 --- a/engine/src/flutter/compositor/layer_host.h +++ b/engine/src/flutter/compositor/layer_host.h @@ -41,6 +41,8 @@ class LayerHost : public SurfaceHolder::Client { void SetNeedsAnimate(); void SetRootLayer(scoped_refptr layer); + void GetPixelsForTesting(std::vector* pixels); + private: enum State { kWaitingForSurfaceService, diff --git a/engine/src/flutter/tests/harness/reftest-expected.sky b/engine/src/flutter/tests/harness/reftest-expected.sky new file mode 100644 index 00000000000..7a7e586c04f --- /dev/null +++ b/engine/src/flutter/tests/harness/reftest-expected.sky @@ -0,0 +1,12 @@ + +This is a reftest. + + diff --git a/engine/src/flutter/tests/harness/reftest-mismatch-expected-mismatch.sky b/engine/src/flutter/tests/harness/reftest-mismatch-expected-mismatch.sky new file mode 100644 index 00000000000..25e46f88ac0 --- /dev/null +++ b/engine/src/flutter/tests/harness/reftest-mismatch-expected-mismatch.sky @@ -0,0 +1,12 @@ + +This is a mismatch reftest. + + diff --git a/engine/src/flutter/tests/harness/reftest-mismatch.sky b/engine/src/flutter/tests/harness/reftest-mismatch.sky new file mode 100644 index 00000000000..a016a24f693 --- /dev/null +++ b/engine/src/flutter/tests/harness/reftest-mismatch.sky @@ -0,0 +1,12 @@ + +This is a mismatch reftest. Should not match. + + diff --git a/engine/src/flutter/tests/harness/reftest.sky b/engine/src/flutter/tests/harness/reftest.sky new file mode 100644 index 00000000000..7a7e586c04f --- /dev/null +++ b/engine/src/flutter/tests/harness/reftest.sky @@ -0,0 +1,12 @@ + +This is a reftest. + + diff --git a/engine/src/flutter/tools/tester/test_harness_impl.cc b/engine/src/flutter/tools/tester/test_harness_impl.cc index 79a3f75f723..882aff34f21 100644 --- a/engine/src/flutter/tools/tester/test_harness_impl.cc +++ b/engine/src/flutter/tools/tester/test_harness_impl.cc @@ -20,9 +20,10 @@ TestHarnessImpl::TestHarnessImpl(TestRunner* test_runner) TestHarnessImpl::~TestHarnessImpl() { } -void TestHarnessImpl::OnTestComplete(const mojo::String& test_result) { +void TestHarnessImpl::OnTestComplete(const mojo::String& test_result, + const mojo::Array pixels) { if (test_runner_) - test_runner_->OnTestComplete(test_result); + test_runner_->OnTestComplete(test_result, pixels); } void TestHarnessImpl::DispatchInputEvent(mojo::EventPtr event) { diff --git a/engine/src/flutter/tools/tester/test_harness_impl.h b/engine/src/flutter/tools/tester/test_harness_impl.h index 01d0c758381..8a597035b86 100644 --- a/engine/src/flutter/tools/tester/test_harness_impl.h +++ b/engine/src/flutter/tools/tester/test_harness_impl.h @@ -21,7 +21,8 @@ class TestHarnessImpl : public mojo::InterfaceImpl { private: // TestHarness implementation. - void OnTestComplete(const mojo::String& test_result) override; + void OnTestComplete(const mojo::String& test_result, + const mojo::Array pixels) override; void DispatchInputEvent(mojo::EventPtr event) override; base::WeakPtr test_runner_; diff --git a/engine/src/flutter/tools/tester/test_runner.cc b/engine/src/flutter/tools/tester/test_runner.cc index 78125ee67fd..4f948be4c0b 100644 --- a/engine/src/flutter/tools/tester/test_runner.cc +++ b/engine/src/flutter/tools/tester/test_runner.cc @@ -17,10 +17,11 @@ TestRunnerClient::~TestRunnerClient() { } TestRunner::TestRunner(TestRunnerClient* client, mojo::View* container, - const std::string& url) + const std::string& url, bool enable_pixel_dumping) : test_harness_factory_(this), client_(client), - weak_ptr_factory_(this) { + weak_ptr_factory_(this), + enable_pixel_dumping_(enable_pixel_dumping) { CHECK(client); scoped_ptr exported_services( @@ -42,10 +43,22 @@ void TestRunner::OnTestStart() { std::cout.flush(); } -void TestRunner::OnTestComplete(const std::string& test_result) { +void TestRunner::OnTestComplete(const std::string& test_result, + const mojo::Array& pixels) { std::cout << "Content-Type: text/plain\n"; std::cout << test_result << "\n"; std::cout << "#EOF\n"; + + // TODO(ojan): Don't generate the pixels if enable_pixel_dumping_ is false. + if (enable_pixel_dumping_) { + // TODO(ojan): Add real hashes here once we want to do pixel tests. + std::cout << "\nActualHash: FAKEHASHSTUB\n"; + std::cout << "Content-Type: image/png\n"; + std::cout << "Content-Length: " << pixels.size() << "\n"; + std::cout.write( + reinterpret_cast(&pixels[0]), pixels.size()); + } + std::cout << "#EOF\n"; std::cout.flush(); std::cerr << "#EOF\n"; diff --git a/engine/src/flutter/tools/tester/test_runner.h b/engine/src/flutter/tools/tester/test_runner.h index 3c52f850621..7e2d45b9234 100644 --- a/engine/src/flutter/tools/tester/test_runner.h +++ b/engine/src/flutter/tools/tester/test_runner.h @@ -27,7 +27,7 @@ class TestRunnerClient { class TestRunner { public: TestRunner(TestRunnerClient* client, mojo::View* container, - const std::string& url); + const std::string& url, bool enable_pixel_dumping); virtual ~TestRunner(); TestRunnerClient* client() const { return client_; } @@ -35,12 +35,14 @@ class TestRunner { base::WeakPtr GetWeakPtr(); void OnTestStart(); - void OnTestComplete(const std::string& test_result); + void OnTestComplete(const std::string& test_result, + const mojo::Array& pixels); private: TestHarnessFactory test_harness_factory_; TestRunnerClient* client_; base::WeakPtrFactory weak_ptr_factory_; + bool enable_pixel_dumping_; MOJO_DISALLOW_COPY_AND_ASSIGN(TestRunner); }; diff --git a/engine/src/flutter/tools/tester/tester.cc b/engine/src/flutter/tools/tester/tester.cc index 19f0db83628..4ee889ee06f 100644 --- a/engine/src/flutter/tools/tester/tester.cc +++ b/engine/src/flutter/tools/tester/tester.cc @@ -23,10 +23,32 @@ namespace sky { namespace tester { namespace { -std::string WaitForURL() { +struct UrlData { std::string url; - std::cin >> url; - return url; + std::string expected_pixel_hash; + bool enable_pixel_dumping = false; +}; + +void WaitForURL(UrlData& data) { + // A test name is formated like file:///path/to/test'--pixel-test'pixelhash + std::cin >> data.url; + + std::string pixel_switch; + std::string::size_type separator_position = data.url.find('\''); + if (separator_position != std::string::npos) { + pixel_switch = data.url.substr(separator_position + 1); + data.url.erase(separator_position); + } + + std::string pixel_hash; + separator_position = pixel_switch.find('\''); + if (separator_position != std::string::npos) { + pixel_hash = pixel_switch.substr(separator_position + 1); + pixel_switch.erase(separator_position); + } + + data.enable_pixel_dumping = pixel_switch == "--pixel-test"; + data.expected_pixel_hash = pixel_hash; } } // namespace @@ -110,8 +132,16 @@ class SkyTester : public mojo::ApplicationDelegate, void Run() { DCHECK(!test_runner_); - std::string url = url_from_args_.length() ? url_from_args_ : WaitForURL(); - test_runner_.reset(new TestRunner(this, content_, url)); + + UrlData data; + if (url_from_args_.length()) { + data.url = url_from_args_; + } else { + WaitForURL(data); + } + + test_runner_.reset(new TestRunner(this, content_, data.url, + data.enable_pixel_dumping)); } void OnTestComplete() override {