From 7f189e4d10f87f7f6ae4bdbc259683928356a41b Mon Sep 17 00:00:00 2001 From: sabrina-shanman Date: Mon, 24 Sep 2018 17:15:56 -0700 Subject: [PATCH 1/6] Make visualPickResult used by Pointers a private copy --- libraries/pointers/src/Pointer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/pointers/src/Pointer.cpp b/libraries/pointers/src/Pointer.cpp index 031baece5f..bdf1250a8d 100644 --- a/libraries/pointers/src/Pointer.cpp +++ b/libraries/pointers/src/Pointer.cpp @@ -68,7 +68,7 @@ void Pointer::update(unsigned int pointerID) { // This only needs to be a read lock because update won't change any of the properties that can be modified from scripts withReadLock([&] { auto pickResult = getPrevPickResult(); - auto visualPickResult = getVisualPickResult(pickResult); + auto visualPickResult = getVisualPickResult(std::make_shared(*pickResult.get())); updateVisuals(visualPickResult); generatePointerEvents(pointerID, visualPickResult); }); From 5dbebd4aae4652fa0109d3da9e11f93cf85151a1 Mon Sep 17 00:00:00 2001 From: sabrina-shanman Date: Mon, 24 Sep 2018 17:36:24 -0700 Subject: [PATCH 2/6] Add comment explaining why visualPickResult is a copy --- libraries/pointers/src/Pointer.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/pointers/src/Pointer.cpp b/libraries/pointers/src/Pointer.cpp index bdf1250a8d..852a83c192 100644 --- a/libraries/pointers/src/Pointer.cpp +++ b/libraries/pointers/src/Pointer.cpp @@ -68,6 +68,7 @@ void Pointer::update(unsigned int pointerID) { // This only needs to be a read lock because update won't change any of the properties that can be modified from scripts withReadLock([&] { auto pickResult = getPrevPickResult(); + // Pointer needs its own PickResult object so it doesn't modify the cached pick result auto visualPickResult = getVisualPickResult(std::make_shared(*pickResult.get())); updateVisuals(visualPickResult); generatePointerEvents(pointerID, visualPickResult); From eca31e7a994a5a0c7841e82eda987f99f972ba5c Mon Sep 17 00:00:00 2001 From: sabrina-shanman Date: Tue, 25 Sep 2018 09:20:57 -0700 Subject: [PATCH 3/6] Fix instantiating abstract class when creating visual pick result for pointers --- interface/src/raypick/LaserPointer.cpp | 5 +++++ interface/src/raypick/LaserPointer.h | 2 ++ interface/src/raypick/ParabolaPointer.cpp | 5 +++++ interface/src/raypick/ParabolaPointer.h | 2 ++ interface/src/raypick/StylusPointer.cpp | 5 +++++ interface/src/raypick/StylusPointer.h | 1 + libraries/pointers/src/Pointer.cpp | 2 +- libraries/pointers/src/Pointer.h | 1 + 8 files changed, 22 insertions(+), 1 deletion(-) diff --git a/interface/src/raypick/LaserPointer.cpp b/interface/src/raypick/LaserPointer.cpp index 5fbe3a90b5..64faf5f9bf 100644 --- a/interface/src/raypick/LaserPointer.cpp +++ b/interface/src/raypick/LaserPointer.cpp @@ -35,6 +35,11 @@ void LaserPointer::editRenderStatePath(const std::string& state, const QVariant& } } +PickResultPointer LaserPointer::getPickResultCopy(const PickResultPointer& pickResult) const { + auto rayPickResult = std::static_pointer_cast(pickResult); + return std::make_shared(*rayPickResult.get()); +} + QVariantMap LaserPointer::toVariantMap() const { QVariantMap qVariantMap; diff --git a/interface/src/raypick/LaserPointer.h b/interface/src/raypick/LaserPointer.h index c0ac3259d9..b391f60f85 100644 --- a/interface/src/raypick/LaserPointer.h +++ b/interface/src/raypick/LaserPointer.h @@ -47,6 +47,8 @@ public: static std::shared_ptr buildRenderState(const QVariantMap& propMap); protected: + PickResultPointer getPickResultCopy(const PickResultPointer& pickResult) const override; + void editRenderStatePath(const std::string& state, const QVariant& pathProps) override; glm::vec3 getPickOrigin(const PickResultPointer& pickResult) const override; diff --git a/interface/src/raypick/ParabolaPointer.cpp b/interface/src/raypick/ParabolaPointer.cpp index 888b3ddbe8..e7f54d2e97 100644 --- a/interface/src/raypick/ParabolaPointer.cpp +++ b/interface/src/raypick/ParabolaPointer.cpp @@ -30,6 +30,11 @@ ParabolaPointer::ParabolaPointer(const QVariant& rayProps, const RenderStateMap& { } +PickResultPointer ParabolaPointer::getPickResultCopy(const PickResultPointer& pickResult) const { + auto stylusPickResult = std::static_pointer_cast(pickResult); + return std::make_shared(*stylusPickResult.get()); +} + void ParabolaPointer::editRenderStatePath(const std::string& state, const QVariant& pathProps) { auto renderState = std::static_pointer_cast(_renderStates[state]); if (renderState) { diff --git a/interface/src/raypick/ParabolaPointer.h b/interface/src/raypick/ParabolaPointer.h index 526abe3b0d..8fb864c07b 100644 --- a/interface/src/raypick/ParabolaPointer.h +++ b/interface/src/raypick/ParabolaPointer.h @@ -102,6 +102,8 @@ public: static std::shared_ptr buildRenderState(const QVariantMap& propMap); protected: + virtual PickResultPointer getPickResultCopy(const PickResultPointer& pickResult) const override; + void editRenderStatePath(const std::string& state, const QVariant& pathProps) override; glm::vec3 getPickOrigin(const PickResultPointer& pickResult) const override; diff --git a/interface/src/raypick/StylusPointer.cpp b/interface/src/raypick/StylusPointer.cpp index 06e3e52d21..7f05a706a4 100644 --- a/interface/src/raypick/StylusPointer.cpp +++ b/interface/src/raypick/StylusPointer.cpp @@ -147,6 +147,11 @@ bool StylusPointer::shouldTrigger(const PickResultPointer& pickResult) { return false; } +PickResultPointer StylusPointer::getPickResultCopy(const PickResultPointer& pickResult) const { + auto stylusPickResult = std::static_pointer_cast(pickResult); + return std::make_shared(*stylusPickResult.get()); +} + Pointer::PickedObject StylusPointer::getHoveredObject(const PickResultPointer& pickResult) { auto stylusPickResult = std::static_pointer_cast(pickResult); if (!stylusPickResult) { diff --git a/interface/src/raypick/StylusPointer.h b/interface/src/raypick/StylusPointer.h index 4095acb529..ff60fd78e5 100644 --- a/interface/src/raypick/StylusPointer.h +++ b/interface/src/raypick/StylusPointer.h @@ -42,6 +42,7 @@ protected: Buttons getPressedButtons(const PickResultPointer& pickResult) override; bool shouldHover(const PickResultPointer& pickResult) override; bool shouldTrigger(const PickResultPointer& pickResult) override; + virtual PickResultPointer getPickResultCopy(const PickResultPointer& pickResult) const override; PointerEvent buildPointerEvent(const PickedObject& target, const PickResultPointer& pickResult, const std::string& button = "", bool hover = true) override; diff --git a/libraries/pointers/src/Pointer.cpp b/libraries/pointers/src/Pointer.cpp index 852a83c192..26460cbdd7 100644 --- a/libraries/pointers/src/Pointer.cpp +++ b/libraries/pointers/src/Pointer.cpp @@ -69,7 +69,7 @@ void Pointer::update(unsigned int pointerID) { withReadLock([&] { auto pickResult = getPrevPickResult(); // Pointer needs its own PickResult object so it doesn't modify the cached pick result - auto visualPickResult = getVisualPickResult(std::make_shared(*pickResult.get())); + auto visualPickResult = getVisualPickResult(getPickResultCopy(pickResult)); updateVisuals(visualPickResult); generatePointerEvents(pointerID, visualPickResult); }); diff --git a/libraries/pointers/src/Pointer.h b/libraries/pointers/src/Pointer.h index 4264a60079..173163374f 100644 --- a/libraries/pointers/src/Pointer.h +++ b/libraries/pointers/src/Pointer.h @@ -91,6 +91,7 @@ protected: virtual bool shouldHover(const PickResultPointer& pickResult) { return true; } virtual bool shouldTrigger(const PickResultPointer& pickResult) { return true; } + virtual PickResultPointer getPickResultCopy(const PickResultPointer& pickResult) const = 0; virtual PickResultPointer getVisualPickResult(const PickResultPointer& pickResult) { return pickResult; }; static const float POINTER_MOVE_DELAY; From 660bdf36d01b83996aacc93e9997eac41b70f463 Mon Sep 17 00:00:00 2001 From: sabrina-shanman Date: Tue, 25 Sep 2018 09:56:11 -0700 Subject: [PATCH 4/6] Check if pick result is null when building visual pick result --- interface/src/raypick/LaserPointer.cpp | 3 +++ interface/src/raypick/ParabolaPointer.cpp | 3 +++ interface/src/raypick/StylusPointer.cpp | 3 +++ 3 files changed, 9 insertions(+) diff --git a/interface/src/raypick/LaserPointer.cpp b/interface/src/raypick/LaserPointer.cpp index 64faf5f9bf..79bca0449f 100644 --- a/interface/src/raypick/LaserPointer.cpp +++ b/interface/src/raypick/LaserPointer.cpp @@ -36,6 +36,9 @@ void LaserPointer::editRenderStatePath(const std::string& state, const QVariant& } PickResultPointer LaserPointer::getPickResultCopy(const PickResultPointer& pickResult) const { + if (!pickResult) { + std::make_shared(); + } auto rayPickResult = std::static_pointer_cast(pickResult); return std::make_shared(*rayPickResult.get()); } diff --git a/interface/src/raypick/ParabolaPointer.cpp b/interface/src/raypick/ParabolaPointer.cpp index 4614b81cbb..ec4222d5f6 100644 --- a/interface/src/raypick/ParabolaPointer.cpp +++ b/interface/src/raypick/ParabolaPointer.cpp @@ -31,6 +31,9 @@ ParabolaPointer::ParabolaPointer(const QVariant& rayProps, const RenderStateMap& } PickResultPointer ParabolaPointer::getPickResultCopy(const PickResultPointer& pickResult) const { + if (!pickResult) { + std::make_shared(); + } auto stylusPickResult = std::static_pointer_cast(pickResult); return std::make_shared(*stylusPickResult.get()); } diff --git a/interface/src/raypick/StylusPointer.cpp b/interface/src/raypick/StylusPointer.cpp index 7f05a706a4..2742c68d1d 100644 --- a/interface/src/raypick/StylusPointer.cpp +++ b/interface/src/raypick/StylusPointer.cpp @@ -148,6 +148,9 @@ bool StylusPointer::shouldTrigger(const PickResultPointer& pickResult) { } PickResultPointer StylusPointer::getPickResultCopy(const PickResultPointer& pickResult) const { + if (!pickResult) { + std::make_shared(); + } auto stylusPickResult = std::static_pointer_cast(pickResult); return std::make_shared(*stylusPickResult.get()); } From 6e83e96d1cff5c000fe30f22e052ec4815749c60 Mon Sep 17 00:00:00 2001 From: sabrina-shanman Date: Tue, 25 Sep 2018 12:05:47 -0700 Subject: [PATCH 5/6] Check type in Pointer::getPickResultCopy --- interface/src/raypick/LaserPointer.cpp | 4 ++-- interface/src/raypick/ParabolaPointer.cpp | 6 +++--- interface/src/raypick/StylusPointer.cpp | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/interface/src/raypick/LaserPointer.cpp b/interface/src/raypick/LaserPointer.cpp index 79bca0449f..577978cc88 100644 --- a/interface/src/raypick/LaserPointer.cpp +++ b/interface/src/raypick/LaserPointer.cpp @@ -36,10 +36,10 @@ void LaserPointer::editRenderStatePath(const std::string& state, const QVariant& } PickResultPointer LaserPointer::getPickResultCopy(const PickResultPointer& pickResult) const { - if (!pickResult) { + auto rayPickResult = std::dynamic_pointer_cast(pickResult); + if (!rayPickResult) { std::make_shared(); } - auto rayPickResult = std::static_pointer_cast(pickResult); return std::make_shared(*rayPickResult.get()); } diff --git a/interface/src/raypick/ParabolaPointer.cpp b/interface/src/raypick/ParabolaPointer.cpp index ec4222d5f6..92b82fff7f 100644 --- a/interface/src/raypick/ParabolaPointer.cpp +++ b/interface/src/raypick/ParabolaPointer.cpp @@ -31,11 +31,11 @@ ParabolaPointer::ParabolaPointer(const QVariant& rayProps, const RenderStateMap& } PickResultPointer ParabolaPointer::getPickResultCopy(const PickResultPointer& pickResult) const { - if (!pickResult) { + auto parabolaPickResult = std::dynamic_pointer_cast(pickResult); + if (!parabolaPickResult) { std::make_shared(); } - auto stylusPickResult = std::static_pointer_cast(pickResult); - return std::make_shared(*stylusPickResult.get()); + return std::make_shared(*parabolaPickResult.get()); } void ParabolaPointer::editRenderStatePath(const std::string& state, const QVariant& pathProps) { diff --git a/interface/src/raypick/StylusPointer.cpp b/interface/src/raypick/StylusPointer.cpp index 2742c68d1d..0b44b2705d 100644 --- a/interface/src/raypick/StylusPointer.cpp +++ b/interface/src/raypick/StylusPointer.cpp @@ -148,10 +148,10 @@ bool StylusPointer::shouldTrigger(const PickResultPointer& pickResult) { } PickResultPointer StylusPointer::getPickResultCopy(const PickResultPointer& pickResult) const { - if (!pickResult) { + auto stylusPickResult = std::dynamic_pointer_cast(pickResult); + if (!stylusPickResult) { std::make_shared(); } - auto stylusPickResult = std::static_pointer_cast(pickResult); return std::make_shared(*stylusPickResult.get()); } From 55ab0a394b81706e7d5776cf253258e688d014a8 Mon Sep 17 00:00:00 2001 From: sabrina-shanman Date: Tue, 25 Sep 2018 12:21:00 -0700 Subject: [PATCH 6/6] Return std::make_shared --- interface/src/raypick/LaserPointer.cpp | 2 +- interface/src/raypick/ParabolaPointer.cpp | 2 +- interface/src/raypick/StylusPointer.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/interface/src/raypick/LaserPointer.cpp b/interface/src/raypick/LaserPointer.cpp index 577978cc88..3c66923b4e 100644 --- a/interface/src/raypick/LaserPointer.cpp +++ b/interface/src/raypick/LaserPointer.cpp @@ -38,7 +38,7 @@ void LaserPointer::editRenderStatePath(const std::string& state, const QVariant& PickResultPointer LaserPointer::getPickResultCopy(const PickResultPointer& pickResult) const { auto rayPickResult = std::dynamic_pointer_cast(pickResult); if (!rayPickResult) { - std::make_shared(); + return std::make_shared(); } return std::make_shared(*rayPickResult.get()); } diff --git a/interface/src/raypick/ParabolaPointer.cpp b/interface/src/raypick/ParabolaPointer.cpp index 92b82fff7f..33fa8738d9 100644 --- a/interface/src/raypick/ParabolaPointer.cpp +++ b/interface/src/raypick/ParabolaPointer.cpp @@ -33,7 +33,7 @@ ParabolaPointer::ParabolaPointer(const QVariant& rayProps, const RenderStateMap& PickResultPointer ParabolaPointer::getPickResultCopy(const PickResultPointer& pickResult) const { auto parabolaPickResult = std::dynamic_pointer_cast(pickResult); if (!parabolaPickResult) { - std::make_shared(); + return std::make_shared(); } return std::make_shared(*parabolaPickResult.get()); } diff --git a/interface/src/raypick/StylusPointer.cpp b/interface/src/raypick/StylusPointer.cpp index 0b44b2705d..b648e125bf 100644 --- a/interface/src/raypick/StylusPointer.cpp +++ b/interface/src/raypick/StylusPointer.cpp @@ -150,7 +150,7 @@ bool StylusPointer::shouldTrigger(const PickResultPointer& pickResult) { PickResultPointer StylusPointer::getPickResultCopy(const PickResultPointer& pickResult) const { auto stylusPickResult = std::dynamic_pointer_cast(pickResult); if (!stylusPickResult) { - std::make_shared(); + return std::make_shared(); } return std::make_shared(*stylusPickResult.get()); }