Peter Clifton writes: > [...] > > The question in the first place is WTF are these functions doing here? > > Where is the screen coords get_bounds function for complex? And > > believe it or not they are likely to be merged as is in HEAD. > > I'm not sure I understand that last point. As I understand your comment, > your WTF is regarding the fact that these functions exist (or are in > gschem rather then libgeda?). I agree that they aren't ideally placed, > and can be made better.
These functions are working on a OBJECT or a list of OBJECTS that may or may not be COMPLEXes are in a file named o_complex_basic.c. For these kind of function, we have o_basic.c and o_list.c. get_object_list_bounds() is a rename of an existing function that basically did the same but had complex in his name. On this point this is an improvement. But renaming it and leaving it where it is is definitely not. What is a function working on a generic OBJECT is doing in o_complex_basic.c? Even worse, instead of fixing the problem, some other totally unrelated functions have been added there too (get_bounds fro glist). > [...] > The noscreen branch actually removes all get_bounds functions in screen > coordinates, so the complex won't miss its lack of a function. We are working on pseudo-objects. Why, for the time between this merge and your merge for no-screen, the complex object will not have a screen-coords get_bounds function while every other object has? It had had one until a few weeks ago. > [...] > > Really apart from the get_bounds problem (the commit you mention and > > the changes on prototypes to work on objects) and the changes on > > function casts by Dan in HEAD (considering you are rebasing on HEAD) > > there is not much conflicts. > > Many of the 3-way merge conflicts I highlighted before are not difficult > to resolve. Some are more complicated than others though! What is more > difficult is making sure all code is updated as necessary. > > For example, in noscreen (later changes) objects re-calc their > world-bounds as and when they are altered. Instead of get_...._bounds > recalcing every time, you can just retrieve the stored value. Draw > functions no-longer recalc before drawing. > > It means I need to ensure that all places an object may have its bounds > modified, a recalc is called. In the past I have had a look at what changing for noscreen would mean. If the changes are incremental and small, the code cleaned before applying, especially the get_bounds() mess, there is no problem in re-basing it on HEAD. And since the action stuff is pretty much independant it becomes easy to merge it after. I can do that if I am given a few days. Testing everything is updated is not really difficult. The objects are modified when they are loaded, translated/rotated/mirrored, added/removed an attribute or modified for text and complex. Providing these functions ends with a call to o_recalc_x() we are safe. In fact it is a bug if they are not already doing so. > [...] > (I may introduce a regression test in my code which before drawing says: > 1. What does the stored bounds say? > 2. Recalc the object > 3. Are the stored bounds still the same, or was there a bug? > ) That's a good idea. Regards, Patrick _______________________________________________ geda-dev mailing list [email protected] http://www.seul.org/cgi-bin/mailman/listinfo/geda-dev
