From 8fb3015ec8677b71f8315f62ec83e0501c18d6ff Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Mon, 5 Mar 2018 16:51:05 -0800 Subject: [PATCH 1/4] Fix for missing avatar on second replay Fixes a logic problem in the handshake request handling; deals with KillAvatar from the Agent by removing the node-linked data, not the actual Node; tweaks some of the network debug output. --- assignment-client/src/avatars/AvatarMixer.cpp | 4 +++- libraries/networking/src/udt/Connection.cpp | 3 ++- libraries/networking/src/udt/Socket.cpp | 4 ++++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 3ca924b007..dd7db5a21d 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -605,7 +605,9 @@ void AvatarMixer::handleAvatarIdentityPacket(QSharedPointer mes void AvatarMixer::handleKillAvatarPacket(QSharedPointer message, SharedNodePointer node) { auto start = usecTimestampNow(); - DependencyManager::get()->processKillNode(*message); + node->stopPingTimer(); + emit(DependencyManager::get()->nodeKilled(node)); + node->setLinkedData(nullptr); auto end = usecTimestampNow(); _handleKillAvatarPacketElapsedTime += (end - start); diff --git a/libraries/networking/src/udt/Connection.cpp b/libraries/networking/src/udt/Connection.cpp index 77ed589e0b..c1fe6ccd85 100644 --- a/libraries/networking/src/udt/Connection.cpp +++ b/libraries/networking/src/udt/Connection.cpp @@ -576,8 +576,9 @@ void Connection::processControl(ControlPacketPointer controlPacket) { // where the other end expired our connection. Let's reset. #ifdef UDT_CONNECTION_DEBUG - qCDebug(networking) << "Got handshake request, stopping SendQueue"; + qCDebug(networking) << "Got HandshakeRequest from" << _destination << ", stopping SendQueue"; #endif + _hasReceivedHandshakeACK = false; stopSendQueue(); } break; diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index 8b93a05130..5324946f82 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -402,6 +402,10 @@ void Socket::readPendingDatagrams() { packet->getDataSize(), packet->getPayloadSize())) { // the connection could not be created or indicated that we should not continue processing this packet +#ifdef UDT_CONNECTION_DEBUG + qCDebug(networking) << "Bad packet version" << (unsigned int)NLPacket::versionInHeader(*packet) + << ", type" << NLPacket::typeInHeader(*packet); +#endif continue; } } From 547d86a2d52699d27b0cb553e0e47ecd02a65f2d Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Mon, 5 Mar 2018 18:10:15 -0800 Subject: [PATCH 2/4] Improve debugging diagnostic --- libraries/networking/src/udt/Socket.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/udt/Socket.cpp b/libraries/networking/src/udt/Socket.cpp index 5324946f82..4189cb613c 100644 --- a/libraries/networking/src/udt/Socket.cpp +++ b/libraries/networking/src/udt/Socket.cpp @@ -403,7 +403,7 @@ void Socket::readPendingDatagrams() { packet->getPayloadSize())) { // the connection could not be created or indicated that we should not continue processing this packet #ifdef UDT_CONNECTION_DEBUG - qCDebug(networking) << "Bad packet version" << (unsigned int)NLPacket::versionInHeader(*packet) + qCDebug(networking) << "Can't process packet: version" << (unsigned int)NLPacket::versionInHeader(*packet) << ", type" << NLPacket::typeInHeader(*packet); #endif continue; From 30206ff33e2a4275cc0f4e82029f52c4d341929b Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 6 Mar 2018 12:28:55 -0800 Subject: [PATCH 3/4] Call killed-avatar slot directly rather than by signal Rename nodeKilled slot to handleKilledAvatar since the node isn't killed in this case and call directly from handleKillAvatarPacket(). --- assignment-client/src/avatars/AvatarMixer.cpp | 27 ++++++++++--------- assignment-client/src/avatars/AvatarMixer.h | 2 +- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index dd7db5a21d..929941c05c 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -42,7 +42,7 @@ AvatarMixer::AvatarMixer(ReceivedMessage& message) : ThreadedAssignment(message) { // make sure we hear about node kills so we can tell the other nodes - connect(DependencyManager::get().data(), &NodeList::nodeKilled, this, &AvatarMixer::nodeKilled); + connect(DependencyManager::get().data(), &NodeList::nodeKilled, this, &AvatarMixer::handleAvatarKilled); auto& packetReceiver = DependencyManager::get()->getPacketReceiver(); packetReceiver.registerListener(PacketType::AvatarData, this, "queueIncomingPacket"); @@ -423,14 +423,15 @@ void AvatarMixer::throttle(std::chrono::microseconds duration, int frame) { } } -void AvatarMixer::nodeKilled(SharedNodePointer killedNode) { - if (killedNode->getType() == NodeType::Agent - && killedNode->getLinkedData()) { + +void AvatarMixer::handleAvatarKilled(SharedNodePointer avatarNode) { + if (avatarNode->getType() == NodeType::Agent + && avatarNode->getLinkedData()) { auto nodeList = DependencyManager::get(); { // decrement sessionDisplayNames table and possibly remove - QMutexLocker nodeDataLocker(&killedNode->getLinkedData()->getMutex()); - AvatarMixerClientData* nodeData = dynamic_cast(killedNode->getLinkedData()); + QMutexLocker nodeDataLocker(&avatarNode->getLinkedData()->getMutex()); + AvatarMixerClientData* nodeData = dynamic_cast(avatarNode->getLinkedData()); const QString& baseDisplayName = nodeData->getBaseDisplayName(); // No sense guarding against very rare case of a node with no entry, as this will work without the guard and do one less lookup in the common case. if (--_sessionDisplayNames[baseDisplayName].second <= 0) { @@ -447,12 +448,12 @@ void AvatarMixer::nodeKilled(SharedNodePointer killedNode) { // we relay avatar kill packets to agents that are not upstream // and downstream avatar mixers, if the node that was just killed was being replicated return (node->getType() == NodeType::Agent && !node->isUpstream()) || - (killedNode->isReplicated() && shouldReplicateTo(*killedNode, *node)); + (avatarNode->isReplicated() && shouldReplicateTo(*avatarNode, *node)); }, [&](const SharedNodePointer& node) { if (node->getType() == NodeType::Agent) { if (!killPacket) { killPacket = NLPacket::create(PacketType::KillAvatar, NUM_BYTES_RFC4122_UUID + sizeof(KillAvatarReason)); - killPacket->write(killedNode->getUUID().toRfc4122()); + killPacket->write(avatarNode->getUUID().toRfc4122()); killPacket->writePrimitive(KillAvatarReason::AvatarDisconnected); } @@ -462,7 +463,7 @@ void AvatarMixer::nodeKilled(SharedNodePointer killedNode) { if (!replicatedKillPacket) { replicatedKillPacket = NLPacket::create(PacketType::ReplicatedKillAvatar, NUM_BYTES_RFC4122_UUID + sizeof(KillAvatarReason)); - replicatedKillPacket->write(killedNode->getUUID().toRfc4122()); + replicatedKillPacket->write(avatarNode->getUUID().toRfc4122()); replicatedKillPacket->writePrimitive(KillAvatarReason::AvatarDisconnected); } @@ -479,7 +480,7 @@ void AvatarMixer::nodeKilled(SharedNodePointer killedNode) { return false; } - if (node->getUUID() == killedNode->getUUID()) { + if (node->getUUID() == avatarNode->getUUID()) { return false; } @@ -489,7 +490,7 @@ void AvatarMixer::nodeKilled(SharedNodePointer killedNode) { QMetaObject::invokeMethod(node->getLinkedData(), "cleanupKilledNode", Qt::AutoConnection, - Q_ARG(const QUuid&, QUuid(killedNode->getUUID()))); + Q_ARG(const QUuid&, QUuid(avatarNode->getUUID()))); } ); } @@ -605,8 +606,8 @@ void AvatarMixer::handleAvatarIdentityPacket(QSharedPointer mes void AvatarMixer::handleKillAvatarPacket(QSharedPointer message, SharedNodePointer node) { auto start = usecTimestampNow(); - node->stopPingTimer(); - emit(DependencyManager::get()->nodeKilled(node)); + handleAvatarKilled(node); + node->setLinkedData(nullptr); auto end = usecTimestampNow(); _handleKillAvatarPacketElapsedTime += (end - start); diff --git a/assignment-client/src/avatars/AvatarMixer.h b/assignment-client/src/avatars/AvatarMixer.h index cb5f536faa..1fbfd7338b 100644 --- a/assignment-client/src/avatars/AvatarMixer.h +++ b/assignment-client/src/avatars/AvatarMixer.h @@ -39,7 +39,7 @@ public slots: /// runs the avatar mixer void run() override; - void nodeKilled(SharedNodePointer killedNode); + void handleAvatarKilled(SharedNodePointer killedNode); void sendStatsPacket() override; From 4adc357534a61e93d49d0326796f4a7d16eaffc4 Mon Sep 17 00:00:00 2001 From: SamGondelman Date: Tue, 13 Mar 2018 11:57:14 -0700 Subject: [PATCH 4/4] apply taa to text and web entities/overlays --- interface/src/ui/overlays/Text3DOverlay.cpp | 2 +- .../entities-renderer/src/RenderableTextEntityItem.cpp | 2 +- libraries/render-utils/src/GeometryCache.cpp | 8 ++++---- libraries/render-utils/src/GeometryCache.h | 4 ++-- libraries/render-utils/src/text/Font.cpp | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/interface/src/ui/overlays/Text3DOverlay.cpp b/interface/src/ui/overlays/Text3DOverlay.cpp index bed20be698..9bb1da59d2 100644 --- a/interface/src/ui/overlays/Text3DOverlay.cpp +++ b/interface/src/ui/overlays/Text3DOverlay.cpp @@ -114,7 +114,7 @@ void Text3DOverlay::render(RenderArgs* args) { glm::vec3 topLeft(-halfDimensions.x, -halfDimensions.y, SLIGHTLY_BEHIND); glm::vec3 bottomRight(halfDimensions.x, halfDimensions.y, SLIGHTLY_BEHIND); - DependencyManager::get()->bindSimpleProgram(batch, false, quadColor.a < 1.0f, false, false, false, false); + DependencyManager::get()->bindSimpleProgram(batch, false, quadColor.a < 1.0f, false, false, false); DependencyManager::get()->renderQuad(batch, topLeft, bottomRight, quadColor, _geometryId); // Same font properties as textSize() diff --git a/libraries/entities-renderer/src/RenderableTextEntityItem.cpp b/libraries/entities-renderer/src/RenderableTextEntityItem.cpp index 968c181940..e58eb540e8 100644 --- a/libraries/entities-renderer/src/RenderableTextEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableTextEntityItem.cpp @@ -111,7 +111,7 @@ void TextEntityRenderer::doRender(RenderArgs* args) { if (!_geometryID) { _geometryID = geometryCache->allocateID(); } - geometryCache->bindSimpleProgram(batch, false, transparent, false, false, false, false); + geometryCache->bindSimpleProgram(batch, false, transparent, false, false, false); geometryCache->renderQuad(batch, minCorner, maxCorner, backgroundColor, _geometryID); float scale = _lineHeight / _textRenderer->getFontSize(); diff --git a/libraries/render-utils/src/GeometryCache.cpp b/libraries/render-utils/src/GeometryCache.cpp index 212cf5eae1..bf265fae8c 100644 --- a/libraries/render-utils/src/GeometryCache.cpp +++ b/libraries/render-utils/src/GeometryCache.cpp @@ -2110,7 +2110,7 @@ static void buildWebShader(const gpu::ShaderPointer& vertShader, const gpu::Shad gpu::State::SRC_ALPHA, gpu::State::BLEND_OP_ADD, gpu::State::INV_SRC_ALPHA, gpu::State::FACTOR_ALPHA, gpu::State::BLEND_OP_ADD, gpu::State::ONE); - PrepareStencil::testMaskDrawShapeNoAA(*state); + PrepareStencil::testMaskDrawShape(*state); pipelinePointerOut = gpu::Pipeline::create(shaderPointerOut, state); } @@ -2122,11 +2122,11 @@ void GeometryCache::bindWebBrowserProgram(gpu::Batch& batch, bool transparent) { gpu::PipelinePointer GeometryCache::getWebBrowserProgram(bool transparent) { static std::once_flag once; std::call_once(once, [&]() { - buildWebShader(simple_vert::getShader(), simple_opaque_web_browser_frag::getShader(), false, _simpleOpaqueWebBrowserShader, _simpleOpaqueWebBrowserPipelineNoAA); - buildWebShader(simple_vert::getShader(), simple_transparent_web_browser_frag::getShader(), true, _simpleTransparentWebBrowserShader, _simpleTransparentWebBrowserPipelineNoAA); + buildWebShader(simple_vert::getShader(), simple_opaque_web_browser_frag::getShader(), false, _simpleOpaqueWebBrowserShader, _simpleOpaqueWebBrowserPipeline); + buildWebShader(simple_vert::getShader(), simple_transparent_web_browser_frag::getShader(), true, _simpleTransparentWebBrowserShader, _simpleTransparentWebBrowserPipeline); }); - return transparent ? _simpleTransparentWebBrowserPipelineNoAA : _simpleOpaqueWebBrowserPipelineNoAA; + return transparent ? _simpleTransparentWebBrowserPipeline : _simpleOpaqueWebBrowserPipeline; } void GeometryCache::bindSimpleProgram(gpu::Batch& batch, bool textured, bool transparent, bool culled, bool unlit, bool depthBiased, bool isAntiAliased) { diff --git a/libraries/render-utils/src/GeometryCache.h b/libraries/render-utils/src/GeometryCache.h index 998043b80e..3c6b85bed1 100644 --- a/libraries/render-utils/src/GeometryCache.h +++ b/libraries/render-utils/src/GeometryCache.h @@ -475,9 +475,9 @@ private: static QHash _simplePrograms; gpu::ShaderPointer _simpleOpaqueWebBrowserShader; - gpu::PipelinePointer _simpleOpaqueWebBrowserPipelineNoAA; + gpu::PipelinePointer _simpleOpaqueWebBrowserPipeline; gpu::ShaderPointer _simpleTransparentWebBrowserShader; - gpu::PipelinePointer _simpleTransparentWebBrowserPipelineNoAA; + gpu::PipelinePointer _simpleTransparentWebBrowserPipeline; static render::ShapePipelinePointer getShapePipeline(bool textured = false, bool transparent = false, bool culled = true, bool unlit = false, bool depthBias = false); diff --git a/libraries/render-utils/src/text/Font.cpp b/libraries/render-utils/src/text/Font.cpp index d3d25431d0..cd171db855 100644 --- a/libraries/render-utils/src/text/Font.cpp +++ b/libraries/render-utils/src/text/Font.cpp @@ -243,7 +243,7 @@ void Font::setupGPU() { state->setBlendFunction(false, gpu::State::SRC_ALPHA, gpu::State::BLEND_OP_ADD, gpu::State::INV_SRC_ALPHA, gpu::State::FACTOR_ALPHA, gpu::State::BLEND_OP_ADD, gpu::State::ONE); - PrepareStencil::testMaskDrawShapeNoAA(*state); + PrepareStencil::testMaskDrawShape(*state); _pipeline = gpu::Pipeline::create(program, state); auto transparentState = std::make_shared(); @@ -252,7 +252,7 @@ void Font::setupGPU() { transparentState->setBlendFunction(true, gpu::State::SRC_ALPHA, gpu::State::BLEND_OP_ADD, gpu::State::INV_SRC_ALPHA, gpu::State::FACTOR_ALPHA, gpu::State::BLEND_OP_ADD, gpu::State::ONE); - PrepareStencil::testMaskDrawShapeNoAA(*transparentState); + PrepareStencil::testMaskDrawShape(*transparentState); _transparentPipeline = gpu::Pipeline::create(programTransparent, transparentState); }