Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 2858 - Will no longer attempt to redraw images that are already of the correct rotation #2961

Merged
merged 7 commits into from
Apr 15, 2019

Conversation

MiloTodt
Copy link
Contributor

@MiloTodt MiloTodt commented Apr 2, 2019

Description

Currently, Boostnote calls a "fixRotate" function on all dropped images. This works by creating a canvas, redrawing the image, and then saving the canvas. This means that all images were being saved as base64, which inflated filesizes.

This fix makes it so that any image that is already of the correct rotation (the vast majority of them) does not go through the fixRotate call, and their filesize does not increase.

Before import:
1

During import (the console.logs have been removed before submission):
2

After importing the pictures and exporting the note. The filesize of the image that had correct rotation in the first place is not changed. Note how the one which had the wrong rotation has grown.

3

Issue fixed

fix #2858

Type of changes

  • ✔️ Bug fix (Change that fixed an issue)
  • ⚪ Breaking change (Change that can cause existing functionality to change)
  • ⚪ Improvement (Change that improves the code. Maybe performance or development improvement)
  • ⚪ Feature (Change that adds new functionality)
  • ⚪ Documentation change (Change that modifies documentation. Maybe typo fixes)

Checklist:

  • ✔️ My code follows the project code style
  • ⚪ I have written test for my code and it has been tested
  • ✔️ All existing tests have been passed
  • ✔️ I have attached a screenshot/video to visualize my change if possible

@ZeroX-DG ZeroX-DG added the awaiting review ❇️ Pull request is awaiting a review. label Apr 4, 2019
Copy link
Member

@ZeroX-DG ZeroX-DG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small adjustment needs to be made 😄

browser/main/lib/dataApi/attachmentManagement.js Outdated Show resolved Hide resolved
@ZeroX-DG ZeroX-DG added awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. and removed awaiting review ❇️ Pull request is awaiting a review. labels Apr 4, 2019
@MiloTodt
Copy link
Contributor Author

MiloTodt commented Apr 8, 2019

The pipeline is failing, but I don't think it's my code doing it? Yarn isn't able to find a package on the test server.
image

Copy link
Member

@ZeroX-DG ZeroX-DG left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎉 I have restarted the pipeline to apply the fix and everything is fine now.

@ZeroX-DG ZeroX-DG added approved 👍 Pull request has been approved by sufficient reviewers. and removed awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. labels Apr 13, 2019
@ZeroX-DG ZeroX-DG requested a review from Rokt33r April 13, 2019 23:31
@ZeroX-DG ZeroX-DG added the needs extra review 🔎 Pull request requires review from an additional reviewer. label Apr 13, 2019
@Rokt33r Rokt33r removed needs extra review 🔎 Pull request requires review from an additional reviewer. approved 👍 Pull request has been approved by sufficient reviewers. labels Apr 15, 2019
@Rokt33r Rokt33r added this to the v0.11.15 milestone Apr 15, 2019
@Rokt33r Rokt33r merged commit d416631 into BoostIO:master Apr 15, 2019
@MiloTodt MiloTodt deleted the issue_2858.image_conversion branch April 15, 2019 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Attached image files grow in size and get corrupted meta info
3 participants