[Merged] Refactoring ViewProviderSketch
Forum rules
Be nice to others! Respect the FreeCAD code of conduct!
Be nice to others! Respect the FreeCAD code of conduct!
[Merged] Refactoring ViewProviderSketch
Hi!
This is an announcement that I am initiating a substantial refactoring of ViewProviderSketch.
As it is currently, trying to incorporate Geometry Layers will only result in a bigger chaos. The class has grown so large that keeping track of the different parts is a daunting task and incorporating new functionality in the current state only contributes to a larger class.
My (current) main target is ::draw().
Wish me luck
This is an announcement that I am initiating a substantial refactoring of ViewProviderSketch.
As it is currently, trying to incorporate Geometry Layers will only result in a bigger chaos. The class has grown so large that keeping track of the different parts is a daunting task and incorporating new functionality in the current state only contributes to a larger class.
My (current) main target is ::draw().
Wish me luck
Re: Refactoring ViewProviderSketch
Good luck, and thanks for the heads-up!
Re: Refactoring ViewProviderSketch
Good luck. I already got a tendinitis just scrolling this file top to bottom with the mouse wheel a single time.
Re: Refactoring ViewProviderSketch
Oh! You are a strong man. Normal people can get a tendinitis just by scrolling with the mouse wheel the ::draw() function
I am offloading all the Coin related drawing on a new class CoinManager.
- I have refactored all the filling of coin nodes for the geometry. This is already in quite a nice state. I have systematically unloaded the responsibility for providing the coordinates of points and curves on OCC and have managed to substantially reduce the codesize.
- I have refactored the creation of the information layer out of ViewProviderSketch and into CoinManager. This I still need to rework it to remove code repetitions and reduce the codesize. Now it is just "hidden" somewhere else.
- Then after that is the drawing of constraints which was about half the original draw() function.
I think I might be able to run the creation of the geometry nodes and the constraint nodes in parallel.
When extending that to different layers, I might be able to run the creation of the geometry of the different layers in parallel too. Then join the tasks, and merge the geometry analysis of each layer (so that all BSplines use the same scaling of comb, poles, etc...). Then create the information layers of each geometry layer in parallel too. And all this in parallel with the constraint nodes. Joining at the end of the function. Then of course, some profiling may be needed to see if it actually makes sense to run all this in parallel or not (or from which size of sketch onward it makes sense). But I think that for big sketches it should help.
Re: Refactoring ViewProviderSketch
An update is due, so that other developers can keep track of this refactor effort (at least so that there is awareness it is still being undertaken).
The task to refactor ViewProviderSketch::draw() has become quite a daunting effort. Mainly because it has ended up in becoming a major refactor of about every single aspect of ViewProviderSketch where coin is involved. This is at least 70% of the original weight (and it was heavy).
Now I am going through preselection and I estimate I have done around 60% of the total effort.
The task to refactor ViewProviderSketch::draw() has become quite a daunting effort. Mainly because it has ended up in becoming a major refactor of about every single aspect of ViewProviderSketch where coin is involved. This is at least 70% of the original weight (and it was heavy).
Now I am going through preselection and I estimate I have done around 60% of the total effort.
Re: Refactoring ViewProviderSketch
Summary: Refactoring extends to DrawSketchHandler.
Extra! Extra!
ViewProviderSketch is not longer getting along with DrawSketchHandler. It is filing for divorce. It has already unfriend him, they no longer talk to each other and now they communicate via attorney.
Well, that was just to say that I am extending the refactor to DrawSketchHandler. The reason is that I am cleaning the public interface of ViewProviderSketch and this friendship was very odd. They were friends, but ViewProviderSketch would expose many functions public, so that the inheritors of DrawSketchHandler could access it, which were a violation of the encapsulation of ViewProviderSketch itself (anybody with a pointer to the viewprovider could draw an arbitrary line on the 3D view).
Then DrawSketchHandler had access to all the guts of ViewProviderSketch. Violation of privacy aside, this was promoting an unordered grow of the codebase. The attorney in the middle seems to have calm down the situation, and now the attorney defines the interface and would promote a more loose coupling (they still need to colaborate).
Now ViewProviderSketch.h is 231 lines long and ViewProviderSketch.cpp is only 3558 lines long (coming from 498 lines and 7367 lines respectively). There is much more documentation and diet has not ended yet. There is still quite some work to be done on the place where this difference of implementation lines went though.
Re: Refactoring ViewProviderSketch
Nice metaphor . Thanks for keeping us updated.
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
- adrianinsaval
- Veteran
- Posts: 5553
- Joined: Thu Apr 05, 2018 5:15 pm