publiclab / image-sequencer

A pure JavaScript sequential image processing system, inspired by storyboards
https://sequencer.publiclab.org
GNU General Public License v3.0
110 stars 208 forks source link

Better Approach to Using Modules Internally Inside Other Modules #1560

Open harshkhandeparkar opened 4 years ago

harshkhandeparkar commented 4 years ago

Please describe the problem (or idea)

Code similar to the following was used earlier for EdgeDetect and was used in one of the PRs https://github.com/publiclab/image-sequencer/blob/ef75475238c937994e7184aa0c58c59bbb80d355/src/modules/Mask/Module.js#L33-L39

What happens:

  1. A new instance of IS is created internally.
  2. Internal sequencer loads the data URI of the image that was already loaded by the main sequencer
  3. Internal Sequencer passes the data URI to the resize module.
  4. Resize module reads the data URI and converts it to a ndarray of pixels.
  5. Pixels are manipulated
  6. Pixels are converted back to a data URI
  7. The data URI returned by the internal sequencer is converted back to a ndarray and then used further.

The following code uses the Blur module inside the edge-detect module https://github.com/publiclab/image-sequencer/blob/ecaa3ecd131a794ecf7c5d799542e437d992f25d/src/modules/EdgeDetect/Module.js#L1 https://github.com/publiclab/image-sequencer/blob/ecaa3ecd131a794ecf7c5d799542e437d992f25d/src/modules/EdgeDetect/Module.js#L31-L33

What happens:

  1. Pixels ndarray are passed into the blur module.
  2. Blur module manipulates the pixels ndarray directly.
  3. Returns the new, manipulated pixels.

The first approach is not optimized and has a lot of overhead and unnecessary conversion. The second approach is the best possible approach.

BUT the first approach cannot be used in every scenario. Because:

  1. Not every module exports a function like the Blur function.
  2. In the internal sequencer code, the image returned by the previous step is being accessed via its data URI only. We need a way to access the pixels directly. OR We have to create a new util function that can parse the data URI of the old image on demand and return the pixels directly. This function should be a part of PixelManipulation and should be passed as a parameter to extraManipulation. This is the reason why some modules do not work with GIFs (the old steps' pixels are difficult to access frame by frame).

It would be helpful if someone creates two separate issues out of this.

Thank you!

Your help makes Public Lab better! We deeply appreciate your helping refine and improve this site.

To learn how to write really great issues, which increases the chances they'll be resolved, see:

https://publiclab.org/wiki/developers#Contributing+for+non-coders

harshkhandeparkar commented 4 years ago

@jywarren @publiclab/is-reviewers

jywarren commented 4 years ago

Thanks!!!

For this:

New way to access earlier steps' pixels.

I long ago had an issue which I can't find, which had the idea that if two neighboring steps BOTH agreed, they could pass a different image format, and skip the data-url encoding/decoding step. But both would have to agree on the format, like ndarray, and we'd probably have to clearly and very thoroughly document and test the possible formats to agree upon before allowing this kind of inter-step negotiation. Maybe the same kind of thing could be used for nested modules?