Use coordinates visitor to avoid copying vertex data

Change-Id: Ibab8d6cf8cedc93b0585a3af2d989eef7e8263b4
Reviewed-by: Sean Harmer <sean.harmer@kdab.com>

6 files changed, 477 insertions, 85 deletions

diff --git a/src/render/backend/bufferutils_p.h b/src/render/backend/bufferutils_p.h
new file mode 100644
index 000000000..2bb35fac6
--- /dev/null
+++ b/src/render/backend/bufferutils_p.h
@@ -0,0 +1,115 @@
+/****************************************************************************
+**
+** Copyright (C) 2015 Paul Lemire paul.lemire350@gmail.com
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the Qt3D module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 3 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPL3 included in the
+** packaging of this file. Please review the following information to
+** ensure the GNU Lesser General Public License version 3 requirements
+** will be met: https://www.gnu.org/licenses/lgpl-3.0.html.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 2.0 or (at your option) the GNU General
+** Public license version 3 or any later version approved by the KDE Free
+** Qt Foundation. The licenses are as published by the Free Software
+** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-2.0.html and
+** https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#ifndef QT3DRENDER_RENDER_BUFFERUTILS_P_H
+#define QT3DRENDER_RENDER_BUFFERUTILS_P_H
+
+//
+//  W A R N I N G
+//  -------------
+//
+// This file is not part of the Qt API.  It exists for the convenience
+// of other Qt classes.  This header file may change from version to
+// version without notice, or even be removed.
+//
+// We mean it.
+//
+
+#include <Qt3DRender/QAttribute>
+#include <QByteArray>
+
+QT_BEGIN_NAMESPACE
+
+
+namespace Qt3DRender {
+
+namespace Render {
+
+class GeometryRenderer;
+class NodeManagers;
+class Attribute;
+class Buffer;
+
+struct BufferInfo
+{
+    BufferInfo()
+        : type(QAttribute::VertexBaseType::Float)
+        , dataSize(0)
+        , count(0)
+        , byteStride(0)
+        , byteOffset(0)
+    {}
+
+    QByteArray data;
+    QAttribute::VertexBaseType type;
+    uint dataSize;
+    uint count;
+    uint byteStride;
+    uint byteOffset;
+};
+
+
+namespace BufferTypeInfo {
+
+    template <QAttribute::VertexBaseType> struct EnumToType;
+    template <> struct EnumToType<QAttribute::Byte> { typedef const char type; };
+    template <> struct EnumToType<QAttribute::UnsignedByte> { typedef const uchar type; };
+    template <> struct EnumToType<QAttribute::Short> { typedef const short type; };
+    template <> struct EnumToType<QAttribute::UnsignedShort> { typedef const ushort type; };
+    template <> struct EnumToType<QAttribute::Int> { typedef const int type; };
+    template <> struct EnumToType<QAttribute::UnsignedInt> { typedef const uint type; };
+    template <> struct EnumToType<QAttribute::Float> { typedef const float type; };
+    template <> struct EnumToType<QAttribute::Double> { typedef const double type; };
+
+    template<QAttribute::VertexBaseType v>
+    typename EnumToType<v>::type *castToType(const QByteArray &u, uint byteOffset)
+    {
+        return reinterpret_cast< typename EnumToType<v>::type *>(u.constData() + byteOffset);
+    }
+
+} // namespace BufferTypeInfo
+
+} // namespace Render
+
+} // namespace Qt3DRender
+
+QT_END_NAMESPACE
+
+
+#endif // QT3DRENDER_RENDER_BUFFERUTILS_P_H
diff --git a/src/render/backend/coordinatevisitor_p.h b/src/render/backend/coordinatevisitor_p.h
new file mode 100644
index 000000000..e7687ab3a
--- /dev/null
+++ b/src/render/backend/coordinatevisitor_p.h
@@ -0,0 +1,212 @@
+/****************************************************************************
+**
+** Copyright (C) 2017 Klaralvdalens Datakonsult AB (KDAB).
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the Qt3D module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 3 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPL3 included in the
+** packaging of this file. Please review the following information to
+** ensure the GNU Lesser General Public License version 3 requirements
+** will be met: https://www.gnu.org/licenses/lgpl-3.0.html.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 2.0 or (at your option) the GNU General
+** Public license version 3 or any later version approved by the KDE Free
+** Qt Foundation. The licenses are as published by the Free Software
+** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-2.0.html and
+** https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#ifndef QT3DRENDER_RENDER_COORDINATEVISITOR_P_H
+#define QT3DRENDER_RENDER_COORDINATEVISITOR_P_H
+
+//
+//  W A R N I N G
+//  -------------
+//
+// This file is not part of the Qt API.  It exists for the convenience
+// of other Qt classes.  This header file may change from version to
+// version without notice, or even be removed.
+//
+// We mean it.
+//
+
+#include <Qt3DCore/qnodeid.h>
+#include <Qt3DRender/QAttribute>
+#include <Qt3DRender/private/trianglesvisitor_p.h>
+#include <Qt3DRender/private/attribute_p.h>
+#include <Qt3DRender/private/buffer_p.h>
+#include <Qt3DRender/private/bufferutils_p.h>
+#include <Qt3DRender/private/geometryrenderer_p.h>
+#include <Qt3DRender/private/geometry_p.h>
+#include <Qt3DRender/private/managers_p.h>
+#include <Qt3DRender/private/nodemanagers_p.h>
+
+QT_BEGIN_NAMESPACE
+
+namespace Qt3DCore {
+class QEntity;
+}
+
+namespace Qt3DRender {
+namespace Render {
+
+
+template <typename ValueType, QAttribute::VertexBaseType VertexBaseType, uint dataSize>
+class Q_AUTOTEST_EXPORT CoordinateVisitor
+{
+public:
+    explicit CoordinateVisitor(NodeManagers *manager)
+        : m_manager(manager)
+    {
+    }
+    virtual ~CoordinateVisitor() { }
+
+    virtual void visit(uint ndx, ValueType x) {
+        Q_UNUSED(ndx); Q_UNUSED(x);
+    }
+    virtual void visit(uint ndx, ValueType x, ValueType y) {
+        Q_UNUSED(ndx); Q_UNUSED(x); Q_UNUSED(y);
+    }
+    virtual void visit(uint ndx, ValueType x, ValueType y, ValueType z) {
+        Q_UNUSED(ndx); Q_UNUSED(x); Q_UNUSED(y); Q_UNUSED(z);
+    }
+    virtual void visit(uint ndx, ValueType x, ValueType y, ValueType z, ValueType w) {
+        Q_UNUSED(ndx); Q_UNUSED(x); Q_UNUSED(y); Q_UNUSED(z); Q_UNUSED(w);
+    }
+
+    bool apply(const GeometryRenderer *renderer, const QString &attributeName)
+    {
+        if (renderer == nullptr || renderer->instanceCount() != 1) {
+            return false;
+        }
+
+        Geometry *geom = m_manager->lookupResource<Geometry, GeometryManager>(renderer->geometryId());
+
+        if (!geom)
+            return false;
+
+        Attribute *attribute = nullptr;
+
+        const auto attrIds = geom->attributes();
+        for (const Qt3DCore::QNodeId attrId : attrIds) {
+            attribute = m_manager->lookupResource<Attribute, AttributeManager>(attrId);
+            if (attribute){
+                if (attribute->name() == attributeName
+                        || (attributeName == QStringLiteral("default")
+                            && attribute->name() == QAttribute::defaultTextureCoordinateAttributeName())) {
+                    break;
+                }
+            }
+            attribute = nullptr;
+        }
+
+        if (!attribute)
+            return false;
+
+        return apply(attribute);
+    }
+
+    bool apply(Qt3DRender::Render::Attribute *attribute)
+    {
+        if (attribute->vertexBaseType() != VertexBaseType)
+            return false;
+        if (attribute->vertexSize() < dataSize)
+            return false;
+
+        auto data = m_manager->lookupResource<Buffer, BufferManager>(attribute->bufferId())->data();
+        auto buffer = BufferTypeInfo::castToType<VertexBaseType>(data, attribute->byteOffset());
+        switch (dataSize) {
+        case 1: traverseCoordinates1(buffer, attribute->byteStride(), attribute->count()); break;
+        case 2: traverseCoordinates2(buffer, attribute->byteStride(), attribute->count()); break;
+        case 3: traverseCoordinates3(buffer, attribute->byteStride(), attribute->count()); break;
+        case 4: traverseCoordinates4(buffer, attribute->byteStride(), attribute->count()); break;
+        default: Q_UNREACHABLE();
+        }
+
+        return true;
+    }
+
+protected:
+
+    template <typename Coordinate>
+    void traverseCoordinates1(Coordinate *coordinates,
+                              const uint byteStride,
+                              const uint count)
+    {
+        const uint stride = byteStride / sizeof(Coordinate);
+        for (uint ndx = 0; ndx < count; ++ndx) {
+            visit(ndx, coordinates[0]);
+            coordinates += stride;
+        }
+    }
+
+    template <typename Coordinate>
+    void traverseCoordinates2(Coordinate *coordinates,
+                              const uint byteStride,
+                              const uint count)
+    {
+        const uint stride = byteStride / sizeof(Coordinate);
+        for (uint ndx = 0; ndx < count; ++ndx) {
+            visit(ndx, coordinates[0], coordinates[1]);
+            coordinates += stride;
+        }
+    }
+
+    template <typename Coordinate>
+    void traverseCoordinates3(Coordinate *coordinates,
+                              const uint byteStride,
+                              const uint count)
+    {
+        const uint stride = byteStride / sizeof(Coordinate);
+        for (uint ndx = 0; ndx < count; ++ndx) {
+            visit(ndx, coordinates[0], coordinates[1], coordinates[2]);
+            coordinates += stride;
+        }
+    }
+
+    template <typename Coordinate>
+    void traverseCoordinates4(Coordinate *coordinates,
+                              const uint byteStride,
+                              const uint count)
+    {
+        const uint stride = byteStride / sizeof(Coordinate);
+        for (uint ndx = 0; ndx < count; ++ndx) {
+            visit(ndx, coordinates[0], coordinates[1], coordinates[2], coordinates[3]);
+            coordinates += stride;
+        }
+    }
+
+    NodeManagers *m_manager;
+};
+
+typedef CoordinateVisitor<float, QAttribute::Float, 3> Coordinate3fVisitor;
+
+} // namespace Render
+
+} // namespace Qt3DRender
+
+QT_END_NAMESPACE
+
+
+#endif // QT3DRENDER_RENDER_COORDINATEVISITOR_P_H
diff --git a/src/render/backend/render-backend.pri b/src/render/backend/render-backend.pri
index 1cd911df8..f694e7cea 100644
--- a/src/render/backend/render-backend.pri
+++ b/src/render/backend/render-backend.pri
@@ -29,6 +29,8 @@ HEADERS += \
     $$PWD/triangleboundingvolume_p.h \
     $$PWD/openglvertexarrayobject_p.h \
     $$PWD/trianglesextractor_p.h \
+    $$PWD/coordinatevisitor_p.h \
+    $$PWD/bufferutils_p.h \
     $$PWD/trianglesvisitor_p.h \
     $$PWD/abstractrenderer_p.h \
     $$PWD/computecommand_p.h \
diff --git a/src/render/backend/trianglesvisitor.cpp b/src/render/backend/trianglesvisitor.cpp
index 5453661e0..036f43fa0 100644
--- a/src/render/backend/trianglesvisitor.cpp
+++ b/src/render/backend/trianglesvisitor.cpp
@@ -48,6 +48,7 @@
 #include <Qt3DRender/private/geometry_p.h>
 #include <Qt3DRender/private/attribute_p.h>
 #include <Qt3DRender/private/buffer_p.h>
+#include <Qt3DRender/private/bufferutils_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -321,42 +322,25 @@ QVector4D readCoordinate(const BufferInfo &info, Coordinate *coordinates, uint i
     return ret;
 }
 
-
-template <QAttribute::VertexBaseType> struct EnumToType;
-template <> struct EnumToType<QAttribute::Byte> { typedef const char type; };
-template <> struct EnumToType<QAttribute::UnsignedByte> { typedef const uchar type; };
-template <> struct EnumToType<QAttribute::Short> { typedef const short type; };
-template <> struct EnumToType<QAttribute::UnsignedShort> { typedef const ushort type; };
-template <> struct EnumToType<QAttribute::Int> { typedef const int type; };
-template <> struct EnumToType<QAttribute::UnsignedInt> { typedef const uint type; };
-template <> struct EnumToType<QAttribute::Float> { typedef const float type; };
-template <> struct EnumToType<QAttribute::Double> { typedef const double type; };
-
-template<QAttribute::VertexBaseType v>
-typename EnumToType<v>::type *castToType(const QByteArray &u, uint byteOffset)
-{
-    return reinterpret_cast< typename EnumToType<v>::type *>(u.constData() + byteOffset);
-}
-
 template<typename Func>
 void processBuffer(const BufferInfo &info, Func &f)
 {
     switch (info.type) {
-    case QAttribute::Byte: f(info, castToType<QAttribute::Byte>(info.data, info.byteOffset));
+    case QAttribute::Byte: f(info, BufferTypeInfo::castToType<QAttribute::Byte>(info.data, info.byteOffset));
         return;
-    case QAttribute::UnsignedByte: f(info, castToType<QAttribute::UnsignedByte>(info.data, info.byteOffset));
+    case QAttribute::UnsignedByte: f(info, BufferTypeInfo::castToType<QAttribute::UnsignedByte>(info.data, info.byteOffset));
         return;
-    case QAttribute::Short: f(info, castToType<QAttribute::Short>(info.data, info.byteOffset));
+    case QAttribute::Short: f(info, BufferTypeInfo::castToType<QAttribute::Short>(info.data, info.byteOffset));
         return;
-    case QAttribute::UnsignedShort: f(info, castToType<QAttribute::UnsignedShort>(info.data, info.byteOffset));
+    case QAttribute::UnsignedShort: f(info, BufferTypeInfo::castToType<QAttribute::UnsignedShort>(info.data, info.byteOffset));
         return;
-    case QAttribute::Int: f(info, castToType<QAttribute::Int>(info.data, info.byteOffset));
+    case QAttribute::Int: f(info, BufferTypeInfo::castToType<QAttribute::Int>(info.data, info.byteOffset));
         return;
-    case QAttribute::UnsignedInt: f(info, castToType<QAttribute::UnsignedInt>(info.data, info.byteOffset));
+    case QAttribute::UnsignedInt: f(info, BufferTypeInfo::castToType<QAttribute::UnsignedInt>(info.data, info.byteOffset));
         return;
-    case QAttribute::Float: f(info, castToType<QAttribute::Float>(info.data, info.byteOffset));
+    case QAttribute::Float: f(info, BufferTypeInfo::castToType<QAttribute::Float>(info.data, info.byteOffset));
         return;
-    case QAttribute::Double: f(info, castToType<QAttribute::Double>(info.data, info.byteOffset));
+    case QAttribute::Double: f(info, BufferTypeInfo::castToType<QAttribute::Double>(info.data, info.byteOffset));
         return;
     default:
         return;
@@ -367,21 +351,21 @@ QVector4D readBuffer(const BufferInfo &info, uint index)
 {
     switch (info.type) {
     case QAttribute::Byte:
-        return readCoordinate(info, castToType<QAttribute::Byte>(info.data, info.byteOffset), index);
+        return readCoordinate(info, BufferTypeInfo::castToType<QAttribute::Byte>(info.data, info.byteOffset), index);
     case QAttribute::UnsignedByte:
-        return readCoordinate(info, castToType<QAttribute::UnsignedByte>(info.data, info.byteOffset), index);
+        return readCoordinate(info, BufferTypeInfo::castToType<QAttribute::UnsignedByte>(info.data, info.byteOffset), index);
     case QAttribute::Short:
-        return readCoordinate(info, castToType<QAttribute::Short>(info.data, info.byteOffset), index);
+        return readCoordinate(info, BufferTypeInfo::castToType<QAttribute::Short>(info.data, info.byteOffset), index);
     case QAttribute::UnsignedShort:
-        return readCoordinate(info, castToType<QAttribute::UnsignedShort>(info.data, info.byteOffset), index);
+        return readCoordinate(info, BufferTypeInfo::castToType<QAttribute::UnsignedShort>(info.data, info.byteOffset), index);
     case QAttribute::Int:
-        return readCoordinate(info, castToType<QAttribute::Int>(info.data, info.byteOffset), index);
+        return readCoordinate(info, BufferTypeInfo::castToType<QAttribute::Int>(info.data, info.byteOffset), index);
     case QAttribute::UnsignedInt:
-        return readCoordinate(info, castToType<QAttribute::UnsignedInt>(info.data, info.byteOffset), index);
+        return readCoordinate(info, BufferTypeInfo::castToType<QAttribute::UnsignedInt>(info.data, info.byteOffset), index);
     case QAttribute::Float:
-        return readCoordinate(info, castToType<QAttribute::Float>(info.data, info.byteOffset), index);
+        return readCoordinate(info, BufferTypeInfo::castToType<QAttribute::Float>(info.data, info.byteOffset), index);
     case QAttribute::Double:
-        return readCoordinate(info, castToType<QAttribute::Double>(info.data, info.byteOffset), index);
+        return readCoordinate(info, BufferTypeInfo::castToType<QAttribute::Double>(info.data, info.byteOffset), index);
     default:
         break;
     }
diff --git a/src/render/backend/trianglesvisitor_p.h b/src/render/backend/trianglesvisitor_p.h
index a0fa89efb..149ac2a65 100644
--- a/src/render/backend/trianglesvisitor_p.h
+++ b/src/render/backend/trianglesvisitor_p.h
@@ -53,6 +53,7 @@
 
 #include <Qt3DCore/qnodeid.h>
 #include <Qt3DRender/QAttribute>
+#include <Qt3DRender/private/bufferutils_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -69,24 +70,6 @@ class NodeManagers;
 class Attribute;
 class Buffer;
 
-struct BufferInfo
-{
-    BufferInfo()
-        : type(QAttribute::VertexBaseType::Float)
-        , dataSize(0)
-        , count(0)
-        , byteStride(0)
-        , byteOffset(0)
-    {}
-
-    QByteArray data;
-    QAttribute::VertexBaseType type;
-    uint dataSize;
-    uint count;
-    uint byteStride;
-    uint byteOffset;
-};
-
 class Q_AUTOTEST_EXPORT TrianglesVisitor
 {
 public:
diff --git a/src/render/jobs/calcboundingvolumejob.cpp b/src/render/jobs/calcboundingvolumejob.cpp
index 9e373c655..2f2d471d6 100644
--- a/src/render/jobs/calcboundingvolumejob.cpp
+++ b/src/render/jobs/calcboundingvolumejob.cpp
@@ -50,6 +50,7 @@
 #include <Qt3DRender/private/attribute_p.h>
 #include <Qt3DRender/private/buffer_p.h>
 #include <Qt3DRender/private/sphere_p.h>
+#include <Qt3DRender/private/coordinatevisitor_p.h>
 
 #include <QtCore/qmath.h>
 #include <QtConcurrent/QtConcurrent>
@@ -73,6 +74,114 @@ struct UpdateBoundFunctor {
     }
 };
 
+class BoundingVolumeCalculator
+{
+public:
+    BoundingVolumeCalculator(NodeManagers *manager) : m_manager(manager) { }
+
+    const Sphere& result() { return m_volume; }
+
+    bool apply(Qt3DRender::Render::Attribute *positionAttribute)
+    {
+        FindExtremePoints findExtremePoints(m_manager);
+        if (!findExtremePoints.apply(positionAttribute))
+            return false;
+
+        // Calculate squared distance for the pairs of points
+        const float xDist2 = (findExtremePoints.xMaxPt - findExtremePoints.xMinPt).lengthSquared();
+        const float yDist2 = (findExtremePoints.yMaxPt - findExtremePoints.yMinPt).lengthSquared();
+        const float zDist2 = (findExtremePoints.zMaxPt - findExtremePoints.zMinPt).lengthSquared();
+
+        // Select most distant pair
+        QVector3D p = findExtremePoints.xMinPt;
+        QVector3D q = findExtremePoints.xMaxPt;
+        if (yDist2 > xDist2 && yDist2 > zDist2) {
+            p = findExtremePoints.yMinPt;
+            q = findExtremePoints.yMaxPt;
+        }
+        if (zDist2 > xDist2 && zDist2 > yDist2) {
+            p = findExtremePoints.zMinPt;
+            q = findExtremePoints.zMaxPt;
+        }
+
+        const QVector3D c = 0.5f * (p + q);
+        m_volume.setCenter(c);
+        m_volume.setRadius((q - c).length());
+
+        ExpandSphere expandSphere(m_manager, m_volume);
+        if (!expandSphere.apply(positionAttribute))
+            return false;
+
+        return true;
+    }
+
+private:
+    Sphere m_volume;
+    NodeManagers *m_manager;
+
+    class FindExtremePoints : public Coordinate3fVisitor
+    {
+    public:
+        FindExtremePoints(NodeManagers *manager)
+            : Coordinate3fVisitor(manager)
+            , xMin(0.0f), xMax(0.0f), yMin(0.0f), yMax(0.0f), zMin(0.0f), zMax(0.0f)
+        { }
+
+        float xMin, xMax, yMin, yMax, zMin, zMax;
+        QVector3D xMinPt, xMaxPt, yMinPt, yMaxPt, zMinPt, zMaxPt;
+
+        void visit(uint ndx, float x, float y, float z) override
+        {
+            if (ndx) {
+                if (x < xMin) {
+                    xMin = x;
+                    xMinPt = QVector3D(x, y, z);
+                }
+                if (x > xMax) {
+                    xMax = x;
+                    xMaxPt = QVector3D(x, y, z);
+                }
+                if (y < yMin) {
+                    yMin = y;
+                    yMinPt = QVector3D(x, y, z);
+                }
+                if (y > yMax) {
+                    yMax = y;
+                    yMaxPt = QVector3D(x, y, z);
+                }
+                if (z < zMin) {
+                    zMin = z;
+                    zMinPt = QVector3D(x, y, z);
+                }
+                if (z > zMax) {
+                    zMax = z;
+                    zMaxPt = QVector3D(x, y, z);
+                }
+            } else {
+                xMin = xMax = x;
+                yMin = yMax = y;
+                zMin = zMax = z;
+                xMinPt = xMaxPt = yMinPt = yMaxPt = zMinPt = zMaxPt = QVector3D(x, y, z);
+            }
+        }
+    };
+
+    class ExpandSphere : public Coordinate3fVisitor
+    {
+    public:
+        ExpandSphere(NodeManagers *manager, Sphere& volume)
+            : Coordinate3fVisitor(manager), m_volume(volume)
+        { }
+
+        Sphere& m_volume;
+        void visit(uint ndx, float x, float y, float z) override
+        {
+            Q_UNUSED(ndx);
+            m_volume.expandToContain(QVector3D(x, y, z));
+        }
+    };
+};
+
 void calculateLocalBoundingVolume(NodeManagers *manager, Entity *node)
 {
     // The Bounding volume will only be computed if the position Buffer
@@ -86,32 +195,32 @@ void calculateLocalBoundingVolume(NodeManagers *manager, Entity *node)
         Geometry *geom = manager->lookupResource<Geometry, GeometryManager>(gRenderer->geometryId());
 
         if (geom) {
-            Qt3DRender::Render::Attribute *pickVolumeAttribute = manager->lookupResource<Attribute, AttributeManager>(geom->boundingPositionAttribute());
+            Qt3DRender::Render::Attribute *positionAttribute = manager->lookupResource<Attribute, AttributeManager>(geom->boundingPositionAttribute());
 
             // Use the default position attribute if attribute is null
-            if (!pickVolumeAttribute) {
+            if (!positionAttribute) {
                 const auto attrIds = geom->attributes();
                 for (const Qt3DCore::QNodeId attrId : attrIds) {
-                    pickVolumeAttribute = manager->lookupResource<Attribute, AttributeManager>(attrId);
-                    if (pickVolumeAttribute &&
-                            pickVolumeAttribute->name() == QAttribute::defaultPositionAttributeName())
+                    positionAttribute = manager->lookupResource<Attribute, AttributeManager>(attrId);
+                    if (positionAttribute &&
+                            positionAttribute->name() == QAttribute::defaultPositionAttributeName())
                         break;
                 }
             }
 
-            if (pickVolumeAttribute) {
-                if (!pickVolumeAttribute
-                        || pickVolumeAttribute->attributeType() != QAttribute::VertexAttribute
-                        || pickVolumeAttribute->vertexBaseType() != QAttribute::Float
-                        || pickVolumeAttribute->vertexSize() < 3) {
-                    qWarning() << "QGeometry::boundingVolumePositionAttribute pickVolume Attribute not suited for bounding volume computation";
-                    return;
-                }
+            if (!positionAttribute
+                    || positionAttribute->attributeType() != QAttribute::VertexAttribute
+                    || positionAttribute->vertexBaseType() != QAttribute::Float
+                    || positionAttribute->vertexSize() < 3) {
+                qWarning() << "QGeometry::boundingVolumePositionAttribute position Attribute not suited for bounding volume computation";
+                return;
+            }
 
-                Buffer *buf = manager->lookupResource<Buffer, BufferManager>(pickVolumeAttribute->bufferId());
+            if (positionAttribute) {
+                Buffer *buf = manager->lookupResource<Buffer, BufferManager>(positionAttribute->bufferId());
                 // No point in continuing if the positionAttribute doesn't have a suitable buffer
                 if (!buf) {
-                    qWarning() << "ObjectPicker pickVolume Attribute not referencing a valid buffer";
+                    qWarning() << "ObjectPicker position Attribute not referencing a valid buffer";
                     return;
                 }
 
@@ -121,29 +230,16 @@ void calculateLocalBoundingVolume(NodeManagers *manager, Entity *node)
                 // If anything in the GeometryRenderer has changed
                 if (buf->isDirty() ||
                         node->isBoundingVolumeDirty() ||
-                        pickVolumeAttribute->isDirty() ||
+                        positionAttribute->isDirty() ||
                         geom->isDirty() ||
                         gRenderer->isDirty()) {
 
-                    const QByteArray buffer = buf->data();
-                    const char *rawBuffer = buffer.constData();
-                    rawBuffer += pickVolumeAttribute->byteOffset();
-                    const int stride = pickVolumeAttribute->byteStride() ? pickVolumeAttribute->byteStride() : sizeof(float) * pickVolumeAttribute->vertexSize();
-                    QVector<QVector3D> vertices(pickVolumeAttribute->count());
-
-                    // TODO avoid copying the vertices
-                    for (int c = 0, vC = vertices.size(); c < vC; ++c) {
-                        QVector3D v;
-                        const float *fptr = reinterpret_cast<const float*>(rawBuffer);
-                        // TODO unwrap loop (switch?)
-                        for (uint i = 0, m = qMin(pickVolumeAttribute->vertexSize(), 3U); i < m; ++i)
-                            v[i] = fptr[i];
-                        vertices[c] = v;
-                        rawBuffer += stride;
+                    BoundingVolumeCalculator reader(manager);
+                    if (reader.apply(positionAttribute)) {
+                        node->localBoundingVolume()->setCenter(reader.result().center());
+                        node->localBoundingVolume()->setRadius(reader.result().radius());
+                        node->unsetBoundingVolumeDirty();
                     }
-
-                    node->localBoundingVolume()->initializeFromPoints(vertices);
-                    node->unsetBoundingVolumeDirty();
                 }
             }
         }