Remove stateful concept ( without passing for deprecation first ). #7920
Conversation
| } | ||
| } | ||
| return false; | ||
| }, |
There was a problem hiding this comment.
Why this goes all away?
https://github.com/fabricjs/fabric.js/pull/7920/files#diff-216444c1bc3837d560c8241c4d9643b6de8c147095081c5b38715b9e058c24cbL427
if is not statefulCache, return false, stop there.
The rest of the code was executed only if the group had statefullCache activated. Since it is no more possible to activate it, all of this function becomes just a callSuper, and so it can be removed.
There was a problem hiding this comment.
I don't understand and I don't even know what statefulCache is about
There was a problem hiding this comment.
statefulCache determine that there is a cache invalidation comparing all the properties of the object with a copy of those when the cache was created. It is slow bothersome since you are looking for a fast iteration over objects and render a cached copy instead you have to navigate the object structure and verify if something changed.
There was a problem hiding this comment.
I'm not sure I need to answer this question.
Group invalidation is done in Object#_set, this piece of code is an artifact, and a damaging one too.
There was a problem hiding this comment.
it wasn't a question :D it was more what i was trying to answer to with the comment.
I would expect a reader does not get automatically why a large chunk of code is deleted
There was a problem hiding this comment.
I'm not sure I need to answer this question. Group invalidation is done in
Object#_set, this piece of code is an artifact, and a damaging one too.
Group invalidation works if you use set. if you didn't use it, this piece of code was looking for you, with an extensive comparing of the children objects. From today you are forced to use set
There was a problem hiding this comment.
OK now I get it
Question:
Why this goes all away?
Answer:
asturur
changed the title
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs 😔. Thank you for your contributions. |
Stateful requires cloning and keep track of copies of properties.
It extends to
Now statefulCache requires a lot of deep checks and is a bit slow, is disabled by default since long and is easily not needed if the developer uses the set method to set props and values. An easy let go.
object:modified was weird.
You would set fill = red from blue. Nothing would happen.
Then if you would execute a transformation, with no results color change would still be detected and object modified would fire. Was partial and a bit useless.
As for the cache, we could fire
object:modifedwhen using set method over the sateProperties ( to be discussed )For invalidating text size changes we will be clear in docs that now using the set method is necessary, and we will use the same logic we use for dirty prop.
Missing
[ ] adapt tests
[ ] Write text alternative