From 7ce13691ff2764e317f357fac4931e7bc16d8091 Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Tue, 11 Jan 2022 11:08:49 +0100 Subject: [PATCH] QmlDesigner: Remove generator from AsynchronousImageFactory The generator was introduced to decouple the generation thread from the fetch thread. But for the factory you only generate the image so you can use the factory thread for the collector. Change-Id: If569defba0c52dd85b19451a78f0daf5043b4d01 Reviewed-by: Reviewed-by: Qt CI Bot Reviewed-by: Thomas Hartmann --- .../imagecache/asynchronousimagefactory.cpp | 30 ++--- .../imagecache/asynchronousimagefactory.h | 11 +- .../qmldesigner/qmldesignerprojectmanager.cpp | 3 +- .../asynchronousimagefactory-test.cpp | 106 ++++++++---------- 4 files changed, 70 insertions(+), 80 deletions(-) diff --git a/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.cpp b/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.cpp index 367f53afe54..81c7aa53eef 100644 --- a/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.cpp +++ b/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.cpp @@ -25,6 +25,7 @@ #include "asynchronousimagefactory.h" +#include "imagecachecollector.h" #include "imagecachegenerator.h" #include "imagecachestorage.h" #include "timestampprovider.h" @@ -32,11 +33,11 @@ namespace QmlDesigner { AsynchronousImageFactory::AsynchronousImageFactory(ImageCacheStorageInterface &storage, - ImageCacheGeneratorInterface &generator, - TimeStampProviderInterface &timeStampProvider) + TimeStampProviderInterface &timeStampProvider, + ImageCacheCollectorInterface &collector) : m_storage(storage) - , m_generator(generator) , m_timeStampProvider(timeStampProvider) + , m_collector(collector) { m_backgroundThread = std::thread{[this] { while (isRunning()) { @@ -45,8 +46,8 @@ AsynchronousImageFactory::AsynchronousImageFactory(ImageCacheStorageInterface &s entry->extraId, std::move(entry->auxiliaryData), m_storage, - m_generator, - m_timeStampProvider); + m_timeStampProvider, + m_collector); } waitForEntries(); @@ -107,8 +108,8 @@ void AsynchronousImageFactory::request(Utils::SmallStringView name, Utils::SmallStringView extraId, ImageCache::AuxiliaryData auxiliaryData, ImageCacheStorageInterface &storage, - ImageCacheGeneratorInterface &generator, - TimeStampProviderInterface &timeStampProvider) + TimeStampProviderInterface &timeStampProvider, + ImageCacheCollectorInterface &collector) { const auto id = extraId.empty() ? Utils::PathString{name} : Utils::PathString::join({name, "+", extraId}); @@ -118,19 +119,20 @@ void AsynchronousImageFactory::request(Utils::SmallStringView name, if (currentModifiedTime == storageModifiedTime) return; + auto capture = [=](const QImage &image, const QImage &smallImage) { + m_storage.storeImage(id, currentModifiedTime, image, smallImage); + }; - generator.generateImage(name, - extraId, - currentModifiedTime, - ImageCache::CaptureImageWithSmallImageCallback{}, - ImageCache::AbortCallback{}, - std::move(auxiliaryData)); + collector.start(name, + extraId, + std::move(auxiliaryData), + std::move(capture), + ImageCache::AbortCallback{}); } void AsynchronousImageFactory::clean() { clearEntries(); - m_generator.clean(); } void AsynchronousImageFactory::wait() diff --git a/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.h b/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.h index d9da32d3a5b..1331e1af415 100644 --- a/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.h +++ b/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.h @@ -39,15 +39,14 @@ namespace QmlDesigner { class TimeStampProviderInterface; class ImageCacheStorageInterface; -class ImageCacheGeneratorInterface; class ImageCacheCollectorInterface; class AsynchronousImageFactory { public: AsynchronousImageFactory(ImageCacheStorageInterface &storage, - ImageCacheGeneratorInterface &generator, - TimeStampProviderInterface &timeStampProvider); + TimeStampProviderInterface &timeStampProvider, + ImageCacheCollectorInterface &collector); ~AsynchronousImageFactory(); @@ -83,8 +82,8 @@ private: Utils::SmallStringView extraId, ImageCache::AuxiliaryData auxiliaryData, ImageCacheStorageInterface &storage, - ImageCacheGeneratorInterface &generator, - TimeStampProviderInterface &timeStampProvider); + TimeStampProviderInterface &timeStampProvider, + ImageCacheCollectorInterface &collector); void wait(); void clearEntries(); void stopThread(); @@ -95,8 +94,8 @@ private: std::condition_variable m_condition; std::thread m_backgroundThread; ImageCacheStorageInterface &m_storage; - ImageCacheGeneratorInterface &m_generator; TimeStampProviderInterface &m_timeStampProvider; + ImageCacheCollectorInterface &m_collector; bool m_finishing{false}; }; diff --git a/src/plugins/qmldesigner/qmldesignerprojectmanager.cpp b/src/plugins/qmldesigner/qmldesignerprojectmanager.cpp index 3e5b4bf6297..9ae83b54516 100644 --- a/src/plugins/qmldesigner/qmldesignerprojectmanager.cpp +++ b/src/plugins/qmldesigner/qmldesignerprojectmanager.cpp @@ -83,10 +83,9 @@ public: ImageCacheConnectionManager connectionManager; ImageCacheCollector collector{connectionManager, ImageCacheCollectorNullImageHandling::DontCaptureNullImage}; - ImageCacheGenerator generator{collector, storage}; TimeStampProvider timeStampProvider; AsynchronousExplicitImageCache cache{storage}; - AsynchronousImageFactory factory{storage, generator, timeStampProvider}; + AsynchronousImageFactory factory{storage, timeStampProvider, collector}; }; class QmlDesignerProjectManagerProjectData diff --git a/tests/unit/unittest/asynchronousimagefactory-test.cpp b/tests/unit/unittest/asynchronousimagefactory-test.cpp index ab968f220e3..dfc40ceb5ba 100644 --- a/tests/unit/unittest/asynchronousimagefactory-test.cpp +++ b/tests/unit/unittest/asynchronousimagefactory-test.cpp @@ -25,6 +25,7 @@ #include "googletest.h" +#include "imagecachecollectormock.h" #include "mockimagecachegenerator.h" #include "mockimagecachestorage.h" #include "mocktimestampprovider.h" @@ -41,67 +42,64 @@ class AsynchronousImageFactory : public testing::Test protected: AsynchronousImageFactory() { - ON_CALL(mockTimeStampProvider, timeStamp(Eq("/path/to/Component.qml"))) + ON_CALL(timeStampProviderMock, timeStamp(Eq("/path/to/Component.qml"))) .WillByDefault(Return(Sqlite::TimeStamp{123})); } protected: Notification notification; Notification waitInThread; - NiceMock mockStorage; - NiceMock mockGenerator; - NiceMock mockTimeStampProvider; - QmlDesigner::AsynchronousImageFactory factory{mockStorage, mockGenerator, mockTimeStampProvider}; + NiceMock storageMock; + NiceMock collectorMock; + NiceMock timeStampProviderMock; + QmlDesigner::AsynchronousImageFactory factory{storageMock, timeStampProviderMock, collectorMock}; QImage image1{10, 10, QImage::Format_ARGB32}; QImage smallImage1{1, 1, QImage::Format_ARGB32}; }; -TEST_F(AsynchronousImageFactory, RequestImageRequestImageFromGenerator) +TEST_F(AsynchronousImageFactory, RequestImageRequestImageFromCollector) { - EXPECT_CALL(mockGenerator, - generateImage(Eq("/path/to/Component.qml"), - IsEmpty(), - Eq(Sqlite::TimeStamp{123}), - _, - _, - VariantWith(Utils::monostate{}))) - .WillRepeatedly([&](auto, auto, auto, auto, auto, auto) { notification.notify(); }); + EXPECT_CALL(collectorMock, + start(Eq("/path/to/Component.qml"), + IsEmpty(), + VariantWith(Utils::monostate{}), + _, + _)) + .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); }); factory.generate("/path/to/Component.qml"); notification.wait(); } -TEST_F(AsynchronousImageFactory, RequestImageWithExtraIdRequestImageFromGenerator) +TEST_F(AsynchronousImageFactory, RequestImageWithExtraIdRequestImageFromCollector) { - EXPECT_CALL(mockGenerator, - generateImage(Eq("/path/to/Component.qml"), - Eq("foo"), - Eq(Sqlite::TimeStamp{123}), - _, - _, - VariantWith(Utils::monostate{}))) - .WillRepeatedly([&](auto, auto, auto, auto, auto, auto) { notification.notify(); }); + EXPECT_CALL(collectorMock, + start(Eq("/path/to/Component.qml"), + Eq("foo"), + VariantWith(Utils::monostate{}), + _, + _)) + .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); }); factory.generate("/path/to/Component.qml", "foo"); notification.wait(); } -TEST_F(AsynchronousImageFactory, RequestImageWithAuxiliaryDataRequestImageFromGenerator) +TEST_F(AsynchronousImageFactory, RequestImageWithAuxiliaryDataRequestImageFromCollector) { std::vector sizes{{20, 11}}; - EXPECT_CALL(mockGenerator, - generateImage(Eq("/path/to/Component.qml"), - Eq("foo"), - Eq(Sqlite::TimeStamp{123}), - _, - _, - VariantWith(AllOf( - Field(&FontCollectorSizesAuxiliaryData::sizes, - ElementsAre(QSize{20, 11})), - Field(&FontCollectorSizesAuxiliaryData::colorName, Eq(u"color")), - Field(&FontCollectorSizesAuxiliaryData::text, Eq(u"some text")))))) - .WillRepeatedly([&](auto, auto, auto, auto, auto, auto) { notification.notify(); }); + EXPECT_CALL(collectorMock, + start(Eq("/path/to/Component.qml"), + Eq("foo"), + VariantWith( + AllOf(Field(&FontCollectorSizesAuxiliaryData::sizes, + ElementsAre(QSize{20, 11})), + Field(&FontCollectorSizesAuxiliaryData::colorName, Eq(u"color")), + Field(&FontCollectorSizesAuxiliaryData::text, Eq(u"some text")))), + _, + _)) + .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); }); factory.generate("/path/to/Component.qml", "foo", @@ -111,16 +109,16 @@ TEST_F(AsynchronousImageFactory, RequestImageWithAuxiliaryDataRequestImageFromGe notification.wait(); } -TEST_F(AsynchronousImageFactory, DontRequestImageRequestImageFromGeneratorIfFileWasNotUpdated) +TEST_F(AsynchronousImageFactory, DontRequestImageRequestImageFromCollectorIfFileWasNotUpdated) { - ON_CALL(mockStorage, fetchModifiedImageTime(Eq("/path/to/Component.qml"))).WillByDefault([&](auto) { + ON_CALL(storageMock, fetchModifiedImageTime(Eq("/path/to/Component.qml"))).WillByDefault([&](auto) { notification.notify(); return Sqlite::TimeStamp{124}; }); - ON_CALL(mockTimeStampProvider, timeStamp(Eq("/path/to/Component.qml"))) + ON_CALL(timeStampProviderMock, timeStamp(Eq("/path/to/Component.qml"))) .WillByDefault(Return(Sqlite::TimeStamp{124})); - EXPECT_CALL(mockGenerator, generateImage(_, _, _, _, _, _)).Times(0); + EXPECT_CALL(collectorMock, start(_, _, _, _, _)).Times(0); factory.generate("/path/to/Component.qml"); notification.wait(); @@ -128,36 +126,28 @@ TEST_F(AsynchronousImageFactory, DontRequestImageRequestImageFromGeneratorIfFile TEST_F(AsynchronousImageFactory, CleanRemovesEntries) { - EXPECT_CALL(mockGenerator, generateImage(Eq("/path/to/Component1.qml"), _, _, _, _, _)) - .WillRepeatedly([&](auto, auto, auto, auto, auto, auto) { waitInThread.wait(); }); + EXPECT_CALL(collectorMock, start(Eq("/path/to/Component1.qml"), _, _, _, _)) + .WillRepeatedly([&](auto, auto, auto, auto, auto) { waitInThread.wait(); }); factory.generate("/path/to/Component1.qml"); - EXPECT_CALL(mockGenerator, generateImage(Eq("/path/to/Component3.qml"), _, _, _, _, _)).Times(0); + EXPECT_CALL(collectorMock, start(Eq("/path/to/Component3.qml"), _, _, _, _)).Times(0); factory.generate("/path/to/Component3.qml"); factory.clean(); waitInThread.notify(); } -TEST_F(AsynchronousImageFactory, CleanCallsGeneratorClean) -{ - EXPECT_CALL(mockGenerator, clean()).Times(AtLeast(1)); - - factory.clean(); -} - TEST_F(AsynchronousImageFactory, AfterCleanNewJobsWorks) { factory.clean(); - EXPECT_CALL(mockGenerator, - generateImage(Eq("/path/to/Component.qml"), - IsEmpty(), - Eq(Sqlite::TimeStamp{123}), - _, - _, - VariantWith(Utils::monostate{}))) - .WillRepeatedly([&](auto, auto, auto, auto, auto, auto) { notification.notify(); }); + EXPECT_CALL(collectorMock, + start(Eq("/path/to/Component.qml"), + IsEmpty(), + VariantWith(Utils::monostate{}), + _, + _)) + .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); }); factory.generate("/path/to/Component.qml"); notification.wait();