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

Fixes #3847 Bug Inconsistent zoom behavior #4148

Merged
merged 5 commits into from
Dec 20, 2024

Conversation

jivansh77
Copy link
Contributor

Initially, blocks would get stuck when Zooming In/Out.

Key Changes:

  • Added immediate canvas refresh after block scale changes
  • Added bounds checking to ensure proper block positioning after resize

@jivansh77
Copy link
Contributor Author

jivansh77 commented Dec 16, 2024

@walterbender Please Review, issue is #3847

@walterbender
Copy link
Member

How did you test this?

@jivansh77
Copy link
Contributor Author

@walterbender Initially, the blocks would get stuck when zooming in/out

Screen.Recording.2024-12-19.at.9.52.00.AM.mp4

@jivansh77
Copy link
Contributor Author

jivansh77 commented Dec 19, 2024

@walterbender Just refreshing the canvas did not solve the issue, so I added a clearcache method and stage update. I have tested with multiple blocks, the zoom is working fine now.

Screen.Recording.2024-12-19.at.11.40.18.AM.mp4

@walterbender
Copy link
Member

It seems to work for the most part but maybe there is still an issue with some of the block artwork.
Screenshot From 2024-12-19 16-50-43

@jivansh77
Copy link
Contributor Author

jivansh77 commented Dec 20, 2024

Yes it seems I have to update cache for bitmap also. @walterbender pls review

Screen.Recording.2024-12-20.at.1.42.50.PM.mp4

@jivansh77 jivansh77 marked this pull request as draft December 20, 2024 11:59
@jivansh77
Copy link
Contributor Author

@walterbender Any idea why the smoke test is failing

@jivansh77 jivansh77 marked this pull request as ready for review December 20, 2024 14:09
@walterbender
Copy link
Member

It is unrelated to your PR. It is a misconfiguration somewhere.

@jivansh77
Copy link
Contributor Author

jivansh77 commented Dec 20, 2024

@walterbender Yes, I figured, so far the only issue I am facing is when zooming multiple times in quick succession it cannot update the cache, which causes some of the blocks to get stuck. So shall I add a debounce of 250ms to prevent this. Otherwise, it works as expected

@walterbender
Copy link
Member

A debounce is a good idea.

@jivansh77
Copy link
Contributor Author

@walterbender Pls review

@walterbender
Copy link
Member

Something is not right. It was working much better before. Now it is slow and doesn't complete.
Screenshot From 2024-12-20 12-23-34

@jivansh77
Copy link
Contributor Author

jivansh77 commented Dec 20, 2024

@walterbender Yes, I noticed too, this happens bcos I called the container.cache(). If I don't, it works much better but then I am getting these errors in the console for some reason. I think it might be bcos of some EaseIJS requirement. But I'm trying to find a way around this.
image
image

@jivansh77
Copy link
Contributor Author

jivansh77 commented Dec 20, 2024

@walterbender I have figured out a way, by updating the refreshCanvas function, it now works the way it was but without any errors, pls review

@walterbender walterbender merged commit 29ef513 into sugarlabs:master Dec 20, 2024
2 of 4 checks passed
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.

2 participants