In src/selection-chemistry.cpp, sp_selection_tile() line 1957 and on:
// delete objects so that their clones don't get alerted; this object will be restored shortly for (GSList *i = items; i != NULL; i = i->next) { SPObject *item = SP_OBJECT (i->data); item->deleteObject (false); }
Here's the cause of bug 1238314. If a clone-original is incorporated into a pattern, it's clones are orphaned (because the version in the template is a renamed duplicate).
By replacing the code above with a call to sp_selection_delete_impl(), the bug is fixed, but the clones become regular duplicates.
Is this desirable? Or should it be fixed in a way that preserves clone-relations? (The comment seems to indicate that this is the plan.)
--Nicklas
Nicklas Lindgren:
the bug is fixed, but the clones become regular duplicates.
Is this desirable? Or should it be fixed in a way that preserves clone-relations? (The comment seems to indicate that this is the plan.)
If you mean my original comment in the bug report, no, I would be quite satisfied if the clones become duplicates at the moment of chosing 'Object(s) to pattern', thanks! If someone wants clonicity preserved, (s)he should file an RFE for that, IMHO.
Great work BTW ralf
On 8/10/05, Ralf Stephan <ralf@...748...> wrote:
If you mean my original comment in the bug report, no, I would be quite satisfied if the clones become duplicates at the moment of chosing 'Object(s) to pattern', thanks! If someone wants clonicity preserved, (s)he should file an RFE for that, IMHO.
I agree this would work as the last resort, but let's try to do it correctly first, without breaking clone links. This should not be too difficult (see my prev mail).
On 8/10/05, Nicklas Lindgren <nili@...612...> wrote:
By replacing the code above with a call to sp_selection_delete_impl(), the bug is fixed, but the clones become regular duplicates.
Is this desirable? Or should it be fixed in a way that preserves clone-relations? (The comment seems to indicate that this is the plan.)
Yes, the idea was to preserve the clone links as much as possible. However I still do not understand the reason for the bug. The deleteObject (false) deletes the object without notifying its clones, so they don't get the chance to realize they are orphans. Later, that same object is restored in the pattern, with the same ID, so the clones should just stay, now linked to that restored object. This works when the clone of the object going into pattern is itself outside the pattern; what exactly breaks if it's inside?
Ah, probably what happens is that the clone that goes into the object is restored FIRST, at the time when its original is not yet restored, and thus breaks. So I think the fix should just change the order of restoring objects in the pattern, making sure all clones are restored LAST. Perhaps implementing something like sort_by_cloning(objects) would be nice (and useful in other situations too), that would take a list of objects and sort them in such a way that each original is always closer to the beginning of the list than any of its clones in the same list. Then in restoring object, you iterate over the sorted list, making sure clones are not restored before their originals.
On Wed, Aug 10, 2005 at 03:23:02PM -0300, bulia byak wrote:
On 8/10/05, Nicklas Lindgren <nili@...612...> wrote:
By replacing the code above with a call to sp_selection_delete_impl(), the bug is fixed, but the clones become regular duplicates.
Is this desirable? Or should it be fixed in a way that preserves clone-relations? (The comment seems to indicate that this is the plan.)
Yes, the idea was to preserve the clone links as much as possible. However I still do not understand the reason for the bug. The deleteObject (false) deletes the object without notifying its clones, so they don't get the chance to realize they are orphans. Later, that same object is restored in the pattern, with the same ID, so the clones should just stay, now linked to that restored object. [...]
The problem seems to be, that they aren't restored later, but they are restored first, with the call to pattern_tile just above the mentioned code. Therefore the objects are in effect duplicated instead, and their id-attributes are changed. After that the objects with the original ids are silently deleted.
The problem is solvable by moving the deletion before the call to pattern_tile.
But this exposes another problem. A transform is applied to all objects in the pattern to anchor the top-left in 0,0. Applying a transform to a clone-original affects all the clones, and affects clones included in the pattern twice.
Should perhaps a reverse transform be applied to all the clones in this case?
An alternative that seems to be simpler is to apply x and y attributes to the newly created pattern element, rather than transforming each contained object, but this seems to make pattern-drawing buggy :-(.
--Nicklas
On 8/10/05, Nicklas Lindgren <nili@...612...> wrote:
The problem seems to be, that they aren't restored later, but they are restored first, with the call to pattern_tile just above the mentioned code. Therefore the objects are in effect duplicated instead, and their id-attributes are changed. After that the objects with the original ids are silently deleted.
The problem is solvable by moving the deletion before the call to pattern_tile.
Hmm, I don't remember off hand, but that definitely was the original intent: delete the object, then create it anew with the same id. In fact the same sequence of actions is used in a lot of commands, so this is almost a cliche. I'm not sure why it's broken here, but of course it must be fixed.
But this exposes another problem. A transform is applied to all objects in the pattern to anchor the top-left in 0,0. Applying a transform to a clone-original affects all the clones, and affects clones included in the pattern twice.
I think that here you could use the same hack I did in the Align dialog. It was the same problem: when aligning or distributing an original with its clones, it went awry because movements of the original disturbed clones. So I did this: before the moves I set the preference to "clones unmoved", which results in each clone feeling the move of its original and reversing it automatically, so it stays unmoved. After that the preference is restored to its previous value. Look it up in ui/dialog/align.cpp.
However note that for this to work, you need to actually move objects by the regular transform functions, not just assign it a transform= attribute. The signal for clones to adjust themselves is sent by sp_item_write_transform, so you need to use either it or some function that calls it eventually.
An alternative that seems to be simpler is to apply x and y attributes to the newly created pattern element, rather than transforming each contained object, but this seems to make pattern-drawing buggy :-(.
Yeah, I think I tried that but it didn't work. If you can make it work, however, clones would still be distirbed, even if the parent of the original moves, not the original itself.
participants (3)
-
bulia byak -
Nicklas Lindgren -
Ralf Stephan