Node.js sample (dev) #22

ccpandhare · 2017-04-03T06:31:05Z

Hey @jywarren, I made a small demo for the node.js implementation.
I'm not sure if it should be merged, but I wanted you to have a look at it.
To keep things clean, I have kept the Node.js version's code away from the working browser code. This is not how things will work out eventually, there will only be a single script. This is just for demonstration.

I am adding the suffix Node to all the new files generated in this PR. So there is an 'ImageSequencerNode.js', 'ModulesNode.js', etc. Also, I have added a demo module for Node, called DoNothingNode. It literally does nothing. just takes the input image and returns it as-is. This is all for testing purposes. So the only module which will currently work is DoNothingNode.

Also, this PR contains work from my other PR #17, the removeStep() one too.

First, get the additional dependancies installed (sharp and the likes)

$ npm install

Then run node terminal, requiring the index.js file beforehand.

$ node -r ./index

Now the node terminal opens, with index.js already required.

Type in the following in the node terminal to test ImageSequencer:

> sequencer
> sequencer.steps
> sequencer.addStep('do-nothing')
> sequencer.removeStep(0)
> sequencer.addStep('do-nothing')
> sequencer.steps
> sequencer.steps[0].get()

Note that the removeStep accepts the id parameter, one which I have defined in PR #17 . It is a unique auto-incrementing key which starts from 0.

ccpandhare · 2017-04-03T08:00:35Z

The build fails on Travis as TravisCI uses an older version of the dependancies required by Sharp.

jywarren · 2017-04-03T16:25:01Z

hmm, can you specify the sharp version number more narrowly?

…

On Mon, Apr 3, 2017 at 4:00 AM, Chinmay Pandhare (CCD) < ***@***.***> wrote: The build fails on Travis as TravisCI uses an older version of the dependancies required by Sharp. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#22 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ-fty8oUjEtWTjJlrcoDPcXwf1ykks5rsKckgaJpZM4MxI3f> .

ccpandhare · 2017-04-04T15:41:49Z

I tried doing that... Turns out it doesn't help.
Any suggestions on how to overcome this?

jywarren · 2017-04-04T22:40:34Z

hmm, i'll take a look tonight or tomorrow night, i hope! Sorry, i'm a bit behind on things since I just finished most of my travel, but will do my best to catch up.

…

On Tue, Apr 4, 2017 at 11:41 AM, Chinmay Pandhare (CCD) < ***@***.***> wrote: I tried doing that... Turns out it doesn't help. Any suggestions on how to overcome this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#22 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ8osDoHBtXvtIUQQNXRT_5AvtrM-ks5rsmS9gaJpZM4MxI3f> .

ccpandhare · 2017-04-05T15:44:36Z

Thanks a lot!

ccpandhare · 2017-04-22T15:03:10Z

@jywarren Could you have a look at this? Had University exams so couldn't have a look, sorry.

jywarren · 2017-04-24T15:39:34Z

sure, sorry -- been busy too! Will take a look today or tomorrow, thanks!

jywarren · 2017-04-26T19:19:01Z

Still trying to find a moment, my sincere apologies!

ccpandhare · 2017-04-27T04:28:02Z

Hey!
No issue at all :)

jywarren · 2017-05-04T01:15:43Z

So, I think part of the problem may be that Sharp is not pure JavaScript, so I'm concerned that it bifurcates how things are run too much. I know that we can use the canvas for in-browser usage, but I do think we can bypass some of the problems with Sharp using Jimp. I know of other projects using Jimp too.

I appreciate the analysis done in this comment, but do you think attempting a Jimp based solution would address some of the issues and need for native compilation in this pull request?

jywarren · 2017-05-04T01:17:10Z

Alternatively we could try to debug Sharp by searching for "Sharp in Travis" or looking closely at their documentation here: http://sharp.dimens.io/en/stable/install/

ccpandhare · 2017-05-28T04:51:30Z

Hey @jywarren
Was able to fix the issue.
Turns out all I had to do was set a minimum c++ compiler version in .travis.yml by doing this:

language: node_js
node_js:
  - '4'
  - '5'
env:
  - CXX=g++-4.8
script: npm test
addons:
  apt:
    sources:
      - ubuntu-toolchain-r-test
    packages:
      - g++-4.8

jywarren · 2017-05-31T00:54:01Z

Oh wow! Looking at the changes here now.

jywarren · 2017-05-31T00:54:57Z

build/config.gypi

@@ -0,0 +1,67 @@
+# Do not edit. File was generated by node-gyp's "configure" step


Is this an auto-generated file we should not be git tracking? Would you mind looking this up to see what the best practice is? Thanks!

jywarren · 2017-05-31T00:57:04Z

src/ImageSequencerNode.js

@@ -0,0 +1,142 @@
+ImageSequencer = function ImageSequencer(options) {


Hi, just checking - is this instead of another module, and how much redundancy is there? Or is this just a core module which is then included into another? I just want to be sure we don't replicate too much code if we could be using require() -- and it's been a few weeks :-P -- can you help me understand? Thank you!! Exciting to see work starting up!

jywarren · 2017-05-31T00:57:46Z

src/ImageSequencerNode.js

+    return 'Addded.';
+  }
+
+  function removeStep (id) {


Yes, i'm concerned about redundancy -- isn't this the same code as this: https://github.com/publiclab/image-sequencer/pull/22/files#diff-f887b503b80e10727ac6cee436178fc5R83 ?

jywarren · 2017-05-31T00:58:41Z

src/modules/DoNothingNode.js

+/*
+ * Demo Module. Does nothing.
+ */
+module.exports = function DoNothing(options) {


Hm, was this for testing purposes, or can we add a bit more to the description to explain why it's needed, so future programmers don't get confused? Thanks!

Yes, this was for testing purposes... This is a demo module which gives the output image as it is...

ccpandhare · 2017-06-03T15:07:21Z

I have shifted to a different branches and things have changed quite a lot. Also, as this was a demo, I wish to close this pull request. I am creating another pull request.

ccpandhare added 30 commits March 11, 2017 09:28

Added Crop Module

5cbfca8

lowercase parameters

a5f4484

Fix : add a step in the demo

7557c9e

Change global variable to document.sequencer_image

f695e65

Change global variable to document.sequencer_image

b107f22

Small fix for FireFox; Ref: Issue publiclab#7

633f1f3

Change document.sequencer_image to options.image

6f7d2bf

Change document.sequencer_image to options.initial_image

8b5c834

Change document.sequencer_image to options.initial_image

f2e8062

Completed the addStep issue.

aba3ba1

Completed the addStep issue.

3587e9e

Change options.initial_image to options.initialImage

732df84

Added usage notes, changed a variable name.

2dd47a7

Added Crop Module

6faeca2

lowercase parameters

d8bdbcd

Added usage notes, changed a variable name.

1dfa8f5

Merging Crop.js

7f44792

Merging Crop.js

6e78b90

Merging Crop.js

b1bdbd1

Merging Crop.js

90da255

Final changes to removeStep

f099c1f

Final changes to removeStep

8765f37

Merge branch 'master' of https://github.com/publiclab/image-sequencer

cd19a17

Merging publiclab:master in

99e8f10

Some changes for removeStep()

692e570

buildfix for non-browsers

b802658

Fix build error.

2dbd881

Fix build error.

7c8b320

Final Fixes for removeStep() build error

f5ac3f4

Moved the .remove to UserInterface.js

4cf5bfa

ccpandhare added 3 commits March 31, 2017 02:38

node test

c0ca135

Finishing demo

f11cd4a

buildfix

708b546

Fix TravisCI Dependancies

9a1132c

jywarren requested changes May 31, 2017

View reviewed changes

jywarren mentioned this pull request May 31, 2017

Added removeStep() functionality #17

Merged

ccpandhare closed this Jun 3, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Node.js sample (dev) #22

Node.js sample (dev) #22

ccpandhare commented Apr 3, 2017

ccpandhare commented Apr 3, 2017

jywarren commented Apr 3, 2017 via email

ccpandhare commented Apr 4, 2017

jywarren commented Apr 4, 2017 via email

ccpandhare commented Apr 5, 2017

ccpandhare commented Apr 22, 2017

jywarren commented Apr 24, 2017 via email

jywarren commented Apr 26, 2017

ccpandhare commented Apr 27, 2017

jywarren commented May 4, 2017

jywarren commented May 4, 2017

ccpandhare commented May 28, 2017

jywarren commented May 31, 2017

jywarren May 31, 2017

jywarren May 31, 2017

jywarren May 31, 2017

jywarren May 31, 2017

ccpandhare Jun 3, 2017

ccpandhare commented Jun 3, 2017

		@@ -0,0 +1,67 @@
		# Do not edit. File was generated by node-gyp's "configure" step

		@@ -0,0 +1,142 @@
		ImageSequencer = function ImageSequencer(options) {

Node.js sample (dev) #22

Node.js sample (dev) #22

Conversation

ccpandhare commented Apr 3, 2017

ccpandhare commented Apr 3, 2017

jywarren commented Apr 3, 2017 via email

ccpandhare commented Apr 4, 2017

jywarren commented Apr 4, 2017 via email

ccpandhare commented Apr 5, 2017

ccpandhare commented Apr 22, 2017

jywarren commented Apr 24, 2017 via email

jywarren commented Apr 26, 2017

ccpandhare commented Apr 27, 2017

jywarren commented May 4, 2017

jywarren commented May 4, 2017

ccpandhare commented May 28, 2017

jywarren commented May 31, 2017

jywarren May 31, 2017

Choose a reason for hiding this comment

jywarren May 31, 2017

Choose a reason for hiding this comment

jywarren May 31, 2017

Choose a reason for hiding this comment

jywarren May 31, 2017

Choose a reason for hiding this comment

ccpandhare Jun 3, 2017

Choose a reason for hiding this comment

ccpandhare commented Jun 3, 2017