another pass through the crucible

libraries/entities-renderer/src/EntityTreeRenderer.cpp

@@ -338,42 +338,40 @@ void EntityTreeRenderer::updateChangedEntities(const render::ScenePointer& scene
         // we expect the cost to updating all renderables to exceed available time budget
         // so we first sort by priority and update in order until out of time
 
-        // as per the instructions in PriortySortUtil we first derive from PrioritySortUtil::Prioritizable
+        class SortableRenderer: public PrioritySortUtil::Sortable {
-        class PrioritizableRenderer: public PrioritySortUtil::Prioritizable {
         public:
-            PrioritizableRenderer(const EntityRendererPointer& renderer) : _renderer(renderer) { }
+            SortableRenderer(const EntityRendererPointer& renderer) : _renderer(renderer) { }
+
             glm::vec3 getPosition() const override { return _renderer->getEntity()->getPosition(); }
             float getRadius() const override { return 0.5f * _renderer->getEntity()->getQueryAACube().getScale(); }
             uint64_t getTimestamp() const override { return _renderer->getUpdateTime(); }
+
+            const EntityRendererPointer& getRenderer() const { return _renderer; }
         private:
-            const EntityRendererPointer& _renderer;
+            EntityRendererPointer _renderer;
         };
 
         // prioritize and sort the renderables
         uint64_t sortStart = usecTimestampNow();
-        using SortableRenderer = PrioritySortUtil::Sortable<EntityRendererPointer>;
+        PrioritySortUtil::PriorityQueue<SortableRenderer> sortedRenderables(view);
-        std::priority_queue< SortableRenderer > sortedRenderables;
         {
             PROFILE_RANGE_EX(simulation_physics, "SortRenderables", 0xffff00ff, (uint64_t)_renderablesToUpdate.size());
-            PrioritySortUtil::Prioritizer prioritizer(view);
             std::unordered_map<EntityItemID, EntityRendererPointer>::iterator itr = _renderablesToUpdate.begin();
             while (itr != _renderablesToUpdate.end()) {
-                float priority = prioritizer.computePriority(PrioritizableRenderer(itr->second));
+                sortedRenderables.push(SortableRenderer(itr->second));
-                //SortableRenderer entry(itr->second, priority);
-                sortedRenderables.push(SortableRenderer(itr->second, priority));
                 ++itr;
             }
         }
         {
-            PROFILE_RANGE_EX(simulation_physics, "UpdateRenderables", 0xffff00ff, (uint64_t)_renderablesToUpdate.size());
+            PROFILE_RANGE_EX(simulation_physics, "UpdateRenderables", 0xffff00ff, sortedRenderables.size());
 
             // compute remaining time budget
             uint64_t updateStart = usecTimestampNow();
             const uint64_t MIN_SORTED_UPDATE_RENDERABLES_TIME_BUDGET = 1 * USECS_PER_MSEC;
             uint64_t timeBudget = MIN_SORTED_UPDATE_RENDERABLES_TIME_BUDGET;
-            uint64_t timeForSort = updateStart - sortStart;
+            uint64_t sortCost = updateStart - sortStart;
-            if (timeForSort < MAX_UPDATE_RENDERABLES_TIME_BUDGET - MIN_SORTED_UPDATE_RENDERABLES_TIME_BUDGET) {
+            if (sortCost < MAX_UPDATE_RENDERABLES_TIME_BUDGET - MIN_SORTED_UPDATE_RENDERABLES_TIME_BUDGET) {
-                timeBudget = MAX_UPDATE_RENDERABLES_TIME_BUDGET - timeForSort;
+                timeBudget = MAX_UPDATE_RENDERABLES_TIME_BUDGET - sortCost;
             }
             uint64_t expiry = updateStart + timeBudget;
 
@@ -381,7 +379,7 @@ void EntityTreeRenderer::updateChangedEntities(const render::ScenePointer& scene
             std::unordered_map<EntityItemID, EntityRendererPointer>::iterator itr;
             size_t numSorted = sortedRenderables.size();
             while (!sortedRenderables.empty() && usecTimestampNow() < expiry) {
-                const EntityRendererPointer& renderer = sortedRenderables.top().getThing();
+                const EntityRendererPointer& renderer = sortedRenderables.top().getRenderer();
                 renderer->updateInScene(scene, transaction);
                 _renderablesToUpdate.erase(renderer->getEntity()->getID());
                 sortedRenderables.pop();
@@ -389,7 +387,7 @@ void EntityTreeRenderer::updateChangedEntities(const render::ScenePointer& scene
 
             // compute average per-renderable update cost
             size_t numUpdated = numSorted - sortedRenderables.size() + 1; // add one to avoid divide by zero
-            float cost = (float)(usecTimestampNow() - updateStart) / (float)(numUpdated + 1);
+            float cost = (float)(usecTimestampNow() - updateStart) / (float)(numUpdated);
             const float blend = 0.1f;
             _avgRenderableUpdateCost = (1.0f - blend) * _avgRenderableUpdateCost + blend * cost;
         }

libraries/shared/src/PrioritySortUtil.h

@@ -16,22 +16,21 @@
 
 /*   PrioritySortUtil is a helper for sorting 3D things relative to a ViewFrustum.  To use:
 
-(1) Derive a class from pure-virtual PrioritySortUtil::Prioritizable
+(1) Derive a class from pure-virtual PrioritySortUtil::Sortable that wraps a copy of
-    that wraps the Thing you want to prioritize and sort:
+    the Thing you want to prioritize and sort:
 
-     class PrioritizableThing : public PrioritySortUtil::Prioritizable {
+    class SortableWrapper: public PrioritySortUtil::Sortable {
-     public:
+    public:
-        PrioritizableThing(const Thing& thing) : _thing(thing) {}
+        SortableWrapper(const Thing& thing) : _thing(thing) { }
-        glm::vec3 getPosition() const override { return _thing.getPosition(); }
+        glm::vec3 getPosition() const override { return _thing->getPosition(); }
-        float getRadius() const const override { return _thing.getBoundingRadius(); }
+        float getRadius() const override { return 0.5f * _thing->getBoundingRadius(); }
-        uint64_t getTimestamp() const override { return _thing.getLastUpdated(); }
+        uint64_t getTimestamp() const override { return _thing->getLastTime(); }
-     private:
+        const Thing& getThing() const { return _thing; }
-        // Yes really: the data member is a const reference!
+    private:
-        // PrioritizableThing only needs enough scope to compute a priority.
+        Thing _thing;
-        const Thing& _thing;
+    };
-     }
 
-(2) Loop over your things and insert each into a priority_queue:
+(2) Make a PrioritySortUtil::PriorityQueue<Thing> and add them to the queue:
 
     PrioritySortUtil::Prioritizer prioritizer(viewFrustum);
     std::priority_queue< PrioritySortUtil::Sortable<Thing> > sortedThings;
@@ -40,7 +39,7 @@
         sortedThings.push(PrioritySortUtil::Sortable<Thing> entry(thing, priority));
     }
 
-(4) Loop over your priority queue and do timeboxed work:
+(3) Loop over your priority queue and do timeboxed work:
 
     uint64_t cutoffTime = usecTimestampNow() + TIME_BUDGET;
     while (!sortedThings.empty()) {
@@ -51,7 +50,6 @@
             break;
         }
     }
-
 */
 
 namespace PrioritySortUtil {
@@ -60,39 +58,28 @@ namespace PrioritySortUtil {
     constexpr float DEFAULT_CENTER_COEF { 0.5f };
     constexpr float DEFAULT_AGE_COEF { 0.25f / (float)(USECS_PER_SECOND) };
 
-    template <typename T>
     class Sortable {
-    public:
-        Sortable(const T& thing, float sortPriority) : _thing(thing), _priority(sortPriority) {}
-        const T& getThing() const { return  _thing; }
-        void setPriority(float priority) { _priority = priority; }
-        bool operator<(const Sortable& other) const { return _priority < other._priority; }
-    private:
-        T _thing;
-        float _priority;
-    };
-
-    // Prioritizable isn't a template because templates can't have pure-virtual methods.
-    class Prioritizable {
     public:
         virtual glm::vec3 getPosition() const = 0;
         virtual float getRadius() const = 0;
         virtual uint64_t getTimestamp() const = 0;
+
+        void setPriority(float priority) { _priority = priority; }
+        bool operator<(const Sortable& other) const { return _priority < other._priority; }
+    private:
+        float _priority { 0.0f };
     };
 
-    class Prioritizer {
+    template <typename T>
+    class PriorityQueue {
     public:
-        Prioritizer() = delete;
+        PriorityQueue() = delete;
 
-        Prioritizer(const ViewFrustum& view) : _view(view) {
+        PriorityQueue(const ViewFrustum& view) : _view(view) { }
-            cacheView();
-        }
 
-        Prioritizer(const ViewFrustum& view, float angularWeight, float centerWeight, float ageWeight)
+        PriorityQueue(const ViewFrustum& view, float angularWeight, float centerWeight, float ageWeight)
                 : _view(view), _angularWeight(angularWeight), _centerWeight(centerWeight), _ageWeight(ageWeight)
-        {
+        { }
-            cacheView();
-        }
 
         void setView(const ViewFrustum& view) { _view = view; }
 
@@ -102,24 +89,34 @@ namespace PrioritySortUtil {
             _ageWeight = ageWeight;
         }
 
-        float computePriority(const Prioritizable& prioritizableThing) const {
+        size_t size() const { return _queue.size(); }
+        void push(T thing) {
+            thing.setPriority(computePriority(thing));
+            _queue.push(thing);
+        }
+        const T& top() const { return _queue.top(); }
+        void pop() { return _queue.pop(); }
+        bool empty() const { return _queue.empty(); }
+
+    private:
+        float computePriority(const T& thing) const {
             // priority = weighted linear combination of multiple values:
             //   (a) angular size
             //   (b) proximity to center of view
             //   (c) time since last update
             // where the relative "weights" are tuned to scale the contributing values into units of "priority".
 
-            glm::vec3 position = prioritizableThing.getPosition();
+            glm::vec3 position = thing.getPosition();
-            glm::vec3 offset = position - _viewPosition;
+            glm::vec3 offset = position - _view.getPosition();
             float distance = glm::length(offset) + 0.001f; // add 1mm to avoid divide by zero
-            float radius = prioritizableThing.getRadius();
+            float radius = thing.getRadius();
 
             float priority = _angularWeight * (radius / distance)
-                + _centerWeight * (glm::dot(offset, _viewForward) / distance)
+                + _centerWeight * (glm::dot(offset, _view.getDirection()) / distance)
-                + _ageWeight * (float)(usecTimestampNow() - prioritizableThing.getTimestamp());
+                + _ageWeight * (float)(usecTimestampNow() - thing.getTimestamp());
 
             // decrement priority of things outside keyhole
-            if (distance - radius > _viewRadius) {
+            if (distance - radius > _view.getCenterRadius()) {
                 if (!_view.sphereIntersectsFrustum(position, radius)) {
                     constexpr float OUT_OF_VIEW_PENALTY = -10.0f;
                     priority += OUT_OF_VIEW_PENALTY;
@@ -128,18 +125,8 @@ namespace PrioritySortUtil {
             return priority;
         }
 
-    private:
-        void cacheView() {
-            // assuming we'll prioritize many things: cache these values
-            _viewPosition = _view.getPosition();
-            _viewForward = _view.getDirection();
-            _viewRadius = _view.getCenterRadius();
-        }
-
         ViewFrustum _view;
-        glm::vec3 _viewPosition;
+        std::priority_queue<T> _queue;
-        glm::vec3 _viewForward;
-        float _viewRadius;
         float _angularWeight { DEFAULT_ANGULAR_COEF };
         float _centerWeight { DEFAULT_CENTER_COEF };
         float _ageWeight { DEFAULT_AGE_COEF };