Add missing methods to scatter proxy api

+ remove data mutex

Change-Id: Ie0f150d3ecb51076a998c2bdc2ba6fec46941793
Reviewed-by: Tomi Korpipää <tomi.korpipaa@digia.com>

23 files changed, 167 insertions, 123 deletions

diff --git a/src/datavis3d/data/barrenderitem.cpp b/src/datavis3d/data/barrenderitem.cpp
index 5a9b69c7..404e5027 100644
--- a/src/datavis3d/data/barrenderitem.cpp
+++ b/src/datavis3d/data/barrenderitem.cpp
@@ -32,7 +32,6 @@ BarRenderItem::~BarRenderItem()
 {
 }
 
-// This should be accessed only under data mutex
 void BarRenderItem::formatLabel()
 {
     // Format the string on first access
@@ -41,7 +40,7 @@ void BarRenderItem::formatLabel()
     // TODO actually format instead of just prepending the value
     m_label.clear(); // Just in case
     m_label.append(numStr);
-    m_label.append(m_renderer->m_dataProxy->itemLabelFormat());
+    m_label.append(m_renderer->m_dataProxy->itemLabelFormat()); // TODO format needs to be cached
 }
 
 QT_DATAVIS3D_END_NAMESPACE
diff --git a/src/datavis3d/data/qabstractdataproxy.cpp b/src/datavis3d/data/qabstractdataproxy.cpp
index ce4ae67a..11874edb 100644
--- a/src/datavis3d/data/qabstractdataproxy.cpp
+++ b/src/datavis3d/data/qabstractdataproxy.cpp
@@ -47,12 +47,6 @@ QString QAbstractDataProxy::itemLabelFormat() const
     return d_ptr->m_itemLabelFormat;
 }
 
-QMutex *QAbstractDataProxy::mutex()
-{
-    return &d_ptr->m_mutex;
-}
-
-
 // QAbstractDataProxyPrivate
 
 QAbstractDataProxyPrivate::QAbstractDataProxyPrivate(QAbstractDataProxy *q, QAbstractDataProxy::DataType type)
@@ -68,7 +62,6 @@ QAbstractDataProxyPrivate::~QAbstractDataProxyPrivate()
 
 void QAbstractDataProxyPrivate::setItemLabelFormat(const QString &format)
 {
-    QMutexLocker locker(&m_mutex);
     m_itemLabelFormat = format;
 }
 
diff --git a/src/datavis3d/data/qabstractdataproxy.h b/src/datavis3d/data/qabstractdataproxy.h
index df5edcf6..dbb06c7b 100644
--- a/src/datavis3d/data/qabstractdataproxy.h
+++ b/src/datavis3d/data/qabstractdataproxy.h
@@ -22,7 +22,6 @@
 #include <QtDataVis3D/qdatavis3dnamespace.h>
 #include <QObject>
 #include <QScopedPointer>
-#include <QMutex>
 
 QT_DATAVIS3D_BEGIN_NAMESPACE
 
@@ -55,8 +54,6 @@ public:
     void setItemLabelFormat(const QString &format);
     QString itemLabelFormat() const;
 
-    QMutex *mutex();
-
 signals:
     void itemLabelFormatChanged();
 
diff --git a/src/datavis3d/data/qabstractdataproxy_p.h b/src/datavis3d/data/qabstractdataproxy_p.h
index 363e5d77..eda13b86 100644
--- a/src/datavis3d/data/qabstractdataproxy_p.h
+++ b/src/datavis3d/data/qabstractdataproxy_p.h
@@ -29,7 +29,6 @@
 #include "datavis3dglobal_p.h"
 #include "qabstractdataproxy.h"
 #include <QString>
-#include <QMutex>
 
 #ifndef QABSTRACTDATAPROXY_P_H
 #define QABSTRACTDATAPROXY_P_H
@@ -48,7 +47,6 @@ public:
 protected:
     QAbstractDataProxy *q_ptr;
     QAbstractDataProxy::DataType m_type;
-    QMutex m_mutex;
     QString m_itemLabelFormat;
 
 private:
diff --git a/src/datavis3d/data/qbardataitem.cpp b/src/datavis3d/data/qbardataitem.cpp
index 6aebd346..1e8f3d95 100644
--- a/src/datavis3d/data/qbardataitem.cpp
+++ b/src/datavis3d/data/qbardataitem.cpp
@@ -36,7 +36,14 @@ QT_DATAVIS3D_BEGIN_NAMESPACE
  * Constructs QBarDataItem.
  */
 QBarDataItem::QBarDataItem()
-    : d_ptr(0) // private data doesn't exist by default (optimization)
+    : d_ptr(0), // private data doesn't exist by default (optimization)
+      m_value(0.0)
+{
+}
+
+QBarDataItem::QBarDataItem(qreal value)
+    : d_ptr(0),
+      m_value(value)
 {
 }
 
diff --git a/src/datavis3d/data/qbardataitem.h b/src/datavis3d/data/qbardataitem.h
index 8be34f65..dbc9ade8 100644
--- a/src/datavis3d/data/qbardataitem.h
+++ b/src/datavis3d/data/qbardataitem.h
@@ -29,6 +29,7 @@ class QT_DATAVIS3D_EXPORT QBarDataItem
 {
 public:
     QBarDataItem();
+    QBarDataItem(qreal value);
     QBarDataItem(const QBarDataItem &other);
     ~QBarDataItem();
 
diff --git a/src/datavis3d/data/qbardataproxy.cpp b/src/datavis3d/data/qbardataproxy.cpp
index d830c74b..589ed37a 100644
--- a/src/datavis3d/data/qbardataproxy.cpp
+++ b/src/datavis3d/data/qbardataproxy.cpp
@@ -18,7 +18,6 @@
 
 #include "qbardataproxy.h"
 #include "qbardataproxy_p.h"
-#include <QMutexLocker>
 
 QT_DATAVIS3D_BEGIN_NAMESPACE
 
@@ -50,9 +49,8 @@ void QBarDataProxy::setRow(int rowIndex, QBarDataRow *row)
 
 void QBarDataProxy::setRows(int rowIndex, const QBarDataArray &rows)
 {
-    int count = rows.size();
     dptr()->setRows(rowIndex, rows);
-    emit rowsChanged(rowIndex, count);
+    emit rowsChanged(rowIndex, rows.size());
 }
 
 void QBarDataProxy::setItem(int rowIndex, int columnIndex, const QBarDataItem &item)
@@ -95,7 +93,6 @@ void QBarDataProxy::removeRows(int rowIndex, int removeCount)
     }
 }
 
-// Mutexing data accessors should be done by user, if needed
 int QBarDataProxy::rowCount() const
 {
     return dptrc()->m_dataArray->size();
@@ -147,8 +144,6 @@ QBarDataProxyPrivate::~QBarDataProxyPrivate()
 
 bool QBarDataProxyPrivate::resetArray(QBarDataArray *newArray)
 {
-    QMutexLocker locker(&m_mutex);
-
     if (!m_dataArray->size() && (!newArray || !newArray->size()))
         return false;
 
@@ -164,7 +159,6 @@ bool QBarDataProxyPrivate::resetArray(QBarDataArray *newArray)
 
 void QBarDataProxyPrivate::setRow(int rowIndex, QBarDataRow *row)
 {
-    QMutexLocker locker(&m_mutex);
     Q_ASSERT(rowIndex >= 0 && rowIndex < m_dataArray->size());
     clearRow(rowIndex);
     (*m_dataArray)[rowIndex] = row;
@@ -172,7 +166,6 @@ void QBarDataProxyPrivate::setRow(int rowIndex, QBarDataRow *row)
 
 void QBarDataProxyPrivate::setRows(int rowIndex, const QBarDataArray &rows)
 {
-    QMutexLocker locker(&m_mutex);
     QBarDataArray &dataArray = *m_dataArray;
     Q_ASSERT(rowIndex >= 0 && (rowIndex + rows.size()) <= dataArray.size());
     for (int i = 0; i < rows.size(); i++) {
@@ -184,7 +177,6 @@ void QBarDataProxyPrivate::setRows(int rowIndex, const QBarDataArray &rows)
 
 void QBarDataProxyPrivate::setItem(int rowIndex, int columnIndex, const QBarDataItem &item)
 {
-    QMutexLocker locker(&m_mutex);
     Q_ASSERT(rowIndex >= 0 && rowIndex < m_dataArray->size());
     QBarDataRow &row = *(*m_dataArray)[rowIndex];
     Q_ASSERT(columnIndex < row.size());
@@ -193,7 +185,6 @@ void QBarDataProxyPrivate::setItem(int rowIndex, int columnIndex, const QBarData
 
 int QBarDataProxyPrivate::addRow(QBarDataRow *row)
 {
-    QMutexLocker locker(&m_mutex);
     int currentSize = m_dataArray->size();
     m_dataArray->append(row);
     return currentSize;
@@ -201,7 +192,6 @@ int QBarDataProxyPrivate::addRow(QBarDataRow *row)
 
 int QBarDataProxyPrivate::addRows(const QBarDataArray &rows)
 {
-    QMutexLocker locker(&m_mutex);
     int currentSize = m_dataArray->size();
     for (int i = 0; i < rows.size(); i++)
         m_dataArray->append(rows.at(i));
@@ -210,14 +200,12 @@ int QBarDataProxyPrivate::addRows(const QBarDataArray &rows)
 
 void QBarDataProxyPrivate::insertRow(int rowIndex, QBarDataRow *row)
 {
-    QMutexLocker locker(&m_mutex);
     Q_ASSERT(rowIndex >= 0 && rowIndex <= m_dataArray->size());
     m_dataArray->insert(rowIndex, row);
 }
 
 void QBarDataProxyPrivate::insertRows(int rowIndex, const QBarDataArray &rows)
 {
-    QMutexLocker locker(&m_mutex);
     Q_ASSERT(rowIndex >= 0 && rowIndex <= m_dataArray->size());
     for (int i = 0; i < rows.size(); i++)
         m_dataArray->insert(rowIndex++, rows.at(i));
@@ -225,7 +213,6 @@ void QBarDataProxyPrivate::insertRows(int rowIndex, const QBarDataArray &rows)
 
 void QBarDataProxyPrivate::removeRows(int rowIndex, int removeCount)
 {
-    QMutexLocker locker(&m_mutex);
     Q_ASSERT(rowIndex >= 0);
     int maxRemoveCount = m_dataArray->size() - rowIndex;
     removeCount = qMin(removeCount, maxRemoveCount);
@@ -235,8 +222,6 @@ void QBarDataProxyPrivate::removeRows(int rowIndex, int removeCount)
     }
 }
 
-// Protected & private functions. Do not mutex as these are used from mutexed functions.
-
 void QBarDataProxyPrivate::clearRow(int rowIndex)
 {
     if (m_dataArray->at(rowIndex)) {
@@ -255,7 +240,6 @@ void QBarDataProxyPrivate::clearArray()
 
 QPair<GLfloat, GLfloat> QBarDataProxyPrivate::limitValues(int startRow, int endRow, int startColumn, int endColumn)
 {
-    QMutexLocker locker(&m_mutex);
     QPair<GLfloat, GLfloat> limits = qMakePair(0.0f, 0.0f);
     endRow = qMin(endRow, m_dataArray->size() - 1);
     for (int i = startRow; i <= endRow; i++) {
diff --git a/src/datavis3d/data/qbardataproxy.h b/src/datavis3d/data/qbardataproxy.h
index 14c224ed..e28f8ff0 100644
--- a/src/datavis3d/data/qbardataproxy.h
+++ b/src/datavis3d/data/qbardataproxy.h
@@ -42,36 +42,26 @@ public:
     // BarDataProxy is optimized for adding, inserting, and removing rows of data.
     // Adding a column essentially means modifying every row, which is comparatively very inefficient.
     // Proxy is also optimized to use cases where the only defining characteristic of an individual
-    // bar is its value. Modifying other data such as color or label format of individual bar
-    // requires allocating additional data object for the bar.
+    // bar is its value. Modifying other data that might be added in the future such as color of
+    // individual bar requires allocating additional data object for the bar.
 
-    // If data is accessed from same thread that sets it, access doesn't need to be protected with mutex.
     // Row and item pointers are guaranteed to be valid only until next call that modifies data.
     // Array pointer is guaranteed to be valid for lifetime of proxy.
     int rowCount() const;
     const QBarDataArray *array() const;
     const QBarDataRow *rowAt(int rowIndex) const;
-    const QBarDataItem *itemAt(int rowIndex, int columnIndex) const; // Row and column in said row need to exist or this crashes
+    const QBarDataItem *itemAt(int rowIndex, int columnIndex) const;
 
-    // The data array is a list of list (rows) of QBarDataItem instances.
+    // The data array is a list of vectors (rows) of QBarDataItem instances.
     // Each row can contain different amount of items or even be null.
 
-    // All array/item manipulation functions are internally protected by data mutex.
-
-    // TODO: Should we remove internal mutexing and require user to mutex also on data manipulations?
-    // TODO: Upside would be enabling user to mix data setters and getters within same mutex scope.
-    // TODO: Downside would be signal emissions from inside the mutex scope, which has potential for deadlock.
-
-    // TODO Should data manipulation/access methods be protected rather than public to enforce subclassing use of proxy?
-    // TODO Leaving them public gives user more options.
-
     // QBarDataProxy takes ownership of all QBarDataRows passed to it, whether directly or
     // in a QBarDataArray container.
     // QBarDataRow pointers should not be used to modify data further after they have been passed to
     // the proxy, as such modifications will not trigger proper signals.
 
-    // Clears the existing array and sets it data to new array.
-    // Takes ownership of the array. Passing null array clears all data.
+    // Clears the existing array and takes ownership of the new array.
+    // Passing null array clears all data.
     void resetArray(QBarDataArray *newArray);
 
     // Change existing rows
@@ -114,7 +104,7 @@ signals:
     // TODO void columnsAdded(int startIndex, int count);
     // TODO void columnsRemoved(int startIndex, int count);
     // TODO void columnsInserted(int startIndex, int count);
-    void itemChanged(int rowIndex, int columnIndex);
+    void itemChanged(int rowIndex, int columnIndex); // TODO remove once itemsChanged is added?
     // TODO void itemsChanged(int rowIndex, int columnIndex, int rowCount, int columnCount);
 
 protected:
diff --git a/src/datavis3d/data/qmapdataproxy.cpp b/src/datavis3d/data/qmapdataproxy.cpp
index 9604c589..a7a0e9d5 100644
--- a/src/datavis3d/data/qmapdataproxy.cpp
+++ b/src/datavis3d/data/qmapdataproxy.cpp
@@ -18,7 +18,6 @@
 
 #include "qmapdataproxy.h"
 #include "qmapdataproxy_p.h"
-#include <QMutexLocker>
 
 QT_DATAVIS3D_BEGIN_NAMESPACE
 
@@ -43,7 +42,6 @@ void QMapDataProxy::resetArray(QMapDataArray *newArray)
 }
 
 
-// Mutexing data accessors should be done by user, if needed
 int QMapDataProxy::itemCount() const
 {
     return dptrc()->m_dataArray.size();
@@ -83,8 +81,6 @@ QMapDataProxyPrivate::~QMapDataProxyPrivate()
 
 bool QMapDataProxyPrivate::resetArray(QMapDataArray *newArray)
 {
-    QMutexLocker locker(&m_mutex);
-
     if (!m_dataArray.size() && (!newArray || !newArray->size()))
         return false;
 
@@ -98,11 +94,8 @@ bool QMapDataProxyPrivate::resetArray(QMapDataArray *newArray)
     return true;
 }
 
-// Protected & private functions. Do not mutex as these are used from mutexed functions.
-
 QPair<GLfloat, GLfloat> QMapDataProxyPrivate::limitValues()
 {
-    QMutexLocker locker(&m_mutex);
     QPair<GLfloat, GLfloat> limits = qMakePair(0.0f, 0.0f);
     for (int i = 0; i < m_dataArray.size(); i++) {
         const QMapDataItem &item = m_dataArray.at(i);
diff --git a/src/datavis3d/data/qmapdataproxy.h b/src/datavis3d/data/qmapdataproxy.h
index 4aaec67f..45bb95d5 100644
--- a/src/datavis3d/data/qmapdataproxy.h
+++ b/src/datavis3d/data/qmapdataproxy.h
@@ -41,19 +41,12 @@ public:
     // map item are it's value and position. Modifying other data such as color or mesh of individual bar
     // requires allocating additional data object for the bar.
 
-    // If data is accessed from same thread that sets it, access doesn't need to be protected with mutex.
     // Item pointers are guaranteed to be valid only until next call that modifies data.
     // Array pointer is guaranteed to be valid for lifetime of proxy.
     int itemCount() const;
     const QMapDataArray *array() const;
     const QMapDataItem *itemAt(int index) const; // Index needs to exist or this crashes
 
-    // All array/item manipulation functions are internally protected by data mutex.
-
-    // TODO: Should we remove internal mutexing and require user to mutex also on data manipulations?
-    // TODO: Upside would be enabling user to mix data setters and getters within same mutex scope.
-    // TODO: Downside would be signal emissions from inside the mutex scope, which has potential for deadlock.
-
     // TODO Should data manipulation/access methods be protected rather than public to enforce subclassing use of proxy?
     // TODO Leaving them public gives user more options.
 
diff --git a/src/datavis3d/data/qscatterdataitem.cpp b/src/datavis3d/data/qscatterdataitem.cpp
index f917f962..e2580339 100644
--- a/src/datavis3d/data/qscatterdataitem.cpp
+++ b/src/datavis3d/data/qscatterdataitem.cpp
@@ -37,6 +37,14 @@ QT_DATAVIS3D_BEGIN_NAMESPACE
  * Constructs QScatterDataItem.
  */
 QScatterDataItem::QScatterDataItem()
+    : d_ptr(0) // private data doesn't exist by default (optimization)
+
+{
+}
+
+QScatterDataItem::QScatterDataItem(const QVector3D &position)
+    : d_ptr(0),
+      m_position(position)
 {
 }
 
@@ -57,6 +65,12 @@ QScatterDataItem &QScatterDataItem::operator=(const QScatterDataItem &other)
     m_position = other.m_position;
     //m_size = other.m_size;
 
+    if (other.d_ptr)
+        createExtraData();
+    else
+        d_ptr = 0;
+    // TODO set extra data
+
     return *this;
 }
 
diff --git a/src/datavis3d/data/qscatterdataitem.h b/src/datavis3d/data/qscatterdataitem.h
index 80a8b181..c878e82a 100644
--- a/src/datavis3d/data/qscatterdataitem.h
+++ b/src/datavis3d/data/qscatterdataitem.h
@@ -30,6 +30,7 @@ class QT_DATAVIS3D_EXPORT QScatterDataItem
 {
 public:
     QScatterDataItem();
+    QScatterDataItem(const QVector3D &position);
     QScatterDataItem(const QScatterDataItem &other);
     ~QScatterDataItem();
 
diff --git a/src/datavis3d/data/qscatterdataproxy.cpp b/src/datavis3d/data/qscatterdataproxy.cpp
index 03ee1c80..d2f544ef 100644
--- a/src/datavis3d/data/qscatterdataproxy.cpp
+++ b/src/datavis3d/data/qscatterdataproxy.cpp
@@ -18,7 +18,6 @@
 
 #include "qscatterdataproxy.h"
 #include "qscatterdataproxy_p.h"
-#include <QMutexLocker>
 
 QT_DATAVIS3D_BEGIN_NAMESPACE
 
@@ -42,20 +41,63 @@ void QScatterDataProxy::resetArray(QScatterDataArray *newArray)
         emit arrayReset();
 }
 
-// Mutexing data accessors should be done by user, if needed
+void QScatterDataProxy::setItem(int index, const QScatterDataItem &item)
+{
+    dptr()->setItem(index, item);
+    emit itemsChanged(index, 1);
+}
+
+void QScatterDataProxy::setItems(int index, const QScatterDataArray &items)
+{
+    dptr()->setItems(index, items);
+    emit itemsChanged(index, items.size());
+}
+
+int QScatterDataProxy::addItem(const QScatterDataItem &item)
+{
+    int addIndex = dptr()->addItem(item);
+    emit itemsAdded(addIndex, 1);
+    return addIndex;
+}
+
+int QScatterDataProxy::addItems(const QScatterDataArray &items)
+{
+    int addIndex = dptr()->addItems(items);
+    emit itemsAdded(addIndex, items.size());
+    return addIndex;
+}
+
+void QScatterDataProxy::insertItem(int index, const QScatterDataItem &item)
+{
+    dptr()->insertItem(index, item);
+    emit itemsInserted(index, 1);
+}
+
+void QScatterDataProxy::insertItems(int index, const QScatterDataArray &items)
+{
+    dptr()->insertItems(index, items);
+    emit itemsInserted(index, items.size());
+}
+
+void QScatterDataProxy::removeItems(int index, int removeCount)
+{
+    dptr()->removeItems(index, removeCount);
+    emit itemsRemoved(index, removeCount);
+}
+
 int QScatterDataProxy::itemCount() const
 {
-    return dptrc()->m_dataArray.size();
+    return dptrc()->m_dataArray->size();
 }
 
 const QScatterDataArray *QScatterDataProxy::array() const
 {
-    return &dptrc()->m_dataArray;
+    return dptrc()->m_dataArray;
 }
 
 const QScatterDataItem *QScatterDataProxy::itemAt(int index) const
 {
-    return &dptrc()->m_dataArray.at(index);
+    return &dptrc()->m_dataArray->at(index);
 }
 
 QScatterDataProxyPrivate *QScatterDataProxy::dptr()
@@ -68,43 +110,89 @@ const QScatterDataProxyPrivate *QScatterDataProxy::dptrc() const
     return static_cast<const QScatterDataProxyPrivate *>(d_ptr.data());
 }
 
-// QBarDataProxyPrivate
+// QScatterDataProxyPrivate
 
 QScatterDataProxyPrivate::QScatterDataProxyPrivate(QScatterDataProxy *q)
-    : QAbstractDataProxyPrivate(q, QAbstractDataProxy::DataTypeMap)
+    : QAbstractDataProxyPrivate(q, QAbstractDataProxy::DataTypeScatter),
+      m_dataArray(new QScatterDataArray)
 {
 }
 
 QScatterDataProxyPrivate::~QScatterDataProxyPrivate()
 {
-    m_dataArray.clear();
+    m_dataArray->clear();
+    delete m_dataArray;
 }
 
 bool QScatterDataProxyPrivate::resetArray(QScatterDataArray *newArray)
 {
-    QMutexLocker locker(&m_mutex);
-
-    if (!m_dataArray.size() && (!newArray || !newArray->size()))
+    if (!m_dataArray->size() && (!newArray || !newArray->size()))
         return false;
 
-    m_dataArray.clear();
+    m_dataArray->clear();
+    delete m_dataArray;
 
-    if (newArray) {
-        m_dataArray = *newArray;
-        delete newArray;
-    }
+    if (newArray)
+        m_dataArray = newArray;
+    else
+        m_dataArray = new QScatterDataArray;
 
     return true;
 }
 
-// Protected & private functions. Do not mutex as these are used from mutexed functions.
+void QScatterDataProxyPrivate::setItem(int index, const QScatterDataItem &item)
+{
+    Q_ASSERT(index >= 0 && index < m_dataArray->size());
+    (*m_dataArray)[index] = item;
+}
+
+void QScatterDataProxyPrivate::setItems(int index, const QScatterDataArray &items)
+{
+    Q_ASSERT(index >= 0 && (index + items.size()) <= m_dataArray->size());
+    for (int i = 0; i < items.size(); i++)
+        (*m_dataArray)[index++] = items[i];
+}
+
+int QScatterDataProxyPrivate::addItem(const QScatterDataItem &item)
+{
+    int currentSize = m_dataArray->size();
+    m_dataArray->append(item);
+    return currentSize;
+}
+
+int QScatterDataProxyPrivate::addItems(const QScatterDataArray &items)
+{
+    int currentSize = m_dataArray->size();
+    (*m_dataArray) += items;
+    return currentSize;
+}
+
+void QScatterDataProxyPrivate::insertItem(int index, const QScatterDataItem &item)
+{
+    Q_ASSERT(index >= 0 && index <= m_dataArray->size());
+    m_dataArray->insert(index, item);
+}
+
+void QScatterDataProxyPrivate::insertItems(int index, const QScatterDataArray &items)
+{
+    Q_ASSERT(index >= 0 && index <= m_dataArray->size());
+    for (int i = 0; i < items.size(); i++)
+        m_dataArray->insert(index++, items.at(i));
+}
+
+void QScatterDataProxyPrivate::removeItems(int index, int removeCount)
+{
+    Q_ASSERT(index >= 0);
+    int maxRemoveCount = m_dataArray->size() - index;
+    removeCount = qMin(removeCount, maxRemoveCount);
+    m_dataArray->remove(index, removeCount);
+}
 
 QVector3D QScatterDataProxyPrivate::limitValues()
 {
-    QMutexLocker locker(&m_mutex);
     QVector3D limits;
-    for (int i = 0; i < m_dataArray.size(); i++) {
-        const QScatterDataItem &item = m_dataArray.at(i);
+    for (int i = 0; i < m_dataArray->size(); i++) {
+        const QScatterDataItem &item = m_dataArray->at(i);
         float xValue = qAbs(item.position().x());
         if (limits.x() < xValue)
             limits.setX(xValue);
diff --git a/src/datavis3d/data/qscatterdataproxy.h b/src/datavis3d/data/qscatterdataproxy.h
index ed4993bf..9e139c00 100644
--- a/src/datavis3d/data/qscatterdataproxy.h
+++ b/src/datavis3d/data/qscatterdataproxy.h
@@ -37,41 +37,33 @@ public:
     explicit QScatterDataProxy(QScatterDataProxyPrivate *d);
     virtual ~QScatterDataProxy();
 
-    // QScatterDataProxy is also optimized to use cases where the only defining characteristics of an individual
-    // map item are it's value and position. Modifying other data such as color or mesh of individual bar
-    // requires allocating additional data object for the bar.
+    // QScatterDataProxy is optimized to use cases where the only defining characteristics of an
+    // individual scatter item are it's position and size. Modifying other data that might be
+    // added in the future such as color requires allocating additional data object for the bar.
 
-    // If data is accessed from same thread that sets it, access doesn't need to be protected with mutex.
     // Item pointers are guaranteed to be valid only until next call that modifies data.
     // Array pointer is guaranteed to be valid for lifetime of proxy.
     int itemCount() const;
     const QScatterDataArray *array() const;
-    const QScatterDataItem *itemAt(int index) const; // Index needs to exist or this crashes
+    const QScatterDataItem *itemAt(int index) const;
 
-    // All array/item manipulation functions are internally protected by data mutex.
-
-    // TODO: Should we remove internal mutexing and require user to mutex also on data manipulations?
-    // TODO: Upside would be enabling user to mix data setters and getters within same mutex scope.
-    // TODO: Downside would be signal emissions from inside the mutex scope, which has potential for deadlock.
-
-    // TODO Should data manipulation/access methods be protected rather than public to enforce subclassing use of proxy?
-    // TODO Leaving them public gives user more options.
-
-    // QScatterDataProxy takes ownership of all QScatterDataArrays and QScatterDataItems passed to it.
-
-    // Clears the existing array and sets it data to new array.
+    // Clears the existing array and takes ownership of the new array.
+    // Passing null array clears all data.
     void resetArray(QScatterDataArray *newArray);
 
-    // TODO void setItem(int index, QScatterDataItem *item);
-    // TODO void setItems(int index, QScatterDataArray *items);
+    // Change existing items
+    void setItem(int index, const QScatterDataItem &item);
+    void setItems(int index, const QScatterDataArray &items);
 
-    // TODO int addItem(QScatterDataItem *item); // returns the index of added item
-    // TODO int addItems(QScatterDataArray *items); // returns the index of added item
+    int addItem(const QScatterDataItem &item); // returns the index of added item
+    int addItems(const QScatterDataArray &items); // returns the index of first added item
 
-    // TODO void insertItem(int index, QScatterDataItem *item);
-    // TODO void insertItems(int index, QScatterDataArray *items);
+    // If index is equal to data array size, item(s) are added to the array.
+    void insertItem(int index, const QScatterDataItem &item);
+    void insertItems(int index, const QScatterDataArray &items);
 
-    // TODO void removeItems(int index, int removeCount);
+    // Attempting to remove items past the end of the array does nothing.
+    void removeItems(int index, int removeCount);
 
 signals:
     void arrayReset();
diff --git a/src/datavis3d/data/qscatterdataproxy_p.h b/src/datavis3d/data/qscatterdataproxy_p.h
index ca4720ab..526845fd 100644
--- a/src/datavis3d/data/qscatterdataproxy_p.h
+++ b/src/datavis3d/data/qscatterdataproxy_p.h
@@ -43,11 +43,18 @@ public:
     virtual ~QScatterDataProxyPrivate();
 
     bool resetArray(QScatterDataArray *newArray);
+    void setItem(int index, const QScatterDataItem &item);
+    void setItems(int index, const QScatterDataArray &items);
+    int addItem(const QScatterDataItem &item);
+    int addItems(const QScatterDataArray &items);
+    void insertItem(int index, const QScatterDataItem &item);
+    void insertItems(int index, const QScatterDataArray &items);
+    void removeItems(int index, int removeCount);
 
     QVector3D limitValues();
 
 private:
-    QScatterDataArray m_dataArray;
+    QScatterDataArray *m_dataArray;
     QString m_itemLabelFormat;
 
     friend class QScatterDataProxy;
diff --git a/src/datavis3d/engine/abstract3drenderer.cpp b/src/datavis3d/engine/abstract3drenderer.cpp
index 9a9d572d..caa0fc99 100644
--- a/src/datavis3d/engine/abstract3drenderer.cpp
+++ b/src/datavis3d/engine/abstract3drenderer.cpp
@@ -199,7 +199,6 @@ void Abstract3DRenderer::updateAxisSubSegmentCount(QAbstractAxis::AxisOrientatio
     axisCacheForOrientation(orientation).setSubSegmentCount(count);
 }
 
-// This method needs to be called under the controller-renderer sync mutex
 void Abstract3DRenderer::initializeAxisCache(QAbstractAxis::AxisOrientation orientation, const QAbstractAxis *axis)
 {
     axisCacheForOrientation(orientation).setDrawer(m_drawer);
diff --git a/src/datavis3d/engine/bars3dcontroller.cpp b/src/datavis3d/engine/bars3dcontroller.cpp
index d12becb4..0a48f0ed 100644
--- a/src/datavis3d/engine/bars3dcontroller.cpp
+++ b/src/datavis3d/engine/bars3dcontroller.cpp
@@ -81,10 +81,6 @@ void Bars3dController::render(const GLuint defaultFboHandle)
         return;
 
     // TODO do not give the entire data array, just the data window
-    // TODO Would it be enough to just mutex cache update in renderer?
-    // TODO --> Only if there is no need to store m_dataProxy for later, e.g. for string formatting
-    // TODO Also, m_valuesDirty flag setting needs to be under same mutex
-    QMutexLocker(m_data->mutex());
     m_renderer->render(m_data, m_valuesDirty, m_cameraHelper, defaultFboHandle);
 
     m_valuesDirty = false;
diff --git a/src/datavis3d/engine/bars3drenderer.cpp b/src/datavis3d/engine/bars3drenderer.cpp
index 930cb9f5..140cb4de 100644
--- a/src/datavis3d/engine/bars3drenderer.cpp
+++ b/src/datavis3d/engine/bars3drenderer.cpp
@@ -32,7 +32,6 @@
 #include <QThread>
 #include <qmath.h>
 #include <QDebug>
-#include <QMutexLocker>
 
 // Commenting this draws the shadow map with perspective projection. Otherwise it's drawn in
 // orthographic projection.
@@ -151,7 +150,6 @@ void Bars3dRenderer::initializePreOpenGL()
     QObject::connect(m_controller, &Bars3dController::zoomLevelChanged, this,
                      &Bars3dRenderer::updateZoomLevel);
 
-    // TODO Should all this initial setup be mutexed?
     updateSampleSpace(m_controller->rowCount(), m_controller->columnCount());
     updateSelectionMode(m_controller->selectionMode());
     updateSlicingActive(m_controller->isSlicingActive());
diff --git a/src/datavis3d/engine/bars3drenderer_p.h b/src/datavis3d/engine/bars3drenderer_p.h
index fd74ab3a..97ad5bf4 100644
--- a/src/datavis3d/engine/bars3drenderer_p.h
+++ b/src/datavis3d/engine/bars3drenderer_p.h
@@ -68,7 +68,7 @@ private:
     Bars3dController *m_controller;
 
     // Mutex for sharing resources between render and main threads.
-    // Note: Data access mutex is separate and owned by data proxy.
+    // TODO this mutex needs to go, too...
     QMutex m_mutex;
 
     // Cached state based on emitted signals from the controller
diff --git a/src/datavis3d/engine/maps3dcontroller.cpp b/src/datavis3d/engine/maps3dcontroller.cpp
index 0088c0b1..6050cc05 100644
--- a/src/datavis3d/engine/maps3dcontroller.cpp
+++ b/src/datavis3d/engine/maps3dcontroller.cpp
@@ -265,7 +265,6 @@ void Maps3DController::render(const GLuint defaultFboHandle)
     }
 #endif
 
-    QMutexLocker(m_data->mutex());
     // Update cached values
     if (m_valuesDirty) {
         const QMapDataArray &dataArray = *m_data->array();
diff --git a/src/datavis3d/engine/scatter3dcontroller.cpp b/src/datavis3d/engine/scatter3dcontroller.cpp
index 4660f4ad..abbf0ed5 100644
--- a/src/datavis3d/engine/scatter3dcontroller.cpp
+++ b/src/datavis3d/engine/scatter3dcontroller.cpp
@@ -76,10 +76,6 @@ void Scatter3DController::render(const GLuint defaultFboHandle)
         return;
 
     // TODO do not give the entire data array, just the data window
-    // TODO Would it be enough to just mutex cache update in renderer?
-    // TODO --> Only if there is no need to store m_dataProxy for later, e.g. for string formatting
-    // TODO Also, m_valuesDirty flag setting needs to be under same mutex
-    QMutexLocker(m_data->mutex());
     m_renderer->render(m_data, m_valuesDirty, m_cameraHelper, defaultFboHandle);
 
     m_valuesDirty = false;
diff --git a/src/datavis3d/engine/scatter3drenderer.cpp b/src/datavis3d/engine/scatter3drenderer.cpp
index d21a168b..34b2da39 100644
--- a/src/datavis3d/engine/scatter3drenderer.cpp
+++ b/src/datavis3d/engine/scatter3drenderer.cpp
@@ -29,7 +29,6 @@
 #include <QThread>
 #include <qmath.h>
 #include <QDebug>
-#include <QMutexLocker>
 
 // Commenting this draws the shadow map with perspective projection. Otherwise it's drawn in
 // orthographic projection.
@@ -135,7 +134,6 @@ void Scatter3DRenderer::initializePreOpenGL()
     QObject::connect(m_controller, &Scatter3DController::zoomLevelChanged, this,
                      &Scatter3DRenderer::updateZoomLevel);
 
-    // TODO Should all this initial setup be mutexed?
     updateSelectionMode(m_controller->selectionMode());
     updateZoomLevel(m_controller->zoomLevel());
     updateMeshFileName(m_controller->objFile());
diff --git a/src/datavis3d/engine/scatter3drenderer_p.h b/src/datavis3d/engine/scatter3drenderer_p.h
index ae36b326..4d4c8a19 100644
--- a/src/datavis3d/engine/scatter3drenderer_p.h
+++ b/src/datavis3d/engine/scatter3drenderer_p.h
@@ -70,6 +70,7 @@ private:
     Scatter3DController *m_controller;
 
     // Mutex for sharing resources between render and main threads.
+    // TODO: this mutex needs to go, too
     QMutex m_mutex;
 
     // Cached state based on emitted signals from the controller