From a09baca0dd25d0629036382a51484a4db284e5f3 Mon Sep 17 00:00:00 2001 From: Eric Seidel Date: Wed, 22 Jul 2015 14:53:25 -0700 Subject: [PATCH] Add C++ based support for drawAtlas This is supposed to make Viktor's game faster, but it's not clear to me that it actually does. I've left the code using the dart version of drawAtlas for now until Viktor can verify that it looks correct. I also added a wrapper for SkFilterQuality in the process of debugging SkCanvas.drawAtlas since all drawAtlas examples in Skia use FilterQuality.low. The bug which blocked me for so long turned out to be that SkCanvas.drawAtlas doesn't draw anything if antialiasing is turned on. Issue #138. R=abarth@google.com --- sky/engine/bindings/exception_state.cc | 4 ++ sky/engine/bindings/scripts/dart_types.py | 4 ++ sky/engine/core/core.gni | 5 ++ sky/engine/core/painting/Canvas.cpp | 42 ++++++++++++++ sky/engine/core/painting/Canvas.h | 6 ++ sky/engine/core/painting/Canvas.idl | 5 ++ sky/engine/core/painting/FilterQuality.dart | 14 +++++ sky/engine/core/painting/FilterQuality.h | 26 +++++++++ sky/engine/core/painting/Paint.cpp | 4 ++ sky/engine/core/painting/Paint.h | 2 + sky/engine/core/painting/Paint.idl | 1 + sky/engine/core/painting/RSTransform.cpp | 57 +++++++++++++++++++ sky/engine/core/painting/RSTransform.dart | 22 +++++++ sky/engine/core/painting/RSTransform.h | 32 +++++++++++ sky/engine/core/painting/Rect.cpp | 18 +++--- sky/engine/core/painting/Rect.h | 1 + sky/sdk/example/game/lib/particle_system.dart | 26 ++++----- 17 files changed, 246 insertions(+), 23 deletions(-) create mode 100644 sky/engine/core/painting/FilterQuality.dart create mode 100644 sky/engine/core/painting/FilterQuality.h create mode 100644 sky/engine/core/painting/RSTransform.cpp create mode 100644 sky/engine/core/painting/RSTransform.dart create mode 100644 sky/engine/core/painting/RSTransform.h diff --git a/sky/engine/bindings/exception_state.cc b/sky/engine/bindings/exception_state.cc index 916a38708a510..83128b11c4db9 100644 --- a/sky/engine/bindings/exception_state.cc +++ b/sky/engine/bindings/exception_state.cc @@ -47,6 +47,10 @@ Dart_Handle ExceptionState::GetDartException(Dart_NativeArguments args, // TODO(abarth): Still don't understand autoscope. DCHECK(auto_scope); // TODO(eseidel): This should be a real exception object! + if (!message_.isEmpty()) { + CString utf8 = message_.utf8(); + return Dart_NewStringFromUTF8(reinterpret_cast(utf8.data()), utf8.length()); + } return Dart_NewStringFromCString("Exception support missing. See exception_state.cc"); } diff --git a/sky/engine/bindings/scripts/dart_types.py b/sky/engine/bindings/scripts/dart_types.py index b24f2bab0da25..ad69011347193 100644 --- a/sky/engine/bindings/scripts/dart_types.py +++ b/sky/engine/bindings/scripts/dart_types.py @@ -113,6 +113,7 @@ class methods. 'unrestricted float': 'float', # Pass these by value, not pointer. 'Color': 'SkColor', + 'RSTransform': 'RSTransform', # These direct conversions appear to be working around # dart_value_to_cpp_value using CPP_SPECIAL_CONVERSION_RULES directly # instead of calling cpp_type. @@ -124,6 +125,7 @@ class methods. 'MojoDataPipeConsumer': 'mojo::ScopedDataPipeConsumerHandle', 'TileMode': 'SkShader::TileMode', 'TransferMode': 'SkXfermode::Mode', + 'FilterQuality': 'SkFilterQuality', 'PaintingStyle': 'SkPaint::Style', } @@ -370,10 +372,12 @@ def pass_by_value_format(typename, null_check="{null_check}"): 'Float32List': pass_by_value_format('Float32List'), 'Offset': pass_by_value_format('Offset'), 'Point': pass_by_value_format('Point'), + 'RSTransform': pass_by_value_format('RSTransform'), 'Rect': pass_by_value_format('Rect'), 'Size': pass_by_value_format('Size'), 'TileMode': pass_by_value_format('TileMode', ''), 'TransferMode': pass_by_value_format('TransferMode', ''), + 'FilterQuality': pass_by_value_format('FilterQuality', ''), 'PaintingStyle': pass_by_value_format('PaintingStyle', ''), 'MojoDataPipeConsumer': pass_by_value_format('mojo::ScopedDataPipeConsumerHandle'), } diff --git a/sky/engine/core/core.gni b/sky/engine/core/core.gni index 49c3ef8e50f40..1080a77e5ae8a 100644 --- a/sky/engine/core/core.gni +++ b/sky/engine/core/core.gni @@ -436,6 +436,7 @@ sky_core_files = [ "painting/DrawLooperAddLayerCallback.h", "painting/DrawLooperLayerInfo.cpp", "painting/DrawLooperLayerInfo.h", + "painting/FilterQuality.h", "painting/LayerDrawLooperBuilder.cpp", "painting/LayerDrawLooperBuilder.h", "painting/LayoutRoot.cpp", @@ -463,6 +464,8 @@ sky_core_files = [ "painting/Point.h", "painting/RRect.cpp", "painting/RRect.h", + "painting/RSTransform.cpp", + "painting/RSTransform.h", "painting/Rect.cpp", "painting/Rect.h", "painting/Shader.cpp", @@ -706,12 +709,14 @@ core_dart_files = get_path_info([ "painting/Color.dart", "painting/ColorFilter.dart", "painting/DrawLooperLayerInfo.dart", + "painting/FilterQuality.dart", "painting/Gradient.dart", "painting/MaskFilter.dart", "painting/Offset.dart", "painting/OffsetBase.dart", "painting/PaintingStyle.dart", "painting/Point.dart", + "painting/RSTransform.dart", "painting/Rect.dart", "painting/Size.dart", "painting/TransferMode.dart", diff --git a/sky/engine/core/painting/Canvas.cpp b/sky/engine/core/painting/Canvas.cpp index 7a8d5a2d2070c..296ac9ca4de52 100644 --- a/sky/engine/core/painting/Canvas.cpp +++ b/sky/engine/core/painting/Canvas.cpp @@ -217,4 +217,46 @@ void Canvas::drawPaintingNode(PaintingNode* paintingNode, const Point& p) translate(-p.sk_point.x(), -p.sk_point.y()); } +void Canvas::drawAtlas(CanvasImage* atlas, + const Vector& transforms, const Vector& rects, + const Vector& colors, SkXfermode::Mode mode, + const Rect& cullRect, Paint* paint, ExceptionState& es) +{ + if (!m_canvas) + return; + RefPtr skImage = adoptRef(SkImage::NewFromBitmap(atlas->bitmap())); + if (transforms.size() != rects.size()) + return es.ThrowRangeError("transforms and rects lengths must match"); + if (colors.size() && colors.size() != rects.size()) + return es.ThrowRangeError("if supplied, colors length must match that of transforms and rects"); + + Vector skXForms; + for (size_t x = 0; x < transforms.size(); x++) { + const RSTransform& transform = transforms[x]; + if (transform.is_null) + return es.ThrowRangeError("transforms contained a null"); + skXForms.append(transform.sk_xform); + } + + Vector skRects; + for (size_t x = 0; x < rects.size(); x++) { + const Rect& rect = rects[x]; + if (rect.is_null) + return es.ThrowRangeError("rects contained a null"); + skRects.append(rect.sk_rect); + } + + m_canvas->drawAtlas( + skImage.get(), + skXForms.data(), + skRects.data(), + colors.isEmpty() ? nullptr : colors.data(), + skXForms.size(), + mode, + cullRect.is_null ? nullptr : &cullRect.sk_rect, + paint ? &paint->paint() : nullptr + ); +} + + } // namespace blink diff --git a/sky/engine/core/painting/Canvas.h b/sky/engine/core/painting/Canvas.h index eacb81d5182b7..5cba6cf55b1e1 100644 --- a/sky/engine/core/painting/Canvas.h +++ b/sky/engine/core/painting/Canvas.h @@ -17,6 +17,7 @@ #include "sky/engine/core/painting/RRect.h" #include "sky/engine/core/painting/Rect.h" #include "sky/engine/core/painting/Size.h" +#include "sky/engine/core/painting/RSTransform.h" #include "sky/engine/platform/graphics/DisplayList.h" #include "sky/engine/tonic/dart_wrappable.h" #include "sky/engine/tonic/float32_list.h" @@ -86,6 +87,11 @@ class Canvas : public RefCounted, public DartWrappable { void drawDrawable(Drawable* drawable); void drawPaintingNode(PaintingNode* paintingNode, const Point& p); + void drawAtlas(CanvasImage* atlas, + const Vector& transforms, const Vector& rects, + const Vector& colors, SkXfermode::Mode mode, + const Rect& cullRect, Paint* paint, ExceptionState&); + SkCanvas* skCanvas() { return m_canvas; } void clearSkCanvas() { m_canvas = nullptr; } bool isRecording() const { return !!m_canvas; } diff --git a/sky/engine/core/painting/Canvas.idl b/sky/engine/core/painting/Canvas.idl index 6f822447f02f6..618af67419b1b 100644 --- a/sky/engine/core/painting/Canvas.idl +++ b/sky/engine/core/painting/Canvas.idl @@ -34,4 +34,9 @@ void drawPicture(Picture picture); void drawDrawable(Drawable drawable); void drawPaintingNode(PaintingNode paintingNode, Point p); + + // TODO(eseidel): Paint should be optional, but optional doesn't work. + [RaisesException] void drawAtlas(Image image, + sequence transforms, sequence rects, + sequence colors, TransferMode mode, Rect cullRect, Paint paint); }; diff --git a/sky/engine/core/painting/FilterQuality.dart b/sky/engine/core/painting/FilterQuality.dart new file mode 100644 index 0000000000000..4d0e163efed3f --- /dev/null +++ b/sky/engine/core/painting/FilterQuality.dart @@ -0,0 +1,14 @@ +// 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. + +part of dart.sky; + +/// List of predefined filter quality modes. This list comes from Skia's +/// SkFitlerQuality.h and the values (order) should be kept in sync. +enum FilterQuality { + none, + low, + medium, + high, +} diff --git a/sky/engine/core/painting/FilterQuality.h b/sky/engine/core/painting/FilterQuality.h new file mode 100644 index 0000000000000..adef85a81c866 --- /dev/null +++ b/sky/engine/core/painting/FilterQuality.h @@ -0,0 +1,26 @@ +// 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_PAINTING_FILTERQUALITY_H_ +#define SKY_ENGINE_CORE_PAINTING_FILTERQUALITY_H_ + +#include "sky/engine/tonic/dart_converter.h" +#include "third_party/skia/include/core/SkXfermode.h" + +namespace blink { + +class FilterQuality {}; + +template <> +struct DartConverter + : public DartConverterEnum {}; + +// If this fails, it's because SkFilterQuality has changed. We need to change +// FilterQuality.dart to ensure the FilterQuality enum is in sync with the C++ +// values. +COMPILE_ASSERT(SkFilterQuality::kHigh_SkFilterQuality == 3, Need_to_update_FilterQuality_dart); + +} // namespace blink + +#endif // SKY_ENGINE_CORE_PAINTING_FILTERQUALITY_H_ diff --git a/sky/engine/core/painting/Paint.cpp b/sky/engine/core/painting/Paint.cpp index f97c52d29acff..d92fda79c08bd 100644 --- a/sky/engine/core/painting/Paint.cpp +++ b/sky/engine/core/painting/Paint.cpp @@ -64,6 +64,10 @@ void Paint::setTransferMode(SkXfermode::Mode transfer_mode) { paint_.setXfermodeMode(transfer_mode); } +void Paint::setFilterQuality(SkFilterQuality filter_quality) { + paint_.setFilterQuality(filter_quality); +} + String Paint::toString() const { StringBuilder result; diff --git a/sky/engine/core/painting/Paint.h b/sky/engine/core/painting/Paint.h index 7888905d4250d..3037ea9769c7a 100644 --- a/sky/engine/core/painting/Paint.h +++ b/sky/engine/core/painting/Paint.h @@ -8,6 +8,7 @@ #include "sky/engine/core/painting/CanvasColor.h" #include "sky/engine/core/painting/PaintingStyle.h" #include "sky/engine/core/painting/TransferMode.h" +#include "sky/engine/core/painting/FilterQuality.h" #include "sky/engine/tonic/dart_wrappable.h" #include "sky/engine/wtf/PassRefPtr.h" #include "sky/engine/wtf/RefCounted.h" @@ -44,6 +45,7 @@ class Paint : public RefCounted, public DartWrappable { void setShader(Shader* shader); void setStyle(SkPaint::Style style); void setTransferMode(SkXfermode::Mode transfer_mode); + void setFilterQuality(SkFilterQuality filter_quality); const SkPaint& paint() const { return paint_; } void setPaint(const SkPaint& paint) { paint_ = paint; } diff --git a/sky/engine/core/painting/Paint.idl b/sky/engine/core/painting/Paint.idl index cc2eba99bf45c..cdc64a4f5981e 100644 --- a/sky/engine/core/painting/Paint.idl +++ b/sky/engine/core/painting/Paint.idl @@ -15,6 +15,7 @@ void setShader(Shader shader); void setStyle(PaintingStyle style); void setTransferMode(TransferMode transferMode); + void setFilterQuality(FilterQuality filterQuality); DOMString toString(); }; diff --git a/sky/engine/core/painting/RSTransform.cpp b/sky/engine/core/painting/RSTransform.cpp new file mode 100644 index 0000000000000..df300132876d2 --- /dev/null +++ b/sky/engine/core/painting/RSTransform.cpp @@ -0,0 +1,57 @@ +// 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/core/painting/RSTransform.h" + +#include "sky/engine/core/script/dom_dart_state.h" +#include "sky/engine/tonic/dart_error.h" +#include "sky/engine/tonic/dart_value.h" +#include "base/logging.h" + +namespace blink { + +// Convert dart_xform._value[0...3] ==> RSTransform +RSTransform DartConverter::FromDart(Dart_Handle dart_xform) { + RSTransform result; + result.is_null = true; + if (Dart_IsNull(dart_xform)) + return result; + + Dart_Handle value = + Dart_GetField(dart_xform, DOMDartState::Current()->value_handle()); + if (Dart_IsNull(value)) + return result; + + Dart_TypedData_Type type; + float* data = nullptr; + intptr_t num_elements = 0; + Dart_TypedDataAcquireData( + value, &type, reinterpret_cast(&data), &num_elements); + DCHECK(!LogIfError(value)); + ASSERT(type == Dart_TypedData_kFloat32 && num_elements == 4); + + SkScalar* dest[] = { + &result.sk_xform.fSCos, + &result.sk_xform.fSSin, + &result.sk_xform.fTx, + &result.sk_xform.fTy + }; + for (intptr_t i = 0; i < 4; ++i) + *dest[i] = data[i]; + + Dart_TypedDataReleaseData(value); + + result.is_null = false; + return result; +} + +RSTransform DartConverter::FromArgumentsWithNullCheck(Dart_NativeArguments args, + int index, + Dart_Handle& exception) { + Dart_Handle dart_xform = Dart_GetNativeArgument(args, index); + DCHECK(!LogIfError(dart_xform)); + return FromDart(dart_xform); +} + +} // namespace blink diff --git a/sky/engine/core/painting/RSTransform.dart b/sky/engine/core/painting/RSTransform.dart new file mode 100644 index 0000000000000..5c11e549f3c7c --- /dev/null +++ b/sky/engine/core/painting/RSTransform.dart @@ -0,0 +1,22 @@ +// 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. + +part of dart.sky; + +// Modeled after Skia's SkRSXform +class RSTransform { + RSTransform(double scos, double ssin, double tx, double ty) { + _value + ..[0] = scos + ..[1] = ssin + ..[2] = tx + ..[3] = ty; + } + + final Float32List _value = new Float32List(4); + double get scos => _value[0]; + double get ssin => _value[1]; + double get tx => _value[2]; + double get ty => _value[3]; +} diff --git a/sky/engine/core/painting/RSTransform.h b/sky/engine/core/painting/RSTransform.h new file mode 100644 index 0000000000000..1b47926ecdc9f --- /dev/null +++ b/sky/engine/core/painting/RSTransform.h @@ -0,0 +1,32 @@ +// 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_PAINTING_RSTRANSFORM_H_ +#define SKY_ENGINE_CORE_PAINTING_RSTRANSFORM_H_ + +#include "dart/runtime/include/dart_api.h" +#include "sky/engine/tonic/dart_converter.h" +#include "third_party/skia/include/core/SkPoint.h" +#include "third_party/skia/include/core/SkSize.h" +#include "third_party/skia/include/core/SkRSXform.h" + +namespace blink { +// Very simple wrapper for SkRSXform to add a null state. +class RSTransform { + public: + SkRSXform sk_xform; + bool is_null; +}; + +template <> +struct DartConverter { + static RSTransform FromDart(Dart_Handle handle); + static RSTransform FromArgumentsWithNullCheck(Dart_NativeArguments args, + int index, + Dart_Handle& exception); +}; + +} // namespace blink + +#endif // SKY_ENGINE_CORE_PAINTING_RSTRANSFORM_H_ diff --git a/sky/engine/core/painting/Rect.cpp b/sky/engine/core/painting/Rect.cpp index 677faca62c90e..65dea9592906d 100644 --- a/sky/engine/core/painting/Rect.cpp +++ b/sky/engine/core/painting/Rect.cpp @@ -11,16 +11,10 @@ namespace blink { -// Convert dart_rect._value[0...3] ==> SkRect. -Rect DartConverter::FromArgumentsWithNullCheck(Dart_NativeArguments args, - int index, - Dart_Handle& exception) { +// Convert dart_rect._value[0...3] ==> SkRect +Rect DartConverter::FromDart(Dart_Handle dart_rect) { Rect result; result.is_null = true; - - Dart_Handle dart_rect = Dart_GetNativeArgument(args, index); - DCHECK(!LogIfError(dart_rect)); - if (Dart_IsNull(dart_rect)) return result; @@ -52,4 +46,12 @@ Rect DartConverter::FromArgumentsWithNullCheck(Dart_NativeArguments args, return result; } +Rect DartConverter::FromArgumentsWithNullCheck(Dart_NativeArguments args, + int index, + Dart_Handle& exception) { + Dart_Handle dart_rect = Dart_GetNativeArgument(args, index); + DCHECK(!LogIfError(dart_rect)); + return FromDart(dart_rect); +} + } // namespace blink diff --git a/sky/engine/core/painting/Rect.h b/sky/engine/core/painting/Rect.h index 7a1d837393411..359bcdfdb0257 100644 --- a/sky/engine/core/painting/Rect.h +++ b/sky/engine/core/painting/Rect.h @@ -19,6 +19,7 @@ class Rect { template <> struct DartConverter { + static Rect FromDart(Dart_Handle handle); static Rect FromArgumentsWithNullCheck(Dart_NativeArguments args, int index, Dart_Handle& exception); diff --git a/sky/sdk/example/game/lib/particle_system.dart b/sky/sdk/example/game/lib/particle_system.dart index 01e0d653d90d0..db64a6997221f 100644 --- a/sky/sdk/example/game/lib/particle_system.dart +++ b/sky/sdk/example/game/lib/particle_system.dart @@ -272,16 +272,22 @@ class ParticleSystem extends Node { colors.add(particleColor); } - drawAtlas(canvas, texture.image, transforms, rects, colors, TransferMode.modulate, - new Paint()..setTransferMode(transferMode)); + // TODO(viktork): Verify that the C++ version looks right and is faster! + Paint paint = new Paint()..setTransferMode(transferMode) + ..setFilterQuality(FilterQuality.low) // All Skia examples do this. + ..isAntiAlias = false; // Antialiasing breaks SkCanvas.drawAtlas? + // return canvas.drawAtlas(texture.image, transforms, rects, colors, + // TransferMode.modulate, null, paint); + + dartDrawAtlas(canvas, texture.image, transforms, rects, colors, + TransferMode.modulate, paint); } double randMinus1To1() => _rand.nextDouble() * 2.0 - 1.0; } -// TODO: Needs bindings to Skia method in SkCanvas (exclude canvas parameter) -void drawAtlas(Canvas canvas, Image image, List transforms, List rects, List colors, - TransferMode transferMode, Paint paint) { +void dartDrawAtlas(Canvas canvas, Image image, List transforms, + List rects, List colors, TransferMode transferMode, Paint paint) { assert(transforms.length == rects.length && transforms.length == colors.length); Texture mainTexture = new Texture(image); @@ -308,13 +314,3 @@ void drawAtlas(Canvas canvas, Image image, List transforms, List