diff --git a/src/plugins/qmldesigner/designercore/imagecache/imagecachegenerator.cpp b/src/plugins/qmldesigner/designercore/imagecache/imagecachegenerator.cpp index ea0493176bf..13e1a141c78 100644 --- a/src/plugins/qmldesigner/designercore/imagecache/imagecachegenerator.cpp +++ b/src/plugins/qmldesigner/designercore/imagecache/imagecachegenerator.cpp @@ -56,22 +56,47 @@ void ImageCacheGenerator::generateImage(Utils::SmallStringView name, { { std::lock_guard lock{m_mutex}; - m_tasks.emplace_back(name, - extraId, - std::move(auxiliaryData), - timeStamp, - std::move(captureCallback), - std::move(abortCallback)); + + auto found = std::find_if(m_tasks.begin(), m_tasks.end(), [&](const Task &task) { + return task.filePath == name && task.extraId == extraId; + }); + + if (found != m_tasks.end()) { + found->timeStamp = timeStamp; + found->captureCallbacks.push_back(std::move(captureCallback)); + found->abortCallbacks.push_back(std::move(abortCallback)); + } else { + m_tasks.emplace_back(name, + extraId, + std::move(auxiliaryData), + timeStamp, + std::move(captureCallback), + std::move(abortCallback)); + } } m_condition.notify_all(); } +namespace { +Utils::PathString createId(Utils::SmallStringView name, Utils::SmallStringView extraId) +{ + return extraId.empty() ? Utils::PathString{name} : Utils::PathString{name, "+", extraId}; +} +template +void callCallbacks(const Callbacks &callbacks, Argument &&...arguments) +{ + for (auto &&callback : callbacks) + callback(std::forward(arguments)...); +} + +} // namespace + void ImageCacheGenerator::clean() { std::lock_guard lock{m_mutex}; for (Task &task : m_tasks) - task.abortCallback(); + callCallbacks(task.abortCallbacks); m_tasks.clear(); } @@ -83,12 +108,6 @@ void ImageCacheGenerator::waitForFinished() if (m_backgroundThread) m_backgroundThread->wait(); } -namespace { -Utils::PathString createId(Utils::SmallStringView name, Utils::SmallStringView extraId) -{ - return extraId.empty() ? Utils::PathString{name} : Utils::PathString{name, "+", extraId}; -} -} // namespace void ImageCacheGenerator::startGeneration() { @@ -105,9 +124,9 @@ void ImageCacheGenerator::startGeneration() return; } - task = std::move(m_tasks.back()); + task = std::move(m_tasks.front()); - m_tasks.pop_back(); + m_tasks.pop_front(); } m_collector.start( @@ -116,9 +135,9 @@ void ImageCacheGenerator::startGeneration() std::move(task.auxiliaryData), [this, task](QImage &&image, QImage &&smallImage) { if (image.isNull()) - task.abortCallback(); + callCallbacks(task.abortCallbacks); else - task.captureCallback(image, smallImage); + callCallbacks(task.captureCallbacks, image, smallImage); m_storage.storeImage(createId(task.filePath, task.extraId), task.timeStamp, @@ -126,7 +145,7 @@ void ImageCacheGenerator::startGeneration() smallImage); }, [this, task] { - task.abortCallback(); + callCallbacks(task.abortCallbacks); m_storage.storeImage(createId(task.filePath, task.extraId), task.timeStamp, {}, {}); }); diff --git a/src/plugins/qmldesigner/designercore/imagecache/imagecachegenerator.h b/src/plugins/qmldesigner/designercore/imagecache/imagecachegenerator.h index baa6831754e..78b7c96c937 100644 --- a/src/plugins/qmldesigner/designercore/imagecache/imagecachegenerator.h +++ b/src/plugins/qmldesigner/designercore/imagecache/imagecachegenerator.h @@ -32,6 +32,7 @@ #include +#include #include #include @@ -74,16 +75,16 @@ private: : filePath(filePath) , extraId(std::move(extraId)) , auxiliaryData(std::move(auxiliaryData)) - , captureCallback(std::move(captureCallback)) - , abortCallback(std::move(abortCallback)) + , captureCallbacks({std::move(captureCallback)}) + , abortCallbacks({std::move(abortCallback)}) , timeStamp(timeStamp) {} Utils::PathString filePath; Utils::SmallString extraId; ImageCache::AuxiliaryData auxiliaryData; - CaptureCallback captureCallback; - AbortCallback abortCallback; + std::vector captureCallbacks; + std::vector abortCallbacks; Sqlite::TimeStamp timeStamp; }; @@ -98,7 +99,7 @@ private: std::unique_ptr m_backgroundThread; mutable std::mutex m_mutex; std::condition_variable m_condition; - std::vector m_tasks; + std::deque m_tasks; ImageCacheCollectorInterface &m_collector; ImageCacheStorageInterface &m_storage; bool m_finishing{false}; diff --git a/tests/unit/unittest/imagecachegenerator-test.cpp b/tests/unit/unittest/imagecachegenerator-test.cpp index 9d83f9bbe9f..bcc4a5f47ad 100644 --- a/tests/unit/unittest/imagecachegenerator-test.cpp +++ b/tests/unit/unittest/imagecachegenerator-test.cpp @@ -78,11 +78,11 @@ TEST_F(ImageCacheGenerator, CallsCollectorWithCaptureCallback) TEST_F(ImageCacheGenerator, CallsCollectorOnlyIfNotProcessing) { - EXPECT_CALL(collectorMock, start(Eq("name"), _, _, _, _)) + EXPECT_CALL(collectorMock, start(AnyOf(Eq("name"), Eq("name2")), _, _, _, _)) .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); }); generator.generateImage("name", {}, {}, imageCallbackMock.AsStdFunction(), {}, {}); - generator.generateImage("name", {}, {}, imageCallbackMock.AsStdFunction(), {}, {}); + generator.generateImage("name2", {}, {}, imageCallbackMock.AsStdFunction(), {}, {}); notification.wait(2); } @@ -320,4 +320,134 @@ TEST_F(ImageCacheGenerator, CallsCollectorWithAuxiliaryData) notification.wait(); } +TEST_F(ImageCacheGenerator, MergeTasks) +{ + EXPECT_CALL(collectorMock, start(Eq("waitDummy"), _, _, _, _)) + .WillRepeatedly([&](auto, auto, auto, auto, auto) { waitInThread.wait(); }); + EXPECT_CALL(collectorMock, start(Eq("notificationDummy"), _, _, _, _)) + .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); }); + + EXPECT_CALL(collectorMock, start(Eq("name"), _, _, _, _)); + + generator.generateImage("waitDummy", {}, {}, {}, {}, {}); + generator.generateImage("name", {}, {}, {}, {}, {}); + generator.generateImage("name", {}, {}, {}, {}, {}); + generator.generateImage("notificationDummy", {}, {}, {}, {}, {}); + waitInThread.notify(); + notification.wait(); +} + +TEST_F(ImageCacheGenerator, DontMergeTasksWithDifferentId) +{ + EXPECT_CALL(collectorMock, start(Eq("waitDummy"), _, _, _, _)) + .WillRepeatedly([&](auto, auto, auto, auto, auto) { waitInThread.wait(); }); + + EXPECT_CALL(collectorMock, start(Eq("name"), _, _, _, _)) + .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); }); + EXPECT_CALL(collectorMock, start(Eq("name2"), _, _, _, _)) + .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); }); + + generator.generateImage("waitDummy", {}, {}, {}, {}, {}); + generator.generateImage("name", {}, {}, {}, {}, {}); + generator.generateImage("name2", {}, {}, {}, {}, {}); + waitInThread.notify(); + notification.wait(2); +} + +TEST_F(ImageCacheGenerator, MergeTasksWithSameExtraId) +{ + EXPECT_CALL(collectorMock, start(Eq("waitDummy"), _, _, _, _)) + .WillRepeatedly([&](auto, auto, auto, auto, auto) { waitInThread.wait(); }); + EXPECT_CALL(collectorMock, start(Eq("notificationDummy"), _, _, _, _)) + .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); }); + + EXPECT_CALL(collectorMock, start(Eq("name"), _, _, _, _)); + + generator.generateImage("waitDummy", {}, {}, {}, {}, {}); + generator.generateImage("name", "id1", {}, {}, {}, {}); + generator.generateImage("name", "id1", {}, {}, {}, {}); + waitInThread.notify(); + generator.generateImage("notificationDummy", {}, {}, {}, {}, {}); + notification.wait(); +} + +TEST_F(ImageCacheGenerator, DontMergeTasksWithDifferentExtraId) +{ + EXPECT_CALL(collectorMock, start(Eq("waitDummy"), _, _, _, _)) + .WillRepeatedly([&](auto, auto, auto, auto, auto) { waitInThread.wait(); }); + + EXPECT_CALL(collectorMock, start(Eq("name"), _, _, _, _)) + .Times(2) + .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); }); + + generator.generateImage("waitDummy", {}, {}, {}, {}, {}); + generator.generateImage("name", "id1", {}, {}, {}, {}); + generator.generateImage("name", "id2", {}, {}, {}, {}); + waitInThread.notify(); + notification.wait(2); +} + +TEST_F(ImageCacheGenerator, UseLastTimeStampIfTasksAreMerged) +{ + ON_CALL(collectorMock, start(Eq("waitDummy"), _, _, _, _)) + .WillByDefault([&](auto, auto, auto, auto, auto) { waitInThread.wait(); }); + ON_CALL(collectorMock, start(Eq("notificationDummy"), _, _, _, _)) + .WillByDefault([&](auto, auto, auto, auto, auto) { notification.notify(); }); + ON_CALL(collectorMock, start(Eq("name"), _, _, _, _)) + .WillByDefault([&](auto, auto, auto, auto, auto abortCallback) { abortCallback(); }); + + EXPECT_CALL(storageMock, storeImage(Eq("name"), _, _, _)); + + generator.generateImage("waitDummy", {}, {}, {}, {}, {}); + generator.generateImage("name", {}, {3}, {}, abortCallbackMock.AsStdFunction(), {}); + generator.generateImage("name", {}, {4}, {}, abortCallbackMock.AsStdFunction(), {}); + generator.generateImage("notificationDummy", {}, {}, {}, {}, {}); + waitInThread.notify(); + notification.wait(); +} + +TEST_F(ImageCacheGenerator, MergeCaptureCallbackIfTasksAreMerged) +{ + NiceMock> newerImageCallbackMock; + ON_CALL(collectorMock, start(Eq("waitDummy"), _, _, _, _)) + .WillByDefault([&](auto, auto, auto, auto, auto) { waitInThread.wait(); }); + ON_CALL(collectorMock, start(Eq("notificationDummy"), _, _, _, _)) + .WillByDefault([&](auto, auto, auto, auto, auto) { notification.notify(); }); + ON_CALL(collectorMock, start(Eq("name"), _, _, _, _)) + .WillByDefault([&](auto, auto, auto, auto imageCallback, auto) { + imageCallback(QImage{image1}, QImage{smallImage1}); + }); + + EXPECT_CALL(imageCallbackMock, Call(_, _)); + EXPECT_CALL(newerImageCallbackMock, Call(_, _)); + + generator.generateImage("waitDummy", {}, {}, {}, {}, {}); + generator.generateImage("name", {}, {}, imageCallbackMock.AsStdFunction(), {}, {}); + generator.generateImage("name", {}, {}, newerImageCallbackMock.AsStdFunction(), {}, {}); + generator.generateImage("notificationDummy", {}, {}, {}, {}, {}); + waitInThread.notify(); + notification.wait(); +} + +TEST_F(ImageCacheGenerator, MergeAbortCallbackIfTasksAreMerged) +{ + NiceMock> newerAbortCallbackMock; + ON_CALL(collectorMock, start(Eq("waitDummy"), _, _, _, _)) + .WillByDefault([&](auto, auto, auto, auto, auto) { waitInThread.wait(); }); + ON_CALL(collectorMock, start(Eq("notificationDummy"), _, _, _, _)) + .WillByDefault([&](auto, auto, auto, auto, auto) { notification.notify(); }); + ON_CALL(collectorMock, start(Eq("name"), _, _, _, _)) + .WillByDefault([&](auto, auto, auto, auto, auto abortCallback) { abortCallback(); }); + + EXPECT_CALL(abortCallbackMock, Call()); + EXPECT_CALL(newerAbortCallbackMock, Call()); + + generator.generateImage("waitDummy", {}, {}, {}, {}, {}); + generator.generateImage("name", {}, {}, {}, abortCallbackMock.AsStdFunction(), {}); + generator.generateImage("name", {}, {}, {}, newerAbortCallbackMock.AsStdFunction(), {}); + generator.generateImage("notificationDummy", {}, {}, {}, {}, {}); + waitInThread.notify(); + notification.wait(); +} + } // namespace