Further performance improvements to hex viewer

src/Insight/UserInterfaces/InsightWindow/Widgets/TargetMemoryInspectionPane/HexViewerWidget/ItemGraphicsScene.cpp

@@ -2,9 +2,6 @@
 
 #include <cmath>
 #include <QMenu>
-#include <iterator>
-#include <unordered_set>
-#include <algorithm>
 #include <QApplication>
 #include <QClipboard>
 #include <QByteArray>
@@ -136,15 +133,7 @@ namespace Bloom::Widgets
 
         this->addItem(this->hoverRectX);
         this->addItem(this->hoverRectY);
-
         this->setItemIndexMethod(QGraphicsScene::NoIndex);
-
-        this->allocateGraphicsItemsTimer = new QTimer(this);
-        this->allocateGraphicsItemsTimer->setSingleShot(true);
-        this->allocateGraphicsItemsTimer->setInterval(60);
-        this->allocateGraphicsItemsTimer->callOnTimeout(this, [this] {
-            this->allocateGraphicsItems();
-        });
     }
 
     void ItemGraphicsScene::init() {
@@ -166,15 +155,6 @@ namespace Bloom::Widgets
 
         auto* vScrollBar = this->views().first()->verticalScrollBar();
         vScrollBar->setSingleStep((ByteItem::HEIGHT + (ByteItem::BOTTOM_MARGIN / 2)));
-
-        QObject::connect(
-            vScrollBar,
-            &QScrollBar::valueChanged,
-            this,
-            [this] (int) {
-                this->allocateGraphicsItemsTimer->start();
-            }
-        );
     }
 
     void ItemGraphicsScene::updateStackPointer(std::uint32_t stackPointer) {
@@ -235,7 +215,7 @@ namespace Bloom::Widgets
         const auto itemsRequired = static_cast<std::uint32_t>(
             (availableWidth / (ByteItem::WIDTH + (ByteItem::RIGHT_MARGIN / 2)))
                 * (
-                    (view->viewport()->height() + (40 * ItemGraphicsScene::GRID_SIZE))
+                    (view->viewport()->height() + (4 * ItemGraphicsScene::GRID_SIZE))
                     / (ByteItem::HEIGHT + (ByteItem::BOTTOM_MARGIN / 2))
                 )
         );
@@ -281,6 +261,70 @@ namespace Bloom::Widgets
         return QPointF();
     }
 
+    void ItemGraphicsScene::allocateGraphicsItems() {
+        const auto* view = this->views().first();
+        const auto verticalScrollBarValue = view->verticalScrollBar()->value();
+
+        constexpr auto bufferPointSize = 2;
+        const auto gridPointIndex = static_cast<decltype(this->gridPoints)::size_type>(std::max(
+            static_cast<int>(
+                std::floor(
+                    static_cast<float>(verticalScrollBarValue) / static_cast<float>(ItemGraphicsScene::GRID_SIZE)
+                )
+            ) - 1 - bufferPointSize,
+            0
+        ));
+
+        // Sanity check
+        assert(this->gridPoints.size() > gridPointIndex);
+
+        const auto& allocatableGraphicsItems = this->graphicsItems;
+        auto allocatableGraphicsItemsCount = allocatableGraphicsItems.size();
+
+        const auto allocateRangeStartItemIt = this->gridPoints[gridPointIndex];
+        const auto allocateRangeEndItemIt = allocateRangeStartItemIt + std::min(
+            std::distance(allocateRangeStartItemIt, this->flattenedItems.end() - 1),
+            static_cast<long>(allocatableGraphicsItemsCount)
+        );
+
+        const auto& firstItem = *allocateRangeStartItemIt;
+        const auto& lastItem = *allocateRangeEndItemIt;
+
+        /*
+         * Ensure that a graphics item for each parent, grandparent, etc. is allocated for the first item in the
+         * allocatable range.
+         */
+        auto* parentItem = firstItem->parent;
+
+        while (
+            parentItem != nullptr
+            && parentItem != this->topLevelGroup.get()
+            && allocatableGraphicsItemsCount > 0
+        ) {
+            allocatableGraphicsItems[allocatableGraphicsItemsCount - 1]->setHexViewerItem(parentItem);
+            --allocatableGraphicsItemsCount;
+            parentItem = parentItem->parent;
+        }
+
+        for (auto itemIt = allocateRangeStartItemIt; itemIt != allocateRangeEndItemIt; ++itemIt) {
+            if (allocatableGraphicsItemsCount < 1) {
+                // No more graphics items available to allocate
+                break;
+            }
+
+            allocatableGraphicsItems[allocatableGraphicsItemsCount - 1]->setHexViewerItem(*itemIt);
+            --allocatableGraphicsItemsCount;
+        }
+
+        // If we still have some available graphics items, clear them
+        while (allocatableGraphicsItemsCount > 0) {
+            allocatableGraphicsItems[allocatableGraphicsItemsCount - 1]->setHexViewerItem(nullptr);
+            --allocatableGraphicsItemsCount;
+        }
+
+        this->update();
+    }
+
     bool ItemGraphicsScene::event(QEvent* event) {
         if (event->type() == QEvent::Type::GraphicsSceneLeave && this->state.hoveredByteItem != nullptr) {
             this->onByteItemLeave();
@@ -432,6 +476,7 @@ namespace Bloom::Widgets
 
     void ItemGraphicsScene::mouseReleaseEvent(QGraphicsSceneMouseEvent* mouseEvent) {
         this->clearSelectionRectItem();
+        this->update();
     }
 
     void ItemGraphicsScene::keyPressEvent(QKeyEvent* keyEvent) {
@@ -486,99 +531,6 @@ namespace Bloom::Widgets
         menu->exec(event->screenPos());
     }
 
-    void ItemGraphicsScene::allocateGraphicsItems() {
-        const auto* view = this->views().first();
-        const auto verticalScrollBarValue = view->verticalScrollBar()->value();
-
-        constexpr auto bufferPointSize = 20;
-        const auto gridPointIndex = static_cast<decltype(this->gridPoints)::size_type>(std::max(
-            static_cast<int>(
-                std::floor(
-                    static_cast<float>(verticalScrollBarValue) / static_cast<float>(ItemGraphicsScene::GRID_SIZE)
-                )
-            ) - 1 - bufferPointSize,
-            0
-        ));
-
-        // Sanity check
-        assert(this->gridPoints.size() > gridPointIndex);
-
-        const auto totalGraphicsItems = this->graphicsItems.size();
-
-        auto allocateRangeStartItemIt = this->gridPoints[gridPointIndex];
-        const auto allocateRangeEndItemIt = allocateRangeStartItemIt + std::min(
-            std::distance(allocateRangeStartItemIt, this->flattenedItems.end() - 1),
-            static_cast<long>(totalGraphicsItems)
-        );
-
-        const auto excessAvailableGraphicItems = static_cast<int>(totalGraphicsItems)
-            - std::distance(allocateRangeStartItemIt, allocateRangeEndItemIt);
-
-        if (excessAvailableGraphicItems > 0) {
-            allocateRangeStartItemIt -= std::min(
-                std::distance(this->flattenedItems.begin(), allocateRangeStartItemIt),
-                excessAvailableGraphicItems
-            );
-        }
-
-        const auto allocateRangeStartAddress = (*allocateRangeStartItemIt)->startAddress;
-        const auto allocateRangeEndAddress = (*allocateRangeEndItemIt)->startAddress;
-
-        auto allocatableGraphicsItems = std::unordered_set<GraphicsItem*>();
-        std::copy_if(
-            this->graphicsItems.begin(),
-            this->graphicsItems.end(),
-            std::inserter(allocatableGraphicsItems, allocatableGraphicsItems.begin()),
-            [&allocateRangeStartAddress, &allocateRangeEndAddress] (const GraphicsItem* graphicsItem) {
-                return
-                    graphicsItem->hexViewerItem == nullptr
-                    || graphicsItem->hexViewerItem->startAddress < allocateRangeStartAddress
-                    || graphicsItem->hexViewerItem->startAddress > allocateRangeEndAddress;
-            }
-        );
-
-        /*
-         * Ensure that a graphics item for each parent, grandparent, etc. is allocated for the first item in the
-         * allocatable range.
-         */
-        const auto& firstItem = *allocateRangeStartItemIt;
-        auto* parentItem = firstItem->parent;
-
-        while (
-            parentItem != nullptr
-            && parentItem != this->topLevelGroup.get()
-            && !allocatableGraphicsItems.empty()
-        ) {
-            if (parentItem->allocatedGraphicsItem == nullptr) {
-                (*allocatableGraphicsItems.begin())->setHexViewerItem(parentItem);
-            }
-
-            allocatableGraphicsItems.erase(parentItem->allocatedGraphicsItem);
-            parentItem = parentItem->parent;
-        }
-
-        for (auto itemIt = allocateRangeStartItemIt; itemIt != allocateRangeEndItemIt; ++itemIt) {
-            if (allocatableGraphicsItems.empty()) {
-                // No more graphics items available to allocate
-                break;
-            }
-
-            auto& item = *itemIt;
-
-            if (item->allocatedGraphicsItem != nullptr) {
-                continue;
-            }
-
-            (*allocatableGraphicsItems.begin())->setHexViewerItem(item);
-            allocatableGraphicsItems.erase(item->allocatedGraphicsItem);
-        }
-
-        // If we still have some available graphics items, clear them
-        for (auto& graphicsItem : allocatableGraphicsItems) {
-            graphicsItem->setHexViewerItem(nullptr);
-        }
-    }
-
     void ItemGraphicsScene::refreshItemPositionIndices() {
         const auto pointsRequired = static_cast<std::uint32_t>(
             this->sceneRect().height() / ItemGraphicsScene::GRID_SIZE
@@ -675,6 +627,7 @@ namespace Bloom::Widgets
 
         this->hoverRectX->setVisible(false);
         this->hoverRectY->setVisible(false);
+        this->update();
     }
 
     void ItemGraphicsScene::clearSelectionRectItem() {

src/Insight/UserInterfaces/InsightWindow/Widgets/TargetMemoryInspectionPane/HexViewerWidget/ItemGraphicsScene.hpp

@@ -60,6 +60,7 @@ namespace Bloom::Widgets
         void setEnabled(bool enabled);
         void refreshValues();
         QPointF getByteItemPositionByAddress(Targets::TargetMemoryAddress address);
+        void allocateGraphicsItems();
 
     signals:
         void ready();
@@ -119,8 +120,6 @@ namespace Bloom::Widgets
         QAction* displayRelativeAddressAction = new QAction("Relative", this);
         QAction* displayAbsoluteAddressAction = new QAction("Absolute", this);
 
-        QTimer* allocateGraphicsItemsTimer = nullptr;
-
         int getSceneWidth() {
             /*
              * Minus 2 for the QSS margin on the vertical scrollbar (which isn't accounted for during viewport
@@ -131,7 +130,6 @@ namespace Bloom::Widgets
             return std::max(this->parent->viewport()->width(), 400) - 2;
         }
 
-        void allocateGraphicsItems();
         void refreshItemPositionIndices();
         void onTargetStateChanged(Targets::TargetState newState);
         void onByteItemEnter(ByteItem& byteItem);

src/Insight/UserInterfaces/InsightWindow/Widgets/TargetMemoryInspectionPane/HexViewerWidget/ItemGraphicsView.cpp

@@ -20,10 +20,10 @@ namespace Bloom::Widgets
         this->setVerticalScrollBarPolicy(Qt::ScrollBarPolicy::ScrollBarAlwaysOn);
         this->setHorizontalScrollBarPolicy(Qt::ScrollBarPolicy::ScrollBarAlwaysOff);
         this->setSizePolicy(QSizePolicy::MinimumExpanding, QSizePolicy::MinimumExpanding);
-        this->setViewportUpdateMode(QGraphicsView::MinimalViewportUpdate);
+        this->setViewportUpdateMode(QGraphicsView::NoViewportUpdate);
         this->setOptimizationFlag(QGraphicsView::DontSavePainterState, true);
         this->setOptimizationFlag(QGraphicsView::DontAdjustForAntialiasing, true);
-        this->setCacheMode(QGraphicsView::CacheBackground);
+        this->setCacheMode(QGraphicsView::CacheModeFlag::CacheNone);
         this->setFocusPolicy(Qt::StrongFocus);
 
         this->scene = new ItemGraphicsScene(
@@ -75,4 +75,9 @@ namespace Bloom::Widgets
             this->scene->adjustSize();
         }
     }
+
+    void ItemGraphicsView::scrollContentsBy(int dx, int dy) {
+        this->scene->allocateGraphicsItems();
+        return QGraphicsView::scrollContentsBy(dx, dy);
+    }
 }

src/Insight/UserInterfaces/InsightWindow/Widgets/TargetMemoryInspectionPane/HexViewerWidget/ItemGraphicsView.hpp

@@ -41,6 +41,7 @@ namespace Bloom::Widgets
     protected:
         bool event(QEvent* event) override;
         void resizeEvent(QResizeEvent* event) override;
+        void scrollContentsBy(int dx, int dy) override;
 
     private:
         ItemGraphicsScene* scene = nullptr;