1.
- GSList *sl; - sl = g_slist_copy ((GSList *) sp_selection_repr_list (selection)); - sl = g_slist_sort (sl, (GCompareFunc) sp_repr_compare_position); + const GSList *items = sp_selection_item_list (SP_DT_SELECTION (desktop));
I.e. you removed sorting the copied list of items. Why? The result is that the copied items now do not preserve z-order when pasted; they're pasted in the order in which they were selected, which is wrong (try it). And you cannot sort the list returned by sp_selectiion_{repr_}list, only a copy of it, which is what was done in the original code.
2. You only copy gradients applied to the fill, not to the stroke, so gradient-filled stroke is lost when copying between documents.
3. Here you check that the fill is either pattern or gradient, and then, without any further check, cast it into a gradient:
+ if (style && style->fill.type == SP_PAINT_TYPE_PAINTSERVER) { /* Object has pattern or gradient fill*/ + SPGradient *gradient = SP_GRADIENT (SP_OBJECT_STYLE_FILL_SERVER (items->data));
The result is quite predictable - it crashes when you try to copy an object with pattern fill. (Probably with bitmap fill also.)
4. When you paste copied objects with gradients within the same document, it still adds a copy of the gradient (with different id), although the pasted object still refers to the old gradient. This will lead to an unchecked growth of defs each time you do copy/paste. This should be prevented. E.g. check if the current defs contain an exact copy of the gradient you're about to add (not only same id but also same content), and if so, skip adding it.
5. Same thing when you copy an object with gradient to another document and that document already contains a different gradient with the same id. The result is that the pasted object uses that other document's gradient (different from its original gradient), while the correct gradient is added to the defs with a different id (and thus left unused). Fixing this is more complex: if the document you're pasting to contains a gradient with the same id but different content, you must change the id both of the gradient you're inserting to the defs _and_ the reference in the object style. The difficulty being, to check for identical content, you must check not only the content of the repr, but all other reprs it is linked to, and their links, and so on until the tree is exhausted.
_________________________________________________________________ MSN Premium helps eliminate e-mail viruses. Get 2 months FREE* http://join.msn.com/?pgmarket=en-ca&page=byoa/prem&xAPID=1994&DI...
--- bulia byak <archiver_1@...19...> wrote:
- GSList *sl;
- sl = g_slist_copy ((GSList *)
sp_selection_repr_list (selection));
- sl = g_slist_sort (sl, (GCompareFunc)
sp_repr_compare_position);
- const GSList *items = sp_selection_item_list
(SP_DT_SELECTION (desktop));
I.e. you removed sorting the copied list of items. Why? The result is that the copied items now do not preserve z-order when pasted; they're pasted in the order in which they were selected, which is wrong (try it). And you cannot sort the list returned by sp_selectiion_{repr_}list, only a copy of it, which is what was done in the original code.
Doing the gradient stuff on the copied list didnt work as they were no longer in the document, hence arent being rendered, hence dont return correct values for paintserver (this is just what i worked out from 6 hrs of playing with code if I'm wrong feel free to correct me) I switched to using the selection list cos that worked for the gradients, I didnt realise what the sort was doing, and my tests were just drag selects so i didnt notice the z-order suff. Have seperated out the gradient copy from the object copy, and put back the sort, so z-order once again is maintained.
- You only copy gradients applied to the fill, not
to the stroke, so gradient-filled stroke is lost when copying between documents.
The bug only mentioned fill. :P Seriously, it hadnt occured to me, I'll look at it.
- Here you check that the fill is either pattern or
gradient, and then, without any further check, cast it into a gradient:
The result is quite predictable - it crashes when you try to copy an object with pattern fill. (Probably with bitmap fill also.)
Hadn't worried since we dont do patterns yet, but your right, just me being lazy. Have put a check in to determine the type of paintserver and handle accordingly (and afaik bitmap fill is a pattern fill, its just the svg is an <image> :) ) (btw, I'll sort that patterns patch out for you now)
- When you paste copied objects with gradients
within the same document, it still adds a copy of
the > gradient (with different id), although the pasted
object still refers to the old gradient.
for now have added the check id and skip if exists bit, until I work out how to correct the links in the copied objects to point at the new gradients id theres no point creating them.
- Same thing when you copy an object with gradient
to another document and that document already contains a different gradient with the same id. The result is that the pasted object uses that other document's gradient (different from its original gradient), while the correct gradient is added to the defs with a different id (and thus left unused).
I was semi aware of this problem, as i said in the bug tracker, am not sure how to approach relinking all the objects, and need to talk to someone with a bit more experience. Hadn't realised that it was pasting duplicates, but have that covered now.
will update the patch on SF bug in a minute (will be v3).
__________________________________ Do you Yahoo!? Yahoo! Mail - More reliable, more storage, less spam http://mail.yahoo.com
participants (2)
-
bulia byak -
John Cliff