rotate module #1218

keshav234156 · 2019-08-20T16:18:14Z

Fixes #1137
this PR fixes the clipping of image while using rotate module

image at 30

image at 90

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

tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with npm test
code is in uniquely-named feature branch and has no merge conflicts
PR is descriptively titled
ask @publiclab/is-reviewers for help, in a comment below

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

Thanks!

codecov · 2019-08-20T16:37:19Z

Codecov Report

Merging #1218 into main will decrease coverage by 0.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##             main    #1218      +/-   ##
==========================================
- Coverage   32.07%   32.05%   -0.02%     
==========================================
  Files         107      107              
  Lines        1986     1987       +1     
  Branches      297      296       -1     
==========================================
  Hits          637      637              
- Misses       1349     1350       +1

Impacted Files	Coverage Δ
src/modules/Rotate/Module.js	`20% <0%> (-0.84%)`	⬇️

keshav234156 · 2019-08-20T16:44:54Z

@jywarren @harshkhandeparkar can you please guide me how to fix the issues given by codeclimate

harshkhandeparkar · 2019-08-20T17:03:30Z

I never fixed them till date. They will require a separate dedicated PR. don't worry about it, they can be ignored for now.

…

On Tue, 20 Aug, 2019, 10:14 PM keshav234156, ***@***.***> wrote: @jywarren <https://github.com/jywarren> @harshkhandeparkar <https://github.com/HarshKhandeparkar> can you please guide me how to fix the issues given by codeclimate — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1218?email_source=notifications&email_token=AIJI5H623C5Y3JKBI62BNE3QFQNQRA5CNFSM4INZPRVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4W5RTY#issuecomment-523098319>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIJI5H5M7W3IWL7UIWUTQ2LQFQNQRANCNFSM4INZPRVA> .

aashna27 · 2019-08-20T17:10:43Z

Yeah just make sure that the new code you add is refactored.

…

On Tue, Aug 20, 2019, 10:14 PM keshav234156 ***@***.***> wrote: @jywarren <https://github.com/jywarren> @harshkhandeparkar <https://github.com/HarshKhandeparkar> can you please guide me how to fix the issues given by codeclimate — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1218?email_source=notifications&email_token=AGKQ7SHUF2Y74BKJQK2C3JTQFQNQRA5CNFSM4INZPRVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4W5RTY#issuecomment-523098319>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGKQ7SG6UOEMG7DLP56ZAR3QFQNQRANCNFSM4INZPRVA> .

keshav234156 · 2019-08-20T17:21:55Z

@harshkhandeparkar @jywarren can you please review the changes!!

harshkhandeparkar · 2019-08-20T17:49:12Z

Nice work! I am a bit busy as of now. I guess I will review it tmrw. I'll have to test it out locally. Slowly but surely.

…

On Tue, 20 Aug, 2019, 10:51 PM keshav234156, ***@***.***> wrote: @harshkhandeparkar <https://github.com/HarshKhandeparkar> @jywarren <https://github.com/jywarren> can you please review the changes!! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1218?email_source=notifications&email_token=AIJI5H3MWNOPO2MRO5CDGGLQFQR3LA5CNFSM4INZPRVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4XBCTA#issuecomment-523112780>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIJI5H7MNUH3RA37GZTRNQDQFQR3LANCNFSM4INZPRVA> .

keshav234156 · 2019-08-26T17:29:19Z

@harshkhandeparkar can you please review if you are free

harshkhandeparkar · 2019-08-26T18:10:04Z

I am extremely sorry. I have a test on this Wednesday. I will only be able to review on that day. I will do it as soon as possible.

…

On Mon, 26 Aug, 2019, 10:59 PM keshav234156, ***@***.***> wrote: @harshkhandeparkar <https://github.com/HarshKhandeparkar> can you please review if you are free — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1218?email_source=notifications&email_token=AIJI5H5FS6LPUHLWLDJQY73QGQHHDA5CNFSM4INZPRVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5FB56A#issuecomment-524951288>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIJI5HYD4MSHLAIDTEE56CDQGQHHDANCNFSM4INZPRVA> .

harshkhandeparkar

This is perfect. Only one thing. Is it possible to make the code more readable? You could start by splitting the file into two - Module.js and Rotate.js (take inspiration from other modules)
@keshav234156

keshav234156 · 2019-08-29T17:35:42Z

@harshkhandeparkar Splitting the file inti Module.js and Rotate.js would make the code easy to understand and i think there won't be any issue.So will make change and push it asap.

…sequencer into issue-1137-2

keshav234156 · 2019-09-01T21:03:26Z

@jywarren @harshkhandeparkar have splitted it into 2 files.please have a look

src/modules/Rotate/Module.js

src/modules/Rotate/Rotate.js

harshkhandeparkar · 2019-09-02T05:33:38Z

Then why is it done again in Rotate.js?

…

On Mon, 2 Sep, 2019, 11:00 AM keshav234156, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/modules/Rotate/Module.js <#1218 (comment)> : > - - if (rotate_value % 360 == 0) - return pixels; - - var bitmap = new imagejs.Bitmap({ width: pixels.shape[0], height: pixels.shape[1] }); - bitmap._data.data = pixels.data; - - var rotated = bitmap.rotate({ - degrees: rotate_value, - }); - pixels.data = rotated._data.data; - + var radians = 3.141592653589793 * rotate_value / 180; + var width = pixels.shape[0]; + var height = pixels.shape[1]; + var cos = Math.cos(radians); @harshkhandeparkar <https://github.com/HarshKhandeparkar> i think these would still be required as we are returning pixels2 from the Rotate.js ,so it must be initialised before passing it as argument in module.js. What do you think about this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1218?email_source=notifications&email_token=AIJI5HYLTWWPMWVDBWJ7O7DQHSQHNA5CNFSM4INZPRVKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCDKS32Q#discussion_r319810336>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIJI5HZXIRX4AW7KEDM7CTLQHSQHNANCNFSM4INZPRVA> .

keshav234156 · 2019-09-02T08:28:42Z

@publiclab/is-reviewers @jywarren can you please review the PR

aashna27 · 2019-09-10T05:41:22Z

The work done is really great. Thanks. Can you document the flow so that it becomes easy to understand.

keshav234156 · 2019-09-13T16:04:50Z

@aashna27 have a look..

keshav234156 · 2019-09-13T21:18:48Z

@jywarren Have a look

keshav234156 · 2019-09-20T17:20:10Z

@aashna27 @jywarren can you please review this PR!!

jywarren · 2019-09-20T17:27:29Z

Apologies, @keshav234156 i was in a week of meetings, I'm looking now! First, i'll update to sync with the latest main branch. Then will look through the code. Thanks for your patience, we're still recovering a bit from our Summer of Code wrap-up!

jywarren · 2019-09-20T17:33:05Z

src/modules/Rotate/Rotate.js

+
+  if (rotate_value % 360 == 0)
+    return pixels;
+  function copyPixel(x1, y1, x2, y2){


Hi, couldn't this get extra pixels1, pixels2 parameters and then we'd have only one function instead of 2?

ok, would do the changes to make a 1 function.

@jywarren any other changes that are required??

@jywarren Had done the changes to make a single function instead of 2

jywarren · 2019-09-20T17:35:00Z

src/modules/Rotate/Rotate.js

+  var bitmap = new imagejs.Bitmap({ width: pixels1.shape[0], height: pixels1.shape[1] });
+  bitmap._data.data = pixels1.data;
+
+  var rotated = bitmap.rotate({


Hmm, so why do we need to do so much pixel copying and rotation math if we are still using the Imagejs Bitmap function to perform the rotation? Perhaps some comments would help clarify what's going on here? Sorry, it's just a little hard to follow! Thanks for your help, i love the output, just being sure I understand what's happening inside as well :-)

earlier if we use bitmap on original image then it cropped some of the image.to solve this i have put the image on a larger frame ie is square so that there is no problem of clipping,rotated the larger frame image with the help of imagejs rotation function and then extra white is cropped according to to final image dimensions after rotation

pixel1 is a square frame that contain original image at it's centre.then we rotated the pixel1 with help of imagejs,at the last we copied pixels from pixel1 to pixel2 so that there is no extra whitespace.

keshav234156 · 2019-09-23T15:04:58Z

@jywarren if there any other changes or method in your mind please let me know!!.would love to solve this issue.

jywarren · 2019-09-24T20:44:59Z

Looks great. Thanks so much!

* this PR solves the problem of clipping of the image while rotating * a little clipping was still present .so made the changes * Splitted into two files * ommitted extra declared variables * Update Module.js * Update Rotate.js * Update Rotate.js

this PR solves the problem of clipping of the image while rotating

0c91f92

a little clipping was still present .so made the changes

3d29ead

Merge branch 'main' into issue-1137-2

e66c7cf

harshkhandeparkar reviewed Aug 29, 2019

View reviewed changes

keshav234156 added 2 commits September 2, 2019 02:14

Splitted into two files

7f86056

Merge branch 'issue-1137-2' of https://github.com/keshav234156/image-…

c483a82

…sequencer into issue-1137-2

harshkhandeparkar requested changes Sep 2, 2019

View reviewed changes

src/modules/Rotate/Module.js Show resolved Hide resolved

src/modules/Rotate/Rotate.js Outdated Show resolved Hide resolved

ommitted extra declared variables

2eb4101

harshkhandeparkar approved these changes Sep 2, 2019

View reviewed changes

keshav234156 mentioned this pull request Sep 4, 2019

Bugs in AddQR,Draw-rectangle,Grid-overlay after #1188 #1247

Closed

3 tasks

keshav234156 added 2 commits September 13, 2019 21:28

Update Module.js

d68ea53

Update Rotate.js

908ab66

Merge branch 'main' into issue-1137-2

754cc3b

jywarren reviewed Sep 20, 2019

View reviewed changes

Update Rotate.js

6e618b5

Merge branch 'main' into issue-1137-2

b0e6c53

jywarren approved these changes Sep 24, 2019

View reviewed changes

jywarren merged commit 3eb741c into publiclab:main Sep 24, 2019

keshav234156 mentioned this pull request Nov 30, 2019

"Rotate " module changing original dimensions of image #1322

Closed

harshkhandeparkar added this to Done in v3.6.0 Oct 17, 2020

harshkhandeparkar removed this from Done in v3.6.0 Oct 17, 2020

harshkhandeparkar added this to Done in v3.6.0 Oct 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rotate module #1218

rotate module #1218

keshav234156 commented Aug 20, 2019 •

edited

Loading

codecov bot commented Aug 20, 2019 •

edited

Loading

keshav234156 commented Aug 20, 2019

harshkhandeparkar commented Aug 20, 2019 via email

aashna27 commented Aug 20, 2019 via email

keshav234156 commented Aug 20, 2019

harshkhandeparkar commented Aug 20, 2019 via email

keshav234156 commented Aug 26, 2019

harshkhandeparkar commented Aug 26, 2019 via email

harshkhandeparkar left a comment

keshav234156 commented Aug 29, 2019

keshav234156 commented Sep 1, 2019

harshkhandeparkar commented Sep 2, 2019 via email

keshav234156 commented Sep 2, 2019 •

edited by harshkhandeparkar

Loading

aashna27 commented Sep 10, 2019

keshav234156 commented Sep 13, 2019

keshav234156 commented Sep 13, 2019

keshav234156 commented Sep 20, 2019

jywarren commented Sep 20, 2019

jywarren Sep 20, 2019

keshav234156 Sep 20, 2019

keshav234156 Sep 20, 2019

keshav234156 Sep 20, 2019

jywarren Sep 20, 2019

keshav234156 Sep 20, 2019 •

edited

Loading

keshav234156 Sep 20, 2019 •

edited

Loading

keshav234156 commented Sep 23, 2019

jywarren commented Sep 24, 2019

rotate module #1218

rotate module #1218

Conversation

keshav234156 commented Aug 20, 2019 • edited Loading

codecov bot commented Aug 20, 2019 • edited Loading

Codecov Report

keshav234156 commented Aug 20, 2019

harshkhandeparkar commented Aug 20, 2019 via email

aashna27 commented Aug 20, 2019 via email

keshav234156 commented Aug 20, 2019

harshkhandeparkar commented Aug 20, 2019 via email

keshav234156 commented Aug 26, 2019

harshkhandeparkar commented Aug 26, 2019 via email

harshkhandeparkar left a comment

Choose a reason for hiding this comment

keshav234156 commented Aug 29, 2019

keshav234156 commented Sep 1, 2019

harshkhandeparkar commented Sep 2, 2019 via email

keshav234156 commented Sep 2, 2019 • edited by harshkhandeparkar Loading

aashna27 commented Sep 10, 2019

keshav234156 commented Sep 13, 2019

keshav234156 commented Sep 13, 2019

keshav234156 commented Sep 20, 2019

jywarren commented Sep 20, 2019

jywarren Sep 20, 2019

Choose a reason for hiding this comment

keshav234156 Sep 20, 2019

Choose a reason for hiding this comment

keshav234156 Sep 20, 2019

Choose a reason for hiding this comment

keshav234156 Sep 20, 2019

Choose a reason for hiding this comment

jywarren Sep 20, 2019

Choose a reason for hiding this comment

keshav234156 Sep 20, 2019 • edited Loading

Choose a reason for hiding this comment

keshav234156 Sep 20, 2019 • edited Loading

Choose a reason for hiding this comment

keshav234156 commented Sep 23, 2019

jywarren commented Sep 24, 2019

keshav234156 commented Aug 20, 2019 •

edited

Loading

codecov bot commented Aug 20, 2019 •

edited

Loading

keshav234156 commented Sep 2, 2019 •

edited by harshkhandeparkar

Loading

keshav234156 Sep 20, 2019 •

edited

Loading

keshav234156 Sep 20, 2019 •

edited

Loading