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

[Synfig Studio] Minor cleanup on LayerTree #1979

Merged
merged 3 commits into from Mar 3, 2021

Conversation

rodolforg
Copy link
Contributor

@rodolforg rodolforg commented Jan 12, 2021

LayerTree is a fake widget. Its goal is to provide the (real) layer and parameter TreeViews. It isn't supposed to add it to any dialog or container. However, it contains deprecated and unused widgets from the time it was a real "widget".

I removed its unused/invisible/useless inner widgets and their signal handling. It speeds up a lot the layer movements by drag'n drop. For example, studio::LayerTree::on_blend_method_changed() was called at least 24 times while dragging a layer row in the Layer Panel.

After that, I saw on_dirty_preview() was empty and it wouldn't make sense to update neither Layer nor Parameter Panels if CanvasInterface emits this signal: LayerTreeStore already monitors layer changes (adding/removing/raising/lowering/…) Therefore, I removed on_dirty_preview() too.

Finally, quick_layer was a variable only used in the on_dirty_preview(), but, as everything there, it was commented out. So I deleted it too.

We can clean it up more: split into a real LayerTree and an isolated (ie, in other file) ParamTree. But I thought it would a big change to backport it to 1.4.1 if I did more.

This PR was inspired by Jose-Moreno comments in #540 (comment) and speeds up layer drag'n drop.

@rodolforg
Copy link
Contributor Author

rodolforg commented Jan 12, 2021

Of course, it could/should be squashed before merge.
I stopped here for better/easier review and backporting to 1.4.1

@rodolforg
Copy link
Contributor Author

Maybe it helps #525 too

@rodolforg rodolforg changed the title [Synfig Studio] Simplify LayerTree [Synfig Studio] Minor cleanup on LayerTree Jan 16, 2021
@rodolforg
Copy link
Contributor Author

To be clear: this PR solves the problem mentioned by @Jose-Moreno here: https://youtu.be/A3D3mSo3ff0?t=579

@ice0
Copy link
Collaborator

ice0 commented Feb 22, 2021

To be clear: this PR solves the problem mentioned by @Jose-Moreno here: https://youtu.be/A3D3mSo3ff0?t=579

@rodolforg Does it solve it or just hides because of faster speed?

I tried to reproduce the problem using @Jose-Moreno captured mouse clicks and was able to reproduce it with the stable 1.4.0 version. Unfortunately when I build debug version I cannot reproduce the problem because I have a different GTK version on my local machine, and the GUI is slightly different so script just clicks on wrong places.

P.S. I assume that Synfig crashes when some exception occurred inside signal handler.

@rodolforg
Copy link
Contributor Author

I didn't say it solves the crash ;)

At that specific time on the video, he pointed the strange delay when we move a layer up/down.

That's what I fixed.

@ice0
Copy link
Collaborator

ice0 commented Feb 22, 2021

I didn't say it solves the crash ;)

Ok. I want to ask @Jose-Moreno for help again. After that I will merge this PR.

@Jose-Moreno
Copy link

@ice0 @rodolforg Hey. I've been reading your messages today. I'll gladly test Synfig once you review the other issue and merge any patches. 💪

Regarding this PR I'll test it as well later on when a dev build has been made as I can't currently spare time to setup the compilation environment required for Synfig 😅

@rodolforg
Copy link
Contributor Author

Regarding this PR I'll test it as well later on when a dev build has been made

Hey @Jose-Moreno ! you can test any PR on Windows. Just follow the steps described here: https://synfig.readthedocs.io/en/latest/how_to_help/testing_prs.html

IIRC you'll need the release package extracted in a folder, and overwrite it.

@ice0
Copy link
Collaborator

ice0 commented Feb 23, 2021

as I can't currently spare time to setup the compilation environment required for Synfig

I will prepare all the necessary binaries for you :)

@ice0 ice0 force-pushed the studio-simplify-layer-tree-1 branch from 4951d7f to ef37687 Compare March 2, 2021 09:11
@ice0 ice0 added this to Fixed in v1.4.1 via automation Mar 2, 2021
@ice0 ice0 merged commit c2371f7 into synfig:master Mar 3, 2021
v1.4.1 automation moved this from Fixed to Ready to backport Mar 3, 2021
@ice0
Copy link
Collaborator

ice0 commented Mar 3, 2021

Merged. Thank you!

@rodolforg rodolforg mentioned this pull request Apr 16, 2021
rodolforg added a commit to rodolforg/synfig that referenced this pull request Apr 17, 2021
rodolforg added a commit to rodolforg/synfig that referenced this pull request Apr 21, 2021
rodolforg added a commit to rodolforg/synfig that referenced this pull request Apr 26, 2021
@morevnaproject morevnaproject removed this from Ready to backport in v1.4.1 May 10, 2021
@morevnaproject morevnaproject added this to Ready to backport in v1.4.2 May 10, 2021
rodolforg added a commit to rodolforg/synfig that referenced this pull request May 15, 2021
rodolforg added a commit to rodolforg/synfig that referenced this pull request May 26, 2021
rodolforg added a commit to rodolforg/synfig that referenced this pull request May 27, 2021
@rodolforg rodolforg moved this from Ready to backport to Backported in v1.4.2 May 29, 2021
@morevnaproject morevnaproject moved this from Backported to Release Notes in v1.4.2 Jul 12, 2021
morevnaproject pushed a commit to morevnaproject/synfig that referenced this pull request Aug 9, 2021
morevnaproject pushed a commit to morevnaproject/synfig that referenced this pull request Aug 9, 2021
@rodolforg rodolforg deleted the studio-simplify-layer-tree-1 branch November 2, 2021 12:47
@rodolforg rodolforg removed the Backport label Feb 2, 2024
@rodolforg rodolforg added this to the v1.4.2 milestone Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
v1.4.2
Release Notes
Development

Successfully merging this pull request may close these issues.

None yet

3 participants