No, actually not. In both cases the object is in an inconsistent state before the recompute starts. It then expects that a recompute may solve this but in one case it's the object itself that returns the error flag and in the other case a recompute cannot be performed. Again, the solution can only be:they share some commonalities (both arise during the recompute mechanism, both relate to current limitations to the recompute mechanism,
1. the object guarantees to be consistent all the time, then its methods or its view provider can rely on this and don't have to double-check all accesses
2. the object doesn't guarantee this, then its methods or view provider have to double check each access.
So, the limitation is not the recompute mechanism but the fact that the calling instance (which triggers the recompute) doesn't care about the state of the object beforehand and afterwards.
It's always up to each DocumentObject subclass how it wants to handle unexpected situations. It's just that in many it cases it doesn't make sense to continue because following operations are based on a previously successful operation. But it's also possible that something failed it tries an alternative way. So, there is and was never a design decision that it must return after the first problem.To the first: it is caused by operating by the choice of returning control directly after the first error when recomputing a DocumentObject. I do not know if this was a decision made by design (I was not here at the time), but it is indeed a pattern that can be seen in most execute() methods of DocumentObject derivatives.
All what the Document is interested in is a DocumentObject's interface, i.e. that it returns StdReturn or the error structure or raises a well-defined exception. It doesn't make any assumption about implementation details.
The PR aimed to solve two different problems with what you call a general solution. But in fact the PR only solves one specific problem where a call of the solve() method was missing. And from a DocumentObject's point of view it doesn't even know why onRecomputeFailed() is getting called.In any case, I agree that this ticket can be solved by a sketcher-only solution, only it is a non-sense to do it so, if another mechanism is needed for the other ticket that could solve both. It was done so in the original PR:
Sorry, I thought you meant to interrupt the recompute of Document, not a DocumentObject. But again as said above: a DocumentObject can do what ever it wants to do, i.e. it can give up after the first error or it can try to continue. The Document class doesn't make any assumptions about this.The recompute of a DocumentObject generally as a pattern as said above returns on the first error. I was never referring to the recompute of the Document itself. As a consequence of an error the DocumentObject interrupts the execution of itself and returns the control to the Document, who may or may not decide to continue as you say.
Look at the PR799. It made basically two changes:In any case, I can accept your decision. So let's concentrate in what is acceptable:
1. ViewProviderSketch::drawConstraintIcons() double-checks that it can safely access the SoNode of the constraint group. If not it prints a warning and stops the loop
2. solve() is now called independently if auto-recompute is on of off which makes it consistent with many other cases where it was done like this. And the view provider is notified about important changes of the sketch without relying on a successful or failing recompute of the document. And as far as I understood the auto-recompute option its purpose in the first place is to recompute all other objects that somehow depend on the sketch.
Currently I am getting flooded with PRs and thus have much less time to do my actual work. What I want to do at some point is re-factoring the code of ViewProviderSketch which has grown to a big and unhandable monster over time. There are methods with more than thousand lines of code and similar code all over the places.I am not sure if I understood you correctly a couple of weeks ago, but you said you wanted to undertake this on your own. I have put on hold anything relating to the sketcher and representation until that happens, as it makes no sense to interfere or duplicate effort.
My solution causes way less overhead than yours. How can a direct call of solve() cause more overhead than calling it by several cascades inside a virtual method?That is a very bad solution because there is no need to solve two times and all the rest of the overhead like updating the Geometry in a non-failing case two times.
You can do whatever you want as long as you don't make any assumptions about implementation details of other classes.Question: Is it acceptable that I do not return just after the first error has been detected? For example, it is acceptable to define booleans for the different errors and move all the return statements to the end of the function, so that I return the right error message, but I can still leave my sketch in a sync state?
But that should be the very last option. First we have to try to figure out which objects cause the cyclic dependency, update the others as we do now and then update the problematic objects in a linear or any arbitrary order.wmayer wrote:Or we simply call the recompute for each object in a linear order.