Mercurial > hg > svgui

changeset 1431:af824022bffd single-point

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Begin fixing the various snap operations. Also remove SnapNearest, which is never used and seems to consume more lines of code than the rest!
author Chris Cannam
date Wed, 20 Mar 2019 14:59:34 +0000
parents 31499c3520ee
children 5b9692768beb
files layer/Colour3DPlotLayer.cpp layer/Layer.h layer/SpectrogramLayer.cpp layer/TimeRulerLayer.cpp layer/TimeValueLayer.cpp
diffstat 5 files changed, 98 insertions(+), 183 deletions(-) [+]
line wrap: on
line diff
--- a/layer/Colour3DPlotLayer.cpp	Wed Mar 20 11:18:45 2019 +0000
+++ b/layer/Colour3DPlotLayer.cpp	Wed Mar 20 14:59:34 2019 +0000
@@ -1189,7 +1189,6 @@
     switch (snap) {
     case SnapLeft:  frame = left;  break;
     case SnapRight: frame = right; break;
-    case SnapNearest:
     case SnapNeighbouring:
         if (frame - left > right - frame) frame = right;
         else frame = left;
--- a/layer/Layer.h	Wed Mar 20 11:18:45 2019 +0000
+++ b/layer/Layer.h	Wed Mar 20 14:59:34 2019 +0000
@@ -161,7 +161,6 @@
     enum SnapType {
         SnapLeft,
         SnapRight,
-        SnapNearest,
         SnapNeighbouring
     };
 
@@ -171,13 +170,12 @@
      *
      * If snap is SnapLeft or SnapRight, adjust the frame to match
      * that of the nearest feature in the given direction regardless
-     * of how far away it is.  If snap is SnapNearest, adjust the
-     * frame to that of the nearest feature in either direction.  If
-     * snap is SnapNeighbouring, adjust the frame to that of the
-     * nearest feature if it is close, and leave it alone (returning
-     * false) otherwise.  SnapNeighbouring should always choose the
-     * same feature that would be used in an editing operation through
-     * calls to editStart etc.
+     * of how far away it is. If snap is SnapNeighbouring, adjust the
+     * frame to that of the nearest feature in either direction if it
+     * is close, and leave it alone (returning false) otherwise.
+     * SnapNeighbouring should always choose the same feature that
+     * would be used in an editing operation through calls to
+     * editStart etc.
      *
      * Return true if a suitable feature was found and frame adjusted
      * accordingly.  Return false if no suitable feature was available
--- a/layer/SpectrogramLayer.cpp	Wed Mar 20 11:18:45 2019 +0000
+++ b/layer/SpectrogramLayer.cpp	Wed Mar 20 14:59:34 2019 +0000
@@ -1792,7 +1792,6 @@
     switch (snap) {
     case SnapLeft:  frame = left;  break;
     case SnapRight: frame = right; break;
-    case SnapNearest:
     case SnapNeighbouring:
         if (frame - left > right - frame) frame = right;
         else frame = left;
--- a/layer/TimeRulerLayer.cpp	Wed Mar 20 11:18:45 2019 +0000
+++ b/layer/TimeRulerLayer.cpp	Wed Mar 20 14:59:34 2019 +0000
@@ -87,16 +87,6 @@
     case SnapRight:
         frame = right;
         break;
-        
-    case SnapNearest:
-    {
-        if (llabs(frame - left) > llabs(right - frame)) {
-            frame = right;
-        } else {
-            frame = left;
-        }
-        break;
-    }
 
     case SnapNeighbouring:
     {
--- a/layer/TimeValueLayer.cpp	Wed Mar 20 11:18:45 2019 +0000
+++ b/layer/TimeValueLayer.cpp	Wed Mar 20 14:59:34 2019 +0000
@@ -536,60 +536,63 @@
 {
     if (!m_model) return {};
 
+    // Return all points at a frame f, where f is the closest frame to
+    // pixel coordinate x whose pixel coordinate is both within a
+    // small (but somewhat arbitrary) fuzz distance from x and within
+    // the current view. If there is no such frame, return an empty
+    // vector.
+    
     sv_frame_t frame = v->getFrameForX(x);
+    
+    EventVector exact = m_model->getEventsStartingAt(frame);
+    if (!exact.empty()) return exact;
 
-    //!!! this is not going to be right - we want "nearby" points and
-    //!!! there's no api for that
+    // overspill == 1, so one event either side of the given span
+    EventVector neighbouring = m_model->getEventsWithin
+        (frame, m_model->getResolution(), 1);
+
+    double fuzz = v->scaleSize(2);
+    sv_frame_t suitable = 0;
+    bool have = false;
     
-    EventVector local = m_model->getEventsCovering(frame);
-    if (!local.empty()) return local;
-
-    return {};
-/*!!!
-
-    SparseTimeValueModel::PointList prevPoints =
-        m_model->getPreviousPoints(frame);
-    SparseTimeValueModel::PointList nextPoints =
-        m_model->getNextPoints(frame);
-
-    SparseTimeValueModel::PointList usePoints = prevPoints;
-
-    if (prevPoints.empty()) {
-        usePoints = nextPoints;
-    } else if (nextPoints.empty()) {
-        // stick with prevPoints
-    } else if (prevPoints.begin()->frame < v->getStartFrame() &&
-               !(nextPoints.begin()->frame > v->getEndFrame())) {
-        usePoints = nextPoints;
-    } else if (nextPoints.begin()->frame - frame <
-               frame - prevPoints.begin()->frame) {
-        usePoints = nextPoints;
-    }
-
-    if (!usePoints.empty()) {
-        double fuzz = v->scaleSize(2);
-        int px = v->getXForFrame(usePoints.begin()->frame);
-        if ((px > x && px - x > fuzz) ||
-            (px < x && x - px > fuzz + 3)) {
-            usePoints.clear();
+    for (Event e: neighbouring) {
+        sv_frame_t f = e.getFrame();
+        if (f < v->getStartFrame() || f > v->getEndFrame()) {
+            continue;
+        }
+        int px = v->getXForFrame(f);
+        if ((px > x && px - x > fuzz) || (px < x && x - px > fuzz + 3)) {
+            continue;
+        }
+        if (!have) {
+            suitable = f;
+            have = true;
+        } else if (llabs(frame - f) < llabs(suitable - f)) {
+            suitable = f;
         }
     }
 
-    return usePoints;
-*/
+    if (have) {
+        return m_model->getEventsStartingAt(suitable);
+    } else {
+        return {};
+    }
 }
 
 QString
-TimeValueLayer::getLabelPreceding(sv_frame_t /* frame */) const
+TimeValueLayer::getLabelPreceding(sv_frame_t frame) const
 {
     if (!m_model || !m_model->hasTextLabels()) return "";
-/*!!! no corresponding api yet
-    EventVector points = m_model->getPreviousPoints(frame);
-    for (EventVector::const_iterator i = points.begin();
-         i != points.end(); ++i) {
-        if (i->getLabel() != "") return i->getLabel();
+
+    Event e;
+    if (m_model->getNearestEventMatching
+        (frame,
+         [](Event e) { return e.hasLabel() && e.getLabel() != ""; },
+         EventSeries::Backward,
+         e)) {
+        return e.getLabel();
     }
-*/
+
     return "";
 }
 
@@ -657,71 +660,42 @@
         return Layer::snapToFeatureFrame(v, frame, resolution, snap);
     }
 
+    // SnapLeft / SnapRight: return frame of nearest feature in that
+    // direction no matter how far away
+    // 
+    // SnapNearest: return frame of nearest feature in either
+    // direction no matter how far away - I'm fairly sure this is
+    // never actually used
+    //
+    // SnapNeighbouring: return frame of feature that would be used in
+    // an editing operation, i.e. closest feature in either direction
+    // but only if it is "close enough"
+    
     resolution = m_model->getResolution();
-    EventVector points;
 
     if (snap == SnapNeighbouring) {
-        points = getLocalPoints(v, v->getXForFrame(frame));
+        EventVector points = getLocalPoints(v, v->getXForFrame(frame));
         if (points.empty()) return false;
         frame = points.begin()->getFrame();
         return true;
-    }    
-
-    //!!! again, wrong api - correct one is not here yet
-    points = m_model->getEventsCovering(frame);
-    sv_frame_t snapped = frame;
-    bool found = false;
-
-    for (EventVector::const_iterator i = points.begin();
-         i != points.end(); ++i) {
-
-        if (snap == SnapRight) {
-
-            if (i->getFrame() > frame) {
-                snapped = i->getFrame();
-                found = true;
-                break;
-            }
-
-        } else if (snap == SnapLeft) {
-
-            if (i->getFrame() <= frame) {
-                snapped = i->getFrame();
-                found = true; // don't break, as the next may be better
-            } else {
-                break;
-            }
-
-        } else { // nearest
-
-            EventVector::const_iterator j = i;
-            ++j;
-
-            if (j == points.end()) {
-
-                snapped = i->getFrame();
-                found = true;
-                break;
-
-            } else if (j->getFrame() >= frame) {
-
-                if (j->getFrame() - frame < frame - i->getFrame()) {
-                    snapped = j->getFrame();
-                } else {
-                    snapped = i->getFrame();
-                }
-                found = true;
-                break;
-            }
-        }
     }
 
-    frame = snapped;
-    return found;
+    Event e;
+    if (m_model->getNearestEventMatching
+        (frame,
+         [](Event) { return true; },
+         snap == SnapLeft ? EventSeries::Backward : EventSeries::Forward,
+         e)) {
+        frame = e.getFrame();
+        return true;
+    }
+
+    return false;
 }
 
 bool
-TimeValueLayer::snapToSimilarFeature(LayerGeometryProvider *v, sv_frame_t &frame,
+TimeValueLayer::snapToSimilarFeature(LayerGeometryProvider *v,
+                                     sv_frame_t &frame,
                                      int &resolution,
                                      SnapType snap) const
 {
@@ -731,80 +705,35 @@
 
     resolution = m_model->getResolution();
 
-    /*!!! todo: overhaul the logic of this function (and supporting
-          apis in EventSeries / SparseTimeValueModel)
+    Event ref;
+    Event e;
+    float matchvalue;
+    bool found;
 
-    const EventVector &points = m_model->getPoints();
-    EventVector close = m_model->getPoints(frame, frame);
-    */
-    const EventVector &points = m_model->getAllEvents();
-    EventVector close = {};
+    found = m_model->getNearestEventMatching
+        (frame, [](Event) { return true; }, EventSeries::Backward, ref);
 
-    EventVector::const_iterator i;
-
-    sv_frame_t matchframe = frame;
-    double matchvalue = 0.0;
-
-    for (i = close.begin(); i != close.end(); ++i) {
-        if (i->getFrame() > frame) break;
-        matchvalue = i->getValue();
-        matchframe = i->getFrame();
+    if (!found) {
+        return false;
     }
 
-    sv_frame_t snapped = frame;
-    bool found = false;
-    bool distant = false;
-    double epsilon = 0.0001;
+    matchvalue = ref.getValue();
+    
+    found = m_model->getNearestEventMatching
+        (frame,
+         [matchvalue](Event e) {
+             double epsilon = 0.0001;
+             return fabs(e.getValue() - matchvalue) < epsilon;
+         },
+         snap == SnapLeft ? EventSeries::Backward : EventSeries::Forward,
+         e);
 
-    i = close.begin();
-
-    // Scan through the close points first, then the more distant ones
-    // if no suitable close one is found. So the while-termination
-    // condition here can only happen once i has passed through the
-    // whole of the close container and then the whole of the separate
-    // points container. The two iterators are totally distinct, but
-    // have the same type so we cheekily use the same variable and a
-    // single loop for both.
-
-    while (i != points.end()) {
-
-        if (!distant) {
-            if (i == close.end()) {
-                // switch from the close container to the points container
-                i = points.begin();
-                distant = true;
-            }
-        }
-
-        if (snap == SnapRight) {
-
-            if (i->getFrame() > matchframe &&
-                fabs(i->getValue() - matchvalue) < epsilon) {
-                snapped = i->getFrame();
-                found = true;
-                break;
-            }
-
-        } else if (snap == SnapLeft) {
-
-            if (i->getFrame() < matchframe) {
-                if (fabs(i->getValue() - matchvalue) < epsilon) {
-                    snapped = i->getFrame();
-                    found = true; // don't break, as the next may be better
-                }
-            } else if (found || distant) {
-                break;
-            }
-
-        } else { 
-            // no other snap types supported
-        }
-
-        ++i;
+    if (!found) {
+        return false;
     }
 
-    frame = snapped;
-    return found;
+    frame = e.getFrame();
+    return true;
 }
 
 void