From 9b098a4bf7fd7df095a4913a261e28146be6701e Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Mon, 8 May 2017 10:51:13 -0700 Subject: [PATCH 01/13] Fix randFloat error in ac audio searcher --- tutorial/ACAudioSearchAndInject_tutorial.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tutorial/ACAudioSearchAndInject_tutorial.js b/tutorial/ACAudioSearchAndInject_tutorial.js index 70e936bb1c..6037c5f723 100644 --- a/tutorial/ACAudioSearchAndInject_tutorial.js +++ b/tutorial/ACAudioSearchAndInject_tutorial.js @@ -1,4 +1,5 @@ "use strict"; + /*jslint nomen: true, plusplus: true, vars: true*/ /*global AvatarList, Entities, EntityViewer, Script, SoundCache, Audio, print, randFloat*/ // @@ -41,7 +42,6 @@ var DEFAULT_SOUND_DATA = { //var isACScript = this.EntityViewer !== undefined; var isACScript = true; -Script.include("http://hifi-content.s3.amazonaws.com/ryan/development/utils_ryan.js"); if (isACScript) { Agent.isAvatar = true; // This puts a robot at 0,0,0, but is currently necessary in order to use AvatarList. Avatar.skeletonModelURL = "http://hifi-content.s3.amazonaws.com/ozan/dev/avatars/invisible_avatar/invisible_avatar.fst"; @@ -51,6 +51,10 @@ function debug() { // Display the arguments not just [Object object]. //print.apply(null, [].map.call(arguments, JSON.stringify)); } +function randFloat(low, high) { + return low + Math.random() * (high - low); +} + if (isACScript) { EntityViewer.setCenterRadius(QUERY_RADIUS); } From c73c59baee08865717b057726ebdd0da6440de00 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Mon, 8 May 2017 11:19:05 -0700 Subject: [PATCH 02/13] Move changes in AC audio searcher from tutorial content into git --- tutorial/ACAudioSearchAndInject_tutorial.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tutorial/ACAudioSearchAndInject_tutorial.js b/tutorial/ACAudioSearchAndInject_tutorial.js index 6037c5f723..5e2998ff1e 100644 --- a/tutorial/ACAudioSearchAndInject_tutorial.js +++ b/tutorial/ACAudioSearchAndInject_tutorial.js @@ -39,12 +39,17 @@ var DEFAULT_SOUND_DATA = { playbackGapRange: 0 // in ms }; +//var AGENT_AVATAR_POSITION = { x: -1.5327, y: 0.672515, z: 5.91573 }; +var AGENT_AVATAR_POSITION = { x: -2.83785, y: 1.45243, z: -13.6042 }; + //var isACScript = this.EntityViewer !== undefined; var isACScript = true; if (isACScript) { Agent.isAvatar = true; // This puts a robot at 0,0,0, but is currently necessary in order to use AvatarList. Avatar.skeletonModelURL = "http://hifi-content.s3.amazonaws.com/ozan/dev/avatars/invisible_avatar/invisible_avatar.fst"; + Avatar.position = AGENT_AVATAR_POSITION; + Agent.isListeningToAudioStream = true; } function ignore() {} function debug() { // Display the arguments not just [Object object]. @@ -97,6 +102,7 @@ function EntityDatum(entityIdentifier) { // Just the data of an entity that we n return; } var properties, soundData; // Latest data, pulled from local octree. + // getEntityProperties locks the tree, which competes with the asynchronous processing of queryOctree results. // Most entity updates are fast and only a very few do getEntityProperties. function ensureSoundData() { // We only getEntityProperities when we need to. @@ -119,43 +125,54 @@ function EntityDatum(entityIdentifier) { // Just the data of an entity that we n } } } + // Stumbling on big new pile of entities will do a lot of getEntityProperties. Once. if (that.lastUserDataUpdate < userDataCutoff) { // NO DATA => SOUND DATA ensureSoundData(); } + if (!that.url) { // NO DATA => NO DATA return that.stop(); } + if (!that.sound) { // SOUND DATA => DOWNLOADING that.sound = SoundCache.getSound(soundData.url); // SoundCache can manage duplicates better than we can. } + if (!that.sound.downloaded) { // DOWNLOADING => DOWNLOADING return; } + if (that.playAfter > now) { // DOWNLOADING | WAITING => WAITING return; } + ensureSoundData(); // We'll try to play/setOptions and will need position, so we might as well get soundData, too. if (soundData.url !== that.url) { // WAITING => NO DATA (update next time around) return that.stop(); } + var options = { position: properties.position, loop: soundData.loop || DEFAULT_SOUND_DATA.loop, volume: soundData.volume || DEFAULT_SOUND_DATA.volume }; + function repeat() { return !options.loop && (soundData.playbackGap >= 0); } + function randomizedNextPlay() { // time of next play or recheck, randomized to distribute the work var range = soundData.playbackGapRange || DEFAULT_SOUND_DATA.playbackGapRange, base = repeat() ? ((that.sound.duration * MSEC_PER_SEC) + (soundData.playbackGap || DEFAULT_SOUND_DATA.playbackGap)) : RECHECK_TIME; return now + base + randFloat(-Math.min(base, range), range); } + if (that.injector && soundData.playing === false) { that.injector.stop(); that.injector = null; } + if (!that.injector) { if (soundData.playing === false) { // WAITING => PLAYING | WAITING return; @@ -169,6 +186,7 @@ function EntityDatum(entityIdentifier) { // Just the data of an entity that we n } return; } + that.injector.setOptions(options); // PLAYING => UPDATE POSITION ETC if (!that.injector.playing) { // Subtle: a looping sound will not check playbackGap. if (repeat()) { // WAITING => PLAYING @@ -182,6 +200,7 @@ function EntityDatum(entityIdentifier) { // Just the data of an entity that we n } }; } + function internEntityDatum(entityIdentifier, timestamp, avatarPosition, avatar) { ignore(avatarPosition, avatar); // We could use avatars and/or avatarPositions to prioritize which ones to play. var entitySound = entityCache[entityIdentifier]; @@ -190,7 +209,9 @@ function internEntityDatum(entityIdentifier, timestamp, avatarPosition, avatar) } entitySound.timestamp = timestamp; // Might be updated for multiple avatars. That's fine. } + var nUpdates = UPDATES_PER_STATS_LOG, lastStats = Date.now(); + function updateAllEntityData() { // A fast update of all entities we know about. A few make sounds. var now = Date.now(), expirationCutoff = now - EXPIRATION_TIME, From b2769702583ff2d97746e16d146f08f4d148d1c3 Mon Sep 17 00:00:00 2001 From: "Anthony J. Thibault" Date: Mon, 8 May 2017 14:32:50 -0700 Subject: [PATCH 03/13] Hand controller lasers work better in thirdPerson camera mode. --- scripts/system/controllers/handControllerGrab.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/system/controllers/handControllerGrab.js b/scripts/system/controllers/handControllerGrab.js index b97e1ff049..f5c3e6eafa 100644 --- a/scripts/system/controllers/handControllerGrab.js +++ b/scripts/system/controllers/handControllerGrab.js @@ -1376,7 +1376,9 @@ function MyController(hand) { visible: true, alpha: 1, parentID: AVATAR_SELF_ID, - parentJointIndex: this.controllerJointIndex, + parentJointIndex: MyAvatar.getJointIndex(this.hand === RIGHT_HAND ? + "_CAMERA_RELATIVE_CONTROLLER_RIGHTHAND" : + "_CAMERA_RELATIVE_CONTROLLER_LEFTHAND"), endParentID: farParentID }; this.overlayLine = Overlays.addOverlay("line3d", lineProperties); From 929079df9784d4eb27e114e29c973b022f0525f0 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Mon, 8 May 2017 19:33:09 -0400 Subject: [PATCH 04/13] add launch.tester to user_actions --- interface/src/Application.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index e882bad7e1..9c23fd3b12 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -941,10 +941,12 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo // sessionRunTime will be reset soon by loadSettings. Grab it now to get previous session value. // The value will be 0 if the user blew away settings this session, which is both a feature and a bug. + static const QString TESTER = "HIFI_TESTER"; auto gpuIdent = GPUIdent::getInstance(); auto glContextData = getGLContextData(); QJsonObject properties = { { "version", applicationVersion() }, + { "tester", QProcessEnvironment::systemEnvironment().contains(TESTER) }, { "previousSessionCrashed", _previousSessionCrashed }, { "previousSessionRuntime", sessionRunTime.get() }, { "cpu_architecture", QSysInfo::currentCpuArchitecture() }, From d86071d783630393578046e389850953501df3ac Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 9 May 2017 12:02:12 -0700 Subject: [PATCH 05/13] Fix possible corruption of ktx cache files When creating a file like this: 1. Create file 2. Write file 3. Close file there is an opening before the file is written and closed where, if the application were to crash (or not finish for any other reason), the file would be left in an undefined state. Instead, this change updates it to do this: 1. Create temp file 2. Write temp file 3. Close temp file 4. Move temp file to final path This ensures that at step 3 we have a valid file, before we rename it. We should never end up with a final file in an undefined state, but it is possible to end up with a temp file that is an undefined state if we, for instance, crash during step 2. --- libraries/networking/src/FileCache.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/libraries/networking/src/FileCache.cpp b/libraries/networking/src/FileCache.cpp index 0a859d511b..0ceda7a745 100644 --- a/libraries/networking/src/FileCache.cpp +++ b/libraries/networking/src/FileCache.cpp @@ -110,10 +110,16 @@ FilePointer FileCache::writeFile(const char* data, File::Metadata&& metadata) { return file; } - // write the new file - FILE* saveFile = fopen(filepath.c_str(), "wb"); + // write the data to a temporary file + std::string tmpFilepath = filepath + "_tmp"; + FILE* saveFile = fopen(tmpFilepath.c_str(), "wb"); if (saveFile != nullptr && fwrite(data, metadata.length, 1, saveFile) && fclose(saveFile) == 0) { - file = addFile(std::move(metadata), filepath); + int result = rename(tmpFilepath.c_str(), filepath.c_str()); + if (result == 0) { + file = addFile(std::move(metadata), filepath); + } else { + qCWarning(file_cache, "[%s] Failed to rename %s to %s (%s)", tmpFilepath.c_str(), filepath.c_str(), strerror(errno)); + } } else { qCWarning(file_cache, "[%s] Failed to write %s (%s)", _dirname.c_str(), metadata.key.c_str(), strerror(errno)); errno = 0; From 542001b14f7e6b9ba3ab5dcac329a454c1308b0f Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 9 May 2017 14:08:43 -0700 Subject: [PATCH 06/13] Update FileCache to use QSaveFile for atomic writes --- libraries/networking/src/FileCache.cpp | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/libraries/networking/src/FileCache.cpp b/libraries/networking/src/FileCache.cpp index 0ceda7a745..348206a863 100644 --- a/libraries/networking/src/FileCache.cpp +++ b/libraries/networking/src/FileCache.cpp @@ -17,6 +17,7 @@ #include #include +#include #include @@ -110,19 +111,13 @@ FilePointer FileCache::writeFile(const char* data, File::Metadata&& metadata) { return file; } - // write the data to a temporary file - std::string tmpFilepath = filepath + "_tmp"; - FILE* saveFile = fopen(tmpFilepath.c_str(), "wb"); - if (saveFile != nullptr && fwrite(data, metadata.length, 1, saveFile) && fclose(saveFile) == 0) { - int result = rename(tmpFilepath.c_str(), filepath.c_str()); - if (result == 0) { - file = addFile(std::move(metadata), filepath); - } else { - qCWarning(file_cache, "[%s] Failed to rename %s to %s (%s)", tmpFilepath.c_str(), filepath.c_str(), strerror(errno)); - } + QSaveFile saveFile(QString::fromStdString(filepath)); + saveFile.open(QIODevice::WriteOnly); + saveFile.write(data, metadata.length); + if (saveFile.commit()) { + file = addFile(std::move(metadata), filepath); } else { - qCWarning(file_cache, "[%s] Failed to write %s (%s)", _dirname.c_str(), metadata.key.c_str(), strerror(errno)); - errno = 0; + qCWarning(file_cache, "[%s] Failed to write %s", _dirname.c_str(), metadata.key.c_str()); } return file; From 6e4c5d1ab4bb336d9fbd386602a48b51e221796e Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 9 May 2017 16:27:02 -0700 Subject: [PATCH 07/13] Update FileCache writing to check QSaveFile::write return value --- libraries/networking/src/FileCache.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/libraries/networking/src/FileCache.cpp b/libraries/networking/src/FileCache.cpp index 348206a863..0f90807d08 100644 --- a/libraries/networking/src/FileCache.cpp +++ b/libraries/networking/src/FileCache.cpp @@ -112,9 +112,8 @@ FilePointer FileCache::writeFile(const char* data, File::Metadata&& metadata) { } QSaveFile saveFile(QString::fromStdString(filepath)); - saveFile.open(QIODevice::WriteOnly); - saveFile.write(data, metadata.length); - if (saveFile.commit()) { + if (saveFile.open(QIODevice::WriteOnly) && saveFile.write(data, metadata.length) == metadata.length + && saveFile.commit()) { file = addFile(std::move(metadata), filepath); } else { qCWarning(file_cache, "[%s] Failed to write %s", _dirname.c_str(), metadata.key.c_str()); From a0bc8ae2eeaa0d32adea3d43bb7d1b7f691368a4 Mon Sep 17 00:00:00 2001 From: David Rowe Date: Wed, 10 May 2017 11:40:42 +1200 Subject: [PATCH 08/13] Fix unreachable eye blink code --- libraries/avatars-renderer/src/avatars-renderer/Head.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libraries/avatars-renderer/src/avatars-renderer/Head.cpp b/libraries/avatars-renderer/src/avatars-renderer/Head.cpp index a90c9cd5f7..1c54ea269a 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Head.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Head.cpp @@ -89,8 +89,7 @@ void Head::simulate(float deltaTime) { _timeWithoutTalking += deltaTime; if ((_averageLoudness - _longTermAverageLoudness) > TALKING_LOUDNESS) { _timeWithoutTalking = 0.0f; - - } else if (_timeWithoutTalking < BLINK_AFTER_TALKING && _timeWithoutTalking >= BLINK_AFTER_TALKING) { + } else if (_timeWithoutTalking - deltaTime < BLINK_AFTER_TALKING && _timeWithoutTalking >= BLINK_AFTER_TALKING) { forceBlink = true; } From a45dc673be5893e6119721c7521a6beafc59da2b Mon Sep 17 00:00:00 2001 From: David Kelly Date: Tue, 9 May 2017 17:33:33 -0700 Subject: [PATCH 09/13] minor handshake script update --- scripts/system/makeUserConnection.js | 29 +++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/scripts/system/makeUserConnection.js b/scripts/system/makeUserConnection.js index 52afc8883d..0475c1a398 100644 --- a/scripts/system/makeUserConnection.js +++ b/scripts/system/makeUserConnection.js @@ -366,6 +366,8 @@ return nearestAvatar; } function messageSend(message) { + // we always append whether or not we are logged in... + message.isLoggedIn = Account.isLoggedIn(); Messages.sendMessage(MESSAGE_CHANNEL, JSON.stringify(message)); } function handStringMessageSend(message) { @@ -463,7 +465,9 @@ endHandshakeAnimation(); // No-op if we were successful, but this way we ensure that failures and abandoned handshakes don't leave us // in a weird state. - request({ uri: requestUrl, method: 'DELETE' }, debug); + if (Account.isLoggedIn()) { + request({ uri: requestUrl, method: 'DELETE' }, debug); + } } function updateTriggers(value, fromKeyboard, hand) { @@ -590,7 +594,7 @@ } } - function makeConnection(id) { + function makeConnection(id, isLoggedIn) { // send done to let the connection know you have made connection. messageSend({ key: "done", @@ -606,7 +610,10 @@ // It would be "simpler" to skip this and just look at the response, but: // 1. We don't want to bother the metaverse with request that we know will fail. // 2. We don't want our code here to be dependent on precisely how the metaverse responds (400, 401, etc.) - if (!Account.isLoggedIn()) { + // 3. We also don't want to connect to someone who is anonymous _now_, but was not earlier and still has + // the same node id. Since the messaging doesn't say _who_ isn't logged in, fail the same as if we are + // not logged in. + if (!Account.isLoggedIn() || !isLoggedIn) { handleConnectionResponseAndMaybeRepeat("401:Unauthorized", {statusCode: 401}); return; } @@ -628,8 +635,12 @@ // we change states, start the connectionInterval where we check // to be sure the hand is still close enough. If not, we terminate // the interval, go back to the waiting state. If we make it - // the entire CONNECTING_TIME, we make the connection. - function startConnecting(id, jointIndex) { + // the entire CONNECTING_TIME, we make the connection. We pass in + // whether or not the connecting id is actually logged in, as now we + // will allow to start the connection process but have it stop with a + // fail message before trying to call the backend if the other guy isn't + // logged in. + function startConnecting(id, jointIndex, isLoggedIn) { var count = 0; debug("connecting", id, "hand", jointIndex); // do we need to do this? @@ -671,7 +682,7 @@ startHandshake(); } else if (count > CONNECTING_TIME / CONNECTING_INTERVAL) { debug("made connection with " + id); - makeConnection(id); + makeConnection(id, isLoggedIn); stopConnecting(); } }, CONNECTING_INTERVAL); @@ -736,7 +747,7 @@ if (state === STATES.WAITING && (!connectingId || connectingId === senderID)) { if (message.id === MyAvatar.sessionUUID) { stopWaiting(); - startConnecting(senderID, exisitingOrSearchedJointIndex()); + startConnecting(senderID, exisitingOrSearchedJointIndex(), message.isLoggedIn); } else if (connectingId) { // this is for someone else (we lost race in connectionRequest), // so lets start over @@ -755,7 +766,7 @@ startHandshake(); break; } - startConnecting(senderID, connectingHandJointIndex); + startConnecting(senderID, connectingHandJointIndex, message.isLoggedIn); } break; case "done": @@ -775,7 +786,7 @@ } else { // they just created a connection request to us, and we are connecting to // them, so lets just stop connecting and make connection.. - makeConnection(connectingId); + makeConnection(connectingId, message.isLoggedIn); stopConnecting(); } } else { From ee1fd693161071382fe5dbc589737d115c97df32 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Mon, 8 May 2017 18:53:04 -0700 Subject: [PATCH 10/13] Fixing crash in texture transfer logic, again --- libraries/gpu-gl/CMakeLists.txt | 2 +- libraries/gpu-gl/src/gpu/gl/GLTexture.cpp | 355 +++++++++++++------- libraries/gpu-gl/src/gpu/gl/GLTexture.h | 34 +- libraries/gpu-gl/src/gpu/gl45/GL45Backend.h | 1 - 4 files changed, 247 insertions(+), 145 deletions(-) diff --git a/libraries/gpu-gl/CMakeLists.txt b/libraries/gpu-gl/CMakeLists.txt index 3e3853532a..65130d6d07 100644 --- a/libraries/gpu-gl/CMakeLists.txt +++ b/libraries/gpu-gl/CMakeLists.txt @@ -1,5 +1,5 @@ set(TARGET_NAME gpu-gl) -setup_hifi_library() +setup_hifi_library(Concurrent) link_hifi_libraries(shared gl gpu) if (UNIX) target_link_libraries(${TARGET_NAME} pthread) diff --git a/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp b/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp index 5534419eaa..84dc49deba 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp +++ b/libraries/gpu-gl/src/gpu/gl/GLTexture.cpp @@ -160,8 +160,6 @@ const uvec3 GLVariableAllocationSupport::INITIAL_MIP_TRANSFER_DIMENSIONS { 64, 6 WorkQueue GLVariableAllocationSupport::_transferQueue; WorkQueue GLVariableAllocationSupport::_promoteQueue; WorkQueue GLVariableAllocationSupport::_demoteQueue; -TexturePointer GLVariableAllocationSupport::_currentTransferTexture; -TransferJobPointer GLVariableAllocationSupport::_currentTransferJob; size_t GLVariableAllocationSupport::_frameTexturesCreated { 0 }; #define OVERSUBSCRIBED_PRESSURE_VALUE 0.95f @@ -176,30 +174,19 @@ const uvec3 GLVariableAllocationSupport::MAX_TRANSFER_DIMENSIONS { 1024, 1024, 1 const size_t GLVariableAllocationSupport::MAX_TRANSFER_SIZE = GLVariableAllocationSupport::MAX_TRANSFER_DIMENSIONS.x * GLVariableAllocationSupport::MAX_TRANSFER_DIMENSIONS.y * 4; #if THREADED_TEXTURE_BUFFERING -std::shared_ptr TransferJob::_bufferThread { nullptr }; -std::atomic TransferJob::_shutdownBufferingThread { false }; -Mutex TransferJob::_mutex; -TransferJob::VoidLambdaQueue TransferJob::_bufferLambdaQueue; -void TransferJob::startTransferLoop() { - if (_bufferThread) { - return; - } - _shutdownBufferingThread = false; - _bufferThread = std::make_shared([] { - TransferJob::bufferLoop(); +TexturePointer GLVariableAllocationSupport::_currentTransferTexture; +TransferJobPointer GLVariableAllocationSupport::_currentTransferJob; +QThreadPool* TransferJob::_bufferThreadPool { nullptr }; + +void TransferJob::startBufferingThread() { + static std::once_flag once; + std::call_once(once, [&] { + _bufferThreadPool = new QThreadPool(qApp); + _bufferThreadPool->setMaxThreadCount(1); }); } -void TransferJob::stopTransferLoop() { - if (!_bufferThread) { - return; - } - _shutdownBufferingThread = true; - _bufferThread->join(); - _bufferThread.reset(); - _shutdownBufferingThread = false; -} #endif TransferJob::TransferJob(const GLTexture& parent, uint16_t sourceMip, uint16_t targetMip, uint8_t face, uint32_t lines, uint32_t lineOffset) @@ -233,7 +220,6 @@ TransferJob::TransferJob(const GLTexture& parent, uint16_t sourceMip, uint16_t t // Buffering can invoke disk IO, so it should be off of the main and render threads _bufferingLambda = [=] { _mipData = _parent._gpuObject.accessStoredMipFace(sourceMip, face)->createView(_transferSize, _transferOffset); - _bufferingCompleted = true; }; _transferLambda = [=] { @@ -243,65 +229,66 @@ TransferJob::TransferJob(const GLTexture& parent, uint16_t sourceMip, uint16_t t } TransferJob::TransferJob(const GLTexture& parent, std::function transferLambda) - : _parent(parent), _bufferingCompleted(true), _transferLambda(transferLambda) { + : _parent(parent), _bufferingRequired(false), _transferLambda(transferLambda) { } TransferJob::~TransferJob() { Backend::updateTextureTransferPendingSize(_transferSize, 0); } - bool TransferJob::tryTransfer() { - // Disable threaded texture transfer for now #if THREADED_TEXTURE_BUFFERING // Are we ready to transfer - if (_bufferingCompleted) { - _transferLambda(); + if (!bufferingCompleted()) { + startBuffering(); + return false; + } +#else + if (_bufferingRequired) { + _bufferingLambda(); + } +#endif + _transferLambda(); + return true; +} + +#if THREADED_TEXTURE_BUFFERING +bool TransferJob::bufferingRequired() const { + if (!_bufferingRequired) { + return false; + } + + // The default state of a QFuture is with status Canceled | Started | Finished, + // so we have to check isCancelled before we check the actual state + if (_bufferingStatus.isCanceled()) { return true; } - startBuffering(); - return false; -#else - if (!_bufferingCompleted) { - _bufferingLambda(); - _bufferingCompleted = true; - } - _transferLambda(); - return true; -#endif + return !_bufferingStatus.isStarted(); } -#if THREADED_TEXTURE_BUFFERING +bool TransferJob::bufferingCompleted() const { + if (!_bufferingRequired) { + return true; + } + + // The default state of a QFuture is with status Canceled | Started | Finished, + // so we have to check isCancelled before we check the actual state + if (_bufferingStatus.isCanceled()) { + return false; + } + + return _bufferingStatus.isFinished(); +} void TransferJob::startBuffering() { - if (_bufferingStarted) { - return; - } - _bufferingStarted = true; - { - Lock lock(_mutex); - _bufferLambdaQueue.push(_bufferingLambda); - } -} - -void TransferJob::bufferLoop() { - while (!_shutdownBufferingThread) { - VoidLambdaQueue workingQueue; - { - Lock lock(_mutex); - _bufferLambdaQueue.swap(workingQueue); - } - - if (workingQueue.empty()) { - QThread::msleep(5); - continue; - } - - while (!workingQueue.empty()) { - workingQueue.front()(); - workingQueue.pop(); - } + if (bufferingRequired()) { + assert(_bufferingStatus.isCanceled()); + _bufferingStatus = QtConcurrent::run(_bufferThreadPool, [=] { + _bufferingLambda(); + }); + assert(!_bufferingStatus.isCanceled()); + assert(_bufferingStatus.isStarted()); } } #endif @@ -316,7 +303,9 @@ GLVariableAllocationSupport::~GLVariableAllocationSupport() { void GLVariableAllocationSupport::addMemoryManagedTexture(const TexturePointer& texturePointer) { _memoryManagedTextures.push_back(texturePointer); - addToWorkQueue(texturePointer); + if (MemoryPressureState::Idle != _memoryPressureState) { + addToWorkQueue(texturePointer); + } } void GLVariableAllocationSupport::addToWorkQueue(const TexturePointer& texturePointer) { @@ -345,10 +334,8 @@ void GLVariableAllocationSupport::addToWorkQueue(const TexturePointer& texturePo break; case MemoryPressureState::Idle: - break; - - default: Q_UNREACHABLE(); + break; } } @@ -364,10 +351,10 @@ WorkQueue& GLVariableAllocationSupport::getActiveWorkQueue() { case MemoryPressureState::Transfer: return _transferQueue; - default: + case MemoryPressureState::Idle: + Q_UNREACHABLE(); break; } - Q_UNREACHABLE(); return empty; } @@ -460,16 +447,11 @@ void GLVariableAllocationSupport::updateMemoryPressure() { } if (newState != _memoryPressureState) { + _memoryPressureState = newState; #if THREADED_TEXTURE_BUFFERING if (MemoryPressureState::Transfer == _memoryPressureState) { - TransferJob::stopTransferLoop(); + TransferJob::startBufferingThread(); } - _memoryPressureState = newState; - if (MemoryPressureState::Transfer == _memoryPressureState) { - TransferJob::startTransferLoop(); - } -#else - _memoryPressureState = newState; #endif // Clear the existing queue _transferQueue = WorkQueue(); @@ -487,49 +469,111 @@ void GLVariableAllocationSupport::updateMemoryPressure() { } } +TexturePointer GLVariableAllocationSupport::getNextWorkQueueItem(WorkQueue& workQueue) { + while (!workQueue.empty()) { + auto workTarget = workQueue.top(); + + auto texture = workTarget.first.lock(); + if (!texture) { + workQueue.pop(); + continue; + } + + // Check whether the resulting texture can actually have work performed + GLTexture* gltexture = Backend::getGPUObject(*texture); + GLVariableAllocationSupport* vartexture = dynamic_cast(gltexture); + switch (_memoryPressureState) { + case MemoryPressureState::Oversubscribed: + if (vartexture->canDemote()) { + return texture; + } + break; + + case MemoryPressureState::Undersubscribed: + if (vartexture->canPromote()) { + return texture; + } + break; + + case MemoryPressureState::Transfer: + if (vartexture->hasPendingTransfers()) { + return texture; + } + break; + + case MemoryPressureState::Idle: + Q_UNREACHABLE(); + break; + } + + // If we got here, then the texture has no work to do in the current state, + // so pop it off the queue and continue + workQueue.pop(); + } + + return TexturePointer(); +} + +void GLVariableAllocationSupport::processWorkQueue(WorkQueue& workQueue) { + if (workQueue.empty()) { + return; + } + + // Get the front of the work queue to perform work + auto texture = getNextWorkQueueItem(workQueue); + if (!texture) { + return; + } + + // Grab the first item off the demote queue + PROFILE_RANGE(render_gpu_gl, __FUNCTION__); + + GLTexture* gltexture = Backend::getGPUObject(*texture); + GLVariableAllocationSupport* vartexture = dynamic_cast(gltexture); + switch (_memoryPressureState) { + case MemoryPressureState::Oversubscribed: + vartexture->demote(); + workQueue.pop(); + addToWorkQueue(texture); + break; + + case MemoryPressureState::Undersubscribed: + vartexture->promote(); + workQueue.pop(); + addToWorkQueue(texture); + break; + + case MemoryPressureState::Transfer: + if (vartexture->executeNextTransfer(texture)) { + workQueue.pop(); + addToWorkQueue(texture); + +#if THREADED_TEXTURE_BUFFERING + // Eagerly start the next buffering job if possible + texture = getNextWorkQueueItem(workQueue); + if (texture) { + gltexture = Backend::getGPUObject(*texture); + vartexture = dynamic_cast(gltexture); + vartexture->executeNextBuffer(texture); + } +#endif + } + break; + + case MemoryPressureState::Idle: + Q_UNREACHABLE(); + break; + } +} + void GLVariableAllocationSupport::processWorkQueues() { if (MemoryPressureState::Idle == _memoryPressureState) { return; } auto& workQueue = getActiveWorkQueue(); - PROFILE_RANGE(render_gpu_gl, __FUNCTION__); - while (!workQueue.empty()) { - auto workTarget = workQueue.top(); - workQueue.pop(); - auto texture = workTarget.first.lock(); - if (!texture) { - continue; - } - - // Grab the first item off the demote queue - GLTexture* gltexture = Backend::getGPUObject(*texture); - GLVariableAllocationSupport* vartexture = dynamic_cast(gltexture); - if (MemoryPressureState::Oversubscribed == _memoryPressureState) { - if (!vartexture->canDemote()) { - continue; - } - vartexture->demote(); - _memoryPressureStateStale = true; - } else if (MemoryPressureState::Undersubscribed == _memoryPressureState) { - if (!vartexture->canPromote()) { - continue; - } - vartexture->promote(); - _memoryPressureStateStale = true; - } else if (MemoryPressureState::Transfer == _memoryPressureState) { - if (!vartexture->hasPendingTransfers()) { - continue; - } - vartexture->executeNextTransfer(texture); - } else { - Q_UNREACHABLE(); - } - - // Reinject into the queue if more work to be done - addToWorkQueue(texture); - break; - } + // Do work on the front of the queue + processWorkQueue(workQueue); if (workQueue.empty()) { _memoryPressureState = MemoryPressureState::Idle; @@ -543,28 +587,83 @@ void GLVariableAllocationSupport::manageMemory() { processWorkQueues(); } +bool GLVariableAllocationSupport::executeNextTransfer(const TexturePointer& currentTexture) { +#if THREADED_TEXTURE_BUFFERING + // If a transfer job is active on the buffering thread, but has not completed it's buffering lambda, + // then we need to exit early, since we don't want to have the transfer job leave scope while it's + // being used in another thread -- See https://highfidelity.fogbugz.com/f/cases/4626 + if (_currentTransferJob && !_currentTransferJob->bufferingCompleted()) { + return false; + } +#endif -void GLVariableAllocationSupport::executeNextTransfer(const TexturePointer& currentTexture) { if (_populatedMip <= _allocatedMip) { +#if THREADED_TEXTURE_BUFFERING + _currentTransferJob.reset(); + _currentTransferTexture.reset(); +#endif + return true; + } + + // If the transfer queue is empty, rebuild it + if (_pendingTransfers.empty()) { + populateTransferQueue(); + } + + bool result = false; + if (!_pendingTransfers.empty()) { +#if THREADED_TEXTURE_BUFFERING + // If there is a current transfer, but it's not the top of the pending transfer queue, then it's an orphan, so we want to abandon it. + if (_currentTransferJob && _currentTransferJob != _pendingTransfers.front()) { + _currentTransferJob.reset(); + } + + if (!_currentTransferJob) { + // Keeping hold of a strong pointer to the transfer job ensures that if the pending transfer queue is rebuilt, the transfer job + // doesn't leave scope, causing a crash in the buffering thread + _currentTransferJob = _pendingTransfers.front(); + + // Keeping hold of a strong pointer during the transfer ensures that the transfer thread cannot try to access a destroyed texture + _currentTransferTexture = currentTexture; + } + + // transfer jobs use asynchronous buffering of the texture data because it may involve disk IO, so we execute a try here to determine if the buffering + // is complete + if (_currentTransferJob->tryTransfer()) { + _pendingTransfers.pop(); + // Once a given job is finished, release the shared pointers keeping them alive + _currentTransferTexture.reset(); + _currentTransferJob.reset(); + result = true; + } +#else + if (_pendingTransfers.front()->tryTransfer()) { + _pendingTransfers.pop(); + result = true; + } +#endif + } + return result; +} + +#if THREADED_TEXTURE_BUFFERING +void GLVariableAllocationSupport::executeNextBuffer(const TexturePointer& currentTexture) { + if (_currentTransferJob && !_currentTransferJob->bufferingCompleted()) { return; } + // If the transfer queue is empty, rebuild it if (_pendingTransfers.empty()) { populateTransferQueue(); } if (!_pendingTransfers.empty()) { - // Keeping hold of a strong pointer during the transfer ensures that the transfer thread cannot try to access a destroyed texture - _currentTransferTexture = currentTexture; - // Keeping hold of a strong pointer to the transfer job ensures that if the pending transfer queue is rebuilt, the transfer job - // doesn't leave scope, causing a crash in the buffering thread - _currentTransferJob = _pendingTransfers.front(); - // transfer jobs use asynchronous buffering of the texture data because it may involve disk IO, so we execute a try here to determine if the buffering - // is complete - if (_currentTransferJob->tryTransfer()) { - _pendingTransfers.pop(); - _currentTransferTexture.reset(); - _currentTransferJob.reset(); + if (!_currentTransferJob) { + _currentTransferJob = _pendingTransfers.front(); + _currentTransferTexture = currentTexture; } + + _currentTransferJob->startBuffering(); } } +#endif diff --git a/libraries/gpu-gl/src/gpu/gl/GLTexture.h b/libraries/gpu-gl/src/gpu/gl/GLTexture.h index 877966f2d9..c6ce2a2495 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLTexture.h +++ b/libraries/gpu-gl/src/gpu/gl/GLTexture.h @@ -8,6 +8,9 @@ #ifndef hifi_gpu_gl_GLTexture_h #define hifi_gpu_gl_GLTexture_h +#include +#include + #include "GLShared.h" #include "GLBackend.h" #include "GLTexelFormat.h" @@ -47,24 +50,19 @@ public: class TransferJob { using VoidLambda = std::function; using VoidLambdaQueue = std::queue; - using ThreadPointer = std::shared_ptr; const GLTexture& _parent; Texture::PixelsPointer _mipData; size_t _transferOffset { 0 }; size_t _transferSize { 0 }; - // Indicates if a transfer from backing storage to interal storage has started - bool _bufferingStarted { false }; - bool _bufferingCompleted { false }; + bool _bufferingRequired { true }; VoidLambda _transferLambda; VoidLambda _bufferingLambda; #if THREADED_TEXTURE_BUFFERING - static Mutex _mutex; - static VoidLambdaQueue _bufferLambdaQueue; - static ThreadPointer _bufferThread; - static std::atomic _shutdownBufferingThread; - static void bufferLoop(); + // Indicates if a transfer from backing storage to interal storage has started + QFuture _bufferingStatus; + static QThreadPool* _bufferThreadPool; #endif public: @@ -75,14 +73,13 @@ public: bool tryTransfer(); #if THREADED_TEXTURE_BUFFERING - static void startTransferLoop(); - static void stopTransferLoop(); + void startBuffering(); + bool bufferingRequired() const; + bool bufferingCompleted() const; + static void startBufferingThread(); #endif private: -#if THREADED_TEXTURE_BUFFERING - void startBuffering(); -#endif void transfer(); }; @@ -100,8 +97,10 @@ protected: static WorkQueue _transferQueue; static WorkQueue _promoteQueue; static WorkQueue _demoteQueue; +#if THREADED_TEXTURE_BUFFERING static TexturePointer _currentTransferTexture; static TransferJobPointer _currentTransferJob; +#endif static const uvec3 INITIAL_MIP_TRANSFER_DIMENSIONS; static const uvec3 MAX_TRANSFER_DIMENSIONS; static const size_t MAX_TRANSFER_SIZE; @@ -109,6 +108,8 @@ protected: static void updateMemoryPressure(); static void processWorkQueues(); + static void processWorkQueue(WorkQueue& workQueue); + static TexturePointer getNextWorkQueueItem(WorkQueue& workQueue); static void addToWorkQueue(const TexturePointer& texture); static WorkQueue& getActiveWorkQueue(); @@ -118,7 +119,10 @@ protected: bool canPromote() const { return _allocatedMip > _minAllocatedMip; } bool canDemote() const { return _allocatedMip < _maxAllocatedMip; } bool hasPendingTransfers() const { return _populatedMip > _allocatedMip; } - void executeNextTransfer(const TexturePointer& currentTexture); +#if THREADED_TEXTURE_BUFFERING + void executeNextBuffer(const TexturePointer& currentTexture); +#endif + bool executeNextTransfer(const TexturePointer& currentTexture); virtual void populateTransferQueue() = 0; virtual void promote() = 0; virtual void demote() = 0; diff --git a/libraries/gpu-gl/src/gpu/gl45/GL45Backend.h b/libraries/gpu-gl/src/gpu/gl45/GL45Backend.h index 8319e61382..fe2761b37d 100644 --- a/libraries/gpu-gl/src/gpu/gl45/GL45Backend.h +++ b/libraries/gpu-gl/src/gpu/gl45/GL45Backend.h @@ -17,7 +17,6 @@ #include #define INCREMENTAL_TRANSFER 0 -#define THREADED_TEXTURE_BUFFERING 1 #define GPU_SSBO_TRANSFORM_OBJECT 1 namespace gpu { namespace gl45 { From d92bdbf0ed5c5a33a9a75b40ad536465b1e6fba1 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 10 May 2017 09:43:55 -0700 Subject: [PATCH 11/13] Fix unsigned/signed comparision warning --- libraries/networking/src/FileCache.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libraries/networking/src/FileCache.cpp b/libraries/networking/src/FileCache.cpp index 0f90807d08..43055e7ed6 100644 --- a/libraries/networking/src/FileCache.cpp +++ b/libraries/networking/src/FileCache.cpp @@ -112,8 +112,10 @@ FilePointer FileCache::writeFile(const char* data, File::Metadata&& metadata) { } QSaveFile saveFile(QString::fromStdString(filepath)); - if (saveFile.open(QIODevice::WriteOnly) && saveFile.write(data, metadata.length) == metadata.length + if (saveFile.open(QIODevice::WriteOnly) + && saveFile.write(data, metadata.length) == static_cast(metadata.length) && saveFile.commit()) { + file = addFile(std::move(metadata), filepath); } else { qCWarning(file_cache, "[%s] Failed to write %s", _dirname.c_str(), metadata.key.c_str()); From 4536d3c484d498c95c8b603deee8494a928d52ba Mon Sep 17 00:00:00 2001 From: David Kelly Date: Wed, 10 May 2017 10:09:45 -0700 Subject: [PATCH 12/13] cr feedback --- scripts/system/makeUserConnection.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/system/makeUserConnection.js b/scripts/system/makeUserConnection.js index 0475c1a398..a8afad2e1c 100644 --- a/scripts/system/makeUserConnection.js +++ b/scripts/system/makeUserConnection.js @@ -613,7 +613,7 @@ // 3. We also don't want to connect to someone who is anonymous _now_, but was not earlier and still has // the same node id. Since the messaging doesn't say _who_ isn't logged in, fail the same as if we are // not logged in. - if (!Account.isLoggedIn() || !isLoggedIn) { + if (!Account.isLoggedIn() || isLoggedIn === false) { handleConnectionResponseAndMaybeRepeat("401:Unauthorized", {statusCode: 401}); return; } From 0cae299192f7662df27e0c1c8ca1ed9139ac2bb8 Mon Sep 17 00:00:00 2001 From: Dante Ruiz Date: Thu, 11 May 2017 00:20:21 +0100 Subject: [PATCH 13/13] fix issue when loading file to the input recorder --- .../src/controllers/InputRecorder.cpp | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/libraries/controllers/src/controllers/InputRecorder.cpp b/libraries/controllers/src/controllers/InputRecorder.cpp index 2d2cd40739..7433f181a1 100644 --- a/libraries/controllers/src/controllers/InputRecorder.cpp +++ b/libraries/controllers/src/controllers/InputRecorder.cpp @@ -22,20 +22,20 @@ #include #include - + QString SAVE_DIRECTORY = QStandardPaths::writableLocation(QStandardPaths::AppLocalDataLocation) + "/" + BuildInfo::MODIFIED_ORGANIZATION + "/" + BuildInfo::INTERFACE_NAME + "/hifi-input-recordings/"; QString FILE_PREFIX_NAME = "input-recording-"; QString COMPRESS_EXTENSION = ".tar.gz"; namespace controller { - + QJsonObject poseToJsonObject(const Pose pose) { QJsonObject newPose; - + QJsonArray translation; translation.append(pose.translation.x); translation.append(pose.translation.y); translation.append(pose.translation.z); - + QJsonArray rotation; rotation.append(pose.rotation.x); rotation.append(pose.rotation.y); @@ -69,7 +69,7 @@ namespace controller { QJsonArray angularVelocity = object["angularVelocity"].toArray(); pose.valid = object["valid"].toBool(); - + pose.translation.x = translation[0].toDouble(); pose.translation.y = translation[1].toDouble(); pose.translation.z = translation[2].toDouble(); @@ -89,13 +89,13 @@ namespace controller { return pose; } - + void exportToFile(QJsonObject& object) { if (!QDir(SAVE_DIRECTORY).exists()) { QDir().mkdir(SAVE_DIRECTORY); } - + QString timeStamp = QDateTime::currentDateTime().toString(Qt::ISODate); timeStamp.replace(":", "-"); QString fileName = SAVE_DIRECTORY + FILE_PREFIX_NAME + timeStamp + COMPRESS_EXTENSION; @@ -124,7 +124,7 @@ namespace controller { status = true; return object; } - + InputRecorder::InputRecorder() {} InputRecorder::~InputRecorder() {} @@ -195,16 +195,16 @@ namespace controller { _framesRecorded = data["frameCount"].toInt(); QJsonArray actionArrayList = data["actionList"].toArray(); QJsonArray poseArrayList = data["poseList"].toArray(); - + for (int actionIndex = 0; actionIndex < actionArrayList.size(); actionIndex++) { QJsonArray actionState = actionArrayList[actionIndex].toArray(); for (int index = 0; index < actionState.size(); index++) { - _currentFrameActions[index] = actionState[index].toInt(); + _currentFrameActions[index] = actionState[index].toDouble(); } _actionStateList.push_back(_currentFrameActions); _currentFrameActions = ActionStates(toInt(Action::NUM_ACTIONS)); } - + for (int poseIndex = 0; poseIndex < poseArrayList.size(); poseIndex++) { QJsonArray poseState = poseArrayList[poseIndex].toArray(); for (int index = 0; index < poseState.size(); index++) { @@ -250,13 +250,13 @@ namespace controller { for(auto& channel : _currentFramePoses) { channel = Pose(); } - + for(auto& channel : _currentFrameActions) { channel = 0.0f; } } } - + float InputRecorder::getActionState(controller::Action action) { if (_actionStateList.size() > 0 ) { return _actionStateList[_playCount][toInt(action)];