I've gone ahead and committed this. If the virtual thing is the only issue, and it otherwise looks okay, it's probably better to have it in the codebase where people can start testing it. Looks like Aaron is creating a script to take advantage of this fix to address the marker coloring bug, so this'll get him able to test that.
Please keep an eye out for anything odd showing up with things related to defs like markers, gradients, etc. and report asap.
Bryce
On Wed, Jul 19, 2006 at 07:23:38PM -0700, Bryce Harrington wrote:
On Wed, Jul 19, 2006 at 08:07:27PM -0400, MenTaLguY wrote:
Could you identify the one-line fix please? I'm having trouble isolating the refactoring work from the fix itself.
Sure, indicated below. On reflection, I am not certain it was this one-line fix alone; as I recall, there were one or two refactorings that you identified which may have contributed.
For the most part this appears to be sound, but, I'm a bit worried about _childList no longer being virtual.
I realize you can't currently put virtual methods on an SPObject, but non-virtual polymorphic methods are seldom desirable... I think the approach here is probably to create a CObject analagous to CGroup ( CObject:SPObject::CGroup::SPItemGroup ), have CGroup derive from it, and put the method there.
Yeah, that's exactly the issue I ran into. I figure additional work may be needed in creating CObject, etc. but decided that was too tangential to the marker work I want to stay focused on.
Index: src/sp-defs.cpp
--- src/sp-defs.cpp (revision 12719) +++ src/sp-defs.cpp (working copy) @@ -83,21 +83,14 @@
flags &= SP_OBJECT_MODIFIED_CASCADE;
- GSList *l = NULL;
- for ( SPObject *child = sp_object_first_child(object) ; child != NULL; child = SP_OBJECT_NEXT(child) ) {
g_object_ref(G_OBJECT(child));
l = g_slist_prepend(l, child);
- }
- l = g_slist_reverse(l);
- GSList *l = g_slist_reverse(object->childList(true)); while (l) { SPObject *child = SP_OBJECT(l->data); l = g_slist_remove(l, child);
if (flags || (child->uflags & SP_OBJECT_MODIFIED_FLAG)) {
if (flags || (child->uflags & (SP_OBJECT_MODIFIED_FLAG | SP_OBJECT_CHILD_MODIFIED_FLAG))) {
This is what I originally believed fixed the issue.
However, I think I must have attempted this fix one or two times early, which is why I think one of the other refactorings enabled this fix to actually work. Might be worth testing this in isolation.
child->updateDisplay(ctx, flags); }
g_object_unref(G_OBJECT(child));
}g_object_unref (G_OBJECT (child));
}
Bryce