# HG changeset patch
# User Chris Cannam
# Date 1574344977 0
# Node ID e79731086b0f1f2d59ecdaac91bb201f65b435a8
# Parent  bd6af89982d7486a78a2cb4b20182d0f7f8cefad
Fixes to NoteLayer, particularly to calculation of vertical scale when model unit is not Hz. To avoid inconsistency we now behave as if the unit is always Hz from the point of view of the external API and display, converting at the point where we obtain values from the events themselves. Also various fixes to editing.

diff -r bd6af89982d7 -r e79731086b0f layer/NoteLayer.cpp
--- a/layer/NoteLayer.cpp	Thu Oct 17 14:44:22 2019 +0100
+++ b/layer/NoteLayer.cpp	Thu Nov 21 14:02:57 2019 +0000
@@ -48,6 +48,7 @@
 
 NoteLayer::NoteLayer() :
     SingleColourLayer(),
+    m_modelUsesHz(true),
     m_editing(false),
     m_dragPointX(0),
     m_dragPointY(0),
@@ -86,6 +87,9 @@
 
     if (newModel) {
         connectSignals(m_model);
+
+        QString unit = newModel->getScaleUnits();
+        m_modelUsesHz = (unit.toLower() == "hz");
     }
     
     m_scaleMinimum = 0;
@@ -131,9 +135,7 @@
 QString
 NoteLayer::getScaleUnits() const
 {
-    auto model = ModelById::getAs<NoteModel>(m_model);
-    if (model) return model->getScaleUnits();
-    else return "";
+    return "Hz";
 }
 
 int
@@ -191,8 +193,9 @@
     } else if (name == "Scale Units") {
         auto model = ModelById::getAs<NoteModel>(m_model);
         if (model) {
-            model->setScaleUnits
-                (UnitDatabase::getInstance()->getUnitById(value));
+            QString unit = UnitDatabase::getInstance()->getUnitById(value);
+            model->setScaleUnits(unit);
+            m_modelUsesHz = (unit.toLower() == "hz");
             emit modelChanged(m_model);
         }
     } else {
@@ -215,15 +218,43 @@
     return !v->shouldIlluminateLocalFeatures(this, discard);
 }
 
-bool
-NoteLayer::shouldConvertMIDIToHz() const
+double
+NoteLayer::valueOf(const Event &e) const
 {
-    QString unit = getScaleUnits();
-    return (unit != "Hz");
-//    if (unit == "" ||
-//        unit.startsWith("MIDI") ||
-//        unit.startsWith("midi")) return true;
-//    return false;
+    return convertValueFromEventValue(e.getValue());
+}
+
+Event
+NoteLayer::eventWithValue(const Event &e, double value) const
+{
+    return e.withValue(convertValueToEventValue(value));
+}
+
+double
+NoteLayer::convertValueFromEventValue(float eventValue) const
+{
+    if (m_modelUsesHz) {
+        return eventValue;
+    } else {
+        double v = eventValue;
+        if (v < 0) v = 0;
+        if (v > 127) v = 127;
+        int p = int(round(v));
+        double c = 100.0 * (v - p);
+        return Pitch::getFrequencyForPitch(p, c);
+    }
+}
+
+float
+NoteLayer::convertValueToEventValue(double value) const
+{
+    if (m_modelUsesHz) {
+        return float(value);
+    } else {
+        float c = 0;
+        int p = Pitch::getPitchForFrequency(value, &c);
+        return float(p) + c / 100.f;
+    }
 }
 
 bool
@@ -232,17 +263,14 @@
 {
     auto model = ModelById::getAs<NoteModel>(m_model);
     if (!model) return false;
-    min = model->getValueMinimum();
-    max = model->getValueMaximum();
 
-    if (shouldConvertMIDIToHz()) {
-        unit = "Hz";
-        min = Pitch::getFrequencyForPitch(int(lrint(min)));
-        max = Pitch::getFrequencyForPitch(int(lrint(max + 1)));
-    } else unit = getScaleUnits();
+    min = convertValueFromEventValue(model->getValueMinimum());
+    max = convertValueFromEventValue(model->getValueMaximum());
+    min /= 1.06;
+    max *= 1.06;
+    unit = "Hz";
 
-    if (m_verticalScale == MIDIRangeScale ||
-        m_verticalScale == LogScale) {
+    if (m_verticalScale != LinearScale) {
         logarithmic = true;
     }
 
@@ -262,20 +290,16 @@
     }
 
     if (m_scaleMinimum == m_scaleMaximum) {
-        min = model->getValueMinimum();
-        max = model->getValueMaximum();
+        QString unit;
+        bool log = false;
+        getValueExtents(min, max, log, unit);
     } else {
         min = m_scaleMinimum;
         max = m_scaleMaximum;
     }
 
-    if (shouldConvertMIDIToHz()) {
-        min = Pitch::getFrequencyForPitch(int(lrint(min)));
-        max = Pitch::getFrequencyForPitch(int(lrint(max + 1)));
-    }
-
 #ifdef DEBUG_NOTE_LAYER
-    cerr << "NoteLayer::getDisplayExtents: min = " << min << ", max = " << max << " (m_scaleMinimum = " << m_scaleMinimum << ", m_scaleMaximum = " << m_scaleMaximum << ")" << endl;
+    SVCERR << "NoteLayer::getDisplayExtents: min = " << min << ", max = " << max << " (m_scaleMinimum = " << m_scaleMinimum << ", m_scaleMaximum = " << m_scaleMaximum << ")" << endl;
 #endif
 
     return true;
@@ -298,7 +322,7 @@
     m_scaleMaximum = max;
 
 #ifdef DEBUG_NOTE_LAYER
-    cerr << "NoteLayer::setDisplayExtents: min = " << min << ", max = " << max << endl;
+    SVCERR << "NoteLayer::setDisplayExtents: min = " << min << ", max = " << max << endl;
 #endif
     
     emit layerParametersChanged();
@@ -377,7 +401,7 @@
     }
     
 #ifdef DEBUG_NOTE_LAYER
-    cerr << "NoteLayer::setVerticalZoomStep: " << step << ": " << newmin << " -> " << newmax << " (range " << newdist << ")" << endl;
+    SVCERR << "NoteLayer::setVerticalZoomStep: " << step << ": " << newmin << " -> " << newmax << " (range " << newdist << ")" << endl;
 #endif
 
     setDisplayExtents(newmin, newmax);
@@ -443,7 +467,7 @@
 
     int nearestDistance = -1;
     for (const auto &p: onPoints) {
-        int distance = getYForValue(v, p.getValue()) - y;
+        int distance = getYForValue(v, valueOf(p)) - y;
         if (distance < 0) distance = -distance;
         if (nearestDistance == -1 || distance < nearestDistance) {
             nearestDistance = distance;
@@ -477,12 +501,13 @@
 
     for (i = points.begin(); i != points.end(); ++i) {
 
-        int y = getYForValue(v, i->getValue());
+        int y = getYForValue(v, valueOf(*i));
         int h = 3;
 
         if (model->getValueQuantization() != 0.0) {
             h = y - getYForValue
-                (v, i->getValue() + model->getValueQuantization());
+                (v, convertValueFromEventValue(i->getValue() +
+                                               model->getValueQuantization()));
             if (h < 3) h = 3;
         }
 
@@ -501,28 +526,27 @@
     
     QString pitchText;
 
-    float value = note.getValue();
+    if (m_modelUsesHz) {
+
+        float value = note.getValue();
     
-    if (shouldConvertMIDIToHz()) {
-
-        int mnote = int(lrint(value));
-        int cents = int(lrint((value - float(mnote)) * 100));
-        double freq = Pitch::getFrequencyForPitch(mnote, cents);
-        pitchText = tr("%1 (%2, %3 Hz)")
-            .arg(Pitch::getPitchLabel(mnote, cents))
-            .arg(mnote)
-            .arg(freq);
-
-    } else if (getScaleUnits() == "Hz") {
-
         pitchText = tr("%1 Hz (%2, %3)")
             .arg(value)
             .arg(Pitch::getPitchLabelForFrequency(value))
             .arg(Pitch::getPitchForFrequency(value));
 
     } else {
-        pitchText = tr("%1 %2")
-            .arg(value).arg(getScaleUnits());
+        
+        float eventValue = note.getValue();
+        double value = convertValueFromEventValue(eventValue);
+        
+        int mnote = int(lrint(eventValue));
+        int cents = int(lrint((eventValue - float(mnote)) * 100));
+
+        pitchText = tr("%1 (%2, %3 Hz)")
+            .arg(Pitch::getPitchLabel(mnote, cents))
+            .arg(eventValue)
+            .arg(value);
     }
 
     QString text;
@@ -540,7 +564,8 @@
             .arg(note.getLabel());
     }
 
-    pos = QPoint(v->getXForFrame(note.getFrame()), getYForValue(v, value));
+    pos = QPoint(v->getXForFrame(note.getFrame()),
+                 getYForValue(v, valueOf(note)));
     return text;
 }
 
@@ -593,24 +618,15 @@
     auto model = ModelById::getAs<NoteModel>(m_model);
     if (!model) return;
     
-    QString queryUnits;
-    if (shouldConvertMIDIToHz()) queryUnits = "Hz";
-    else queryUnits = getScaleUnits();
-
     if (shouldAutoAlign()) {
 
-        if (!v->getVisibleExtentsForUnit(queryUnits, min, max, log)) {
+        if (!v->getVisibleExtentsForUnit("Hz", min, max, log)) {
 
-            min = model->getValueMinimum();
-            max = model->getValueMaximum();
-
-            if (shouldConvertMIDIToHz()) {
-                min = Pitch::getFrequencyForPitch(int(lrint(min)));
-                max = Pitch::getFrequencyForPitch(int(lrint(max + 1)));
-            }
+            QString unit;
+            getValueExtents(min, max, log, unit);
 
 #ifdef DEBUG_NOTE_LAYER
-            cerr << "NoteLayer[" << this << "]::getScaleExtents: min = " << min << ", max = " << max << ", log = " << log << endl;
+            SVCERR << "NoteLayer[" << this << "]::getScaleExtents: min = " << min << ", max = " << max << ", log = " << log << endl;
 #endif
 
         } else if (log) {
@@ -618,24 +634,15 @@
             LogRange::mapRange(min, max);
 
 #ifdef DEBUG_NOTE_LAYER
-            cerr << "NoteLayer[" << this << "]::getScaleExtents: min = " << min << ", max = " << max << ", log = " << log << endl;
+            SVCERR << "NoteLayer[" << this << "]::getScaleExtents: min = " << min << ", max = " << max << ", log = " << log << endl;
 #endif
-
         }
 
     } else {
 
         getDisplayExtents(min, max);
 
-        if (m_verticalScale == MIDIRangeScale) {
-            min = Pitch::getFrequencyForPitch(0);
-            max = Pitch::getFrequencyForPitch(127);
-        } else if (shouldConvertMIDIToHz()) {
-            min = Pitch::getFrequencyForPitch(int(lrint(min)));
-            max = Pitch::getFrequencyForPitch(int(lrint(max + 1)));
-        }
-
-        if (m_verticalScale == LogScale || m_verticalScale == MIDIRangeScale) {
+        if (m_verticalScale != LinearScale) {
             LogRange::mapRange(min, max);
             log = true;
         }
@@ -654,27 +661,19 @@
     getScaleExtents(v, min, max, logarithmic);
 
 #ifdef DEBUG_NOTE_LAYER
-    cerr << "NoteLayer[" << this << "]::getYForValue(" << val << "): min = " << min << ", max = " << max << ", log = " << logarithmic << endl;
+    SVCERR << "NoteLayer[" << this << "]::getYForValue(" << val << "): min = " << min << ", max = " << max << ", log = " << logarithmic << endl;
 #endif
 
-    if (shouldConvertMIDIToHz()) {
-        val = Pitch::getFrequencyForPitch(int(lrint(val)),
-                                          int(lrint((val - rint(val)) * 100)));
-#ifdef DEBUG_NOTE_LAYER
-        cerr << "shouldConvertMIDIToHz true, val now = " << val << endl;
-#endif
-    }
-
     if (logarithmic) {
         val = LogRange::map(val);
 #ifdef DEBUG_NOTE_LAYER
-        cerr << "logarithmic true, val now = " << val << endl;
+        SVCERR << "logarithmic true, val now = " << val << endl;
 #endif
     }
 
     int y = int(h - ((val - min) * h) / (max - min)) - 1;
 #ifdef DEBUG_NOTE_LAYER
-    cerr << "y = " << y << endl;
+    SVCERR << "y = " << y << endl;
 #endif
     return y;
 }
@@ -694,10 +693,6 @@
         val = pow(10.0, val);
     }
 
-    if (shouldConvertMIDIToHz()) {
-        val = Pitch::getPitchForFrequency(val);
-    }
-
     return val;
 }
 
@@ -734,20 +729,20 @@
 //    SVDEBUG << "NoteLayer::paint: resolution is "
 //              << model->getResolution() << " frames" << endl;
 
-    double min = model->getValueMinimum();
-    double max = model->getValueMaximum();
+    double min = convertValueFromEventValue(model->getValueMinimum());
+    double max = convertValueFromEventValue(model->getValueMaximum());
     if (max == min) max = min + 1.0;
 
     QPoint localPos;
     Event illuminatePoint;
     bool shouldIlluminate = false;
 
-    if (v->shouldIlluminateLocalFeatures(this, localPos)) {
+    if (m_editing || m_editIsOpen) {
+        shouldIlluminate = true;
+        illuminatePoint = m_editingPoint;
+    } else if (v->shouldIlluminateLocalFeatures(this, localPos)) {
         shouldIlluminate = getPointToDrag(v, localPos.x(), localPos.y(),
                                           illuminatePoint);
-    } else if (m_editIsOpen) {
-        shouldIlluminate = true;
-        illuminatePoint = m_editingPoint;
     }
 
     paint.save();
@@ -759,12 +754,14 @@
         const Event &p(*i);
 
         int x = v->getXForFrame(p.getFrame());
-        int y = getYForValue(v, p.getValue());
+        int y = getYForValue(v, valueOf(p));
         int w = v->getXForFrame(p.getFrame() + p.getDuration()) - x;
         int h = 3;
         
         if (model->getValueQuantization() != 0.0) {
-            h = y - getYForValue(v, p.getValue() + model->getValueQuantization());
+            h = y - getYForValue
+                (v, convertValueFromEventValue
+                 (p.getValue() + model->getValueQuantization()));
             if (h < 3) h = 3;
         }
 
@@ -782,7 +779,17 @@
     // which is too new for us
 #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
 
-            QString vlabel = QString("%1%2").arg(p.getValue()).arg(getScaleUnits());
+            QString vlabel;
+            if (m_modelUsesHz) {
+                vlabel = QString("%1%2")
+                    .arg(p.getValue())
+                    .arg(model->getScaleUnits());
+            } else {
+                vlabel = QString("%1 %2")
+                    .arg(p.getValue())
+                    .arg(model->getScaleUnits());
+            }
+            
             PaintAssistant::drawVisibleText(v, paint, 
                                x - paint.fontMetrics().width(vlabel) - 2,
                                y + paint.fontMetrics().height()/2
@@ -814,7 +821,7 @@
         return 0;
     }
 
-    if (m_verticalScale == LogScale || m_verticalScale == MIDIRangeScale) {
+    if (m_verticalScale != LinearScale) {
         return LogNumericalScale().getWidth(v, paint) + 10; // for piano
     } else {
         return LinearNumericalScale().getWidth(v, paint);
@@ -842,7 +849,7 @@
         LinearNumericalScale().paintVertical(v, this, paint, 0, min, max);
     }
     
-    if (logarithmic && (getScaleUnits() == "Hz")) {
+    if (logarithmic) {
         PianoScale().paintPianoVertical
             (v, paint, QRect(w - 10, 0, 10, h), 
              LogRange::unmap(min), 
@@ -873,8 +880,10 @@
     frame = frame / model->getResolution() * model->getResolution();
 
     double value = getValueForY(v, e->y());
+    float eventValue = convertValueToEventValue(value);
+    eventValue = roundf(eventValue);
 
-    m_editingPoint = Event(frame, float(value), 0, 0.8f, tr("New Point"));
+    m_editingPoint = Event(frame, eventValue, 0, 0.8f, tr("New Point"));
     m_originalPoint = m_editingPoint;
 
     if (m_editingCommand) finish(m_editingCommand);
@@ -897,6 +906,8 @@
     frame = frame / model->getResolution() * model->getResolution();
 
     double newValue = getValueForY(v, e->y());
+    float newEventValue = convertValueToEventValue(newValue);
+    newEventValue = roundf(newEventValue);
 
     sv_frame_t newFrame = m_editingPoint.getFrame();
     sv_frame_t newDuration = frame - newFrame;
@@ -910,8 +921,8 @@
     m_editingCommand->remove(m_editingPoint);
     m_editingPoint = m_editingPoint
         .withFrame(newFrame)
-        .withValue(float(newValue))
-        .withDuration(newDuration);
+        .withDuration(newDuration)
+        .withValue(newEventValue);
     m_editingCommand->add(m_editingPoint);
 }
 
@@ -981,7 +992,7 @@
     m_originalPoint = m_editingPoint;
 
     m_dragPointX = v->getXForFrame(m_editingPoint.getFrame());
-    m_dragPointY = getYForValue(v, m_editingPoint.getValue());
+    m_dragPointY = getYForValue(v, valueOf(m_editingPoint));
 
     if (m_editingCommand) {
         finish(m_editingCommand);
@@ -1010,7 +1021,9 @@
     if (frame < 0) frame = 0;
     frame = frame / model->getResolution() * model->getResolution();
 
-    double value = getValueForY(v, newy);
+    double newValue = getValueForY(v, newy);
+    float newEventValue = convertValueToEventValue(newValue);
+    newEventValue = roundf(newEventValue);
 
     if (!m_editingCommand) {
         m_editingCommand = new ChangeEventsCommand
@@ -1020,7 +1033,7 @@
     m_editingCommand->remove(m_editingPoint);
     m_editingPoint = m_editingPoint
         .withFrame(frame)
-        .withValue(float(value));
+        .withValue(newEventValue);
     m_editingCommand->add(m_editingPoint);
 }
 
@@ -1270,7 +1283,9 @@
 void
 NoteLayer::addNoteOn(sv_frame_t frame, int pitch, int velocity)
 {
-    m_pendingNoteOns.insert(Event(frame, float(pitch), 0,
+    double value = Pitch::getFrequencyForPitch(pitch);
+    float eventValue = convertValueToEventValue(value);
+    m_pendingNoteOns.insert(Event(frame, eventValue, 0,
                                   float(velocity) / 127.f, QString()));
 }
 
@@ -1283,8 +1298,10 @@
          i != m_pendingNoteOns.end(); ++i) {
 
         Event p = *i;
+        double value = valueOf(p);
+        int eventPitch = Pitch::getPitchForFrequency(value);
 
-        if (lrintf(p.getValue()) == pitch) {
+        if (eventPitch == pitch) {
             m_pendingNoteOns.erase(i);
             Event note = p.withDuration(frame - p.getFrame());
             if (model) {
diff -r bd6af89982d7 -r e79731086b0f layer/NoteLayer.h
--- a/layer/NoteLayer.h	Thu Oct 17 14:44:22 2019 +0100
+++ b/layer/NoteLayer.h	Thu Nov 21 14:02:57 2019 +0000
@@ -27,6 +27,17 @@
 class View;
 class QPainter;
 
+/**
+ * Layer for displaying and editing notes, i.e. discrete events with
+ * start time, duration, value that represents pitch, and optionally a
+ * level that represents velocity.
+ *
+ * For the purposes of public API, integration with other classes, and
+ * display alignment, the y-coordinate (value) of the layer always has
+ * a unit of Hz. The model itself may have another unit, such as MIDI
+ * pitch, but the layer always converts to and from Hz behind the
+ * scenes.
+ */
 class NoteLayer : public SingleColourLayer,
                   public VerticalScaleLayer
 {
@@ -65,8 +76,8 @@
     void deleteSelection(Selection s) override;
 
     void copy(LayerGeometryProvider *v, Selection s, Clipboard &to) override;
-    bool paste(LayerGeometryProvider *v, const Clipboard &from, sv_frame_t frameOffset,
-                       bool interactive) override;
+    bool paste(LayerGeometryProvider *v, const Clipboard &from,
+               sv_frame_t frameOffset, bool interactive) override;
 
     ModelId getModel() const override { return m_model; }
     void setModel(ModelId model); // a NoteModel
@@ -76,9 +87,9 @@
     PropertyType getPropertyType(const PropertyName &) const override;
     QString getPropertyGroupName(const PropertyName &) const override;
     int getPropertyRangeAndValue(const PropertyName &,
-                                         int *min, int *max, int *deflt) const override;
+                                 int *min, int *max, int *deflt) const override;
     QString getPropertyValueLabel(const PropertyName &,
-                                          int value) const override;
+                                  int value) const override;
     void setProperty(const PropertyName &, int value) override;
 
     enum VerticalScale {
@@ -98,7 +109,7 @@
     int getCompletion(LayerGeometryProvider *) const override;
 
     bool getValueExtents(double &min, double &max,
-                                 bool &log, QString &unit) const override;
+                         bool &log, QString &unit) const override;
 
     bool getDisplayExtents(double &min, double &max) const override;
     bool setDisplayExtents(double min, double max) override;
@@ -138,7 +149,6 @@
 
 protected:
     void getScaleExtents(LayerGeometryProvider *, double &min, double &max, bool &log) const;
-    bool shouldConvertMIDIToHz() const;
 
     int getDefaultColourHint(bool dark, bool &impose) override;
 
@@ -146,7 +156,14 @@
 
     bool getPointToDrag(LayerGeometryProvider *v, int x, int y, Event &) const;
 
+    double convertValueFromEventValue(float eventValue) const;
+    float convertValueToEventValue(double value) const;
+    
+    double valueOf(const Event &e) const;
+    Event eventWithValue(const Event &e, double value) const;
+    
     ModelId m_model;
+    bool m_modelUsesHz;
     bool m_editing;
     int m_dragPointX;
     int m_dragPointY;