Enforce stricter interval algebra rules for overlaps and contains (#760)

KarthikRIyer · meshula · web-flow · commit 65ffe2a31b97 · 2020-08-21T16:10:29.000-07:00
* Create python-package.yml

* Update flake8 config

* Enforce stricter interval algebra rules for overlaps operator
and update trackAlgorithm accordingly.

* Enforce stricter interval algebra rules for contains operator
and update trackAlgorithm accordingly

* Minor fixes

* Add intersects predicate, constexpr functions to compare time and reformatting

* Make greater_than() &amp; less_than() just inline

* Delete python-package.yml

Co-authored-by: Nick Porcino &lt;meshula@hotmail.com&gt;
diff --git a/src/opentime/timeRange.h b/src/opentime/timeRange.h
@@ -18,7 +18,7 @@ namespace opentime {
  */
 
 /**
- * This default epsilon value is used in comparison between floating numbers.
+ * This default epsilon_s value is used in comparison between floating numbers.
  * It is computed to be twice 192khz, the fastest commonly used audio rate.
  * It can be changed in the future if necessary due to higher sampling rates
  * or some other kind of numeric tolerance detected in the library.
@@ -92,8 +92,9 @@ class TimeRange {
      */
 
     /**
-     * In the relations that follow, epsilon indicates the tolerance,in the sense that if abs(a-b) < epsilon,
-     * we consider a and b to be equal
+     * In the relations that follow, epsilon_s indicates the tolerance,in the sense that if abs(a-b) < epsilon_s,
+     * we consider a and b to be equal.
+     * The time comparison is done in double seconds.
      */
 
     /**
@@ -117,8 +118,12 @@ class TimeRange {
      * The converse would be <em>other.contains(this)</em>
      * @param other
      */
-    bool contains(TimeRange other) const {
-        return _start_time <= other._start_time && end_time_exclusive() >= other.end_time_exclusive();
+    bool contains(TimeRange other, double epsilon_s = DEFAULT_EPSILON_s) const {
+        double thisStart = _start_time.to_seconds();
+        double thisEnd = end_time_exclusive().to_seconds();
+        double otherStart = other._start_time.to_seconds();
+        double otherEnd = other.end_time_exclusive().to_seconds();
+        return greater_than(otherStart, thisStart, epsilon_s) && lesser_than(otherEnd, thisEnd, epsilon_s);
     }
 
     /**
@@ -134,48 +139,49 @@ class TimeRange {
     }
 
     /**
-     * The start of <b>this</b> strictly precedes end of <b>other</b> by a value >= <b>epsilon</b>.
-     * The end of <b>this</b> strictly antecedes start of <b>other</b> by a value >= <b>epsilon</b>.
+     * The start of <b>this</b> strictly precedes end of <b>other</b> by a value >= <b>epsilon_s</b>.
+     * The end of <b>this</b> strictly antecedes start of <b>other</b> by a value >= <b>epsilon_s</b>.
      *              [ this ]
      *                  [ other ]
      * The converse would be <em>other.overlaps(this)</em>
      * @param other
-     * @param epsilon
+     * @param epsilon_s
      */
-    bool overlaps(TimeRange other, double epsilon = DEFAULT_EPSILON_s) const {
+    bool overlaps(TimeRange other, double epsilon_s = DEFAULT_EPSILON_s) const {
         double thisStart = _start_time.to_seconds();
         double thisEnd = end_time_exclusive().to_seconds();
         double otherStart = other._start_time.to_seconds();
         double otherEnd = other.end_time_exclusive().to_seconds();
-        return (otherEnd - thisStart >= epsilon) &&
-               (thisEnd - otherStart >= epsilon);
+        return lesser_than(thisStart, otherStart, epsilon_s) &&
+                greater_than(thisEnd, otherStart, epsilon_s) &&
+                greater_than(otherEnd, thisEnd, epsilon_s);
     }
 
     /**
-     * The end of <b>this</b> strictly precedes the start of <b>other</b> by a value >= <b>epsilon</b>.
+     * The end of <b>this</b> strictly precedes the start of <b>other</b> by a value >= <b>epsilon_s</b>.
      *              [ this ]    [ other ]
      * The converse would be <em>other.before(this)</em>
      * @param other
-     * @param epsilon
+     * @param epsilon_s
      */
-    bool before(TimeRange other, double epsilon = DEFAULT_EPSILON_s) const {
+    bool before(TimeRange other, double epsilon_s = DEFAULT_EPSILON_s) const {
         double thisEnd = end_time_exclusive().to_seconds();
         double otherStart = other._start_time.to_seconds();
-        return otherStart - thisEnd >= epsilon;
+        return greater_than(otherStart, thisEnd, epsilon_s);
     }
 
     /**
-     * The end of <b>this</b> strictly precedes <b>other</b> by a value >= <b>epsilon</b>.
+     * The end of <b>this</b> strictly precedes <b>other</b> by a value >= <b>epsilon_s</b>.
      *                        other
      *                          ↓
      *              [ this ]    *
      * @param other
-     * @param epsilon
+     * @param epsilon_s
      */
-    bool before(RationalTime other, double epsilon = DEFAULT_EPSILON_s) const {
+    bool before(RationalTime other, double epsilon_s = DEFAULT_EPSILON_s) const {
         double thisEnd = end_time_exclusive().to_seconds();
         double otherTime = other.to_seconds();
-        return otherTime - thisEnd >= epsilon;
+        return lesser_than(thisEnd, otherTime, epsilon_s);
     }
 
     /**
@@ -184,29 +190,29 @@ class TimeRange {
      *              [this][other]
      * The converse would be <em>other.meets(this)</em>
      * @param other
-     * @param epsilon
+     * @param epsilon_s
      */
-    bool meets(TimeRange other, double epsilon = DEFAULT_EPSILON_s) const {
+    bool meets(TimeRange other, double epsilon_s = DEFAULT_EPSILON_s) const {
         double thisEnd = end_time_exclusive().to_seconds();
         double otherStart = other._start_time.to_seconds();
-        return otherStart - thisEnd <= epsilon && otherStart - thisEnd >= 0;
+        return otherStart - thisEnd <= epsilon_s && otherStart - thisEnd >= 0;
     }
 
     /**
      * The start of <b>this</b> strictly equals the start of <b>other</b>.
-     * The end of <b>this</b> strictly precedes the end of <b>other</b> by a value >= <b>epsilon</b>.
+     * The end of <b>this</b> strictly precedes the end of <b>other</b> by a value >= <b>epsilon_s</b>.
      *              [ this ]
      *              [    other    ]
      * The converse would be <em>other.begins(this)</em>
      * @param other
-     * @param epsilon
+     * @param epsilon_s
      */
-    bool begins(TimeRange other, double epsilon = DEFAULT_EPSILON_s) const {
+    bool begins(TimeRange other, double epsilon_s = DEFAULT_EPSILON_s) const {
         double thisStart = _start_time.to_seconds();
         double thisEnd = end_time_exclusive().to_seconds();
         double otherStart = other._start_time.to_seconds();
         double otherEnd = other.end_time_exclusive().to_seconds();
-        return fabs(otherStart - thisStart) <= epsilon && otherEnd - thisEnd >= epsilon;
+        return fabs(otherStart - thisStart) <= epsilon_s && lesser_than(thisEnd, otherEnd, epsilon_s);
     }
 
     /**
@@ -217,27 +223,27 @@ class TimeRange {
      *              [ this ]
      * @param other
      */
-    bool begins(RationalTime other, double epsilon = DEFAULT_EPSILON_s) const {
+    bool begins(RationalTime other, double epsilon_s = DEFAULT_EPSILON_s) const {
         double thisStart = _start_time.to_seconds();
         double otherStart = other.to_seconds();
-        return fabs(otherStart - thisStart) <= epsilon;
+        return fabs(otherStart - thisStart) <= epsilon_s;
     }
 
     /**
-     * The start of <b>this</b> strictly antecedes the start of <b>other</b> by a value >= <b>epsilon</b>.
+     * The start of <b>this</b> strictly antecedes the start of <b>other</b> by a value >= <b>epsilon_s</b>.
      * The end of <b>this</b> strictly equals the end of <b>other</b>.
      *                      [ this ]
      *              [     other    ]
      * The converse would be <em>other.finishes(this)</em>
      * @param other
-     * @param epsilon
+     * @param epsilon_s
      */
-    bool finishes(TimeRange other, double epsilon = DEFAULT_EPSILON_s) const {
+    bool finishes(TimeRange other, double epsilon_s = DEFAULT_EPSILON_s) const {
         double thisStart = _start_time.to_seconds();
         double thisEnd = end_time_exclusive().to_seconds();
         double otherStart = other._start_time.to_seconds();
         double otherEnd = other.end_time_exclusive().to_seconds();
-        return fabs(thisEnd - otherEnd) <= epsilon && thisStart - otherStart >= epsilon;
+        return fabs(thisEnd - otherEnd) <= epsilon_s && greater_than(thisStart, otherStart, epsilon_s);
     }
 
     /**
@@ -247,12 +253,29 @@ class TimeRange {
      *                     *
      *              [ this ]
      * @param other
-     * @param epsilon
+     * @param epsilon_s
      */
-    bool finishes(RationalTime other, double epsilon = DEFAULT_EPSILON_s) const {
+    bool finishes(RationalTime other, double epsilon_s = DEFAULT_EPSILON_s) const {
         double thisEnd = end_time_exclusive().to_seconds();
         double otherEnd = other.to_seconds();
-        return fabs(thisEnd - otherEnd) <= epsilon;
+        return fabs(thisEnd - otherEnd) <= epsilon_s;
+    }
+
+    /**
+     * The start of <b>this</b> precedes or equals the end of <b>other</b> by a value >= <b>epsilon_s</b>.
+     * The end of <b>this</b> antecedes or equals the start of <b>other</b> by a value >= <b>epsilon_s</b>.
+     *         [    this    ]           OR      [    other    ]
+     *              [     other    ]                    [     this    ]
+     * The converse would be <em>other.finishes(this)</em>
+     * @param other
+     * @param epsilon_s
+     */
+    bool intersects(TimeRange other, double epsilon_s = DEFAULT_EPSILON_s) const {
+        double thisStart = _start_time.to_seconds();
+        double thisEnd = end_time_exclusive().to_seconds();
+        double otherStart = other._start_time.to_seconds();
+        double otherEnd = other.end_time_exclusive().to_seconds();
+        return lesser_than(thisStart, otherEnd, epsilon_s) && greater_than(thisEnd, otherStart, epsilon_s);
     }
 
 
@@ -288,6 +311,14 @@ class TimeRange {
 private:
     RationalTime _start_time, _duration;
     friend class TimeTransform;
+
+    inline bool greater_than(double lhs, double rhs, double epsilon) const{
+        return lhs - rhs >= epsilon;
+    }
+
+    inline bool lesser_than(double lhs, double rhs, double epsilon) const{
+        return rhs - lhs >= epsilon;
+    }
 };
 
 } }
diff --git a/src/opentimelineio/trackAlgorithm.cpp b/src/opentimelineio/trackAlgorithm.cpp
@@ -27,7 +27,7 @@ Track* track_trimmed_to_range(Track* in_track, TimeRange trim_range, ErrorStatus
         }
         
         auto child_range = child_range_it->second;
-        if (!trim_range.overlaps(child_range)) {
+      if (!trim_range.intersects(child_range)) {
             new_track->remove_child(static_cast<int>(i), error_status);
             if (*error_status) {
                 return nullptr;
diff --git a/src/py-opentimelineio/opentime-bindings/opentime_timeRange.cpp b/src/py-opentimelineio/opentime-bindings/opentime_timeRange.cpp
@@ -43,16 +43,17 @@ void opentime_timeRange_bindings(py::module m) {
         .def("clamped", (RationalTime (TimeRange::*)(RationalTime) const) &TimeRange::clamped, "other"_a)
         .def("clamped", (TimeRange (TimeRange::*)(TimeRange) const) &TimeRange::clamped, "other"_a)
         .def("contains", (bool (TimeRange::*)(RationalTime) const) &TimeRange::contains, "other"_a)
-        .def("contains", (bool (TimeRange::*)(TimeRange) const) &TimeRange::contains, "other"_a)
+        .def("contains", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::contains, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s)
         .def("overlaps", (bool (TimeRange::*)(RationalTime) const) &TimeRange::overlaps, "other"_a)
-        .def("overlaps", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::overlaps, "other"_a, "epsilon"_a=opentime::DEFAULT_EPSILON_s)
-        .def("before", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::before, "other"_a, "epsilon"_a=opentime::DEFAULT_EPSILON_s)
-        .def("before", (bool (TimeRange::*)(RationalTime, double ) const) &TimeRange::before, "other"_a, "epsilon"_a=opentime::DEFAULT_EPSILON_s)
-        .def("meets", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::meets, "other"_a, "epsilon"_a=opentime::DEFAULT_EPSILON_s)
-        .def("begins", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::begins, "other"_a, "epsilon"_a=opentime::DEFAULT_EPSILON_s)
-        .def("begins", (bool (TimeRange::*)(RationalTime, double) const) &TimeRange::begins, "other"_a, "epsilon"_a=opentime::DEFAULT_EPSILON_s)
-        .def("finishes", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::finishes, "other"_a, "epsilon"_a=opentime::DEFAULT_EPSILON_s)
-        .def("finishes", (bool (TimeRange::*)(RationalTime, double) const) &TimeRange::finishes, "other"_a, "epsilon"_a=opentime::DEFAULT_EPSILON_s)
+        .def("overlaps", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::overlaps, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s)
+        .def("before", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::before, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s)
+        .def("before", (bool (TimeRange::*)(RationalTime, double ) const) &TimeRange::before, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s)
+        .def("meets", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::meets, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s)
+        .def("begins", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::begins, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s)
+        .def("begins", (bool (TimeRange::*)(RationalTime, double) const) &TimeRange::begins, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s)
+        .def("finishes", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::finishes, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s)
+        .def("finishes", (bool (TimeRange::*)(RationalTime, double) const) &TimeRange::finishes, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s)
+        .def("intersects", (bool (TimeRange::*)(TimeRange, double) const) &TimeRange::intersects, "other"_a, "epsilon_s"_a=opentime::DEFAULT_EPSILON_s)
         .def("__copy__", [](TimeRange tr) {
                 return tr;
             })
diff --git a/src/py-opentimelineio/opentimelineio/algorithms/track_algo.py b/src/py-opentimelineio/opentimelineio/algorithms/track_algo.py
@@ -47,7 +47,7 @@ def track_trimmed_to_range(in_track, trim_range):
     # iterate backwards so we can delete items
     for c, child in reversed(list(enumerate(new_track))):
         child_range = track_map[child]
-        if not trim_range.overlaps(child_range):
+        if not trim_range.intersects(child_range):
             # completely outside the trim range, so we discard it
             del new_track[c]
         elif trim_range.contains(child_range):
diff --git a/tests/test_opentime.py b/tests/test_opentime.py
@@ -886,7 +886,7 @@ def test_contains(self):
         self.assertFalse(tr.contains(tstart + tdur))
         self.assertFalse(tr.contains(tstart - tdur))
 
-        self.assertTrue(tr.contains(tr))
+        self.assertFalse(tr.contains(tr))
 
         tr_2 = otio.opentime.TimeRange(tstart - tdur, tdur)
         self.assertFalse(tr.contains(tr_2))
@@ -915,25 +915,25 @@ def test_overlaps_timerange(self):
         tdur = otio.opentime.RationalTime(3, 25)
         tr_t = otio.opentime.TimeRange(tstart, tdur)
 
-        self.assertTrue(tr.overlaps(tr_t))
+        self.assertFalse(tr.overlaps(tr_t))
 
         tstart = otio.opentime.RationalTime(13, 25)
         tdur = otio.opentime.RationalTime(1, 25)
         tr_t = otio.opentime.TimeRange(tstart, tdur)
 
-        self.assertTrue(tr.overlaps(tr_t))
+        self.assertFalse(tr.overlaps(tr_t))
 
         tstart = otio.opentime.RationalTime(2, 25)
         tdur = otio.opentime.RationalTime(30, 25)
         tr_t = otio.opentime.TimeRange(tstart, tdur)
 
-        self.assertTrue(tr.overlaps(tr_t))
+        self.assertFalse(tr.overlaps(tr_t))
 
         tstart = otio.opentime.RationalTime(2, 50)
         tdur = otio.opentime.RationalTime(60, 50)
         tr_t = otio.opentime.TimeRange(tstart, tdur)
 
-        self.assertTrue(tr.overlaps(tr_t))
+        self.assertFalse(tr.overlaps(tr_t))
 
         tstart = otio.opentime.RationalTime(2, 50)
         tdur = otio.opentime.RationalTime(14, 50)
@@ -945,14 +945,85 @@ def test_overlaps_timerange(self):
         tdur = otio.opentime.RationalTime(400, 50)
         tr_t = otio.opentime.TimeRange(tstart, tdur)
 
-        self.assertTrue(tr.overlaps(tr_t))
+        self.assertFalse(tr.overlaps(tr_t))
 
         tstart = otio.opentime.RationalTime(100, 50)
         tdur = otio.opentime.RationalTime(400, 50)
         tr_t = otio.opentime.TimeRange(tstart, tdur)
 
         self.assertFalse(tr.overlaps(tr_t))
 
+    def test_intersects_timerange(self):
+        tstart = otio.opentime.RationalTime(12, 25)
+        tdur = otio.opentime.RationalTime(3, 25)
+        tr = otio.opentime.TimeRange(tstart, tdur)
+
+        tstart = otio.opentime.RationalTime(0, 25)
+        tdur = otio.opentime.RationalTime(3, 25)
+        tr_t = otio.opentime.TimeRange(tstart, tdur)
+
+        self.assertFalse(tr.intersects(tr_t))
+
+        tstart = otio.opentime.RationalTime(10, 25)
+        tdur = otio.opentime.RationalTime(3, 25)
+        tr_t = otio.opentime.TimeRange(tstart, tdur)
+
+        self.assertTrue(tr.intersects(tr_t))
+
+        tstart = otio.opentime.RationalTime(10, 25)
+        tdur = otio.opentime.RationalTime(2, 25)
+        tr_t = otio.opentime.TimeRange(tstart, tdur)
+
+        self.assertFalse(tr.intersects(tr_t))
+
+        tstart = otio.opentime.RationalTime(14, 25)
+        tdur = otio.opentime.RationalTime(2, 25)
+        tr_t = otio.opentime.TimeRange(tstart, tdur)
+
+        self.assertTrue(tr.intersects(tr_t))
+
+        tstart = otio.opentime.RationalTime(15, 25)
+        tdur = otio.opentime.RationalTime(2, 25)
+        tr_t = otio.opentime.TimeRange(tstart, tdur)
+
+        self.assertFalse(tr.intersects(tr_t))
+
+        tstart = otio.opentime.RationalTime(13, 25)
+        tdur = otio.opentime.RationalTime(1, 25)
+        tr_t = otio.opentime.TimeRange(tstart, tdur)
+
+        self.assertTrue(tr.intersects(tr_t))
+
+        tstart = otio.opentime.RationalTime(2, 25)
+        tdur = otio.opentime.RationalTime(30, 25)
+        tr_t = otio.opentime.TimeRange(tstart, tdur)
+
+        self.assertTrue(tr.intersects(tr_t))
+
+        tstart = otio.opentime.RationalTime(2, 50)
+        tdur = otio.opentime.RationalTime(60, 50)
+        tr_t = otio.opentime.TimeRange(tstart, tdur)
+
+        self.assertTrue(tr.intersects(tr_t))
+
+        tstart = otio.opentime.RationalTime(2, 50)
+        tdur = otio.opentime.RationalTime(14, 50)
+        tr_t = otio.opentime.TimeRange(tstart, tdur)
+
+        self.assertFalse(tr.intersects(tr_t))
+
+        tstart = otio.opentime.RationalTime(-100, 50)
+        tdur = otio.opentime.RationalTime(400, 50)
+        tr_t = otio.opentime.TimeRange(tstart, tdur)
+
+        self.assertTrue(tr.intersects(tr_t))
+
+        tstart = otio.opentime.RationalTime(100, 50)
+        tdur = otio.opentime.RationalTime(400, 50)
+        tr_t = otio.opentime.TimeRange(tstart, tdur)
+
+        self.assertFalse(tr.intersects(tr_t))
+
     def test_before_timerange(self):
         tstart = otio.opentime.RationalTime(12, 25)
         tdur = otio.opentime.RationalTime(3, 25)

Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,7 @@ Track* track_trimmed_to_range(Track* in_track, TimeRange trim_range, ErrorStatus`
`27`	`27`	`}`
`28`	`28`
`29`	`29`	`auto child_range = child_range_it->second;`
`30`		`- if (!trim_range.overlaps(child_range)) {`
	`30`	`+ if (!trim_range.intersects(child_range)) {`
`31`	`31`	`new_track->remove_child(static_cast<int>(i), error_status);`
`32`	`32`	`if (*error_status) {`
`33`	`33`	`return nullptr;`