[vdr] Re: BUGFIX: cOsd::CanHandleAreas()

[Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de>]

Reinhard Nissl wrote:
> 
> Hi,
> 
> Klaus Schmidinger wrote:
> 
> >>To have overlap detection work properly and to return reasonable error codes
> >>to the user, CanHandleAreas() now first checks the alignment of all areas and
> >>performs overlap detection afterwards.
> >
> > I don't quite see what the advantage is in having an extra 'for' loop.
> > The error messages are still the same, aren't they?
> >
> > Shouldn't
> >
> >   for (int i = 0; i < NumAreas; i++) {
> >       if (Areas[i].x1 > Areas[i].x2 || Areas[i].y1 > Areas[i].y2 || Areas[i].x1 < 0 || Areas[i].y1 < 0)
> >          return oeWrongAlignment;
> >       for (int j = i + 1; j < NumAreas; j++) {
> >           if (Areas[i].Intersects(Areas[j]))
> >              return oeAreasOverlap;
> >           }
> >       }
> >
> > do just the same?
> 
> Well, the difference is that your version might return early with
> oeAreasOverlap without having checked Area[j] for correct alignment first.
> 
> I think, it's better to first check all areas for alignment and then for
> overlapping as the latter "requires" the former.

Ok, the I'll do it this way:

--- osd.c       2004/06/15 20:29:42     1.55
+++ osd.c       2004/07/18 09:23:03
@@ -602,15 +602,18 @@
 
 eOsdError cOsd::CanHandleAreas(const tArea *Areas, int NumAreas)
 {
+  eOsdError Result = oeOk;
   for (int i = 0; i < NumAreas; i++) {
+      if (Areas[i].x1 > Areas[i].x2 || Areas[i].y1 > Areas[i].y2 || Areas[i].x1 < 0 || Areas[i].y1 < 0)
+         return oeWrongAlignment;
       for (int j = i + 1; j < NumAreas; j++) {
-          if (Areas[i].Intersects(Areas[j]))
-             return oeAreasOverlap;
-          if (Areas[i].x1 > Areas[i].x2 || Areas[i].y1 > Areas[i].y2 || Areas[i].x1 < 0 || Areas[i].y1 < 0)
-             return oeWrongAlignment;
+          if (Areas[i].Intersects(Areas[j])) {
+             Result = oeAreasOverlap;
+             break;
+             }
           }
       }
-  return oeOk;
+  return Result;
 }
 
 eOsdError cOsd::SetAreas(const tArea *Areas, int NumAreas)


This avoids the extra 'for' loop and reports oeWrongAlignment before any
oeAreasOverlap.

Klaus