From 2ff84ffe5fb7223b9ed78a6d8cfef754578342bb Mon Sep 17 00:00:00 2001 From: Collin Jackson Date: Mon, 1 Jun 2015 16:21:21 -0700 Subject: [PATCH] Refactor image handling in Sky to expose the loader and image as separate classes to Dart code. This makes it possible to avoid unnecessary paints, by only painting once when the image has loaded. Now that we've separated the loader and image classes, we can implement an image cache in Dart. R=abarth@chromium.org, abarth Review URL: https://codereview.chromium.org/1156003007 --- engine/core/core.gni | 7 ++- engine/core/loader/CanvasImageLoader.cpp | 54 ++++++++++++++++++++++ engine/core/loader/CanvasImageLoader.h | 53 +++++++++++++++++++++ engine/core/loader/ImageLoader.idl | 10 ++++ engine/core/loader/ImageLoaderCallback.h | 20 ++++++++ engine/core/loader/ImageLoaderCallback.idl | 7 +++ engine/core/loader/NewImageLoader.cpp | 53 --------------------- engine/core/loader/NewImageLoader.h | 49 -------------------- engine/core/painting/CanvasImage.cpp | 17 +------ engine/core/painting/CanvasImage.h | 12 +---- engine/core/painting/Image.idl | 1 - examples/raw/spinning_image.dart | 17 +++++-- 12 files changed, 163 insertions(+), 137 deletions(-) create mode 100644 engine/core/loader/CanvasImageLoader.cpp create mode 100644 engine/core/loader/CanvasImageLoader.h create mode 100644 engine/core/loader/ImageLoader.idl create mode 100644 engine/core/loader/ImageLoaderCallback.h create mode 100644 engine/core/loader/ImageLoaderCallback.idl delete mode 100644 engine/core/loader/NewImageLoader.cpp delete mode 100644 engine/core/loader/NewImageLoader.h diff --git a/engine/core/core.gni b/engine/core/core.gni index 536494015d2..43ee00e2438 100644 --- a/engine/core/core.gni +++ b/engine/core/core.gni @@ -798,6 +798,8 @@ sky_core_files = [ "inspector/ScriptAsyncCallStack.h", "inspector/ScriptGCEventListener.h", "layout/LayoutCallback.h", + "loader/CanvasImageLoader.cpp", + "loader/CanvasImageLoader.h", "loader/DocumentLoadTiming.cpp", "loader/DocumentLoadTiming.h", "loader/EmptyClients.cpp", @@ -810,11 +812,10 @@ sky_core_files = [ "loader/FrameLoaderTypes.h", "loader/ImageLoader.cpp", "loader/ImageLoader.h", + "loader/ImageLoaderCallback.h", "loader/MojoLoader.cpp", "loader/MojoLoader.h", "loader/NavigationPolicy.h", - "loader/NewImageLoader.cpp", - "loader/NewImageLoader.h", "loader/UniqueIdentifier.cpp", "loader/UniqueIdentifier.h", "page/ChromeClient.h", @@ -1127,6 +1128,8 @@ core_idl_files = get_path_info([ "html/TextMetrics.idl", "html/VoidCallback.idl", "layout/LayoutCallback.idl", + "loader/ImageLoader.idl", + "loader/ImageLoaderCallback.idl", "painting/Canvas.idl", "painting/ColorFilter.idl", "painting/DrawLooper.idl", diff --git a/engine/core/loader/CanvasImageLoader.cpp b/engine/core/loader/CanvasImageLoader.cpp new file mode 100644 index 00000000000..2879130d55f --- /dev/null +++ b/engine/core/loader/CanvasImageLoader.cpp @@ -0,0 +1,54 @@ +// Copyright 2015 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/engine/config.h" +#include "sky/engine/core/loader/CanvasImageLoader.h" +#include "sky/engine/core/painting/CanvasImage.h" +#include "sky/engine/platform/image-decoders/ImageDecoder.h" +#include "sky/engine/platform/SharedBuffer.h" + +namespace blink { + +CanvasImageLoader::CanvasImageLoader(const String& src, PassOwnPtr callback) + : callback_(callback) { + // TODO(jackson): Figure out how to determine the proper base URL here + url_ = KURL(KURL(), src); +} + +CanvasImageLoader::~CanvasImageLoader() { +} + +void CanvasImageLoader::load() { + fetcher_ = adoptPtr(new MojoFetcher(this, url_)); +} + +void CanvasImageLoader::OnReceivedResponse(mojo::URLResponsePtr response) { + if (response->status_code != 200) { + callback_->handleEvent(nullptr); + return; + } + buffer_ = SharedBuffer::create(); + drainer_ = + adoptPtr(new mojo::common::DataPipeDrainer(this, response->body.Pass())); +} + +void CanvasImageLoader::OnDataAvailable(const void* data, size_t num_bytes) { + buffer_->append(static_cast(data), num_bytes); +} + +void CanvasImageLoader::OnDataComplete() { + OwnPtr decoder = + ImageDecoder::create(*buffer_.get(), ImageSource::AlphaPremultiplied, + ImageSource::GammaAndColorProfileIgnored); + decoder->setData(buffer_.get(), true); + if (!decoder->failed() && decoder->frameCount() > 0) { + RefPtr resultImage = CanvasImage::create(); + resultImage->setBitmap(decoder->frameBufferAtIndex(0)->getSkBitmap()); + callback_->handleEvent(resultImage.get()); + } else { + callback_->handleEvent(nullptr); + } +} + +} // namespace blink \ No newline at end of file diff --git a/engine/core/loader/CanvasImageLoader.h b/engine/core/loader/CanvasImageLoader.h new file mode 100644 index 00000000000..63db7656574 --- /dev/null +++ b/engine/core/loader/CanvasImageLoader.h @@ -0,0 +1,53 @@ +// SKY_ENGINE_CORE_PAINTING_CANVASIMAGE_H_ +// Copyright 2015 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_ENGINE_CORE_LOADER_CANVASIMAGELOADER_H_ +#define SKY_ENGINE_CORE_LOADER_CANVASIMAGELOADER_H_ + +#include "mojo/common/data_pipe_drainer.h" +#include "sky/engine/core/loader/ImageLoaderCallback.h" +#include "sky/engine/platform/SharedBuffer.h" +#include "sky/engine/platform/fetcher/MojoFetcher.h" +#include "sky/engine/tonic/dart_wrappable.h" +#include "sky/engine/wtf/OwnPtr.h" +#include "sky/engine/wtf/text/AtomicString.h" + +namespace blink { + +class CanvasImageLoader : public MojoFetcher::Client, + public mojo::common::DataPipeDrainer::Client, + public RefCounted, + public DartWrappable { + DEFINE_WRAPPERTYPEINFO(); + public: + static PassRefPtr create(const String& src, PassOwnPtr callback) + { + return adoptRef(new CanvasImageLoader(src, callback)); + } + virtual ~CanvasImageLoader(); + + void load(); + + // MojoFetcher::Client + void OnReceivedResponse(mojo::URLResponsePtr) override; + + // mojo::common::DataPipeDrainer::Client + void OnDataAvailable(const void*, size_t) override; + void OnDataComplete() override; + + private: + explicit CanvasImageLoader(const String& src, PassOwnPtr callback); + + OwnPtr fetcher_; + OwnPtr drainer_; + RefPtr buffer_; + + KURL url_; + OwnPtr callback_; +}; + +} // namespace blink + +#endif // SKY_ENGINE_CORE_LOADER_CANVASIMAGELOADER_H_ diff --git a/engine/core/loader/ImageLoader.idl b/engine/core/loader/ImageLoader.idl new file mode 100644 index 00000000000..c4f7723958a --- /dev/null +++ b/engine/core/loader/ImageLoader.idl @@ -0,0 +1,10 @@ +// Copyright 2013 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. + +[ + Constructor(DOMString src, ImageLoaderCallback callback), + ImplementedAs=CanvasImageLoader, +] interface ImageLoader { + void load(); +}; diff --git a/engine/core/loader/ImageLoaderCallback.h b/engine/core/loader/ImageLoaderCallback.h new file mode 100644 index 00000000000..036c2061023 --- /dev/null +++ b/engine/core/loader/ImageLoaderCallback.h @@ -0,0 +1,20 @@ +// Copyright 2015 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_ENGINE_CORE_LOADER_IMAGELOADERCALLBACK_H_ +#define SKY_ENGINE_CORE_LOADER_IMAGELOADERCALLBACK_H_ + +#include "sky/engine/core/painting/CanvasImage.h" + +namespace blink { + +class ImageLoaderCallback { +public: + virtual ~ImageLoaderCallback() {} + virtual void handleEvent(CanvasImage* result) = 0; +}; + +} // namespace blink + +#endif // SKY_ENGINE_CORE_LOADER_IMAGELOADERCALLBACK_H_ diff --git a/engine/core/loader/ImageLoaderCallback.idl b/engine/core/loader/ImageLoaderCallback.idl new file mode 100644 index 00000000000..a8463add7f1 --- /dev/null +++ b/engine/core/loader/ImageLoaderCallback.idl @@ -0,0 +1,7 @@ +// Copyright 2015 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. + +callback interface ImageLoaderCallback { + void handleEvent(Image result); +}; diff --git a/engine/core/loader/NewImageLoader.cpp b/engine/core/loader/NewImageLoader.cpp deleted file mode 100644 index 7906c8dd9fa..00000000000 --- a/engine/core/loader/NewImageLoader.cpp +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright 2015 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/engine/config.h" -#include "sky/engine/core/loader/NewImageLoader.h" -#include "sky/engine/platform/image-decoders/ImageDecoder.h" -#include "sky/engine/platform/SharedBuffer.h" - -namespace blink { - -NewImageLoader::NewImageLoader(NewImageLoaderClient* client) : client_(client) { -} - -NewImageLoader::~NewImageLoader() { -} - -void NewImageLoader::Load(const KURL& src) { - fetcher_ = adoptPtr(new MojoFetcher(this, src)); -} - -void NewImageLoader::OnReceivedResponse(mojo::URLResponsePtr response) { - if (response->status_code != 200) { - client_->OnLoadFinished(SkBitmap()); - return; - } - buffer_ = SharedBuffer::create(); - drainer_ = - adoptPtr(new mojo::common::DataPipeDrainer(this, response->body.Pass())); -} - -void NewImageLoader::OnDataAvailable(const void* data, size_t num_bytes) { - buffer_->append(static_cast(data), num_bytes); -} - -void NewImageLoader::OnDataComplete() { - SkBitmap bitmap; - OwnPtr decoder = - ImageDecoder::create(*buffer_.get(), ImageSource::AlphaPremultiplied, - ImageSource::GammaAndColorProfileIgnored); - decoder->setData(buffer_.get(), true); - - if (decoder->failed()) { - client_->OnLoadFinished(bitmap); - } else { - if (decoder->frameCount() > 0) { - bitmap = decoder->frameBufferAtIndex(0)->getSkBitmap(); - } - client_->OnLoadFinished(bitmap); - } -} - -} // namespace blink \ No newline at end of file diff --git a/engine/core/loader/NewImageLoader.h b/engine/core/loader/NewImageLoader.h deleted file mode 100644 index d2bacca69ba..00000000000 --- a/engine/core/loader/NewImageLoader.h +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright 2015 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_ENGINE_CORE_LOADER_NEWIMAGELOADER_H_ -#define SKY_ENGINE_CORE_LOADER_NEWIMAGELOADER_H_ - -#include "mojo/common/data_pipe_drainer.h" -#include "sky/engine/platform/SharedBuffer.h" -#include "sky/engine/platform/fetcher/MojoFetcher.h" -#include "sky/engine/wtf/OwnPtr.h" -#include "sky/engine/wtf/text/AtomicString.h" -#include "third_party/skia/include/core/SkBitmap.h" - -namespace blink { - -class NewImageLoaderClient { - public: - virtual void OnLoadFinished(const SkBitmap& result) = 0; - - protected: - NewImageLoaderClient() {} -}; - -class NewImageLoader : public MojoFetcher::Client, - public mojo::common::DataPipeDrainer::Client { - public: - explicit NewImageLoader(NewImageLoaderClient* client); - virtual ~NewImageLoader(); - - void Load(const KURL& src); - - // MojoFetcher::Client - void OnReceivedResponse(mojo::URLResponsePtr) override; - - // mojo::common::DataPipeDrainer::Client - void OnDataAvailable(const void*, size_t) override; - void OnDataComplete() override; - - private: - NewImageLoaderClient* client_; - OwnPtr fetcher_; - OwnPtr drainer_; - RefPtr buffer_; -}; - -} // namespace blink - -#endif // SKY_ENGINE_CORE_LOADER_NEWIMAGELOADER_H_ diff --git a/engine/core/painting/CanvasImage.cpp b/engine/core/painting/CanvasImage.cpp index b9bed98dc15..f21d588f62c 100644 --- a/engine/core/painting/CanvasImage.cpp +++ b/engine/core/painting/CanvasImage.cpp @@ -7,7 +7,7 @@ namespace blink { -CanvasImage::CanvasImage() : imageLoader_(adoptPtr(new NewImageLoader(this))) { +CanvasImage::CanvasImage() { } CanvasImage::~CanvasImage() { @@ -21,19 +21,4 @@ int CanvasImage::height() const { return bitmap_.height(); } -void CanvasImage::setSrc(const String& url) { - // TODO(jackson): Figure out how to determine the proper base URL here - KURL newSrcURL = KURL(KURL(), url); - if (srcURL_ != newSrcURL) { - srcURL_ = newSrcURL; - imageLoader_->Load(srcURL_); - } -} - -void CanvasImage::OnLoadFinished(const SkBitmap& result) { - // TODO(jackson): We'll eventually need a notification pathway for - // when the image load is complete so that we know to repaint, etc. - bitmap_ = result; -} - } // namespace blink \ No newline at end of file diff --git a/engine/core/painting/CanvasImage.h b/engine/core/painting/CanvasImage.h index 1f40d7bab83..c92e9ebd988 100644 --- a/engine/core/painting/CanvasImage.h +++ b/engine/core/painting/CanvasImage.h @@ -5,7 +5,6 @@ #ifndef SKY_ENGINE_CORE_PAINTING_CANVASIMAGE_H_ #define SKY_ENGINE_CORE_PAINTING_CANVASIMAGE_H_ -#include "sky/engine/core/loader/NewImageLoader.h" #include "sky/engine/platform/weborigin/KURL.h" #include "sky/engine/tonic/dart_wrappable.h" #include "sky/engine/wtf/PassRefPtr.h" @@ -15,8 +14,7 @@ namespace blink { class CanvasImage final : public RefCounted, - public DartWrappable, - public NewImageLoaderClient { + public DartWrappable { DEFINE_WRAPPERTYPEINFO(); public: ~CanvasImage() override; @@ -25,21 +23,13 @@ class CanvasImage final : public RefCounted, int width() const; int height() const; - KURL src() const { return srcURL_; } - void setSrc(const String&); - const SkBitmap& bitmap() const { return bitmap_; } void setBitmap(const SkBitmap& bitmap) { bitmap_ = bitmap; } private: CanvasImage(); - // NewImageLoaderClient - void OnLoadFinished(const SkBitmap& result) override; - - KURL srcURL_; SkBitmap bitmap_; - OwnPtr imageLoader_; }; } // namespace blink diff --git a/engine/core/painting/Image.idl b/engine/core/painting/Image.idl index 3d24b9aa40a..69fc8b35b0a 100644 --- a/engine/core/painting/Image.idl +++ b/engine/core/painting/Image.idl @@ -8,5 +8,4 @@ ] interface Image { readonly attribute long width; readonly attribute long height; - attribute DOMString src; }; diff --git a/examples/raw/spinning_image.dart b/examples/raw/spinning_image.dart index 53fe140460f..cd5339fbf01 100644 --- a/examples/raw/spinning_image.dart +++ b/examples/raw/spinning_image.dart @@ -6,7 +6,7 @@ import 'dart:sky'; double timeBase = null; -Image image; +Image image = null; void beginFrame(double timeStamp) { if (timeBase == null) timeBase = timeStamp; @@ -16,14 +16,21 @@ void beginFrame(double timeStamp) { canvas.rotateDegrees(delta / 10); canvas.scale(0.2, 0.2); Paint paint = new Paint()..setARGB(255, 0, 255, 0); - canvas.drawImage(image, -image.width / 2.0, -image.height / 2.0, paint); + if (image != null) + canvas.drawImage(image, -image.width / 2.0, -image.height / 2.0, paint); view.picture = canvas.endRecording(); view.scheduleFrame(); } void main() { - image = new Image(); - image.src = "https://www.dartlang.org/logos/dart-logo.png"; + new ImageLoader("https://www.dartlang.org/logos/dart-logo.png", (result) { + if (result != null) { + print("${result.width}x${result.width} image loaded!"); + image = result; + view.scheduleFrame(); + } else { + print("Image failed to load"); + } + }).load(); view.setBeginFrameCallback(beginFrame); - view.scheduleFrame(); }