Hello noel,
I'd like you to do a code review. Please execute
g4 diff -c 10719360
or point your web browser to
http://mondrian/10719360
to review the following code:
Change 10719360 by nigel...@nigeltao-srcgears2 on 2009/04/06 17:16:50 *pending*
Implement path ops in the Gears canvas.
PRESUBMIT=passed
R=noel
[email protected]
DELTA=193 (182 added, 0 deleted, 11 changed)
OCL=10719360
Affected files ...
...
//depot/googleclient/gears/opensource/gears/canvas/canvas_rendering_context_2d.cc#22
edit
...
//depot/googleclient/gears/opensource/gears/canvas/canvas_rendering_context_2d.h#16
edit
... //depot/googleclient/gears/opensource/gears/test/manual/canvas.html#6 edit
193 delta lines: 182 added, 0 deleted, 11 changed
Also consider running:
g4 lint -c 10719360
which verifies that the changelist doesn't introduce new style violations.
If you can't do the review, please let me know as soon as possible. During
your review, please ensure that all new code has corresponding unit tests and
that existing unit tests are updated appropriately. Visit
http://www/eng/code_review.html for more information.
This is a semiautomated message from "g4 mail". Complaints or suggestions?
Mail [email protected].
Change 10719360 by nigel...@nigeltao-srcgears2 on 2009/04/06 17:16:50 *pending*
Implement path ops in the Gears canvas.
Affected files ...
...
//depot/googleclient/gears/opensource/gears/canvas/canvas_rendering_context_2d.cc#22
edit
...
//depot/googleclient/gears/opensource/gears/canvas/canvas_rendering_context_2d.h#16
edit
... //depot/googleclient/gears/opensource/gears/test/manual/canvas.html#6 edit
====
//depot/googleclient/gears/opensource/gears/canvas/canvas_rendering_context_2d.cc#22
-
/home/nigeltao/srcgears2/googleclient/gears/opensource/gears/canvas/canvas_rendering_context_2d.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/canvas/canvas_rendering_context_2d.cc
2009-04-06 17:14:24.000000000 +1000
+++ googleclient/gears/opensource/gears/canvas/canvas_rendering_context_2d.cc
2009-04-06 17:15:37.000000000 +1000
@@ -163,17 +163,19 @@
RegisterMethod("strokeRect", &GearsCanvasRenderingContext2D::StrokeRect);
// path API
- // Missing wrt HTML5: beginPath.
- // Missing wrt HTML5: closePath.
- // Missing wrt HTML5: moveTo.
- // Missing wrt HTML5: lineTo.
- // Missing wrt HTML5: quadraticCurveTo.
- // Missing wrt HTML5: bezierCurveTo.
- // Missing wrt HTML5: arcTo.
- // Missing wrt HTML5: rect.
- // Missing wrt HTML5: arc.
- // Missing wrt HTML5: fill.
- // Missing wrt HTML5: stroke.
+ RegisterMethod("beginPath", &GearsCanvasRenderingContext2D::BeginPath);
+ RegisterMethod("closePath", &GearsCanvasRenderingContext2D::ClosePath);
+ RegisterMethod("moveTo", &GearsCanvasRenderingContext2D::MoveTo);
+ RegisterMethod("lineTo", &GearsCanvasRenderingContext2D::LineTo);
+ RegisterMethod("quadraticCurveTo",
+ &GearsCanvasRenderingContext2D::QuadraticCurveTo);
+ RegisterMethod("bezierCurveTo",
+ &GearsCanvasRenderingContext2D::BezierCurveTo);
+ RegisterMethod("arcTo", &GearsCanvasRenderingContext2D::ArcTo);
+ RegisterMethod("rect", &GearsCanvasRenderingContext2D::Rect);
+ RegisterMethod("arc", &GearsCanvasRenderingContext2D::Arc);
+ RegisterMethod("fill", &GearsCanvasRenderingContext2D::Fill);
+ RegisterMethod("stroke", &GearsCanvasRenderingContext2D::Stroke);
// Missing wrt HTML5: clip.
// Missing wrt HTML5: isPointInPath.
@@ -528,6 +530,159 @@
void GearsCanvasRenderingContext2D::StrokeRect(JsCallContext *context) {
PaintRect(context, &stroke_style_as_paint_);
+}
+
+void GearsCanvasRenderingContext2D::BeginPath(JsCallContext *context) {
+ path_.reset();
+}
+
+void GearsCanvasRenderingContext2D::ClosePath(JsCallContext *context) {
+ path_.close();
+}
+
+void GearsCanvasRenderingContext2D::MoveTo(JsCallContext *context) {
+ double x, y;
+ JsArgument args[] = {
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &x },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &y }
+ };
+ context->GetArguments(ARRAYSIZE(args), args);
+ if (context->is_exception_set())
+ return;
+
+ path_.moveTo(SkDoubleToScalar(x), SkDoubleToScalar(y));
+}
+
+void GearsCanvasRenderingContext2D::LineTo(JsCallContext *context) {
+ double x, y;
+ JsArgument args[] = {
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &x },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &y }
+ };
+ context->GetArguments(ARRAYSIZE(args), args);
+ if (context->is_exception_set())
+ return;
+
+ path_.lineTo(SkDoubleToScalar(x), SkDoubleToScalar(y));
+}
+
+void GearsCanvasRenderingContext2D::QuadraticCurveTo(JsCallContext *context) {
+ double x1, y1, x2, y2;
+ JsArgument args[] = {
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &x1 },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &y1 },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &x2 },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &y2 }
+ };
+ context->GetArguments(ARRAYSIZE(args), args);
+ if (context->is_exception_set())
+ return;
+
+ path_.quadTo(SkDoubleToScalar(x1), SkDoubleToScalar(y1),
+ SkDoubleToScalar(x2), SkDoubleToScalar(y2));
+}
+
+void GearsCanvasRenderingContext2D::BezierCurveTo(JsCallContext *context) {
+ double x1, y1, x2, y2, x3, y3;
+ JsArgument args[] = {
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &x1 },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &y1 },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &x2 },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &y2 },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &x3 },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &y3 }
+ };
+ context->GetArguments(ARRAYSIZE(args), args);
+ if (context->is_exception_set())
+ return;
+
+ path_.cubicTo(SkDoubleToScalar(x1), SkDoubleToScalar(y1),
+ SkDoubleToScalar(x2), SkDoubleToScalar(y2),
+ SkDoubleToScalar(x3), SkDoubleToScalar(y3));
+}
+
+void GearsCanvasRenderingContext2D::ArcTo(JsCallContext *context) {
+ // This implementation mirrors WebKit's behavior, as per a manual (visual)
+ // comparison with Safari and Chromium. Note that Firefox (incorrectly) does
+ // something different.
+ double x1, y1, x2, y2, r;
+ JsArgument args[] = {
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &x1 },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &y1 },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &x2 },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &y2 },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &r }
+ };
+ context->GetArguments(ARRAYSIZE(args), args);
+ if (context->is_exception_set())
+ return;
+
+ path_.arcTo(SkDoubleToScalar(x1), SkDoubleToScalar(y1),
+ SkDoubleToScalar(x2), SkDoubleToScalar(y2), SkDoubleToScalar(r));
+}
+
+void GearsCanvasRenderingContext2D::Rect(JsCallContext *context) {
+ double x, y, w, h;
+ JsArgument args[] = {
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &x },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &y },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &w },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &h }
+ };
+ context->GetArguments(ARRAYSIZE(args), args);
+ if (context->is_exception_set())
+ return;
+
+ path_.addRect(SkDoubleToScalar(x), SkDoubleToScalar(y),
+ SkDoubleToScalar(x + w), SkDoubleToScalar(y + h));
+}
+
+void GearsCanvasRenderingContext2D::Arc(JsCallContext *context) {
+ // This method's implementation follows the one in WebKit, specifically
+ // Path::addArc in WebCore/platform/graphics/skia/PathSkia.cpp.
+ double x, y, r, start_angle, end_angle;
+ bool anticlockwise;
+ JsArgument args[] = {
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &x },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &y },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &r },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &start_angle },
+ { JSPARAM_REQUIRED, JSPARAM_DOUBLE, &end_angle },
+ { JSPARAM_REQUIRED, JSPARAM_BOOL, &anticlockwise }
+ };
+ context->GetArguments(ARRAYSIZE(args), args);
+ if (context->is_exception_set())
+ return;
+
+ SkRect oval;
+ oval.set(x - r, y - r, x + r, y + r);
+ double sweep = end_angle - start_angle;
+ if (sweep >= 2 * M_PI || sweep <= -2 * M_PI) {
+ path_.addOval(oval);
+ } else {
+ SkScalar start_degrees = SkDoubleToScalar(start_angle * 180 / M_PI);
+ SkScalar sweep_degrees = SkDoubleToScalar(sweep * 180 / M_PI);
+ // Counterclockwise arcs should be drawn with negative sweeps, while
+ // clockwise arcs should be drawn with positive sweeps. Check to see
+ // if the situation is reversed and correct it by adding or subtracting
+ // a full circle
+ // TODO(nigeltao): is one in/decrement of 360 enough? What if sweep_degrees
+ // started off greater than 360?
+ if (anticlockwise && sweep_degrees > 0) {
+ sweep_degrees -= SkIntToScalar(360);
+ } else if (!anticlockwise && sweep_degrees < 0) {
+ sweep_degrees += SkIntToScalar(360);
+ }
+ path_.arcTo(oval, start_degrees, sweep_degrees, false);
+ }
+}
+
+void GearsCanvasRenderingContext2D::Fill(JsCallContext *context) {
+ skia_canvas_->drawPath(path_, fill_style_as_paint_);
+}
+
+void GearsCanvasRenderingContext2D::Stroke(JsCallContext *context) {
+ skia_canvas_->drawPath(path_, stroke_style_as_paint_);
}
void GearsCanvasRenderingContext2D::DrawImage(JsCallContext *context) {
====
//depot/googleclient/gears/opensource/gears/canvas/canvas_rendering_context_2d.h#16
-
/home/nigeltao/srcgears2/googleclient/gears/opensource/gears/canvas/canvas_rendering_context_2d.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/canvas/canvas_rendering_context_2d.h
2009-04-06 17:14:24.000000000 +1000
+++ googleclient/gears/opensource/gears/canvas/canvas_rendering_context_2d.h
2009-04-06 16:12:29.000000000 +1000
@@ -143,6 +143,19 @@
// Strokes the rectangle with the current stroke style.
void StrokeRect(JsCallContext *context);
+ // Path operations.
+ void BeginPath(JsCallContext *context);
+ void ClosePath(JsCallContext *context);
+ void MoveTo(JsCallContext *context);
+ void LineTo(JsCallContext *context);
+ void QuadraticCurveTo(JsCallContext *context);
+ void BezierCurveTo(JsCallContext *context);
+ void ArcTo(JsCallContext *context);
+ void Rect(JsCallContext *context);
+ void Arc(JsCallContext *context);
+ void Fill(JsCallContext *context);
+ void Stroke(JsCallContext *context);
+
// Draws a canvas onto this canvas. sx, sy, sw, and sh identify a rectangular
// subset of the source canvas, and dx, dy, dw, dh identify the target area
// in the destination canvas (this canvas) into which pixels from the source
@@ -206,6 +219,8 @@
std::string16 fill_style_as_string_;
std::string16 stroke_style_as_string_;
+ SkPath path_;
+
scoped_ptr<JsEventMonitor> unload_monitor_;
DISALLOW_EVIL_CONSTRUCTORS(GearsCanvasRenderingContext2D);
==== //depot/googleclient/gears/opensource/gears/test/manual/canvas.html#6 -
/home/nigeltao/srcgears2/googleclient/gears/opensource/gears/test/manual/canvas.html
====
# action=edit type=text
--- googleclient/gears/opensource/gears/test/manual/canvas.html 2009-04-06
17:16:58.000000000 +1000
+++ googleclient/gears/opensource/gears/test/manual/canvas.html 2009-04-06
17:14:05.000000000 +1000
@@ -42,6 +42,18 @@
c.transform(1, 0.3, 0, 0.8, 7, 0);
c.clearRect(25, 25, 15, 15);
+
+ c.fillStyle = '#BFBFFF';
+ c.strokeStyle = 'black';
+ c.lineWidth = 2.0;
+ c.moveTo(5, 5);
+ c.lineTo(25, 5);
+ c.lineTo(90, 50);
+ c.quadraticCurveTo(60, 60, 50, 30);
+ c.bezierCurveTo(40, 60, 30, 15, 5, 15);
+ c.closePath();
+ c.stroke();
+ c.fill();
}
