image-sequencer icon indicating copy to clipboard operation
image-sequencer copied to clipboard

Add arbitrary crop module

Open rishabhshuklax opened this issue 5 years ago • 9 comments

Concerns #1080

Screenshot from 2020-02-01 16-37-15

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • [x] tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with npm run test-all
  • [x] code is in uniquely-named feature branch and has no merge conflicts
  • [x] PR is descriptively titled
  • [x] ask @publiclab/is-reviewers for help, in a comment below
  • [x] Insert-step functionality is working correct as expected.

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to 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!

rishabhshuklax avatar Feb 01 '20 11:02 rishabhshuklax

@tech4GT @jywarren please review this

Also could you suggest some other name for this module, I am really bad at this :sweat_smile:

rishabhshuklax avatar Feb 01 '20 11:02 rishabhshuklax

Codecov Report

Merging #1607 into main will increase coverage by 9.64%. The diff coverage is 61.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1607      +/-   ##
==========================================
+ Coverage   55.11%   64.76%   +9.64%     
==========================================
  Files         117      134      +17     
  Lines        2344     2784     +440     
  Branches      360      447      +87     
==========================================
+ Hits         1292     1803     +511     
+ Misses       1052      981      -71     
Impacted Files Coverage Δ
examples/lib/scopeQuery.js 18.51% <ø> (ø)
src/Modules.js 100.00% <ø> (ø)
src/modules/WebglDistort/Module.js 2.29% <0.00%> (ø)
src/ui/SetInputStep.js 12.90% <0.00%> (-1.39%) :arrow_down:
src/modules/ColorHalftone/Module.js 3.63% <3.63%> (ø)
examples/lib/defaultHtmlStepUi.js 11.35% <4.08%> (-0.85%) :arrow_down:
examples/lib/intermediateHtmlStepUi.js 11.11% <5.55%> (+0.94%) :arrow_up:
examples/lib/insertPreview.js 13.15% <20.00%> (-0.36%) :arrow_down:
src/util/getImageDimensions.js 20.00% <20.00%> (ø)
src/util/isGif.js 20.00% <20.00%> (ø)
... and 104 more

codecov[bot] avatar Feb 01 '20 12:02 codecov[bot]

But there's an issue here, we can't just crop in one step and then overlay in another, coz once you export the image you'll get the black borders again. Since Images are rectangular only

@tech4GT I fixed that problem by making the alpha channel of other pixels to be 0, after this a meta module can be created for the problem in #1080 using this and overlay module, i've fixed the working of overlay module with transparent image in #1606, after this gets merged that problem can be easily solved by using this meta module

rishabhshuklax avatar Feb 01 '20 15:02 rishabhshuklax

@tech4GT I am getting this as cli output.

image_1

These were the inputs

Screenshot from 2020-02-01 21-48-24

rishabhshuklax avatar Feb 01 '20 16:02 rishabhshuklax

@tech4GT I've also added the functionality to crop the image so that the quadilateral lies in smallest possible rectangle so we don't have to compute for the extra pixels.

Screenshot from 2020-02-01 22-09-11

This is not showing desired output because extramanipulation is running before changePixel function and generating output according to it, this bug will be fixed in #1564 and it will work normally also I'd really appreciate if you could also review #1564

rishabhshuklax avatar Feb 01 '20 16:02 rishabhshuklax

@tech4GT I am getting this as cli output

image_2

This will be fixed when we merge #1606

To make overlay-subpart module we need to first add support for transparent image for overlay module then we need to develop arbitrary-crop and then the module described in #1080 can be easily made by making meta-module of arbitrary-crop and overlay! What do you think?

rishabhshuklax avatar Feb 03 '20 18:02 rishabhshuklax

@tech4GT I am getting this as cli output

image_2

This will be fixed when we merge #1606

To make overlay-subpart module we need to first add support for transparent image for overlay module then we need to develop arbitrary-crop and then the module described in #1080 can be easily made by making meta-module of arbitrary-crop and overlay! What do you think?

Yeah, this is what I was worried about! Let's merge the other PR first then!

tech4GT avatar Feb 05 '20 11:02 tech4GT

Yeah, this is what I was worried about! Let's merge the other PR first then!

Yes we need to first merge #1606 and then #1564 then finally this one

rishabhshuklax avatar Feb 05 '20 11:02 rishabhshuklax

gitpod-io[bot] avatar Jul 07 '20 16:07 gitpod-io[bot]