Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(622)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 121303004: Fleshed out a lot of the Path interfac (Closed)

Created:
6 years, 11 months ago by jcgregorio
Modified:
6 years, 11 months ago
CC:
skia-review_googlegroups.com
Base URL:
https://skia.googlesource.com/skia.git@master
Visibility:
Public.

Description

Renamed JsCanvas to JsContext and moved into it's own file. Fleshed out a lot of the Path interface. BUG= Committed: http://code.google.com/p/skia/source/detail?r=12913

Patch Set 1 #

Patch Set 2 : more cleanup #

Patch Set 3 : final cleanupo #

Patch Set 4 : #

Patch Set 5 : #

Total comments: 36

Patch Set 6 : review feedback #

Total comments: 6

Patch Set 7 : review feedback #

Patch Set 8 : #

Total comments: 4

Patch Set 9 : unused imports #

Unified diffs Side-by-side diffs Delta from patch set Stats (+563 lines, -440 lines) Patch
M experimental/SkV8Example/Global.h View 1 2 3 4 5 2 chunks +2 lines, -0 lines 0 comments Download
M experimental/SkV8Example/Global.cpp View 4 chunks +13 lines, -9 lines 0 comments Download
A experimental/SkV8Example/JsContext.h View 1 2 3 4 5 1 chunk +96 lines, -0 lines 0 comments Download
A + experimental/SkV8Example/JsContext.cpp View 1 2 3 4 5 12 chunks +146 lines, -136 lines 0 comments Download
A experimental/SkV8Example/Path.h View 1 2 3 4 5 6 7 1 chunk +50 lines, -0 lines 0 comments Download
A experimental/SkV8Example/Path.cpp View 1 2 3 4 5 6 1 chunk +192 lines, -0 lines 0 comments Download
M experimental/SkV8Example/SkV8Example.h View 1 2 3 4 5 6 7 8 2 chunks +3 lines, -69 lines 0 comments Download
M experimental/SkV8Example/SkV8Example.cpp View 4 chunks +9 lines, -217 lines 0 comments Download
A experimental/SkV8Example/path.js View 1 2 3 4 5 1 chunk +34 lines, -0 lines 0 comments Download
M experimental/SkV8Example/sample.js View 1 1 chunk +3 lines, -3 lines 0 comments Download
A + experimental/SkV8Example/speed.js View 1 chunk +10 lines, -6 lines 0 comments Download
M gyp/v8.gyp View 1 chunk +4 lines, -0 lines 0 comments Download
M include/core/SkScalar.h View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 17 (0 generated)
jcgregorio
6 years, 11 months ago (2014-01-06 15:04:11 UTC) #1
robertphillips
lgtm + nits/questions https://codereview.chromium.org/121303004/diff/110001/experimental/SkV8Example/Global.h File experimental/SkV8Example/Global.h (right): https://codereview.chromium.org/121303004/diff/110001/experimental/SkV8Example/Global.h#newcode35 experimental/SkV8Example/Global.h:35: gGlobal = this; this->initialize()? https://codereview.chromium.org/121303004/diff/110001/experimental/SkV8Example/JsContext.cpp File ...
6 years, 11 months ago (2014-01-06 15:37:03 UTC) #2
jcgregorio
https://codereview.chromium.org/121303004/diff/110001/experimental/SkV8Example/Global.h File experimental/SkV8Example/Global.h (right): https://codereview.chromium.org/121303004/diff/110001/experimental/SkV8Example/Global.h#newcode35 experimental/SkV8Example/Global.h:35: gGlobal = this; On 2014/01/06 15:37:04, robertphillips wrote: > ...
6 years, 11 months ago (2014-01-06 15:57:31 UTC) #3
tfarina
v8.gyp needs some love! https://codereview.chromium.org/121303004/diff/110001/gyp/v8.gyp File gyp/v8.gyp (right): https://codereview.chromium.org/121303004/diff/110001/gyp/v8.gyp#newcode11 gyp/v8.gyp:11: ], while you are here, ...
6 years, 11 months ago (2014-01-06 15:58:13 UTC) #4
tfarina
https://codereview.chromium.org/121303004/diff/200001/experimental/SkV8Example/Path.h File experimental/SkV8Example/Path.h (right): https://codereview.chromium.org/121303004/diff/200001/experimental/SkV8Example/Path.h#newcode15 experimental/SkV8Example/Path.h:15: using namespace v8; this polutes the global namespace. I ...
6 years, 11 months ago (2014-01-06 15:59:45 UTC) #5
robertphillips
https://codereview.chromium.org/121303004/diff/110001/experimental/SkV8Example/Path.cpp File experimental/SkV8Example/Path.cpp (right): https://codereview.chromium.org/121303004/diff/110001/experimental/SkV8Example/Path.cpp#newcode12 experimental/SkV8Example/Path.cpp:12: I think so.
6 years, 11 months ago (2014-01-06 16:00:01 UTC) #6
tfarina
https://codereview.chromium.org/121303004/diff/200001/experimental/SkV8Example/SkV8Example.h File experimental/SkV8Example/SkV8Example.h (right): https://codereview.chromium.org/121303004/diff/200001/experimental/SkV8Example/SkV8Example.h#newcode16 experimental/SkV8Example/SkV8Example.h:16: #include "SkPaint.h" you can remove this. https://codereview.chromium.org/121303004/diff/200001/experimental/SkV8Example/SkV8Example.h#newcode20 experimental/SkV8Example/SkV8Example.h:20: class ...
6 years, 11 months ago (2014-01-06 16:03:55 UTC) #7
jcgregorio
On 2014/01/06 15:58:13, tfarina wrote: > v8.gyp needs some love! > > https://codereview.chromium.org/121303004/diff/110001/gyp/v8.gyp > File ...
6 years, 11 months ago (2014-01-06 16:31:24 UTC) #8
jcgregorio
https://codereview.chromium.org/121303004/diff/110001/experimental/SkV8Example/Path.cpp File experimental/SkV8Example/Path.cpp (right): https://codereview.chromium.org/121303004/diff/110001/experimental/SkV8Example/Path.cpp#newcode12 experimental/SkV8Example/Path.cpp:12: On 2014/01/06 16:00:01, robertphillips wrote: > I think so. ...
6 years, 11 months ago (2014-01-06 16:32:47 UTC) #9
tfarina
https://codereview.chromium.org/121303004/diff/300002/experimental/SkV8Example/SkV8Example.h File experimental/SkV8Example/SkV8Example.h (right): https://codereview.chromium.org/121303004/diff/300002/experimental/SkV8Example/SkV8Example.h#newcode13 experimental/SkV8Example/SkV8Example.h:13: #include <v8.h> you can remove this https://codereview.chromium.org/121303004/diff/300002/experimental/SkV8Example/SkV8Example.h#newcode17 experimental/SkV8Example/SkV8Example.h:17: using ...
6 years, 11 months ago (2014-01-06 16:50:59 UTC) #10
jcgregorio
https://codereview.chromium.org/121303004/diff/300002/experimental/SkV8Example/SkV8Example.h File experimental/SkV8Example/SkV8Example.h (right): https://codereview.chromium.org/121303004/diff/300002/experimental/SkV8Example/SkV8Example.h#newcode13 experimental/SkV8Example/SkV8Example.h:13: #include <v8.h> On 2014/01/06 16:50:59, tfarina wrote: > you ...
6 years, 11 months ago (2014-01-06 17:16:10 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/jcgregorio@google.com/121303004/480001
6 years, 11 months ago (2014-01-06 17:23:58 UTC) #12
commit-bot: I haz the power
Presubmit check for 121303004-480001 failed and returned exit status 1. Running presubmit commit checks ...
6 years, 11 months ago (2014-01-06 17:24:05 UTC) #13
jcgregorio
Need LGTM from reed for changes to SkScalar.h.
6 years, 11 months ago (2014-01-06 17:30:49 UTC) #14
reed1
lgtm
6 years, 11 months ago (2014-01-06 18:02:35 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/jcgregorio@google.com/121303004/480001
6 years, 11 months ago (2014-01-06 18:03:00 UTC) #16
commit-bot: I haz the power
6 years, 11 months ago (2014-01-06 18:17:35 UTC) #17
Message was sent while issue was closed. 
Change committed as 12913

Powered by Google App Engine
This is Rietveld 408576698