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: <github-actions-qt-creator@cristianadam.eu>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Thomas Hartmann <thomas.hartmann@qt.io>
This commit is contained in:
Marco Bubke
2022-01-11 11:08:49 +01:00
parent 3d0d9e8e0c
commit 7ce13691ff
4 changed files with 70 additions and 80 deletions

View File

@@ -25,6 +25,7 @@
#include "asynchronousimagefactory.h" #include "asynchronousimagefactory.h"
#include "imagecachecollector.h"
#include "imagecachegenerator.h" #include "imagecachegenerator.h"
#include "imagecachestorage.h" #include "imagecachestorage.h"
#include "timestampprovider.h" #include "timestampprovider.h"
@@ -32,11 +33,11 @@
namespace QmlDesigner { namespace QmlDesigner {
AsynchronousImageFactory::AsynchronousImageFactory(ImageCacheStorageInterface &storage, AsynchronousImageFactory::AsynchronousImageFactory(ImageCacheStorageInterface &storage,
ImageCacheGeneratorInterface &generator, TimeStampProviderInterface &timeStampProvider,
TimeStampProviderInterface &timeStampProvider) ImageCacheCollectorInterface &collector)
: m_storage(storage) : m_storage(storage)
, m_generator(generator)
, m_timeStampProvider(timeStampProvider) , m_timeStampProvider(timeStampProvider)
, m_collector(collector)
{ {
m_backgroundThread = std::thread{[this] { m_backgroundThread = std::thread{[this] {
while (isRunning()) { while (isRunning()) {
@@ -45,8 +46,8 @@ AsynchronousImageFactory::AsynchronousImageFactory(ImageCacheStorageInterface &s
entry->extraId, entry->extraId,
std::move(entry->auxiliaryData), std::move(entry->auxiliaryData),
m_storage, m_storage,
m_generator, m_timeStampProvider,
m_timeStampProvider); m_collector);
} }
waitForEntries(); waitForEntries();
@@ -107,8 +108,8 @@ void AsynchronousImageFactory::request(Utils::SmallStringView name,
Utils::SmallStringView extraId, Utils::SmallStringView extraId,
ImageCache::AuxiliaryData auxiliaryData, ImageCache::AuxiliaryData auxiliaryData,
ImageCacheStorageInterface &storage, ImageCacheStorageInterface &storage,
ImageCacheGeneratorInterface &generator, TimeStampProviderInterface &timeStampProvider,
TimeStampProviderInterface &timeStampProvider) ImageCacheCollectorInterface &collector)
{ {
const auto id = extraId.empty() ? Utils::PathString{name} const auto id = extraId.empty() ? Utils::PathString{name}
: Utils::PathString::join({name, "+", extraId}); : Utils::PathString::join({name, "+", extraId});
@@ -118,19 +119,20 @@ void AsynchronousImageFactory::request(Utils::SmallStringView name,
if (currentModifiedTime == storageModifiedTime) if (currentModifiedTime == storageModifiedTime)
return; return;
auto capture = [=](const QImage &image, const QImage &smallImage) {
m_storage.storeImage(id, currentModifiedTime, image, smallImage);
};
generator.generateImage(name, collector.start(name,
extraId, extraId,
currentModifiedTime, std::move(auxiliaryData),
ImageCache::CaptureImageWithSmallImageCallback{}, std::move(capture),
ImageCache::AbortCallback{}, ImageCache::AbortCallback{});
std::move(auxiliaryData));
} }
void AsynchronousImageFactory::clean() void AsynchronousImageFactory::clean()
{ {
clearEntries(); clearEntries();
m_generator.clean();
} }
void AsynchronousImageFactory::wait() void AsynchronousImageFactory::wait()

View File

@@ -39,15 +39,14 @@ namespace QmlDesigner {
class TimeStampProviderInterface; class TimeStampProviderInterface;
class ImageCacheStorageInterface; class ImageCacheStorageInterface;
class ImageCacheGeneratorInterface;
class ImageCacheCollectorInterface; class ImageCacheCollectorInterface;
class AsynchronousImageFactory class AsynchronousImageFactory
{ {
public: public:
AsynchronousImageFactory(ImageCacheStorageInterface &storage, AsynchronousImageFactory(ImageCacheStorageInterface &storage,
ImageCacheGeneratorInterface &generator, TimeStampProviderInterface &timeStampProvider,
TimeStampProviderInterface &timeStampProvider); ImageCacheCollectorInterface &collector);
~AsynchronousImageFactory(); ~AsynchronousImageFactory();
@@ -83,8 +82,8 @@ private:
Utils::SmallStringView extraId, Utils::SmallStringView extraId,
ImageCache::AuxiliaryData auxiliaryData, ImageCache::AuxiliaryData auxiliaryData,
ImageCacheStorageInterface &storage, ImageCacheStorageInterface &storage,
ImageCacheGeneratorInterface &generator, TimeStampProviderInterface &timeStampProvider,
TimeStampProviderInterface &timeStampProvider); ImageCacheCollectorInterface &collector);
void wait(); void wait();
void clearEntries(); void clearEntries();
void stopThread(); void stopThread();
@@ -95,8 +94,8 @@ private:
std::condition_variable m_condition; std::condition_variable m_condition;
std::thread m_backgroundThread; std::thread m_backgroundThread;
ImageCacheStorageInterface &m_storage; ImageCacheStorageInterface &m_storage;
ImageCacheGeneratorInterface &m_generator;
TimeStampProviderInterface &m_timeStampProvider; TimeStampProviderInterface &m_timeStampProvider;
ImageCacheCollectorInterface &m_collector;
bool m_finishing{false}; bool m_finishing{false};
}; };

View File

@@ -83,10 +83,9 @@ public:
ImageCacheConnectionManager connectionManager; ImageCacheConnectionManager connectionManager;
ImageCacheCollector collector{connectionManager, ImageCacheCollector collector{connectionManager,
ImageCacheCollectorNullImageHandling::DontCaptureNullImage}; ImageCacheCollectorNullImageHandling::DontCaptureNullImage};
ImageCacheGenerator generator{collector, storage};
TimeStampProvider timeStampProvider; TimeStampProvider timeStampProvider;
AsynchronousExplicitImageCache cache{storage}; AsynchronousExplicitImageCache cache{storage};
AsynchronousImageFactory factory{storage, generator, timeStampProvider}; AsynchronousImageFactory factory{storage, timeStampProvider, collector};
}; };
class QmlDesignerProjectManagerProjectData class QmlDesignerProjectManagerProjectData

View File

@@ -25,6 +25,7 @@
#include "googletest.h" #include "googletest.h"
#include "imagecachecollectormock.h"
#include "mockimagecachegenerator.h" #include "mockimagecachegenerator.h"
#include "mockimagecachestorage.h" #include "mockimagecachestorage.h"
#include "mocktimestampprovider.h" #include "mocktimestampprovider.h"
@@ -41,67 +42,64 @@ class AsynchronousImageFactory : public testing::Test
protected: protected:
AsynchronousImageFactory() AsynchronousImageFactory()
{ {
ON_CALL(mockTimeStampProvider, timeStamp(Eq("/path/to/Component.qml"))) ON_CALL(timeStampProviderMock, timeStamp(Eq("/path/to/Component.qml")))
.WillByDefault(Return(Sqlite::TimeStamp{123})); .WillByDefault(Return(Sqlite::TimeStamp{123}));
} }
protected: protected:
Notification notification; Notification notification;
Notification waitInThread; Notification waitInThread;
NiceMock<MockImageCacheStorage> mockStorage; NiceMock<MockImageCacheStorage> storageMock;
NiceMock<MockImageCacheGenerator> mockGenerator; NiceMock<ImageCacheCollectorMock> collectorMock;
NiceMock<MockTimeStampProvider> mockTimeStampProvider; NiceMock<MockTimeStampProvider> timeStampProviderMock;
QmlDesigner::AsynchronousImageFactory factory{mockStorage, mockGenerator, mockTimeStampProvider}; QmlDesigner::AsynchronousImageFactory factory{storageMock, timeStampProviderMock, collectorMock};
QImage image1{10, 10, QImage::Format_ARGB32}; QImage image1{10, 10, QImage::Format_ARGB32};
QImage smallImage1{1, 1, QImage::Format_ARGB32}; QImage smallImage1{1, 1, QImage::Format_ARGB32};
}; };
TEST_F(AsynchronousImageFactory, RequestImageRequestImageFromGenerator) TEST_F(AsynchronousImageFactory, RequestImageRequestImageFromCollector)
{ {
EXPECT_CALL(mockGenerator, EXPECT_CALL(collectorMock,
generateImage(Eq("/path/to/Component.qml"), start(Eq("/path/to/Component.qml"),
IsEmpty(), IsEmpty(),
Eq(Sqlite::TimeStamp{123}), VariantWith<Utils::monostate>(Utils::monostate{}),
_, _,
_, _))
VariantWith<Utils::monostate>(Utils::monostate{}))) .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); });
.WillRepeatedly([&](auto, auto, auto, auto, auto, auto) { notification.notify(); });
factory.generate("/path/to/Component.qml"); factory.generate("/path/to/Component.qml");
notification.wait(); notification.wait();
} }
TEST_F(AsynchronousImageFactory, RequestImageWithExtraIdRequestImageFromGenerator) TEST_F(AsynchronousImageFactory, RequestImageWithExtraIdRequestImageFromCollector)
{ {
EXPECT_CALL(mockGenerator, EXPECT_CALL(collectorMock,
generateImage(Eq("/path/to/Component.qml"), start(Eq("/path/to/Component.qml"),
Eq("foo"), Eq("foo"),
Eq(Sqlite::TimeStamp{123}), VariantWith<Utils::monostate>(Utils::monostate{}),
_, _,
_, _))
VariantWith<Utils::monostate>(Utils::monostate{}))) .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); });
.WillRepeatedly([&](auto, auto, auto, auto, auto, auto) { notification.notify(); });
factory.generate("/path/to/Component.qml", "foo"); factory.generate("/path/to/Component.qml", "foo");
notification.wait(); notification.wait();
} }
TEST_F(AsynchronousImageFactory, RequestImageWithAuxiliaryDataRequestImageFromGenerator) TEST_F(AsynchronousImageFactory, RequestImageWithAuxiliaryDataRequestImageFromCollector)
{ {
std::vector<QSize> sizes{{20, 11}}; std::vector<QSize> sizes{{20, 11}};
EXPECT_CALL(mockGenerator, EXPECT_CALL(collectorMock,
generateImage(Eq("/path/to/Component.qml"), start(Eq("/path/to/Component.qml"),
Eq("foo"), Eq("foo"),
Eq(Sqlite::TimeStamp{123}), VariantWith<FontCollectorSizesAuxiliaryData>(
_, AllOf(Field(&FontCollectorSizesAuxiliaryData::sizes,
_, ElementsAre(QSize{20, 11})),
VariantWith<FontCollectorSizesAuxiliaryData>(AllOf( Field(&FontCollectorSizesAuxiliaryData::colorName, Eq(u"color")),
Field(&FontCollectorSizesAuxiliaryData::sizes, Field(&FontCollectorSizesAuxiliaryData::text, Eq(u"some text")))),
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(); });
.WillRepeatedly([&](auto, auto, auto, auto, auto, auto) { notification.notify(); });
factory.generate("/path/to/Component.qml", factory.generate("/path/to/Component.qml",
"foo", "foo",
@@ -111,16 +109,16 @@ TEST_F(AsynchronousImageFactory, RequestImageWithAuxiliaryDataRequestImageFromGe
notification.wait(); 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(); notification.notify();
return Sqlite::TimeStamp{124}; 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})); .WillByDefault(Return(Sqlite::TimeStamp{124}));
EXPECT_CALL(mockGenerator, generateImage(_, _, _, _, _, _)).Times(0); EXPECT_CALL(collectorMock, start(_, _, _, _, _)).Times(0);
factory.generate("/path/to/Component.qml"); factory.generate("/path/to/Component.qml");
notification.wait(); notification.wait();
@@ -128,36 +126,28 @@ TEST_F(AsynchronousImageFactory, DontRequestImageRequestImageFromGeneratorIfFile
TEST_F(AsynchronousImageFactory, CleanRemovesEntries) TEST_F(AsynchronousImageFactory, CleanRemovesEntries)
{ {
EXPECT_CALL(mockGenerator, generateImage(Eq("/path/to/Component1.qml"), _, _, _, _, _)) EXPECT_CALL(collectorMock, start(Eq("/path/to/Component1.qml"), _, _, _, _))
.WillRepeatedly([&](auto, auto, auto, auto, auto, auto) { waitInThread.wait(); }); .WillRepeatedly([&](auto, auto, auto, auto, auto) { waitInThread.wait(); });
factory.generate("/path/to/Component1.qml"); 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.generate("/path/to/Component3.qml");
factory.clean(); factory.clean();
waitInThread.notify(); waitInThread.notify();
} }
TEST_F(AsynchronousImageFactory, CleanCallsGeneratorClean)
{
EXPECT_CALL(mockGenerator, clean()).Times(AtLeast(1));
factory.clean();
}
TEST_F(AsynchronousImageFactory, AfterCleanNewJobsWorks) TEST_F(AsynchronousImageFactory, AfterCleanNewJobsWorks)
{ {
factory.clean(); factory.clean();
EXPECT_CALL(mockGenerator, EXPECT_CALL(collectorMock,
generateImage(Eq("/path/to/Component.qml"), start(Eq("/path/to/Component.qml"),
IsEmpty(), IsEmpty(),
Eq(Sqlite::TimeStamp{123}), VariantWith<Utils::monostate>(Utils::monostate{}),
_, _,
_, _))
VariantWith<Utils::monostate>(Utils::monostate{}))) .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); });
.WillRepeatedly([&](auto, auto, auto, auto, auto, auto) { notification.notify(); });
factory.generate("/path/to/Component.qml"); factory.generate("/path/to/Component.qml");
notification.wait(); notification.wait();