Added Sketch Module #1450
Added Sketch Module #1450
Conversation
|
@publiclab/is-reviewers Will refactor after your reviews |
Codecov Report
@@ Coverage Diff @@
## main #1450 +/- ##
==========================================
+ Coverage 55.11% 64.29% +9.17%
==========================================
Files 117 129 +12
Lines 2344 2627 +283
Branches 360 419 +59
==========================================
+ Hits 1292 1689 +397
+ Misses 1052 938 -114
|
src/modules/Sketch/Sketch.js
Outdated
| }; | ||
| } | ||
| Sketcher.prototype = { | ||
| transformCanvas: function (canvas) { |
There was a problem hiding this comment.
This is using canvas transform, would it work in node?
There was a problem hiding this comment.
I will have to see that, @harshkhandeparkar do you know anything about this?
There was a problem hiding this comment.
It won't work in node. You either need to use puppeteer (refer to the WebGLDistort module) or temporarily, just add a requires field to the info.json which will skip the tests for the module (refer to TextOverlay).
Ideally, we want every module to work in node, and use puppeteer to make it work.
Thanks.
src/modules/Sketch/Sketch.js
Outdated
| var imageData = context.getImageData(0, 0, width, height); | ||
| var pixels = imageData.data; | ||
|
|
||
| this.sendProgressUpdate(0, 'Calculating required textures'); |
There was a problem hiding this comment.
Is this being used anywhere?
There was a problem hiding this comment.
Yes it is visible in the console.
There was a problem hiding this comment.
Is it required though? Or only for debugging purposes?
There was a problem hiding this comment.
Then I think it should be removed. We don't want unnecessary logs.
| "description": "Converts image to its sketch.", | ||
| "inputs": { | ||
| }, | ||
| "docs-link":"" |
There was a problem hiding this comment.
It'd be great if you could also write the docs for this module, You can use https://github.com/geraintluff/canvas-sketch/blob/master/README.md for reference and also It'd be great if we could give credits to geraintluff somewhere.
| @@ -0,0 +1,8 @@ | |||
| { | |||
| "name": "sketch", | |||
There was a problem hiding this comment.
I think you are using canvas so please add a requires field in the info.json. Please refer to the info.json of text overlay and other modules which use canvas.
|
|
Fixes #993
Concerns #694
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
npm test@publiclab/is-reviewersfor help, in a comment belowIf tests do fail, click on the red
Xto learn why by reading the logs.Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Please make sure to get at least two reviews before asking for merging the PR as that would make the PR more reliable on our part
Thanks!