On 22 déc. 2012, at 09:46, d...@gnu.org wrote: > On 2012/12/22 08:29:59, mike7 wrote: >> Where in the new code are things being allocated and thrown away > temporarily on >> the heap? > > What do you think extract_grob_set does?
Got it. > >> If there is a more efficient way to implement this patch, let me know. > It >> seems, though, that the only way to prevent grobs being elements of > other grobs >> is to prevent grobs from being added to elements lists à la this > patch. > > That is rubbish. We can't add checks for any hypothetical programming > error that can't come about as user error. True. > Here we have one situation, > namely axis groups, where there is a possibility of user error. So I > proposed setting a context property when having an axis group engraver. It sounds like you're mixing together "axis-groups" and "Axis group engraver." The axis group engraver creates VerticalAxisGroups, which implement the axis-group-interface. There are, however, numerous grobs that implement the axis-group-interface that are not created in the Axis_group_engraver. To avoid future confusion, it may be worth renaming the engraver with a convert-ly rule. My patch treats all grobs with an element array (meaning that they implement axis-group-interface). > > If you think that adding something to an existing set twice is a > fundamental problem, you can easily enough create a hashtable for each > set and check before adding to it that the element is not a member of > this set. This is one manifestation of the problem. The deeper problem is that if we have grobs A, B, C, and D and the following happens : A adds B as an element. B adds C C adds D D adds A A hash structure cannot avoid this. > > In this particular case, however, the problem is adding an axis group to > an axis group. _Any_ old axis group to _any_ old axis group. The check > for that is trivial: in the code adding a grob to an axis group, check > that what you add is not an axis group (assuming that axis groups are > top-level containers). It seems like you're mixing up axis-group-interface with Axis_group_engraver. When you say _any_ old axis group, note that NoteColumn, Ambitus, and DynamicLineSpanner are all axis groups with element lists. A user can make an Ambitus the element of a NoteColumn which is the element of a DynamicLineSpanner which is then the element of the Ambitus if she so chooses. This will result in a segfault when the elements lists are recursively iterated through. My patch is intended to avoid this segfault by not allowing recursive nesting of elements in the elements list. I am positive that your proposal would work in this specific case, but I also know that you advocate general, holistic, simple and "boring" solutions that cover many things. I feel that my solution is that. It seems that the only issue, as expressed before, is the efficiency one. I will do benchmarks later today. Cheers, MS _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
