From 421160eeb6cca69111a29dc804874580d4a2f497 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Mon, 8 Feb 2016 23:40:36 -0800 Subject: [PATCH 1/5] Make the file parsing threads run at lower priority --- libraries/animation/src/AnimationCache.cpp | 5 +++++ .../model-networking/src/model-networking/ModelCache.cpp | 7 +++++++ .../model-networking/src/model-networking/TextureCache.cpp | 7 +++++++ 3 files changed, 19 insertions(+) diff --git a/libraries/animation/src/AnimationCache.cpp b/libraries/animation/src/AnimationCache.cpp index bf5ad233d6..7b15f0df26 100644 --- a/libraries/animation/src/AnimationCache.cpp +++ b/libraries/animation/src/AnimationCache.cpp @@ -47,6 +47,10 @@ AnimationReader::AnimationReader(const QUrl& url, const QByteArray& data) : } void AnimationReader::run() { + auto originalPriority = QThread::currentThread()->priority(); + if (originalPriority == QThread::InheritPriority) { + originalPriority = QThread::NormalPriority; + } try { if (_data.isEmpty()) { throw QString("Reply is NULL ?!"); @@ -73,6 +77,7 @@ void AnimationReader::run() { } catch (const QString& error) { emit onError(299, error); } + QThread::currentThread()->setPriority(originalPriority); } bool Animation::isLoaded() const { diff --git a/libraries/model-networking/src/model-networking/ModelCache.cpp b/libraries/model-networking/src/model-networking/ModelCache.cpp index 2b425351f1..c4dc7ebccf 100644 --- a/libraries/model-networking/src/model-networking/ModelCache.cpp +++ b/libraries/model-networking/src/model-networking/ModelCache.cpp @@ -50,6 +50,11 @@ GeometryReader::GeometryReader(const QUrl& url, const QByteArray& data, const QV } void GeometryReader::run() { + auto originalPriority = QThread::currentThread()->priority(); + if (originalPriority == QThread::InheritPriority) { + originalPriority = QThread::NormalPriority; + } + QThread::currentThread()->setPriority(QThread::LowPriority); try { if (_data.isEmpty()) { throw QString("Reply is NULL ?!"); @@ -82,6 +87,8 @@ void GeometryReader::run() { qCDebug(modelnetworking) << "Error reading " << _url << ": " << error; emit onError(NetworkGeometry::ModelParseError, error); } + + QThread::currentThread()->setPriority(originalPriority); } NetworkGeometry::NetworkGeometry(const QUrl& url, bool delayLoad, const QVariantHash& mapping, const QUrl& textureBaseUrl) : diff --git a/libraries/model-networking/src/model-networking/TextureCache.cpp b/libraries/model-networking/src/model-networking/TextureCache.cpp index 60f6d50d59..8201339e93 100644 --- a/libraries/model-networking/src/model-networking/TextureCache.cpp +++ b/libraries/model-networking/src/model-networking/TextureCache.cpp @@ -268,6 +268,12 @@ void ImageReader::listSupportedImageFormats() { } void ImageReader::run() { + auto originalPriority = QThread::currentThread()->priority(); + if (originalPriority == QThread::InheritPriority) { + originalPriority = QThread::NormalPriority; + } + QThread::currentThread()->setPriority(QThread::LowPriority); + auto texture = _texture.toStrongRef(); if (!texture) { qCWarning(modelnetworking) << "Could not get strong ref"; @@ -306,6 +312,7 @@ void ImageReader::run() { Q_ARG(const QImage&, image), Q_ARG(void*, theTexture), Q_ARG(int, originalWidth), Q_ARG(int, originalHeight)); + QThread::currentThread()->setPriority(originalPriority); } void NetworkTexture::setImage(const QImage& image, void* voidTexture, int originalWidth, From 6a66d74e19f4397f6a9371a1d0a5005ddd4320c5 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Mon, 8 Feb 2016 23:45:04 -0800 Subject: [PATCH 2/5] Limit the number of concurrent file parsing threads --- interface/src/Application.cpp | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index ef0c2a588e..efc482a3b4 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -12,13 +12,13 @@ #include "Application.h" #include - #include #include #include #include #include +#include #include #include #include @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -419,6 +420,14 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer) : _maxOctreePPS(maxOctreePacketsPerSecond.get()), _lastFaceTrackerUpdate(0) { + // FIXME this may be excessivly conservative. On the other hand + // maybe I'm used to having an 8-core machine + // Perhaps find the ideal thread count and subtract 2 or 3 + // (main thread, present thread, random OS load) + // More threads == faster concurrent loads, but also more concurrent + // load on the GPU until we can serialize GPU transfers (off the main thread) + QThreadPool::globalInstance()->setMaxThreadCount(2); + thread()->setPriority(QThread::HighPriority); thread()->setObjectName("Main Thread"); setInstance(this); @@ -1287,6 +1296,9 @@ void Application::initializeUi() { } void Application::paintGL() { + + + // paintGL uses a queued connection, so we can get messages from the queue even after we've quit // and the plugins have shutdown if (_aboutToQuit) { @@ -4762,8 +4774,11 @@ void Application::updateDisplayMode() { foreach(auto displayPlugin, standard) { addDisplayPluginToMenu(displayPlugin, first); // This must be a queued connection to avoid a deadlock - QObject::connect(displayPlugin.get(), &DisplayPlugin::requestRender, - this, &Application::paintGL, Qt::QueuedConnection); + QObject::connect(displayPlugin.get(), &DisplayPlugin::requestRender, [=] { + postEvent(this, new LambdaEvent([=] { + paintGL(); + }), Qt::HighEventPriority); + }); QObject::connect(displayPlugin.get(), &DisplayPlugin::recommendedFramebufferSizeChanged, [this](const QSize & size) { resizeGL(); From 2b127720f88b35c3aa39881f08a942dfd6f8c01c Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Mon, 8 Feb 2016 23:53:30 -0800 Subject: [PATCH 3/5] Run the present thread at highest priority --- .../src/display-plugins/OpenGLDisplayPlugin.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp index 084a164deb..2da60e4205 100644 --- a/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp +++ b/libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp @@ -28,6 +28,11 @@ #include #if THREADED_PRESENT + +// FIXME, for display plugins that don't block on something like vsync, just +// cap the present rate at 200 +// const static unsigned int MAX_PRESENT_RATE = 200; + class PresentThread : public QThread, public Dependency { using Mutex = std::mutex; using Condition = std::condition_variable; @@ -66,8 +71,10 @@ public: _context->moveToThread(this); } + virtual void run() override { OpenGLDisplayPlugin* currentPlugin{ nullptr }; + thread()->setPriority(QThread::HighestPriority); Q_ASSERT(_context); while (!_shutdown) { if (_pendingMainThreadOperation) { @@ -450,4 +457,4 @@ void OpenGLDisplayPlugin::enableDeactivate() { _uncustomized = true; _deactivateWait.notify_one(); } -#endif \ No newline at end of file +#endif From 839b8d432e571a67e0847c1d901e43e3ae254381 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Mon, 8 Feb 2016 23:54:10 -0800 Subject: [PATCH 4/5] Don't burn CPU cycles calculating an average color we don't use --- libraries/model/src/model/TextureMap.cpp | 81 +++++++----------------- 1 file changed, 24 insertions(+), 57 deletions(-) diff --git a/libraries/model/src/model/TextureMap.cpp b/libraries/model/src/model/TextureMap.cpp index a86c7cdbec..e59d9716e6 100755 --- a/libraries/model/src/model/TextureMap.cpp +++ b/libraries/model/src/model/TextureMap.cpp @@ -50,88 +50,55 @@ void TextureMap::setLightmapOffsetScale(float offset, float scale) { } - - +// FIXME why is this in the model library? Move to GPU or GPU_GL gpu::Texture* TextureUsage::create2DTextureFromImage(const QImage& srcImage, const std::string& srcImageName) { QImage image = srcImage; - - int imageArea = image.width() * image.height(); - - int opaquePixels = 0; - int translucentPixels = 0; - //bool isTransparent = false; - int redTotal = 0, greenTotal = 0, blueTotal = 0, alphaTotal = 0; - const int EIGHT_BIT_MAXIMUM = 255; - QColor averageColor(EIGHT_BIT_MAXIMUM, EIGHT_BIT_MAXIMUM, EIGHT_BIT_MAXIMUM); - - if (!image.hasAlphaChannel()) { - if (image.format() != QImage::Format_RGB888) { - image = image.convertToFormat(QImage::Format_RGB888); - } - // int redTotal = 0, greenTotal = 0, blueTotal = 0; - for (int y = 0; y < image.height(); y++) { - for (int x = 0; x < image.width(); x++) { - QRgb rgb = image.pixel(x, y); - redTotal += qRed(rgb); - greenTotal += qGreen(rgb); - blueTotal += qBlue(rgb); - } - } - if (imageArea > 0) { - averageColor.setRgb(redTotal / imageArea, greenTotal / imageArea, blueTotal / imageArea); - } - } else { + if (image.hasAlphaChannel()) { if (image.format() != QImage::Format_ARGB32) { image = image.convertToFormat(QImage::Format_ARGB32); } - // check for translucency/false transparency - // int opaquePixels = 0; - // int translucentPixels = 0; - // int redTotal = 0, greenTotal = 0, blueTotal = 0, alphaTotal = 0; - for (int y = 0; y < image.height(); y++) { - for (int x = 0; x < image.width(); x++) { - QRgb rgb = image.pixel(x, y); - redTotal += qRed(rgb); - greenTotal += qGreen(rgb); - blueTotal += qBlue(rgb); - int alpha = qAlpha(rgb); - alphaTotal += alpha; - if (alpha == EIGHT_BIT_MAXIMUM) { - opaquePixels++; - } else if (alpha != 0) { - translucentPixels++; + // Actual alpha channel? + bool transparent { false }; + for (int y = 0; y < image.height(); ++y) { + const QRgb* data = reinterpret_cast(image.constScanLine(y)); + for (int x = 0; x < image.width(); ++x) { + auto alpha = qAlpha(data[x]); + if (alpha != 255) { + transparent = true; + break; } } } - if (opaquePixels == imageArea) { + + // or bullshit alpha channel? + if (!transparent) { qCDebug(modelLog) << "Image with alpha channel is completely opaque:" << QString(srcImageName.c_str()); image = image.convertToFormat(QImage::Format_RGB888); } - - averageColor = QColor(redTotal / imageArea, - greenTotal / imageArea, blueTotal / imageArea, alphaTotal / imageArea); - - //isTransparent = (translucentPixels >= imageArea / 2); + } + + if (!image.hasAlphaChannel() && image.format() != QImage::Format_RGB888) { + image = image.convertToFormat(QImage::Format_RGB888); } gpu::Texture* theTexture = nullptr; if ((image.width() > 0) && (image.height() > 0)) { - // bool isLinearRGB = true; //(_type == NORMAL_TEXTURE) || (_type == EMISSIVE_TEXTURE); bool isLinearRGB = false; //(_type == NORMAL_TEXTURE) || (_type == EMISSIVE_TEXTURE); - + gpu::Element formatGPU = gpu::Element(gpu::VEC3, gpu::NUINT8, (isLinearRGB ? gpu::RGB : gpu::SRGB)); gpu::Element formatMip = gpu::Element(gpu::VEC3, gpu::NUINT8, (isLinearRGB ? gpu::RGB : gpu::SRGB)); if (image.hasAlphaChannel()) { formatGPU = gpu::Element(gpu::VEC4, gpu::NUINT8, (isLinearRGB ? gpu::RGBA : gpu::SRGBA)); formatMip = gpu::Element(gpu::VEC4, gpu::NUINT8, (isLinearRGB ? gpu::BGRA : gpu::SBGRA)); } - - theTexture = (gpu::Texture::create2D(formatGPU, image.width(), image.height(), gpu::Sampler(gpu::Sampler::FILTER_MIN_MAG_MIP_LINEAR))); - theTexture->assignStoredMip(0, formatMip, image.byteCount(), image.constBits()); - theTexture->autoGenerateMips(-1); + theTexture = (gpu::Texture::create2D(formatGPU, image.width(), image.height(), gpu::Sampler(gpu::Sampler::FILTER_MIN_MAG_MIP_LINEAR))); + theTexture->assignStoredMip(0, formatMip, image.byteCount(), image.constBits()); + theTexture->autoGenerateMips(-1); + + // FIXME queue for transfer to GPU and block on completion } return theTexture; From 7d99f9e72f574aee8bc88f86bd98f7dbafec3fbb Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Tue, 9 Feb 2016 10:16:44 -0800 Subject: [PATCH 5/5] PR comments --- libraries/animation/src/AnimationCache.cpp | 1 + libraries/model/src/model/TextureMap.cpp | 12 +++--------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/libraries/animation/src/AnimationCache.cpp b/libraries/animation/src/AnimationCache.cpp index 7b15f0df26..149e1a3761 100644 --- a/libraries/animation/src/AnimationCache.cpp +++ b/libraries/animation/src/AnimationCache.cpp @@ -51,6 +51,7 @@ void AnimationReader::run() { if (originalPriority == QThread::InheritPriority) { originalPriority = QThread::NormalPriority; } + QThread::currentThread()->setPriority(QThread::LowPriority); try { if (_data.isEmpty()) { throw QString("Reply is NULL ?!"); diff --git a/libraries/model/src/model/TextureMap.cpp b/libraries/model/src/model/TextureMap.cpp index e59d9716e6..cd42419f6a 100755 --- a/libraries/model/src/model/TextureMap.cpp +++ b/libraries/model/src/model/TextureMap.cpp @@ -53,32 +53,26 @@ void TextureMap::setLightmapOffsetScale(float offset, float scale) { // FIXME why is this in the model library? Move to GPU or GPU_GL gpu::Texture* TextureUsage::create2DTextureFromImage(const QImage& srcImage, const std::string& srcImageName) { QImage image = srcImage; + bool validAlpha = false; if (image.hasAlphaChannel()) { if (image.format() != QImage::Format_ARGB32) { image = image.convertToFormat(QImage::Format_ARGB32); } // Actual alpha channel? - bool transparent { false }; for (int y = 0; y < image.height(); ++y) { const QRgb* data = reinterpret_cast(image.constScanLine(y)); for (int x = 0; x < image.width(); ++x) { auto alpha = qAlpha(data[x]); if (alpha != 255) { - transparent = true; + validAlpha = true; break; } } } - - // or bullshit alpha channel? - if (!transparent) { - qCDebug(modelLog) << "Image with alpha channel is completely opaque:" << QString(srcImageName.c_str()); - image = image.convertToFormat(QImage::Format_RGB888); - } } - if (!image.hasAlphaChannel() && image.format() != QImage::Format_RGB888) { + if (!validAlpha && image.format() != QImage::Format_RGB888) { image = image.convertToFormat(QImage::Format_RGB888); }