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

fix: handle differing source and result sizes in updateCanvas #270

Closed
wants to merge 1 commit into from

Conversation

DerZade
Copy link
Contributor

@DerZade DerZade commented Mar 16, 2024

updateCanvas has a bug which only really comes to light if the size of the canvas and the size of the original image differ drastically and the image offset is positive. The dx and dy are not adjusted for the size difference.

Here is a visual representation:
Frame 22 (1)
Since the output (100px) is half the size of the input (200px) the dx and dy have to be halfed.


The same issue seems to exist in the master branch. I'll gladly also create a PR for the master branch, once this is merged :)

handle differing source and results sizes in updateCanvas when the coordinates left and top are less than 0
@DerZade
Copy link
Contributor Author

DerZade commented Apr 17, 2024

This has been open for over a month now.

@Norserium anything I can do to get this merged?

@DerZade
Copy link
Contributor Author

DerZade commented May 13, 2024

This has been open for over two months now.

@Norserium anything I can do to get this merged?

@Norserium
Copy link
Collaborator

@DerZade, I will look this pull request on this weekend.

@Norserium
Copy link
Collaborator

@DerZade, your pull request is great, thank your for the important fix!

There is only one nuance. The changes that common to both versions are pushed to master, then I rebase vue-next on master. It's not the perfect flow, but it works.

Don't you mind if I commit this change by myself? If you do mind, please create the pull request from master branch.

@DerZade
Copy link
Contributor Author

DerZade commented Jun 8, 2024

Closed in favor of #276

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.

None yet

2 participants