Sky: different tack for custom Rect type in the Canvas API.

Rect is now a dart-only type, defined in its own .dart file. I use
DartConverter when passing a Rect into C++. I also special-cased Rect in the
IDL compiler so that it's passed by value, instead of allocating a new
Rect object on the heap.

This also adds a mechanism to add custom .dart files to dart_sky.dart - used by
Rect.dart.

R=eseidel@chromium.org

Review URL: https://codereview.chromium.org/1151673002

engine/bindings/BUILD.gn

@@ -331,8 +331,15 @@ action("generate_dart_globals") {
   ]
 }
 
+copy("copy_core_dart_files") {
+  sources = core_dart_files
+  outputs = [
+    "$bindings_output_dir/{{source_file_part}}",
+  ]
+}
+
 action("generate_dart_sky") {
-  sources = core_idl_files
+  sources = core_idl_files + core_dart_files
   script = "$bindings_scripts_dir/compiler.py"
 
   file_list = "$target_gen_dir/${target_name}_file_list.txt"
@@ -355,6 +362,7 @@ action("generate_dart_sky") {
   ]
 
   deps = [
+    ":copy_core_dart_files",
     ":compile_idls",
   ]
 }

engine/bindings/scripts/code_generator_dart.py

@@ -149,15 +149,6 @@ class CodeGeneratorDart(object):
 
         template_contents['cpp_includes'] = sorted(includes)
 
-        # If CustomDart is set, read the custom dart file and add it to our
-        # template parameters.
-        if 'CustomDart' in interface.extended_attributes:
-          dart_filename = os.path.join(os.path.dirname(idl_filename),
-                                       interface.name + ".dart")
-          with open(dart_filename) as dart_file:
-              custom_dartcode = dart_file.read()
-              template_contents['custom_dartcode'] = custom_dartcode
-
         idl_world = {'interface': None, 'callback': None}
 
         # Load the pickle file for this IDL.
@@ -196,8 +187,14 @@ class CodeGeneratorDart(object):
 
         # Load all pickled data for each interface.
         for (directory, file_list) in global_entries:
-            for idl_filename in file_list:
-                interface_name = idl_filename_to_interface_name(idl_filename)
+            for filename in file_list:
+                if os.path.splitext(filename)[1] == '.dart':
+                    # Special case: any .dart files in the list should be added
+                    # to dart_sky.dart directly, but don't need to be processed.
+                    interface_name = os.path.splitext(os.path.basename(filename))[0]
+                    world['interfaces'].append({'name': interface_name})
+                    continue
+                interface_name = idl_filename_to_interface_name(filename)
                 idl_pickle_filename = interface_name + "_globals.pickle"
                 idl_pickle_filename = os.path.join(directory, idl_pickle_filename)
                 if not os.path.exists(idl_pickle_filename):

engine/bindings/scripts/dart_types.py

@@ -114,6 +114,7 @@ CPP_SPECIAL_CONVERSION_RULES = {
     'boolean': 'bool',
     'unrestricted double': 'double',
     'unrestricted float': 'float',
+    'Rect': 'Rect',  # Pass Rect by value, not pointer.
 }
 
 
@@ -352,6 +353,7 @@ DART_TO_CPP_VALUE = {
     # FIXME(vsm): This is an enum type (defined in StorageQuota.idl).
     # We should handle it automatically, but map to a String for now.
     'StorageType': 'DartUtilities::dartToString(args, {index}, exception, {auto_scope})',
+    'Rect': 'DartConverter<{implemented_as}>::FromArguments{null_check}(args, {index}, exception)',
 }
 
 

engine/bindings/scripts/templates/dart_blink.template

@@ -6,6 +6,7 @@
 library dart.sky;
 
 import "dart:nativewrappers";
+import 'dart:typed_data';
 
 {% for interface in interfaces %}
 part '{{interface.name}}.dart';

engine/bindings/scripts/templates/interface_dart.template

@@ -19,9 +19,8 @@ part of dart.sky;
     {%- endfor -%}
 {%- endmacro -%}
 
-{%- set prefix = "_" if custom_dartcode else "" -%}
 {% if not constructors and not custom_constructors %}abstract {% endif -%}
-class {{prefix}}{{interface_name}} extends
+class {{interface_name}} extends
     {{ parent_interface if parent_interface else 'NativeFieldWrapperClass2' }} {
     // Constructors
 {# TODO(eseidel): We only ever have one constructor. #}
@@ -40,15 +39,15 @@ class {{prefix}}{{interface_name}} extends
 
     // Attributes
 {% for attribute in attributes %}
-    {{ attribute.dart_type }} get {{prefix}}{{ attribute.name }} native "{{interface_name}}_{{ attribute.name }}_Getter";
+    {{ attribute.dart_type }} get {{ attribute.name }} native "{{interface_name}}_{{ attribute.name }}_Getter";
 {% if not attribute.is_read_only %}
-    void set {{prefix}}{{ attribute.name }}({{ attribute.dart_type }} value) native "{{interface_name}}_{{ attribute.name }}_Setter";
+    void set {{ attribute.name }}({{ attribute.dart_type }} value) native "{{interface_name}}_{{ attribute.name }}_Setter";
 {% endif %}
 {% endfor %}
 
     // Methods
 {% for method in methods %}
-    {{method.dart_type}} {{prefix}}{{method.name}}({{ args_macro(method.arguments)}}) native "{{interface_name}}_{{ method.name }}_Callback";
+    {{method.dart_type}} {{method.name}}({{ args_macro(method.arguments)}}) native "{{interface_name}}_{{ method.name }}_Callback";
 {% endfor %}
 
     // Operators
@@ -59,5 +58,3 @@ class {{prefix}}{{interface_name}} extends
     void operator[]=(String name, String value) native "{{interface_name}}___setter___Callback";
 {% endif %}
 }
-
-{{custom_dartcode}}

engine/core/core.gni

@@ -847,6 +847,8 @@ sky_core_files = [
   "painting/Picture.h",
   "painting/PictureRecorder.cpp",
   "painting/PictureRecorder.h",
+  "painting/Rect.cpp",
+  "painting/Rect.h",
   "rendering/BidiRun.h",
   "rendering/BidiRunForLine.cpp",
   "rendering/BidiRunForLine.h",
@@ -1114,6 +1116,8 @@ core_idl_files = get_path_info([
                                ],
                                "abspath")
 
+core_dart_files = get_path_info([ "painting/Rect.dart" ], "abspath")
+
 # Files for which bindings (.cpp and .h files) will be generated
 # 'partial interface', target (right side of) 'implements', and
 # interfaces with static bindings (in bindings/core/v8/)

engine/core/painting/Canvas.cpp

@@ -3,7 +3,7 @@
 // found in the LICENSE file.
 
 #include "sky/engine/config.h"
-#include "sky/engine/core/painting/PictureRecorder.h"
+#include "sky/engine/core/painting/Canvas.h"
 
 #include "sky/engine/core/dom/Document.h"
 #include "sky/engine/core/dom/Element.h"
@@ -13,14 +13,6 @@
 
 namespace blink {
 
-SkRect toSkRect(const Vector<float>& rect)
-{
-    ASSERT(rect.size() == 4);
-    SkRect sk_rect;
-    sk_rect.set(rect[0], rect[1], rect[2], rect[3]);
-    return sk_rect;
-}
-
 Canvas::Canvas(const FloatSize& size)
     : m_size(size)
 {
@@ -40,15 +32,12 @@ void Canvas::save()
     m_canvas->save();
 }
 
-void Canvas::saveLayer(const Vector<float>& bounds, const Paint* paint)
+void Canvas::saveLayer(const Rect& bounds, const Paint* paint)
 {
     if (!m_canvas)
         return;
     ASSERT(m_displayList->isRecording());
-    SkRect sk_bounds;
-    if (!bounds.isEmpty())
-      sk_bounds = toSkRect(bounds);
-    m_canvas->saveLayer(!bounds.isEmpty() ? &sk_bounds : nullptr,
+    m_canvas->saveLayer(!bounds.is_null ? &bounds.sk_rect : nullptr,
                         paint ? &paint->paint() : nullptr);
 }
 
@@ -103,12 +92,12 @@ void Canvas::concat(const Vector<float>& matrix)
     m_canvas->concat(sk_matrix);
 }
 
-void Canvas::clipRect(const Vector<float>& rect)
+void Canvas::clipRect(const Rect& rect)
 {
     if (!m_canvas)
         return;
     ASSERT(m_displayList->isRecording());
-    m_canvas->clipRect(toSkRect(rect));
+    m_canvas->clipRect(rect.sk_rect);
 }
 
 void Canvas::drawPicture(Picture* picture)
@@ -129,22 +118,22 @@ void Canvas::drawPaint(const Paint* paint)
     m_canvas->drawPaint(paint->paint());
 }
 
-void Canvas::drawRect(const Vector<float>& rect, const Paint* paint)
+void Canvas::drawRect(const Rect& rect, const Paint* paint)
 {
     if (!m_canvas)
         return;
     ASSERT(paint);
     ASSERT(m_displayList->isRecording());
-    m_canvas->drawRect(toSkRect(rect), paint->paint());
+    m_canvas->drawRect(rect.sk_rect, paint->paint());
 }
 
-void Canvas::drawOval(const Vector<float>& rect, const Paint* paint)
+void Canvas::drawOval(const Rect& rect, const Paint* paint)
 {
     if (!m_canvas)
         return;
     ASSERT(paint);
     ASSERT(m_displayList->isRecording());
-    m_canvas->drawOval(toSkRect(rect), paint->paint());
+    m_canvas->drawOval(rect.sk_rect, paint->paint());
 }
 
 void Canvas::drawCircle(float x, float y, float radius, const Paint* paint)

engine/core/painting/Canvas.dart

-// 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.
-
-// This extends the Canvas.dart generated from Canvas.idl.
-
-class Rect {
-  List<double> _value;
-  double get left => _value[0];
-  double get top => _value[1];
-  double get right => _value[2];
-  double get bottom => _value[3];
-
-  void setLTRB(double left, double top, double right, double bottom) {
-    _value = [left, top, right, bottom];
-  }
-}
-
-class Canvas extends _Canvas {
-  double get height => _height;
-  double get width => _width;
-
-  void save() => _save();
-  void saveLayer(Rect bounds, Paint paint) =>
-      _saveLayer(bounds._value, Paint paint);
-  void restore() => _restore();
-  void translate(double dx, double dy) => _translate(dx, dy);
-  void scale(double sx, double sy) => _scale(sx, sy);
-  void rotateDegrees(double degrees) => _rotateDegrees(degrees);
-  void skew(double sx, double sy) => _skew(sx, sy);
-  void concat(List<double> matrix9) => _concat(matrix9);
-  void clipRect(Rect rect) => _clipRect(rect._value);
-
-  void drawPicture(Picture picture) => _drawPicture(picture);
-  void drawPaint(Paint paint) => _drawPaint(paint);
-  void drawRect(Rect rect, Paint paint) => _drawRect(rect._value, paint);
-  void drawOval(Rect rect, Paint paint) => _drawOval(rect._value, paint);
-  void drawCircle(double x, double y, double radius, Paint paint) =>
-      _drawCircle(x, y, radius, paint);
-}

engine/core/painting/Canvas.h

@@ -7,6 +7,7 @@
 
 #include "sky/engine/core/painting/Paint.h"
 #include "sky/engine/core/painting/Picture.h"
+#include "sky/engine/core/painting/Rect.h"
 #include "sky/engine/platform/graphics/DisplayList.h"
 #include "sky/engine/tonic/dart_wrappable.h"
 #include "sky/engine/wtf/PassRefPtr.h"
@@ -27,7 +28,7 @@ public:
     float height() const { return m_size.height(); }
 
     void save();
-    void saveLayer(const Vector<float>& bounds, const Paint* paint);
+    void saveLayer(const Rect& bounds, const Paint* paint);
     void restore();
 
     void translate(float dx, float dy);
@@ -36,12 +37,12 @@ public:
     void skew(float sx, float sy);
     void concat(const Vector<float>& matrix);
 
-    void clipRect(const Vector<float>& rect);
+    void clipRect(const Rect& rect);
 
     void drawPicture(Picture* picture);
     void drawPaint(const Paint* paint);
-    void drawRect(const Vector<float>& rect, const Paint* paint);
-    void drawOval(const Vector<float>& rect, const Paint* paint);
+    void drawRect(const Rect& rect, const Paint* paint);
+    void drawOval(const Rect& rect, const Paint* paint);
     void drawCircle(float x, float y, float radius, const Paint* paint);
 
     SkCanvas* skCanvas() { return m_canvas; }

engine/core/painting/Canvas.idl

@@ -3,14 +3,14 @@
 // found in the LICENSE file.
 
 // TODO(mpcomplete): Figure out a better SkMatrix representation.
-[CustomDart] interface Canvas {
+interface Canvas {
   // Height and width are used for culling optimizations and do not necessarily
   // imply that the Canvas is backed by a buffer with any specific bounds.
   readonly attribute float height;
   readonly attribute float width;
 
   void save();
-  void saveLayer(float[] bounds4 /* optional */, Paint paint /* optional */);
+  void saveLayer(Rect bounds /* optional */, Paint paint /* optional */);
   void restore();
 
   void translate(float dx, float dy);
@@ -19,12 +19,12 @@
   void skew(float sx, float sy);
   void concat(float[] matrix9);
 
-  void clipRect(float[] rect4);
+  void clipRect(Rect rect);
 
   void drawPicture(Picture picture);
 
   void drawPaint(Paint paint);
-  void drawRect(float[] rect4, Paint paint);
-  void drawOval(float[] rect4, Paint paint);
+  void drawRect(Rect rect, Paint paint);
+  void drawOval(Rect rect, Paint paint);
   void drawCircle(float x, float y, float radius, Paint paint);
 };

engine/core/painting/Rect.cpp

+// 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/painting/Rect.h"
+
+#include "sky/engine/tonic/dart_error.h"
+#include "sky/engine/tonic/dart_state.h"
+#include "base/logging.h"
+
+namespace blink {
+
+// Convert dart_rect._value[0...3] ==> SkRect.
+Rect DartConverter<Rect, void>::FromArgumentsWithNullCheck(
+    Dart_NativeArguments args,
+    int index,
+    Dart_Handle& exception) {
+  Rect result;
+  result.is_null = true;
+
+  Dart_Handle dart_rect = Dart_GetNativeArgument(args, index);
+  DCHECK(!LogIfError(dart_rect));
+
+  Dart_Handle value = Dart_GetField(dart_rect,
+                                    Dart_NewStringFromCString("_value"));
+  if (Dart_IsNull(value))
+    return result;
+
+  Dart_TypedData_Type type;
+  float* data = nullptr;
+  intptr_t num_elements = 0;
+  Dart_TypedDataAcquireData(
+      value, &type, reinterpret_cast<void**>(&data), &num_elements);
+  DCHECK(!LogIfError(value));
+  ASSERT(type == Dart_TypedData_kFloat32 && num_elements == 4);
+
+  SkScalar* dest[] = {
+    &result.sk_rect.fLeft,
+    &result.sk_rect.fTop,
+    &result.sk_rect.fRight,
+    &result.sk_rect.fBottom
+  };
+  for (intptr_t i = 0; i < 4; ++i)
+    *dest[i] = data[i];
+
+  Dart_TypedDataReleaseData(value);
+
+  result.is_null = false;
+  return result;
+}
+
+} // namespace blink

engine/core/painting/Rect.dart

+// 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;
+
+class Rect {
+  Float32List _value;
+  double get left => _value[0];
+  double get top => _value[1];
+  double get right => _value[2];
+  double get bottom => _value[3];
+
+  void setLTRB(double left, double top, double right, double bottom) {
+    _value = new Float32List.fromList([left, top, right, bottom]);
+  }
+}

engine/core/painting/Rect.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_PAINTING_RECT_H_
+#define SKY_ENGINE_CORE_PAINTING_RECT_H_
+
+#include "dart/runtime/include/dart_api.h"
+#include "sky/engine/tonic/dart_converter.h"
+#include "third_party/skia/include/core/SkRect.h"
+
+namespace blink {
+// Very simple wrapper for SkRect to add a null state.
+class Rect {
+ public:
+  SkRect sk_rect;
+  bool is_null;
+};
+
+template<>
+struct DartConverter<Rect, void> {
+  static Rect FromArgumentsWithNullCheck(Dart_NativeArguments args,
+                                         int index,
+                                         Dart_Handle& exception);
+};
+
+} // namespace blink
+
+#endif  // SKY_ENGINE_CORE_PAINTING_RECT_H_