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

No goocanvas #652

Merged
merged 4 commits into from
Aug 20, 2021
Merged

No goocanvas #652

merged 4 commits into from
Aug 20, 2021

Conversation

mbfraga
Copy link
Collaborator

@mbfraga mbfraga commented Aug 19, 2021

Summary / How this PR fixes the problem?

This PR implements a custom Canvas that does not rely on goocanvas (although it is heavily insipired by it and takes code directly from it--so we still need attribution).

The goal is to have more control over drawables and performance.

Some important changes:

  • Added ViewLayer concept and rewrote managers and previous CanvasItems. This actuallly reduced code by a lot.
    • Grid (not 1:1 with master)
    • Hover (now displays the path rather than the bounding box)
    • Nobs and Selection
    • Snaps
  • Added a BaseCanvas to replace Goo.Canvas. In the future we should probably restructure and rename it.
  • Since don't use GooCanvas, the view is a direct reflection of the model...so all synchronization code can go away! This speeds up removal and sorting SIGNIFICANTLY. For remove, we go from >O(n^2) to <O(n) roughly. It scales very well! Re-stacking will be similar. And the code complexity will go down by a lot.
  • The model now takes the canvas to be able to properly request redraws. In the future it would be nice to decouple these, but for now the usage is minimal and optional.

Known Issues / Things To Do

  • There are drawing artifacts (that also existed in goocanvas. These happen when you move items around--once the full canvas redraws they go away. So they are fairly harmless for now. Still something that would be nice to get rid of.
    artifacts

@Alecaddd Alecaddd self-requested a review August 19, 2021 21:31
Copy link
Member

@Alecaddd Alecaddd left a comment

Choose a reason for hiding this comment

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

Awesome work!
This is a massive milestone!

.gitignore Outdated Show resolved Hide resolved
@Alecaddd Alecaddd merged commit 05a4317 into main Aug 20, 2021
@Alecaddd Alecaddd deleted the no_goocanvas branch August 20, 2021 02:27
@albfan
Copy link
Collaborator

albfan commented Aug 20, 2021

Congrats on this @mbfraga

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

3 participants