[prev in list] [next in list] [prev in thread] [next in thread] 

List:       koffice-devel
Subject:    Re: koffice/libs/flake/tests
From:       jaham () gmx ! net
Date:       2009-09-27 15:48:25
Message-ID: 200909271748.25806.jaham () gmx ! net
[Download RAW message or body]

On Sunday 27 September 2009 12:29:32 Thorsten Zachmann wrote:
> On Sat September 26 2009, Thomas Zander wrote:
> > On Friday 25. September 2009 19.38.15 jaham@gmx.net wrote:
> > > I don't think it is too complex, just a recursive function which gets a
> > > list of shapes to paint and when stumbling over a shape container calls
> > > itself with the containers child shapes.
> > > But if you have a better idea to get the same result by sorting the
> > > list of shapes, i am all for it. The important thing is that it works.
> >
> > I think the attached patch is a good way to do it, sorting the list makes
> > sure the painting order is correct.
> > I committed a (failing) test to show the *painting* issue in the fullest.
> >
> > What do you think about this patch?
> 
> That is in the way what I had in mind too. However that is only a part of
>  what needs fixing. It is still possible that two shapes can get the same
>  z-index.
> 
> I was comparing your compare algorithm to one a different one too see
> what is faster, when I found a problem with your algorithem:
> 
> The following
> 
> layer 1
> group 1.1
>   member 1.1.1
>   member 1.1.2
> group 1.2
>     member 1.2.1
>     member 1.2.2
> 
> is sorted as:
> 
> layer 1
> group 1.1
> group 1.2
>   member 1.1.1
>   member 1.1.2
>   member 1.2.1
>   member 1.2.2
> 
> which is wrong.

Attached patch adds a sorting algorithm which does the above example and the 
ones in the unit test correctly.
What do you think?

Ciao Jan

["fix_shape_sorting_by_zindex.patch" (text/x-patch)]

Index: libs/flake/KoShape.cpp
===================================================================
--- libs/flake/KoShape.cpp	(revision 1028493)
+++ libs/flake/KoShape.cpp	(working copy)
@@ -383,22 +383,42 @@
 
 bool KoShape::compareShapeZIndex(KoShape *s1, KoShape *s2)
 {
-    int diff = s1->zIndex() - s2->zIndex();
-    if (diff == 0) {
-        KoShape *s = s1->parent();
-        while (s) {
-            if (s == s2) // s1 is a child of s2
-                return false; // children are always on top of their parents.
-            s = s->parent();
+    // assemble hierarchy of shape s1
+    QList<KoShape*> hierarchy1;
+    hierarchy1.append(s1);
+    while (s1->parent()) {
+        s1 = s1->parent();
+        hierarchy1.prepend(s1);
+    }
+    
+    // assemble hierarchy of shape s2
+    QList<KoShape*> hierarchy2;
+    hierarchy2.append(s2);
+    while (s2->parent()) {
+        s2 = s2->parent();
+        hierarchy2.prepend(s2);
+    }
+
+    KoShape * topLevel1 = hierarchy1.first();
+    KoShape * topLevel2 = hierarchy2.first();
+    
+    // top level shapes are the same
+    // -> go down the hierarchy below the last common parent
+    if (topLevel1 == topLevel2) {
+        int level = 1;
+        while (topLevel1 == topLevel2) {
+            if (level < hierarchy1.count())
+                topLevel1 = hierarchy1[level];
+            if (level < hierarchy2.count())
+                topLevel2 = hierarchy2[level];
+            level++;
         }
-        s = s2->parent();
-        while (s) {
-            if (s == s1) // s2 is a child of s1
-                return true;
-            s = s->parent();
-        }
+        if (topLevel1 == topLevel2->parent())
+            return true;
+        if (topLevel2 == topLevel1->parent())
+            return false;
     }
-    return diff < 0;
+    return topLevel1->zIndex() < topLevel2->zIndex();
 }
 
 void KoShape::setParent(KoShapeContainer *parent)
@@ -421,8 +441,6 @@
 int KoShape::zIndex() const
 {
     Q_D(const KoShape);
-    if (parent()) // we can't be under our parent...
-        return qMax((int) d->zIndex, parent()->zIndex());
     return d->zIndex;
 }
 


_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel


[prev in list] [next in list] [prev in thread] [next in thread]