From d03fcb5ed8a50ac4ba1bf813f450945445bea62c Mon Sep 17 00:00:00 2001 From: David Back Date: Tue, 10 Apr 2018 19:06:27 -0700 Subject: [PATCH 01/47] desktop equip --- .../controllerModules/equipEntity.js | 117 +++++++++++++++++- scripts/system/controllers/grab.js | 2 +- 2 files changed, 115 insertions(+), 4 deletions(-) diff --git a/scripts/system/controllers/controllerModules/equipEntity.js b/scripts/system/controllers/controllerModules/equipEntity.js index 252f6efa9e..33091696f3 100644 --- a/scripts/system/controllers/controllerModules/equipEntity.js +++ b/scripts/system/controllers/controllerModules/equipEntity.js @@ -21,6 +21,7 @@ Script.include("/~/system/libraries/cloneEntityUtils.js"); var DEFAULT_SPHERE_MODEL_URL = "http://hifi-content.s3.amazonaws.com/alan/dev/equip-Fresnel-3.fbx"; var EQUIP_SPHERE_SCALE_FACTOR = 0.65; +var EMPTY_PARENT_ID = "{00000000-0000-0000-0000-000000000000}"; // Each overlayInfoSet describes a single equip hotspot. @@ -176,6 +177,8 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa var TRIGGER_OFF_VALUE = 0.1; var TRIGGER_ON_VALUE = TRIGGER_OFF_VALUE + 0.05; // Squeezed just enough to activate search or near grab var BUMPER_ON_VALUE = 0.5; + + var UNEQUIP_KEY = "u"; function getWearableData(props) { @@ -270,6 +273,8 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa this.shouldSendStart = false; this.equipedWithSecondary = false; this.handHasBeenRightsideUp = false; + this.mouseEquip = false; + this.mouseEquipAnimationHandler; this.parameters = makeDispatcherModuleParameters( 300, @@ -279,10 +284,11 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa var equipHotspotBuddy = new EquipHotspotBuddy(); - this.setMessageGrabData = function(entityProperties) { + this.setMessageGrabData = function(entityProperties, mouseEquip) { if (entityProperties) { this.messageGrabEntity = true; this.grabEntityProps = entityProperties; + this.mouseEquip = mouseEquip; } }; @@ -552,6 +558,15 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa // 100 ms seems to be sufficient time to force the check even occur after the object has been initialized. Script.setTimeout(grabEquipCheck, 100); } + + if (this.mouseEquip) { + this.removeMouseEquipAnimation(); + if (this.hand === RIGHT_HAND) { + this.mouseEquipAnimationHandler = MyAvatar.addAnimationStateHandler(this.rightHandMouseEquipAnimation, []); + } else { + this.mouseEquipAnimationHandler = MyAvatar.addAnimationStateHandler(this.leftHandMouseEquipAnimation, []); + } + } }; this.endEquipEntity = function () { @@ -574,6 +589,11 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa this.targetEntityID = null; this.messageGrabEntity = false; this.grabEntityProps = null; + + if (this.mouseEquip) { + this.removeMouseEquipAnimation(); + this.mouseEquip = false; + } }; this.updateInputs = function (controllerData) { @@ -650,7 +670,7 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa var timestamp = Date.now(); this.updateInputs(controllerData); - if (!this.isTargetIDValid(controllerData)) { + if (!this.mouseEquip && !this.isTargetIDValid(controllerData)) { this.endEquipEntity(); return makeRunningValues(false, [], []); } @@ -740,6 +760,40 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa this.endEquipEntity(); } }; + + this.removeMouseEquipAnimation = function() { + if (this.mouseEquipAnimationHandler) { + this.mouseEquipAnimationHandler = MyAvatar.removeAnimationStateHandler(this.mouseEquipAnimationHandler); + } + }; + + this.leftHandMouseEquipAnimation = function() { + var result = {}; + var leftHandPosition = MyAvatar.getJointPosition("LeftHand"); + var leftShoulderPosition = MyAvatar.getJointPosition("LeftShoulder"); + var cameraToLeftShoulder = Vec3.subtract(leftShoulderPosition, Camera.position); + var cameraToLeftShoulderNormalized = Vec3.normalize(cameraToLeftShoulder); + var leftHandPositionNew = Vec3.sum(leftShoulderPosition, cameraToLeftShoulderNormalized); + var leftHandPositionNewAvatarFrame = Vec3.subtract(leftHandPositionNew, MyAvatar.position); + result.leftHandType = 1; + result.leftHandPosition = leftHandPositionNewAvatarFrame; + result.leftHandRotation = Quat.multiply(Quat.lookAtSimple(leftHandPositionNew, Camera.position), Quat.fromPitchYawRollDegrees(90, 0, -90)); + return result; + }; + + this.rightHandMouseEquipAnimation = function() { + var result = {}; + var rightHandPosition = MyAvatar.getJointPosition("RightHand"); + var rightShoulderPosition = MyAvatar.getJointPosition("RightShoulder"); + var cameraToRightShoulder = Vec3.subtract(rightShoulderPosition, Camera.position); + var cameraToRightShoulderNormalized = Vec3.normalize(cameraToRightShoulder); + var rightHandPositionNew = Vec3.sum(rightShoulderPosition, cameraToRightShoulderNormalized); + var rightHandPositionNewAvatarFrame = Vec3.subtract(rightHandPositionNew, MyAvatar.position); + result.rightHandType = 1; + result.rightHandPosition = rightHandPositionNewAvatarFrame; + result.rightHandRotation = Quat.multiply(Quat.lookAtSimple(rightHandPositionNew, Camera.position), Quat.fromPitchYawRollDegrees(90, 0, 90)); + return result; + }; } var handleMessage = function(channel, message, sender) { @@ -751,7 +805,8 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa var equipModule = (data.hand === "left") ? leftEquipEntity : rightEquipEntity; var entityProperties = Entities.getEntityProperties(data.entityID, DISPATCHER_PROPERTIES); entityProperties.id = data.entityID; - equipModule.setMessageGrabData(entityProperties); + var mouseEquip = false; + equipModule.setMessageGrabData(entityProperties, mouseEquip); } catch (e) { print("WARNING: equipEntity.js -- error parsing Hifi-Hand-Grab message: " + message); @@ -768,10 +823,63 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa } } }; + + var clearGrabActions = function(entityID) { + var actionIDs = Entities.getActionIDs(entityID); + for (var actionIndex = 0; actionIndex < actionIDs.length; actionIndex++) { + var actionID = actionIDs[actionIndex]; + var actionArguments = Entities.getActionArguments(entityID, actionID); + var tag = actionArguments.tag; + if (tag.slice(0, 5) == "grab-") { + Entities.deleteAction(entityID, actionID); + } + } + }; + + var onMousePress = function(event) { + if (isInEditMode()) { // ignore any mouse clicks on the entity while create/edit is open + return; + } + var pickRay = Camera.computePickRay(event.x, event.y); + var intersection = Entities.findRayIntersection(pickRay, true); + if (intersection.intersects) { + var entityProperties = Entities.getEntityProperties(intersection.entityID, DISPATCHER_PROPERTIES); + if (entityProperties.parentID === EMPTY_PARENT_ID) { + entityProperties.id = intersection.entityID; + var rightHandPosition = MyAvatar.getJointPosition("RightHand"); + var leftHandPosition = MyAvatar.getJointPosition("LeftHand"); + var distanceToRightHand = Vec3.distance(entityProperties.position, rightHandPosition); + var distanceToLeftHand = Vec3.distance(entityProperties.position, leftHandPosition); + var leftHandAvailable = leftEquipEntity.targetEntityID === null; + var rightHandAvailable = rightEquipEntity.targetEntityID === null; + var mouseEquip = true; + if (rightHandAvailable && (distanceToRightHand < distanceToLeftHand || !leftHandAvailable)) { + clearGrabActions(intersection.entityID); + rightEquipEntity.setMessageGrabData(entityProperties, mouseEquip); + } else if (leftHandAvailable && (distanceToLeftHand < distanceToRightHand || !rightHandAvailable)) { + clearGrabActions(intersection.entityID); + leftEquipEntity.setMessageGrabData(entityProperties, mouseEquip); + } + } + } + }; + + var onKeyPress = function(event) { + if (event.text === UNEQUIP_KEY) { + if (rightEquipEntity.mouseEquip) { + rightEquipEntity.endEquipEntity(); + } + if (leftEquipEntity.mouseEquip) { + leftEquipEntity.endEquipEntity(); + } + } + }; Messages.subscribe('Hifi-Hand-Grab'); Messages.subscribe('Hifi-Hand-Drop'); Messages.messageReceived.connect(handleMessage); + Controller.mousePressEvent.connect(onMousePress); + Controller.keyPressEvent.connect(onKeyPress); var leftEquipEntity = new EquipEntity(LEFT_HAND); var rightEquipEntity = new EquipEntity(RIGHT_HAND); @@ -785,6 +893,9 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa disableDispatcherModule("LeftEquipEntity"); disableDispatcherModule("RightEquipEntity"); clearAttachPoints(); + Messages.messageReceived.disconnect(handleMessage); + Controller.mousePressEvent.disconnect(onMousePress); + Controller.keyPressEvent.disconnect(onKeyPress); } Script.scriptEnding.connect(cleanup); }()); diff --git a/scripts/system/controllers/grab.js b/scripts/system/controllers/grab.js index 1171703847..b32c64d189 100644 --- a/scripts/system/controllers/grab.js +++ b/scripts/system/controllers/grab.js @@ -567,7 +567,7 @@ Grabber.prototype.moveEventProcess = function() { } if (!this.actionID) { - if (!entityIsGrabbedByOther(this.entityID)) { + if (!entityIsGrabbedByOther(this.entityID) ) && Entities.getEntityProperties(this.entityID, ['parentID']).parentID !== MyAvatar.SELF_ID) { this.actionID = Entities.addAction("far-grab", this.entityID, actionArgs); } } else { From de4aff630ade00fe38d379d34c7575740b7e2a3a Mon Sep 17 00:00:00 2001 From: David Back Date: Thu, 12 Apr 2018 12:41:36 -0700 Subject: [PATCH 02/47] fix grab.js change --- scripts/system/controllers/grab.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/system/controllers/grab.js b/scripts/system/controllers/grab.js index b32c64d189..4af2d97a8f 100644 --- a/scripts/system/controllers/grab.js +++ b/scripts/system/controllers/grab.js @@ -567,7 +567,7 @@ Grabber.prototype.moveEventProcess = function() { } if (!this.actionID) { - if (!entityIsGrabbedByOther(this.entityID) ) && Entities.getEntityProperties(this.entityID, ['parentID']).parentID !== MyAvatar.SELF_ID) { + if (!entityIsGrabbedByOther(this.entityID) && Entities.getEntityProperties(this.entityID, ['parentID']).parentID !== MyAvatar.SELF_ID) { this.actionID = Entities.addAction("far-grab", this.entityID, actionArgs); } } else { From 4446ef853f8c396171a4d51a4cec4b23fd8d880c Mon Sep 17 00:00:00 2001 From: David Back Date: Mon, 16 Apr 2018 12:56:22 -0700 Subject: [PATCH 03/47] remove animations --- .../controllerModules/equipEntity.js | 60 ++----------------- 1 file changed, 5 insertions(+), 55 deletions(-) diff --git a/scripts/system/controllers/controllerModules/equipEntity.js b/scripts/system/controllers/controllerModules/equipEntity.js index 33091696f3..7b9640047c 100644 --- a/scripts/system/controllers/controllerModules/equipEntity.js +++ b/scripts/system/controllers/controllerModules/equipEntity.js @@ -22,6 +22,7 @@ Script.include("/~/system/libraries/cloneEntityUtils.js"); var DEFAULT_SPHERE_MODEL_URL = "http://hifi-content.s3.amazonaws.com/alan/dev/equip-Fresnel-3.fbx"; var EQUIP_SPHERE_SCALE_FACTOR = 0.65; var EMPTY_PARENT_ID = "{00000000-0000-0000-0000-000000000000}"; +var UNEQUIP_KEY = "u"; // Each overlayInfoSet describes a single equip hotspot. @@ -176,10 +177,8 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa var TRIGGER_SMOOTH_RATIO = 0.1; // Time averaging of trigger - 0.0 disables smoothing var TRIGGER_OFF_VALUE = 0.1; var TRIGGER_ON_VALUE = TRIGGER_OFF_VALUE + 0.05; // Squeezed just enough to activate search or near grab - var BUMPER_ON_VALUE = 0.5; - - var UNEQUIP_KEY = "u"; - + var BUMPER_ON_VALUE = 0.5 + function getWearableData(props) { var wearable = {}; @@ -274,7 +273,6 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa this.equipedWithSecondary = false; this.handHasBeenRightsideUp = false; this.mouseEquip = false; - this.mouseEquipAnimationHandler; this.parameters = makeDispatcherModuleParameters( 300, @@ -558,15 +556,6 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa // 100 ms seems to be sufficient time to force the check even occur after the object has been initialized. Script.setTimeout(grabEquipCheck, 100); } - - if (this.mouseEquip) { - this.removeMouseEquipAnimation(); - if (this.hand === RIGHT_HAND) { - this.mouseEquipAnimationHandler = MyAvatar.addAnimationStateHandler(this.rightHandMouseEquipAnimation, []); - } else { - this.mouseEquipAnimationHandler = MyAvatar.addAnimationStateHandler(this.leftHandMouseEquipAnimation, []); - } - } }; this.endEquipEntity = function () { @@ -575,7 +564,7 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa parentID: Uuid.NULL, parentJointIndex: -1 }); - +; var args = [this.hand === RIGHT_HAND ? "right" : "left", MyAvatar.sessionUUID]; Entities.callEntityMethod(this.targetEntityID, "releaseEquip", args); @@ -589,11 +578,6 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa this.targetEntityID = null; this.messageGrabEntity = false; this.grabEntityProps = null; - - if (this.mouseEquip) { - this.removeMouseEquipAnimation(); - this.mouseEquip = false; - } }; this.updateInputs = function (controllerData) { @@ -760,40 +744,6 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa this.endEquipEntity(); } }; - - this.removeMouseEquipAnimation = function() { - if (this.mouseEquipAnimationHandler) { - this.mouseEquipAnimationHandler = MyAvatar.removeAnimationStateHandler(this.mouseEquipAnimationHandler); - } - }; - - this.leftHandMouseEquipAnimation = function() { - var result = {}; - var leftHandPosition = MyAvatar.getJointPosition("LeftHand"); - var leftShoulderPosition = MyAvatar.getJointPosition("LeftShoulder"); - var cameraToLeftShoulder = Vec3.subtract(leftShoulderPosition, Camera.position); - var cameraToLeftShoulderNormalized = Vec3.normalize(cameraToLeftShoulder); - var leftHandPositionNew = Vec3.sum(leftShoulderPosition, cameraToLeftShoulderNormalized); - var leftHandPositionNewAvatarFrame = Vec3.subtract(leftHandPositionNew, MyAvatar.position); - result.leftHandType = 1; - result.leftHandPosition = leftHandPositionNewAvatarFrame; - result.leftHandRotation = Quat.multiply(Quat.lookAtSimple(leftHandPositionNew, Camera.position), Quat.fromPitchYawRollDegrees(90, 0, -90)); - return result; - }; - - this.rightHandMouseEquipAnimation = function() { - var result = {}; - var rightHandPosition = MyAvatar.getJointPosition("RightHand"); - var rightShoulderPosition = MyAvatar.getJointPosition("RightShoulder"); - var cameraToRightShoulder = Vec3.subtract(rightShoulderPosition, Camera.position); - var cameraToRightShoulderNormalized = Vec3.normalize(cameraToRightShoulder); - var rightHandPositionNew = Vec3.sum(rightShoulderPosition, cameraToRightShoulderNormalized); - var rightHandPositionNewAvatarFrame = Vec3.subtract(rightHandPositionNew, MyAvatar.position); - result.rightHandType = 1; - result.rightHandPosition = rightHandPositionNewAvatarFrame; - result.rightHandRotation = Quat.multiply(Quat.lookAtSimple(rightHandPositionNew, Camera.position), Quat.fromPitchYawRollDegrees(90, 0, 90)); - return result; - }; } var handleMessage = function(channel, message, sender) { @@ -874,7 +824,7 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa } } }; - + Messages.subscribe('Hifi-Hand-Grab'); Messages.subscribe('Hifi-Hand-Drop'); Messages.messageReceived.connect(handleMessage); From 0f2679b725ec0ae28dbda35c2ec016688807b348 Mon Sep 17 00:00:00 2001 From: David Back Date: Mon, 16 Apr 2018 13:12:33 -0700 Subject: [PATCH 04/47] pre PR adjustments --- .../controllerModules/equipEntity.js | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/scripts/system/controllers/controllerModules/equipEntity.js b/scripts/system/controllers/controllerModules/equipEntity.js index 7b9640047c..ccd0c750df 100644 --- a/scripts/system/controllers/controllerModules/equipEntity.js +++ b/scripts/system/controllers/controllerModules/equipEntity.js @@ -21,8 +21,6 @@ Script.include("/~/system/libraries/cloneEntityUtils.js"); var DEFAULT_SPHERE_MODEL_URL = "http://hifi-content.s3.amazonaws.com/alan/dev/equip-Fresnel-3.fbx"; var EQUIP_SPHERE_SCALE_FACTOR = 0.65; -var EMPTY_PARENT_ID = "{00000000-0000-0000-0000-000000000000}"; -var UNEQUIP_KEY = "u"; // Each overlayInfoSet describes a single equip hotspot. @@ -177,9 +175,13 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa var TRIGGER_SMOOTH_RATIO = 0.1; // Time averaging of trigger - 0.0 disables smoothing var TRIGGER_OFF_VALUE = 0.1; var TRIGGER_ON_VALUE = TRIGGER_OFF_VALUE + 0.05; // Squeezed just enough to activate search or near grab - var BUMPER_ON_VALUE = 0.5 - + var BUMPER_ON_VALUE = 0.5; + + var EMPTY_PARENT_ID = "{00000000-0000-0000-0000-000000000000}"; + + var UNEQUIP_KEY = "u"; + function getWearableData(props) { var wearable = {}; try { @@ -564,7 +566,7 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa parentID: Uuid.NULL, parentJointIndex: -1 }); -; + var args = [this.hand === RIGHT_HAND ? "right" : "left", MyAvatar.sessionUUID]; Entities.callEntityMethod(this.targetEntityID, "releaseEquip", args); @@ -780,14 +782,14 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa var actionID = actionIDs[actionIndex]; var actionArguments = Entities.getActionArguments(entityID, actionID); var tag = actionArguments.tag; - if (tag.slice(0, 5) == "grab-") { + if (tag.slice(0, 5) === "grab-") { Entities.deleteAction(entityID, actionID); } } }; var onMousePress = function(event) { - if (isInEditMode()) { // ignore any mouse clicks on the entity while create/edit is open + if (isInEditMode()) { // don't consider any mouse clicks on the entity while in edit return; } var pickRay = Camera.computePickRay(event.x, event.y); @@ -804,9 +806,11 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa var rightHandAvailable = rightEquipEntity.targetEntityID === null; var mouseEquip = true; if (rightHandAvailable && (distanceToRightHand < distanceToLeftHand || !leftHandAvailable)) { + // clear any existing grab actions on the entity now (their later removal could affect bootstrapping flags) clearGrabActions(intersection.entityID); rightEquipEntity.setMessageGrabData(entityProperties, mouseEquip); - } else if (leftHandAvailable && (distanceToLeftHand < distanceToRightHand || !rightHandAvailable)) { + } else if (leftHandAvailable && (distanceToLeftHand < distanceToRightHand || !rightHandAvailable)) + // clear any existing grab actions on the entity now (their later removal could affect bootstrapping flags) clearGrabActions(intersection.entityID); leftEquipEntity.setMessageGrabData(entityProperties, mouseEquip); } @@ -824,7 +828,7 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa } } }; - + Messages.subscribe('Hifi-Hand-Grab'); Messages.subscribe('Hifi-Hand-Drop'); Messages.messageReceived.connect(handleMessage); From a869b58dfb3cfa525dba1ba1dd0112304f29bb28 Mon Sep 17 00:00:00 2001 From: David Back Date: Mon, 16 Apr 2018 13:33:17 -0700 Subject: [PATCH 05/47] fix bracket --- scripts/system/controllers/controllerModules/equipEntity.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/system/controllers/controllerModules/equipEntity.js b/scripts/system/controllers/controllerModules/equipEntity.js index ccd0c750df..00fc1e581a 100644 --- a/scripts/system/controllers/controllerModules/equipEntity.js +++ b/scripts/system/controllers/controllerModules/equipEntity.js @@ -181,7 +181,6 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa var UNEQUIP_KEY = "u"; - function getWearableData(props) { var wearable = {}; try { @@ -809,7 +808,7 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa // clear any existing grab actions on the entity now (their later removal could affect bootstrapping flags) clearGrabActions(intersection.entityID); rightEquipEntity.setMessageGrabData(entityProperties, mouseEquip); - } else if (leftHandAvailable && (distanceToLeftHand < distanceToRightHand || !rightHandAvailable)) + } else if (leftHandAvailable && (distanceToLeftHand < distanceToRightHand || !rightHandAvailable)) { // clear any existing grab actions on the entity now (their later removal could affect bootstrapping flags) clearGrabActions(intersection.entityID); leftEquipEntity.setMessageGrabData(entityProperties, mouseEquip); From 3a35fa1c08c7802e4af20466b1f4c54be2a239f1 Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 17 Apr 2018 16:13:10 -0700 Subject: [PATCH 06/47] missing reset flag on end equip, remove mouseEquip check on unequip via U --- .../system/controllers/controllerModules/equipEntity.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scripts/system/controllers/controllerModules/equipEntity.js b/scripts/system/controllers/controllerModules/equipEntity.js index 00fc1e581a..4e960c37f1 100644 --- a/scripts/system/controllers/controllerModules/equipEntity.js +++ b/scripts/system/controllers/controllerModules/equipEntity.js @@ -579,6 +579,7 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa this.targetEntityID = null; this.messageGrabEntity = false; this.grabEntityProps = null; + this.mouseEquip = false; }; this.updateInputs = function (controllerData) { @@ -796,7 +797,7 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa if (intersection.intersects) { var entityProperties = Entities.getEntityProperties(intersection.entityID, DISPATCHER_PROPERTIES); if (entityProperties.parentID === EMPTY_PARENT_ID) { - entityProperties.id = intersection.entityID; + entityProperties.id = intersection.entityID; var rightHandPosition = MyAvatar.getJointPosition("RightHand"); var leftHandPosition = MyAvatar.getJointPosition("LeftHand"); var distanceToRightHand = Vec3.distance(entityProperties.position, rightHandPosition); @@ -819,10 +820,10 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa var onKeyPress = function(event) { if (event.text === UNEQUIP_KEY) { - if (rightEquipEntity.mouseEquip) { + if (rightEquipEntity.targetEntityID) { rightEquipEntity.endEquipEntity(); } - if (leftEquipEntity.mouseEquip) { + if (leftEquipEntity.targetEntityID) { leftEquipEntity.endEquipEntity(); } } From 4a08a6c147a4ab29bde9af35edec8d151c7873e4 Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 17 Apr 2018 16:26:33 -0700 Subject: [PATCH 07/47] ensure target mouse entity has equip data --- scripts/system/controllers/controllerModules/equipEntity.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/system/controllers/controllerModules/equipEntity.js b/scripts/system/controllers/controllerModules/equipEntity.js index 4e960c37f1..6406b8593f 100644 --- a/scripts/system/controllers/controllerModules/equipEntity.js +++ b/scripts/system/controllers/controllerModules/equipEntity.js @@ -796,7 +796,8 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa var intersection = Entities.findRayIntersection(pickRay, true); if (intersection.intersects) { var entityProperties = Entities.getEntityProperties(intersection.entityID, DISPATCHER_PROPERTIES); - if (entityProperties.parentID === EMPTY_PARENT_ID) { + var hasEquipData = getWearableData(entityProperties).joints || getEquipHotspotsData(entityProperties).length > 0; + if (hasEquipData && entityProperties.parentID === EMPTY_PARENT_ID) { entityProperties.id = intersection.entityID; var rightHandPosition = MyAvatar.getJointPosition("RightHand"); var leftHandPosition = MyAvatar.getJointPosition("LeftHand"); From 07fb604090795e837f348eb471b38d85b65251eb Mon Sep 17 00:00:00 2001 From: David Back Date: Wed, 18 Apr 2018 15:00:10 -0700 Subject: [PATCH 08/47] add entityIsEquipped utils for grab.js check --- scripts/system/controllers/grab.js | 2 +- scripts/system/libraries/controllerDispatcherUtils.js | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/scripts/system/controllers/grab.js b/scripts/system/controllers/grab.js index 4af2d97a8f..12cc1a2f7c 100644 --- a/scripts/system/controllers/grab.js +++ b/scripts/system/controllers/grab.js @@ -567,7 +567,7 @@ Grabber.prototype.moveEventProcess = function() { } if (!this.actionID) { - if (!entityIsGrabbedByOther(this.entityID) && Entities.getEntityProperties(this.entityID, ['parentID']).parentID !== MyAvatar.SELF_ID) { + if (!entityIsGrabbedByOther(this.entityID) && !entityIsEquipped(this.entityID)) { this.actionID = Entities.addAction("far-grab", this.entityID, actionArgs); } } else { diff --git a/scripts/system/libraries/controllerDispatcherUtils.js b/scripts/system/libraries/controllerDispatcherUtils.js index 75e1d6668b..41b4458bc5 100644 --- a/scripts/system/libraries/controllerDispatcherUtils.js +++ b/scripts/system/libraries/controllerDispatcherUtils.js @@ -384,6 +384,14 @@ distanceBetweenPointAndEntityBoundingBox = function(point, entityProps) { return Vec3.distance(v, localPoint); }; +entityIsEquipped = function(entityID) { + var rightEquipEntity = getEnabledModuleByName("RightEquipEntity"); + var leftEquipEntity = getEnabledModuleByName("LeftEquipEntity"); + var equippedInRightHand = rightEquipEntity ? rightEquipEntity.targetEntityID === entityID : false; + var equippedInLeftHand = leftEquipEntity ? leftEquipEntity.targetEntityID === entityID : false; + return equippedInRightHand || equippedInLeftHand; +}; + if (typeof module !== 'undefined') { module.exports = { makeDispatcherModuleParameters: makeDispatcherModuleParameters, From 5bf48041df0c1dc8d0eecbb98b30bf1460346e3e Mon Sep 17 00:00:00 2001 From: David Back Date: Wed, 18 Apr 2018 15:01:14 -0700 Subject: [PATCH 09/47] tabs --- scripts/system/libraries/controllerDispatcherUtils.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/system/libraries/controllerDispatcherUtils.js b/scripts/system/libraries/controllerDispatcherUtils.js index 41b4458bc5..61b54ca156 100644 --- a/scripts/system/libraries/controllerDispatcherUtils.js +++ b/scripts/system/libraries/controllerDispatcherUtils.js @@ -385,11 +385,11 @@ distanceBetweenPointAndEntityBoundingBox = function(point, entityProps) { }; entityIsEquipped = function(entityID) { - var rightEquipEntity = getEnabledModuleByName("RightEquipEntity"); - var leftEquipEntity = getEnabledModuleByName("LeftEquipEntity"); - var equippedInRightHand = rightEquipEntity ? rightEquipEntity.targetEntityID === entityID : false; - var equippedInLeftHand = leftEquipEntity ? leftEquipEntity.targetEntityID === entityID : false; - return equippedInRightHand || equippedInLeftHand; + var rightEquipEntity = getEnabledModuleByName("RightEquipEntity"); + var leftEquipEntity = getEnabledModuleByName("LeftEquipEntity"); + var equippedInRightHand = rightEquipEntity ? rightEquipEntity.targetEntityID === entityID : false; + var equippedInLeftHand = leftEquipEntity ? leftEquipEntity.targetEntityID === entityID : false; + return equippedInRightHand || equippedInLeftHand; }; if (typeof module !== 'undefined') { From 6a3303fe3ead895273cfd7a596b5c8bb1b5dce44 Mon Sep 17 00:00:00 2001 From: David Back Date: Wed, 18 Apr 2018 15:05:29 -0700 Subject: [PATCH 10/47] tabs --- scripts/system/controllers/controllerModules/equipEntity.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/system/controllers/controllerModules/equipEntity.js b/scripts/system/controllers/controllerModules/equipEntity.js index 6406b8593f..5807465abb 100644 --- a/scripts/system/controllers/controllerModules/equipEntity.js +++ b/scripts/system/controllers/controllerModules/equipEntity.js @@ -579,7 +579,7 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa this.targetEntityID = null; this.messageGrabEntity = false; this.grabEntityProps = null; - this.mouseEquip = false; + this.mouseEquip = false; }; this.updateInputs = function (controllerData) { @@ -796,7 +796,7 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa var intersection = Entities.findRayIntersection(pickRay, true); if (intersection.intersects) { var entityProperties = Entities.getEntityProperties(intersection.entityID, DISPATCHER_PROPERTIES); - var hasEquipData = getWearableData(entityProperties).joints || getEquipHotspotsData(entityProperties).length > 0; + var hasEquipData = getWearableData(entityProperties).joints || getEquipHotspotsData(entityProperties).length > 0; if (hasEquipData && entityProperties.parentID === EMPTY_PARENT_ID) { entityProperties.id = intersection.entityID; var rightHandPosition = MyAvatar.getJointPosition("RightHand"); From 3c441f0312b4559d12cde9c9242ad2d26238f118 Mon Sep 17 00:00:00 2001 From: David Back Date: Thu, 19 Apr 2018 11:17:26 -0700 Subject: [PATCH 11/47] safer clear grab actions --- scripts/system/controllers/controllerModules/equipEntity.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/system/controllers/controllerModules/equipEntity.js b/scripts/system/controllers/controllerModules/equipEntity.js index 5807465abb..3134ec6736 100644 --- a/scripts/system/controllers/controllerModules/equipEntity.js +++ b/scripts/system/controllers/controllerModules/equipEntity.js @@ -778,11 +778,12 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa var clearGrabActions = function(entityID) { var actionIDs = Entities.getActionIDs(entityID); + var myGrabTag = "grab-" + MyAvatar.sessionUUID; for (var actionIndex = 0; actionIndex < actionIDs.length; actionIndex++) { var actionID = actionIDs[actionIndex]; var actionArguments = Entities.getActionArguments(entityID, actionID); var tag = actionArguments.tag; - if (tag.slice(0, 5) === "grab-") { + if (tag === myGrabTag) { Entities.deleteAction(entityID, actionID); } } From 86b64fe3ab2a8b83216d4ecd4de16fc4a5b6eaca Mon Sep 17 00:00:00 2001 From: David Back Date: Thu, 19 Apr 2018 16:29:49 -0700 Subject: [PATCH 12/47] fix mouse equipping while someone else far grabbing --- .../controllerModules/equipEntity.js | 11 +++++----- .../libraries/controllerDispatcherUtils.js | 20 +++++++++++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/scripts/system/controllers/controllerModules/equipEntity.js b/scripts/system/controllers/controllerModules/equipEntity.js index 3134ec6736..0dca6f11f4 100644 --- a/scripts/system/controllers/controllerModules/equipEntity.js +++ b/scripts/system/controllers/controllerModules/equipEntity.js @@ -796,10 +796,11 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa var pickRay = Camera.computePickRay(event.x, event.y); var intersection = Entities.findRayIntersection(pickRay, true); if (intersection.intersects) { - var entityProperties = Entities.getEntityProperties(intersection.entityID, DISPATCHER_PROPERTIES); + var entityID = intersection.entityID; + var entityProperties = Entities.getEntityProperties(entityID, DISPATCHER_PROPERTIES); var hasEquipData = getWearableData(entityProperties).joints || getEquipHotspotsData(entityProperties).length > 0; - if (hasEquipData && entityProperties.parentID === EMPTY_PARENT_ID) { - entityProperties.id = intersection.entityID; + if (hasEquipData && entityProperties.parentID === EMPTY_PARENT_ID && !entityIsFarGrabbedByOther(entityID)) { + entityProperties.id = entityID; var rightHandPosition = MyAvatar.getJointPosition("RightHand"); var leftHandPosition = MyAvatar.getJointPosition("LeftHand"); var distanceToRightHand = Vec3.distance(entityProperties.position, rightHandPosition); @@ -809,11 +810,11 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa var mouseEquip = true; if (rightHandAvailable && (distanceToRightHand < distanceToLeftHand || !leftHandAvailable)) { // clear any existing grab actions on the entity now (their later removal could affect bootstrapping flags) - clearGrabActions(intersection.entityID); + clearGrabActions(entityID); rightEquipEntity.setMessageGrabData(entityProperties, mouseEquip); } else if (leftHandAvailable && (distanceToLeftHand < distanceToRightHand || !rightHandAvailable)) { // clear any existing grab actions on the entity now (their later removal could affect bootstrapping flags) - clearGrabActions(intersection.entityID); + clearGrabActions(entityID); leftEquipEntity.setMessageGrabData(entityProperties, mouseEquip); } } diff --git a/scripts/system/libraries/controllerDispatcherUtils.js b/scripts/system/libraries/controllerDispatcherUtils.js index 61b54ca156..a8a7c9da2c 100644 --- a/scripts/system/libraries/controllerDispatcherUtils.js +++ b/scripts/system/libraries/controllerDispatcherUtils.js @@ -392,6 +392,26 @@ entityIsEquipped = function(entityID) { return equippedInRightHand || equippedInLeftHand; }; +entityIsFarGrabbedByOther = function(entityID) { + // by convention, a far grab sets the tag of its action to be far-grab-*owner-session-id*. + var actionIDs = Entities.getActionIDs(entityID); + var myFarGrabTag = "far-grab-" + MyAvatar.sessionUUID; + for (var actionIndex = 0; actionIndex < actionIDs.length; actionIndex++) { + var actionID = actionIDs[actionIndex]; + var actionArguments = Entities.getActionArguments(entityID, actionID); + var tag = actionArguments.tag; + if (tag == myFarGrabTag) { + // we see a far-grab-*uuid* shaped tag, but it's our tag, so that's okay. + continue; + } + if (tag.slice(0, 9) == "far-grab-") { + // we see a far-grab-*uuid* shaped tag and it's not ours, so someone else is grabbing it. + return true; + } + } + return false; +}; + if (typeof module !== 'undefined') { module.exports = { makeDispatcherModuleParameters: makeDispatcherModuleParameters, From f30e34e8648ba02b33c5616848f75bc15b590518 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 25 Apr 2018 18:18:19 -0700 Subject: [PATCH 13/47] update serverless content with uncompressed KTX skyboxes --- cmake/externals/serverless-content/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmake/externals/serverless-content/CMakeLists.txt b/cmake/externals/serverless-content/CMakeLists.txt index a08b589ec2..3514260840 100644 --- a/cmake/externals/serverless-content/CMakeLists.txt +++ b/cmake/externals/serverless-content/CMakeLists.txt @@ -4,8 +4,8 @@ set(EXTERNAL_NAME serverless-content) ExternalProject_Add( ${EXTERNAL_NAME} - URL http://cdn.highfidelity.com/content-sets/serverless-tutorial-RC66-v4.zip - URL_MD5 d4f42f630986c83427ff39e1fe9908c6 + URL http://cdn.highfidelity.com/content-sets/serverless-tutorial-RC66-v5.zip + URL_MD5 4e1837bdbf0ee053b413ac92adce94b5 CONFIGURE_COMMAND "" BUILD_COMMAND "" INSTALL_COMMAND "" From 03e03727dbfa7ca30996419821faf7f65f6a1678 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 26 Apr 2018 12:55:28 -0700 Subject: [PATCH 14/47] fix bug: interface sends too many updates on settle --- libraries/physics/src/EntityMotionState.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index c2bacd4949..68f21eea87 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -447,7 +447,12 @@ bool EntityMotionState::shouldSendUpdate(uint32_t simulationStep) { // this case is prevented by setting _ownershipState to UNOWNABLE in EntityMotionState::ctor assert(!(_entity->getClientOnly() && _entity->getOwningAvatarID() != Physics::getSessionUUID())); - if (_entity->dynamicDataNeedsTransmit() || _entity->queryAACubeNeedsUpdate()) { + // shouldSendUpdate() sould NOT be triggering updates to maintain the queryAACube of dynamic entities. + // The server is supposed to predict the transform of such moving things. The client performs a "double prediction" + // where it predicts what the the server is doing, and only sends updates whent the entity's true transform + // differs significantly. That is what the remoteSimulationOutOfSync() logic is all about. + if (_entity->dynamicDataNeedsTransmit() || + (!_entity->getDynamic() && _entity->queryAACubeNeedsUpdate())) { return true; } From c8d7dc9b915c6217ba3931c69753a5c96136491c Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 26 Apr 2018 11:38:50 -0700 Subject: [PATCH 15/47] hide default port from addresses, fix localhost scheme --- libraries/networking/src/AddressManager.cpp | 10 ++++++---- libraries/networking/src/DomainHandler.cpp | 15 +++++++++------ 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/libraries/networking/src/AddressManager.cpp b/libraries/networking/src/AddressManager.cpp index daebcfb40d..72bca8ba3b 100644 --- a/libraries/networking/src/AddressManager.cpp +++ b/libraries/networking/src/AddressManager.cpp @@ -329,12 +329,14 @@ bool AddressManager::handleUrl(const QUrl& lookupUrl, LookupTrigger trigger) { return false; } +static const QString LOCALHOST = "localhost"; + bool isPossiblePlaceName(QString possiblePlaceName) { bool result { false }; int length = possiblePlaceName.length(); static const int MINIMUM_PLACENAME_LENGTH = 1; static const int MAXIMUM_PLACENAME_LENGTH = 64; - if (possiblePlaceName.toLower() != "localhost" && + if (possiblePlaceName.toLower() != LOCALHOST && length >= MINIMUM_PLACENAME_LENGTH && length <= MAXIMUM_PLACENAME_LENGTH) { const QRegExp PLACE_NAME_REGEX = QRegExp("^[0-9A-Za-z](([0-9A-Za-z]|-(?!-))*[^\\W_]$|$)"); result = PLACE_NAME_REGEX.indexIn(possiblePlaceName) == 0; @@ -354,7 +356,7 @@ void AddressManager::handleLookupString(const QString& lookupString, bool fromSu sanitizedString = sanitizedString.remove(HIFI_SCHEME_REGEX); lookupURL = QUrl(sanitizedString); - if (lookupURL.scheme().isEmpty()) { + if (lookupURL.scheme().isEmpty() || lookupURL.scheme().toLower() == LOCALHOST) { lookupURL = QUrl("hifi://" + sanitizedString); } } else { @@ -603,7 +605,7 @@ bool AddressManager::handleNetworkAddress(const QString& lookupString, LookupTri if (ipAddressRegex.indexIn(lookupString) != -1) { QString domainIPString = ipAddressRegex.cap(1); - quint16 domainPort = DEFAULT_DOMAIN_SERVER_PORT; + quint16 domainPort = 0; if (!ipAddressRegex.cap(2).isEmpty()) { domainPort = (quint16) ipAddressRegex.cap(2).toInt(); } @@ -625,7 +627,7 @@ bool AddressManager::handleNetworkAddress(const QString& lookupString, LookupTri if (hostnameRegex.indexIn(lookupString) != -1) { QString domainHostname = hostnameRegex.cap(1); - quint16 domainPort = DEFAULT_DOMAIN_SERVER_PORT; + quint16 domainPort = 0; if (!hostnameRegex.cap(2).isEmpty()) { domainPort = (quint16)hostnameRegex.cap(2).toInt(); diff --git a/libraries/networking/src/DomainHandler.cpp b/libraries/networking/src/DomainHandler.cpp index cd8064c4c0..871dc26899 100644 --- a/libraries/networking/src/DomainHandler.cpp +++ b/libraries/networking/src/DomainHandler.cpp @@ -166,7 +166,12 @@ void DomainHandler::setURLAndID(QUrl domainURL, QUuid domainID) { } } - if (_domainURL != domainURL || _sockAddr.getPort() != domainURL.port()) { + auto domainPort = domainURL.port(); + if (domainPort == -1) { + domainPort = DEFAULT_DOMAIN_SERVER_PORT; + } + + if (_domainURL != domainURL || _sockAddr.getPort() != domainPort) { // re-set the domain info so that auth information is reloaded hardReset(); @@ -192,12 +197,10 @@ void DomainHandler::setURLAndID(QUrl domainURL, QUuid domainID) { emit domainURLChanged(_domainURL); - if (_sockAddr.getPort() != domainURL.port()) { - qCDebug(networking) << "Updated domain port to" << domainURL.port(); + if (_sockAddr.getPort() != domainPort) { + qCDebug(networking) << "Updated domain port to" << domainPort; + _sockAddr.setPort(domainPort); } - - // grab the port by reading the string after the colon - _sockAddr.setPort(domainURL.port()); } } From d87adcec92da330476ec3529fe561aeeb8167c86 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 26 Apr 2018 17:47:12 -0700 Subject: [PATCH 16/47] make home go to DEFAULT_HIFI_ADDRESS by default --- interface/src/ui/AddressBarDialog.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/interface/src/ui/AddressBarDialog.cpp b/interface/src/ui/AddressBarDialog.cpp index 87bf09a252..789a2a2bdf 100644 --- a/interface/src/ui/AddressBarDialog.cpp +++ b/interface/src/ui/AddressBarDialog.cpp @@ -58,9 +58,8 @@ void AddressBarDialog::loadHome() { qDebug() << "Called LoadHome"; auto locationBookmarks = DependencyManager::get(); QString homeLocation = locationBookmarks->addressForBookmark(LocationBookmarks::HOME_BOOKMARK); - const QString DEFAULT_HOME_LOCATION = "localhost"; if (homeLocation == "") { - homeLocation = DEFAULT_HOME_LOCATION; + homeLocation = DEFAULT_HIFI_ADDRESS; } DependencyManager::get()->handleLookupString(homeLocation); } From 2662dc9b4e5ba665556ed60d791b3803ff5eb18f Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Fri, 27 Apr 2018 08:03:48 -0700 Subject: [PATCH 17/47] Suppress tracing log spam when tracing is not active --- libraries/shared/src/Trace.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libraries/shared/src/Trace.h b/libraries/shared/src/Trace.h index 93e2c6c4c2..1e1326968f 100644 --- a/libraries/shared/src/Trace.h +++ b/libraries/shared/src/Trace.h @@ -102,6 +102,9 @@ private: }; inline void traceEvent(const QLoggingCategory& category, const QString& name, EventType type, const QString& id = "", const QVariantMap& args = {}, const QVariantMap& extra = {}) { + if (!DependencyManager::isSet()) { + return; + } const auto& tracer = DependencyManager::get(); if (tracer) { tracer->traceEvent(category, name, type, id, args, extra); From 4db83230ce94b966efdfe07553d368be68a6e893 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Fri, 27 Apr 2018 08:09:31 -0700 Subject: [PATCH 18/47] Fix bad virtual cast on qml surface destruction --- libraries/ui/src/ui/OffscreenQmlSurface.cpp | 23 +++++++++++++-------- libraries/ui/src/ui/OffscreenQmlSurface.h | 4 +++- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/libraries/ui/src/ui/OffscreenQmlSurface.cpp b/libraries/ui/src/ui/OffscreenQmlSurface.cpp index 43b573a169..b3c1c486e9 100644 --- a/libraries/ui/src/ui/OffscreenQmlSurface.cpp +++ b/libraries/ui/src/ui/OffscreenQmlSurface.cpp @@ -223,6 +223,17 @@ void AudioHandler::run() { qDebug() << "QML Audio changed to " << _newTargetDevice; } +OffscreenQmlSurface::~OffscreenQmlSurface() { + clearFocusItem(); +} + +void OffscreenQmlSurface::clearFocusItem() { + if (_currentFocusItem) { + disconnect(_currentFocusItem, &QObject::destroyed, this, &OffscreenQmlSurface::focusDestroyed); + } + _currentFocusItem = nullptr; +} + void OffscreenQmlSurface::initializeEngine(QQmlEngine* engine) { Parent::initializeEngine(engine); new QQmlFileSelector(engine); @@ -545,17 +556,15 @@ bool OffscreenQmlSurface::handlePointerEvent(const PointerEvent& event, class QT } void OffscreenQmlSurface::focusDestroyed(QObject* obj) { - if (_currentFocusItem) { - disconnect(_currentFocusItem, &QObject::destroyed, this, &OffscreenQmlSurface::focusDestroyed); - } - _currentFocusItem = nullptr; + clearFocusItem(); } void OffscreenQmlSurface::onFocusObjectChanged(QObject* object) { + clearFocusItem(); + QQuickItem* item = static_cast(object); if (!item) { setFocusText(false); - _currentFocusItem = nullptr; return; } @@ -563,10 +572,6 @@ void OffscreenQmlSurface::onFocusObjectChanged(QObject* object) { qApp->sendEvent(object, &query); setFocusText(query.value(Qt::ImEnabled).toBool()); - if (_currentFocusItem) { - disconnect(_currentFocusItem, &QObject::destroyed, this, 0); - } - // Raise and lower keyboard for QML text fields. // HTML text fields are handled in emitWebEvent() methods - testing READ_ONLY_PROPERTY prevents action for HTML files. const char* READ_ONLY_PROPERTY = "readOnly"; diff --git a/libraries/ui/src/ui/OffscreenQmlSurface.h b/libraries/ui/src/ui/OffscreenQmlSurface.h index 9fa86f12a3..b95a8f117d 100644 --- a/libraries/ui/src/ui/OffscreenQmlSurface.h +++ b/libraries/ui/src/ui/OffscreenQmlSurface.h @@ -22,7 +22,8 @@ class OffscreenQmlSurface : public hifi::qml::OffscreenSurface { Q_OBJECT Q_PROPERTY(bool focusText READ isFocusText NOTIFY focusTextChanged) public: - + ~OffscreenQmlSurface(); + static void addWhitelistContextHandler(const std::initializer_list& urls, const QmlContextCallback& callback); static void addWhitelistContextHandler(const QUrl& url, const QmlContextCallback& callback) { addWhitelistContextHandler({ { url } }, callback); }; @@ -58,6 +59,7 @@ public slots: void sendToQml(const QVariant& message); protected: + void clearFocusItem(); void setFocusText(bool newFocusText); void initializeEngine(QQmlEngine* engine) override; void onRootContextCreated(QQmlContext* qmlContext) override; From 6640313256e0fadc6401628f6e0316518a01119a Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Fri, 27 Apr 2018 08:10:22 -0700 Subject: [PATCH 19/47] Fix memory leak in QML surfaces --- libraries/ui/src/ui/OffscreenQmlSurface.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/ui/src/ui/OffscreenQmlSurface.cpp b/libraries/ui/src/ui/OffscreenQmlSurface.cpp index b3c1c486e9..0d8e22cebb 100644 --- a/libraries/ui/src/ui/OffscreenQmlSurface.cpp +++ b/libraries/ui/src/ui/OffscreenQmlSurface.cpp @@ -115,6 +115,7 @@ private: class UrlHandler : public QObject { Q_OBJECT public: + UrlHandler(QObject* parent = nullptr) : QObject(parent) {} Q_INVOKABLE bool canHandleUrl(const QString& url) { static auto handler = dynamic_cast(qApp); return handler && handler->canAcceptURL(url); @@ -257,7 +258,7 @@ void OffscreenQmlSurface::initializeEngine(QQmlEngine* engine) { auto rootContext = engine->rootContext(); rootContext->setContextProperty("GL", ::getGLContextData()); - rootContext->setContextProperty("urlHandler", new UrlHandler()); + rootContext->setContextProperty("urlHandler", new UrlHandler(rootContext)); rootContext->setContextProperty("resourceDirectoryUrl", QUrl::fromLocalFile(PathUtils::resourcesPath())); rootContext->setContextProperty("ApplicationInterface", qApp); auto javaScriptToInject = getEventBridgeJavascript(); From 1b612d373f845ea4dab1698fb36b12d5814d88ff Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Fri, 27 Apr 2018 08:11:11 -0700 Subject: [PATCH 20/47] Explicitly delete QML context before releasing engine --- libraries/qml/src/qml/impl/SharedObject.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/libraries/qml/src/qml/impl/SharedObject.cpp b/libraries/qml/src/qml/impl/SharedObject.cpp index 9253c41b39..7bcb216ba8 100644 --- a/libraries/qml/src/qml/impl/SharedObject.cpp +++ b/libraries/qml/src/qml/impl/SharedObject.cpp @@ -97,12 +97,13 @@ SharedObject::~SharedObject() { } // _rootItem is parented to the quickWindow, so needs no explicit destruction - //if (_rootItem) { - // delete _rootItem; - // _rootItem = nullptr; - //} - releaseEngine(_qmlContext->engine()); + if (_qmlContext) { + auto engine = _qmlContext->engine(); + delete _qmlContext; + _qmlContext = nullptr; + releaseEngine(engine); + } } void SharedObject::create(OffscreenSurface* surface) { @@ -210,9 +211,9 @@ QQmlEngine* SharedObject::acquireEngine(OffscreenSurface* surface) { if (!globalEngine) { Q_ASSERT(0 == globalEngineRefCount); globalEngine = new QQmlEngine(); - surface->initializeQmlEngine(result); - ++globalEngineRefCount; + surface->initializeEngine(result); } + ++globalEngineRefCount; result = globalEngine; #else result = new QQmlEngine(); From e892694bf5bdcf56e5892a71fe7fb2492f7d42ec Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Fri, 27 Apr 2018 08:13:35 -0700 Subject: [PATCH 21/47] Don't modify URL on web entity on QML shutdown, remove tablet related code from web entity --- .../src/RenderableWebEntityItem.cpp | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp index f333e805ce..7ad74d1eee 100644 --- a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp @@ -308,12 +308,7 @@ bool WebEntityRenderer::buildWebSurface(const TypedEntityPointer& entity) { item->setProperty(URL_PROPERTY, _lastSourceUrl); }); } else if (_contentType == ContentType::QmlContent) { - _webSurface->load(_lastSourceUrl, [this](QQmlContext* context, QObject* item) { - if (item && item->objectName() == "tabletRoot") { - auto tabletScriptingInterface = DependencyManager::get(); - tabletScriptingInterface->setQmlTabletRoot("com.highfidelity.interface.tablet.system", _webSurface.data()); - } - }); + _webSurface->load(_lastSourceUrl); } _fadeStartTime = usecTimestampNow(); _webSurface->resume(); @@ -330,16 +325,6 @@ void WebEntityRenderer::destroyWebSurface() { if (webSurface) { --_currentWebCount; QQuickItem* rootItem = webSurface->getRootItem(); - // Explicitly set the web URL to an empty string, in an effort to get a - // faster shutdown of any chromium processes interacting with audio - if (rootItem && _contentType == ContentType::HtmlContent) { - rootItem->setProperty(URL_PROPERTY, ""); - } - - if (rootItem && rootItem->objectName() == "tabletRoot") { - auto tabletScriptingInterface = DependencyManager::get(); - tabletScriptingInterface->setQmlTabletRoot("com.highfidelity.interface.tablet.system", nullptr); - } // Fix for crash in QtWebEngineCore when rapidly switching domains // Call stop on the QWebEngineView before destroying OffscreenQMLSurface. From f2172d84a00d8add357029821c45f4d74d15d412 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Fri, 27 Apr 2018 08:35:20 -0700 Subject: [PATCH 22/47] Add stress test for QML web content --- tests/qml/qml/controls/WebEntityView.qml | 32 ++++ tests/qml/src/main.cpp | 198 ++++++++++++++++++----- 2 files changed, 187 insertions(+), 43 deletions(-) create mode 100644 tests/qml/qml/controls/WebEntityView.qml diff --git a/tests/qml/qml/controls/WebEntityView.qml b/tests/qml/qml/controls/WebEntityView.qml new file mode 100644 index 0000000000..a6c38f4abd --- /dev/null +++ b/tests/qml/qml/controls/WebEntityView.qml @@ -0,0 +1,32 @@ +// +// WebEntityView.qml +// +// Created by Kunal Gosar on 16 March 2017 +// Copyright 2017 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +import QtQuick 2.5 +import QtWebEngine 1.5 + +WebEngineView { + id: webViewCore + objectName: "webEngineView" + width: parent !== null ? parent.width : undefined + height: parent !== null ? parent.height : undefined + + onFeaturePermissionRequested: { + grantFeaturePermission(securityOrigin, feature, true); + } + + //disable popup + onContextMenuRequested: { + request.accepted = true; + } + + onNewViewRequested: { + newViewRequestedCallback(request) + } +} diff --git a/tests/qml/src/main.cpp b/tests/qml/src/main.cpp index 022f7290f4..c23a958da7 100644 --- a/tests/qml/src/main.cpp +++ b/tests/qml/src/main.cpp @@ -28,52 +28,82 @@ #include #include #include - +#include #include #include +#include + #include -#include #include #include #include #include #include #include +#include +#include + +#pragma optimize("", off) + +namespace gl { + extern void initModuleGl(); +} -class OffscreenQmlSurface : public hifi::qml::OffscreenSurface { +QUrl getTestResource(const QString& relativePath) { + static QString dir; + if (dir.isEmpty()) { + QDir path(__FILE__); + path.cdUp(); + dir = path.cleanPath(path.absoluteFilePath("../")) + "/"; + qDebug() << "Resources Path: " << dir; + } + return QUrl::fromLocalFile(dir + relativePath); +} + +#define DIVISIONS_X 5 +#define DIVISIONS_Y 5 + +using QmlPtr = QSharedPointer; +using TextureAndFence = hifi::qml::OffscreenSurface::TextureAndFence; + +struct QmlInfo { + QmlPtr surface; + GLuint texture{ 0 }; + uint64_t lifetime{ 0 }; }; class TestWindow : public QWindow { - public: TestWindow(); - private: - using TextureAndFence = hifi::qml::OffscreenSurface::TextureAndFence; QOpenGLContext _glContext; OffscreenGLCanvas _sharedContext; - OffscreenQmlSurface _offscreenQml; + std::array, DIVISIONS_X> _surfaces; + QOpenGLFunctions_4_5_Core _glf; - uint32_t _currentTexture{ 0 }; - GLsync _readFence{ 0 }; std::function _discardLamdba; QSize _size; + size_t _surfaceCount{ 0 }; GLuint _fbo{ 0 }; const QSize _qmlSize{ 640, 480 }; bool _aboutToQuit{ false }; void initGl(); + void updateSurfaces(); + void buildSurface(QmlInfo& qmlInfo); + void destroySurface(QmlInfo& qmlInfo); void resizeWindow(const QSize& size); void draw(); void resizeEvent(QResizeEvent* ev) override; }; TestWindow::TestWindow() { - setSurfaceType(QSurface::OpenGLSurface); + Setting::init(); + setSurfaceType(QSurface::OpenGLSurface); QSurfaceFormat format; format.setDepthBufferSize(24); format.setStencilBufferSize(8); @@ -85,11 +115,12 @@ TestWindow::TestWindow() { show(); + resize(QSize(800, 600)); auto timer = new QTimer(this); timer->setTimerType(Qt::PreciseTimer); - timer->setInterval(5); + timer->setInterval(30); connect(timer, &QTimer::timeout, [&] { draw(); }); timer->start(); @@ -97,7 +128,6 @@ TestWindow::TestWindow() { timer->stop(); _aboutToQuit = true; }); - } void TestWindow::initGl() { @@ -105,6 +135,7 @@ void TestWindow::initGl() { if (!_glContext.create() || !_glContext.makeCurrent(this)) { qFatal("Unable to intialize Window GL context"); } + gl::initModuleGl(); _glf.initializeOpenGLFunctions(); _glf.glCreateFramebuffers(1, &_fbo); @@ -113,15 +144,104 @@ void TestWindow::initGl() { qFatal("Unable to intialize Shared GL context"); } hifi::qml::OffscreenSurface::setSharedContext(_sharedContext.getContext()); - _discardLamdba = _offscreenQml.getDiscardLambda(); - _offscreenQml.resize({ 640, 480 }); - _offscreenQml.load(QUrl::fromLocalFile("C:/Users/bdavi/Git/hifi/tests/qml/qml/main.qml")); + _discardLamdba = hifi::qml::OffscreenSurface::getDiscardLambda(); } void TestWindow::resizeWindow(const QSize& size) { _size = size; } +static const int DEFAULT_MAX_FPS = 10; +static const int YOUTUBE_MAX_FPS = 30; +static const QString CONTROL_URL{ "/qml/controls/WebEntityView.qml" }; +static const char* URL_PROPERTY{ "url" }; + +QString getSourceUrl() { + static const std::vector SOURCE_URLS{ + "https://www.reddit.com/wiki/random", + "https://en.wikipedia.org/wiki/Wikipedia:Random", + "https://slashdot.org/", + //"https://www.youtube.com/watch?v=gDXwhHm4GhM", + //"https://www.youtube.com/watch?v=Ch_hoYPPeGc", + }; + + auto index = rand() % SOURCE_URLS.size(); + return SOURCE_URLS[index]; +} + + + +void TestWindow::buildSurface(QmlInfo& qmlInfo) { + ++_surfaceCount; + auto lifetimeSecs = (uint32_t)(2.0f + (randFloat() * 10.0f)); + auto lifetimeUsecs = (USECS_PER_SECOND * lifetimeSecs); + qmlInfo.lifetime = lifetimeUsecs + usecTimestampNow(); + qmlInfo.texture = 0; + auto& surface = qmlInfo.surface; + surface.reset(new hifi::qml::OffscreenSurface()); + surface->setMaxFps(DEFAULT_MAX_FPS); + surface->resize(_qmlSize); + surface->setMaxFps(DEFAULT_MAX_FPS); + hifi::qml::QmlContextObjectCallback callback = [](QQmlContext* context, QQuickItem* item) { + item->setProperty(URL_PROPERTY, getSourceUrl()); + }; + surface->load(getTestResource(CONTROL_URL), callback); + surface->resume(); +} + +void TestWindow::destroySurface(QmlInfo& qmlInfo) { + auto& surface = qmlInfo.surface; + QQuickItem* rootItem = surface->getRootItem(); + if (rootItem) { + QObject* obj = rootItem->findChild("webEngineView"); + if (!obj && rootItem->objectName() == "webEngineView") { + obj = rootItem; + } + if (obj) { + // stop loading + QMetaObject::invokeMethod(obj, "stop"); + } + } + surface->pause(); + surface.reset(); +} + +void TestWindow::updateSurfaces() { + auto now = usecTimestampNow(); + // Fetch any new textures + for (size_t x = 0; x < DIVISIONS_X; ++x) { + for (size_t y = 0; y < DIVISIONS_Y; ++y) { + auto& qmlInfo = _surfaces[x][y]; + if (!qmlInfo.surface) { + if (randFloat() > 0.99f) { + buildSurface(qmlInfo); + } else { + continue; + } + } + + if (now > qmlInfo.lifetime) { + destroySurface(qmlInfo); + continue; + } + + auto& surface = qmlInfo.surface; + auto& currentTexture = qmlInfo.texture; + + TextureAndFence newTextureAndFence; + if (surface->fetchTexture(newTextureAndFence)) { + if (currentTexture != 0) { + auto readFence = _glf.glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0); + glFlush(); + _discardLamdba(currentTexture, readFence); + } + currentTexture = newTextureAndFence.first; + _glf.glWaitSync((GLsync)newTextureAndFence.second, 0, GL_TIMEOUT_IGNORED); + } + } + } +} + void TestWindow::draw() { if (_aboutToQuit) { return; @@ -140,36 +260,31 @@ void TestWindow::draw() { return; } + updateSurfaces(); + + auto size = this->geometry().size(); + auto incrementX = size.width() / DIVISIONS_X; + auto incrementY = size.height() / DIVISIONS_Y; + _glf.glViewport(0, 0, size.width(), size.height()); _glf.glClearColor(1, 0, 0, 1); _glf.glClear(GL_COLOR_BUFFER_BIT); - TextureAndFence newTextureAndFence; - if (_offscreenQml.fetchTexture(newTextureAndFence)) { - if (_currentTexture) { - _discardLamdba(_currentTexture, _readFence); - _readFence = 0; + for (uint32_t x = 0; x < DIVISIONS_X; ++x) { + for (uint32_t y = 0; y < DIVISIONS_Y; ++y) { + auto& qmlInfo = _surfaces[x][y]; + if (!qmlInfo.surface || !qmlInfo.texture) { + continue; + } + _glf.glNamedFramebufferTexture(_fbo, GL_COLOR_ATTACHMENT0, qmlInfo.texture, 0); + _glf.glBlitNamedFramebuffer(_fbo, 0, + // src coordinates + 0, 0, _qmlSize.width() - 1, _qmlSize.height() - 1, + // dst coordinates + incrementX * x, incrementY * y, incrementX * (x + 1), incrementY * (y + 1), + // blit mask and filter + GL_COLOR_BUFFER_BIT, GL_NEAREST); } - - _currentTexture = newTextureAndFence.first; - _glf.glWaitSync((GLsync)newTextureAndFence.second, 0, GL_TIMEOUT_IGNORED); - _glf.glNamedFramebufferTexture(_fbo, GL_COLOR_ATTACHMENT0, _currentTexture, 0); } - - auto diff = _size - _qmlSize; - diff /= 2; - auto qmlExtent = diff + _qmlSize; - - if (_currentTexture) { - _glf.glBlitNamedFramebuffer(_fbo, 0, - 0, 0, _qmlSize.width() - 1, _qmlSize.height() - 1, - diff.width(), diff.height(), qmlExtent.width() - 1, qmlExtent.height() - 2, - GL_COLOR_BUFFER_BIT, GL_NEAREST); - } - - if (_readFence) { - _glf.glDeleteSync(_readFence); - } - _readFence = _glf.glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0); _glf.glFlush(); _glContext.swapBuffers(this); @@ -180,11 +295,8 @@ void TestWindow::resizeEvent(QResizeEvent* ev) { } int main(int argc, char** argv) { - setupHifiApplication("QML Test"); - QGuiApplication app(argc, argv); TestWindow window; app.exec(); return 0; } - From c163ae63b8954439cebe2d11c8d8902e807ef910 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Fri, 27 Apr 2018 10:20:55 -0700 Subject: [PATCH 23/47] Allow gifting of Pending items; remove PoP verification rescheduling on Pending items --- .../hifi/commerce/purchases/PurchasedItem.qml | 1 - libraries/entities/src/EntityTree.cpp | 33 ++----------------- libraries/entities/src/EntityTree.h | 3 +- 3 files changed, 4 insertions(+), 33 deletions(-) diff --git a/interface/resources/qml/hifi/commerce/purchases/PurchasedItem.qml b/interface/resources/qml/hifi/commerce/purchases/PurchasedItem.qml index 7d7a882ee0..4db98091c1 100644 --- a/interface/resources/qml/hifi/commerce/purchases/PurchasedItem.qml +++ b/interface/resources/qml/hifi/commerce/purchases/PurchasedItem.qml @@ -239,7 +239,6 @@ Item { width: 62; onLoaded: { - item.enabled = (root.purchaseStatus === "confirmed"); item.buttonGlyphText = hifi.glyphs.gift; item.buttonText = "Gift"; item.buttonClicked = function() { diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index d391dc8be1..b56f367e0a 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1172,16 +1172,6 @@ void EntityTree::startChallengeOwnershipTimer(const EntityItemID& entityItemID) _challengeOwnershipTimeoutTimer->start(5000); } -void EntityTree::startPendingTransferStatusTimer(const QString& certID, const EntityItemID& entityItemID, const SharedNodePointer& senderNode) { - qCDebug(entities) << "'transfer_status' is 'pending', checking again in 90 seconds..." << entityItemID; - QTimer* transferStatusRetryTimer = new QTimer(this); - connect(transferStatusRetryTimer, &QTimer::timeout, this, [=]() { - validatePop(certID, entityItemID, senderNode, true); - }); - transferStatusRetryTimer->setSingleShot(true); - transferStatusRetryTimer->start(90000); -} - QByteArray EntityTree::computeNonce(const QString& certID, const QString ownerKey) { QUuid nonce = QUuid::createUuid(); //random, 5-hex value, separated by "-" QByteArray nonceBytes = nonce.toByteArray(); @@ -1321,7 +1311,7 @@ void EntityTree::sendChallengeOwnershipRequestPacket(const QByteArray& certID, c nodeList->sendPacket(std::move(challengeOwnershipPacket), *(nodeList->nodeWithUUID(QUuid::fromRfc4122(nodeToChallenge)))); } -void EntityTree::validatePop(const QString& certID, const EntityItemID& entityItemID, const SharedNodePointer& senderNode, bool isRetryingValidation) { +void EntityTree::validatePop(const QString& certID, const EntityItemID& entityItemID, const SharedNodePointer& senderNode) { // Start owner verification. auto nodeList = DependencyManager::get(); // First, asynchronously hit "proof_of_purchase_status?transaction_type=transfer" endpoint. @@ -1352,30 +1342,13 @@ void EntityTree::validatePop(const QString& certID, const EntityItemID& entityIt withWriteLock([&] { deleteEntity(entityItemID, true); }); - } else if (jsonObject["transfer_status"].toArray().first().toString() == "pending") { - if (isRetryingValidation) { - qCDebug(entities) << "'transfer_status' is 'pending' after retry, deleting entity" << entityItemID; - withWriteLock([&] { - deleteEntity(entityItemID, true); - }); - } else { - if (thread() != QThread::currentThread()) { - QMetaObject::invokeMethod(this, "startPendingTransferStatusTimer", - Q_ARG(const QString&, certID), - Q_ARG(const EntityItemID&, entityItemID), - Q_ARG(const SharedNodePointer&, senderNode)); - return; - } else { - startPendingTransferStatusTimer(certID, entityItemID, senderNode); - } - } } else { // Second, challenge ownership of the PoP cert + // (ignore pending status; a failure will be cleaned up during DDV) sendChallengeOwnershipPacket(certID, jsonObject["transfer_recipient_key"].toString(), entityItemID, senderNode); - } } else { qCDebug(entities) << "Call to" << networkReply->url() << "failed with error" << networkReply->error() << "; deleting entity" << entityItemID @@ -1619,7 +1592,7 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c // Delete the entity we just added if it doesn't pass static certificate verification deleteEntity(entityItemID, true); } else { - validatePop(properties.getCertificateID(), entityItemID, senderNode, false); + validatePop(properties.getCertificateID(), entityItemID, senderNode); } } diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index a080801a0e..3289101967 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -397,12 +397,11 @@ protected: QHash _entitiesToAdd; Q_INVOKABLE void startChallengeOwnershipTimer(const EntityItemID& entityItemID); - Q_INVOKABLE void startPendingTransferStatusTimer(const QString& certID, const EntityItemID& entityItemID, const SharedNodePointer& senderNode); private: void sendChallengeOwnershipPacket(const QString& certID, const QString& ownerKey, const EntityItemID& entityItemID, const SharedNodePointer& senderNode); void sendChallengeOwnershipRequestPacket(const QByteArray& certID, const QByteArray& text, const QByteArray& nodeToChallenge, const SharedNodePointer& senderNode); - void validatePop(const QString& certID, const EntityItemID& entityItemID, const SharedNodePointer& senderNode, bool isRetryingValidation); + void validatePop(const QString& certID, const EntityItemID& entityItemID, const SharedNodePointer& senderNode); std::shared_ptr _myAvatar{ nullptr }; From acb21cc96a459721bf51638bba15855e36892cd2 Mon Sep 17 00:00:00 2001 From: Bradley Austin Davis Date: Wed, 25 Apr 2018 09:43:20 -0700 Subject: [PATCH 24/47] Revert "HOTFIX version of PR 12969: Attempt to shutdown web surfaces more consistently" --- interface/src/Application.cpp | 13 +--- interface/src/Application.h | 7 +-- interface/src/ui/overlays/Web3DOverlay.cpp | 12 ++-- .../src/RenderableWebEntityItem.cpp | 39 +++++------- libraries/qml/src/qml/OffscreenSurface.cpp | 6 +- .../qml/src/qml/impl/RenderEventHandler.cpp | 11 ++-- libraries/qml/src/qml/impl/SharedObject.cpp | 62 +++++++------------ .../src/AbstractViewStateInterface.h | 19 ++---- tests/render-perf/src/main.cpp | 6 +- 9 files changed, 71 insertions(+), 104 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index c38caca090..789f60bdb1 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -4750,7 +4750,7 @@ void Application::updateLOD(float deltaTime) const { } } -void Application::pushPostUpdateLambda(void* key, const std::function& func) { +void Application::pushPostUpdateLambda(void* key, std::function func) { std::unique_lock guard(_postUpdateLambdasLock); _postUpdateLambdas[key] = func; } @@ -7360,7 +7360,7 @@ void Application::windowMinimizedChanged(bool minimized) { } } -void Application::postLambdaEvent(const std::function& f) { +void Application::postLambdaEvent(std::function f) { if (this->thread() == QThread::currentThread()) { f(); } else { @@ -7368,15 +7368,6 @@ void Application::postLambdaEvent(const std::function& f) { } } -void Application::sendLambdaEvent(const std::function& f) { - if (this->thread() == QThread::currentThread()) { - f(); - } else { - LambdaEvent event(f); - QCoreApplication::sendEvent(this, &event); - } -} - void Application::initPlugins(const QStringList& arguments) { QCommandLineOption display("display", "Preferred displays", "displays"); QCommandLineOption disableDisplays("disable-displays", "Displays to disable", "displays"); diff --git a/interface/src/Application.h b/interface/src/Application.h index 74b0e5a110..769658b0d6 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -136,8 +136,7 @@ public: Application(int& argc, char** argv, QElapsedTimer& startup_time, bool runningMarkerExisted); ~Application(); - void postLambdaEvent(const std::function& f) override; - void sendLambdaEvent(const std::function& f) override; + void postLambdaEvent(std::function f) override; QString getPreviousScriptLocation(); void setPreviousScriptLocation(const QString& previousScriptLocation); @@ -241,7 +240,7 @@ public: qint64 getCurrentSessionRuntime() const { return _sessionRunTimer.elapsed(); } - bool isAboutToQuit() const { return _aboutToQuit; } + bool isAboutToQuit() const override { return _aboutToQuit; } bool isPhysicsEnabled() const { return _physicsEnabled; } // the isHMDMode is true whenever we use the interface from an HMD and not a standard flat display @@ -265,7 +264,7 @@ public: render::EnginePointer getRenderEngine() override { return _renderEngine; } gpu::ContextPointer getGPUContext() const { return _gpuContext; } - virtual void pushPostUpdateLambda(void* key, const std::function& func) override; + virtual void pushPostUpdateLambda(void* key, std::function func) override; void updateMyAvatarLookAtPosition(); diff --git a/interface/src/ui/overlays/Web3DOverlay.cpp b/interface/src/ui/overlays/Web3DOverlay.cpp index fcdac8c00c..1e0b50f091 100644 --- a/interface/src/ui/overlays/Web3DOverlay.cpp +++ b/interface/src/ui/overlays/Web3DOverlay.cpp @@ -65,10 +65,14 @@ const QString Web3DOverlay::TYPE = "web3d"; const QString Web3DOverlay::QML = "Web3DOverlay.qml"; static auto qmlSurfaceDeleter = [](OffscreenQmlSurface* surface) { - AbstractViewStateInterface::instance()->sendLambdaEvent([surface] { - // WebEngineView may run other threads (wasapi), so they must be deleted for a clean shutdown - // if the application has already stopped its event loop, delete must be explicit - delete surface; + AbstractViewStateInterface::instance()->postLambdaEvent([surface] { + if (AbstractViewStateInterface::instance()->isAboutToQuit()) { + // WebEngineView may run other threads (wasapi), so they must be deleted for a clean shutdown + // if the application has already stopped its event loop, delete must be explicit + delete surface; + } else { + surface->deleteLater(); + } }); }; diff --git a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp index f333e805ce..3b46c530cc 100644 --- a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp @@ -25,7 +25,7 @@ #include #include "EntitiesRendererLogging.h" -#include + using namespace render; using namespace render::entities; @@ -45,7 +45,6 @@ static int DEFAULT_MAX_FPS = 10; static int YOUTUBE_MAX_FPS = 30; static QTouchDevice _touchDevice; -static const char* URL_PROPERTY = "url"; WebEntityRenderer::ContentType WebEntityRenderer::getContentType(const QString& urlString) { if (urlString.isEmpty()) { @@ -53,7 +52,7 @@ WebEntityRenderer::ContentType WebEntityRenderer::getContentType(const QString& } const QUrl url(urlString); - if (url.scheme() == URL_SCHEME_HTTP || url.scheme() == URL_SCHEME_HTTPS || + if (url.scheme() == "http" || url.scheme() == "https" || urlString.toLower().endsWith(".htm") || urlString.toLower().endsWith(".html")) { return ContentType::HtmlContent; } @@ -165,8 +164,6 @@ void WebEntityRenderer::doRenderUpdateSynchronousTyped(const ScenePointer& scene if (urlChanged) { if (newContentType != ContentType::HtmlContent || currentContentType != ContentType::HtmlContent) { destroyWebSurface(); - // If we destroyed the surface, the URL change will be implicitly handled by the re-creation - urlChanged = false; } withWriteLock([&] { @@ -188,8 +185,8 @@ void WebEntityRenderer::doRenderUpdateSynchronousTyped(const ScenePointer& scene return; } - if (urlChanged && _contentType == ContentType::HtmlContent) { - _webSurface->getRootItem()->setProperty(URL_PROPERTY, _lastSourceUrl); + if (urlChanged) { + _webSurface->getRootItem()->setProperty("url", _lastSourceUrl); } if (_contextPosition != entity->getWorldPosition()) { @@ -260,14 +257,6 @@ bool WebEntityRenderer::hasWebSurface() { return (bool)_webSurface && _webSurface->getRootItem(); } -static const auto WebSurfaceDeleter = [](OffscreenQmlSurface* webSurface) { - AbstractViewStateInterface::instance()->sendLambdaEvent([webSurface] { - // WebEngineView may run other threads (wasapi), so they must be deleted for a clean shutdown - // if the application has already stopped its event loop, delete must be explicit - delete webSurface; - }); -}; - bool WebEntityRenderer::buildWebSurface(const TypedEntityPointer& entity) { if (_currentWebCount >= MAX_CONCURRENT_WEB_VIEWS) { qWarning() << "Too many concurrent web views to create new view"; @@ -275,9 +264,20 @@ bool WebEntityRenderer::buildWebSurface(const TypedEntityPointer& entity) { } ++_currentWebCount; + auto deleter = [](OffscreenQmlSurface* webSurface) { + AbstractViewStateInterface::instance()->postLambdaEvent([webSurface] { + if (AbstractViewStateInterface::instance()->isAboutToQuit()) { + // WebEngineView may run other threads (wasapi), so they must be deleted for a clean shutdown + // if the application has already stopped its event loop, delete must be explicit + delete webSurface; + } else { + webSurface->deleteLater(); + } + }); + }; // FIXME use the surface cache instead of explicit creation - _webSurface = QSharedPointer(new OffscreenQmlSurface(), WebSurfaceDeleter); + _webSurface = QSharedPointer(new OffscreenQmlSurface(), deleter); // FIXME, the max FPS could be better managed by being dynamic (based on the number of current surfaces // and the current rendering load) _webSurface->setMaxFps(DEFAULT_MAX_FPS); @@ -305,7 +305,7 @@ bool WebEntityRenderer::buildWebSurface(const TypedEntityPointer& entity) { _webSurface->setMaxFps(DEFAULT_MAX_FPS); } _webSurface->load("controls/WebEntityView.qml", [this](QQmlContext* context, QObject* item) { - item->setProperty(URL_PROPERTY, _lastSourceUrl); + item->setProperty("url", _lastSourceUrl); }); } else if (_contentType == ContentType::QmlContent) { _webSurface->load(_lastSourceUrl, [this](QQmlContext* context, QObject* item) { @@ -330,11 +330,6 @@ void WebEntityRenderer::destroyWebSurface() { if (webSurface) { --_currentWebCount; QQuickItem* rootItem = webSurface->getRootItem(); - // Explicitly set the web URL to an empty string, in an effort to get a - // faster shutdown of any chromium processes interacting with audio - if (rootItem && _contentType == ContentType::HtmlContent) { - rootItem->setProperty(URL_PROPERTY, ""); - } if (rootItem && rootItem->objectName() == "tabletRoot") { auto tabletScriptingInterface = DependencyManager::get(); diff --git a/libraries/qml/src/qml/OffscreenSurface.cpp b/libraries/qml/src/qml/OffscreenSurface.cpp index 688f3fdb5f..2da1c41340 100644 --- a/libraries/qml/src/qml/OffscreenSurface.cpp +++ b/libraries/qml/src/qml/OffscreenSurface.cpp @@ -66,10 +66,14 @@ OffscreenSurface::OffscreenSurface() } OffscreenSurface::~OffscreenSurface() { - delete _sharedObject; + disconnect(qApp); + _sharedObject->destroy(); } bool OffscreenSurface::fetchTexture(TextureAndFence& textureAndFence) { + if (!_sharedObject) { + return false; + } hifi::qml::impl::TextureAndFence typedTextureAndFence; bool result = _sharedObject->fetchTexture(typedTextureAndFence); textureAndFence = typedTextureAndFence; diff --git a/libraries/qml/src/qml/impl/RenderEventHandler.cpp b/libraries/qml/src/qml/impl/RenderEventHandler.cpp index 945a469611..6b66ce9314 100644 --- a/libraries/qml/src/qml/impl/RenderEventHandler.cpp +++ b/libraries/qml/src/qml/impl/RenderEventHandler.cpp @@ -49,8 +49,8 @@ RenderEventHandler::RenderEventHandler(SharedObject* shared, QThread* targetThre qFatal("Unable to create new offscreen GL context"); } - _canvas.moveToThreadWithContext(targetThread); moveToThread(targetThread); + _canvas.moveToThreadWithContext(targetThread); } void RenderEventHandler::onInitalize() { @@ -160,8 +160,11 @@ void RenderEventHandler::onQuit() { } _shared->shutdownRendering(_canvas, _currentSize); - _canvas.doneCurrent(); - _canvas.moveToThreadWithContext(qApp->thread()); - moveToThread(qApp->thread()); + // Release the reference to the shared object. This will allow it to + // be destroyed (should happen on it's own thread). + _shared->deleteLater(); + + deleteLater(); + QThread::currentThread()->quit(); } diff --git a/libraries/qml/src/qml/impl/SharedObject.cpp b/libraries/qml/src/qml/impl/SharedObject.cpp index 9253c41b39..b2057117f6 100644 --- a/libraries/qml/src/qml/impl/SharedObject.cpp +++ b/libraries/qml/src/qml/impl/SharedObject.cpp @@ -72,35 +72,26 @@ SharedObject::SharedObject() { QObject::connect(qApp, &QCoreApplication::aboutToQuit, this, &SharedObject::onAboutToQuit); } - SharedObject::~SharedObject() { - // After destroy returns, the rendering thread should be gone - destroy(); - - // _renderTimer is created with `this` as the parent, so need no explicit destruction - - // Destroy the event hand - if (_renderObject) { - delete _renderObject; - _renderObject = nullptr; - } - - if (_renderControl) { - delete _renderControl; - _renderControl = nullptr; - } - if (_quickWindow) { _quickWindow->destroy(); - delete _quickWindow; _quickWindow = nullptr; } - // _rootItem is parented to the quickWindow, so needs no explicit destruction - //if (_rootItem) { - // delete _rootItem; - // _rootItem = nullptr; - //} + if (_renderControl) { + _renderControl->deleteLater(); + _renderControl = nullptr; + } + + if (_renderThread) { + _renderThread->quit(); + _renderThread->deleteLater(); + } + + if (_rootItem) { + _rootItem->deleteLater(); + _rootItem = nullptr; + } releaseEngine(_qmlContext->engine()); } @@ -128,10 +119,6 @@ void SharedObject::create(OffscreenSurface* surface) { } void SharedObject::setRootItem(QQuickItem* rootItem) { - if (_quit) { - return; - } - _rootItem = rootItem; _rootItem->setSize(_quickWindow->size()); @@ -140,6 +127,7 @@ void SharedObject::setRootItem(QQuickItem* rootItem) { _renderThread->setObjectName(objectName()); _renderThread->start(); + // Create event handler for the render thread _renderObject = new RenderEventHandler(this, _renderThread); QCoreApplication::postEvent(this, new OffscreenEvent(OffscreenEvent::Initialize)); @@ -149,43 +137,35 @@ void SharedObject::setRootItem(QQuickItem* rootItem) { } void SharedObject::destroy() { - // `destroy` is idempotent, it can be called multiple times without issues if (_quit) { return; } if (!_rootItem) { + deleteLater(); return; } + _paused = true; if (_renderTimer) { - _renderTimer->stop(); QObject::disconnect(_renderTimer); + _renderTimer->deleteLater(); } - if (_renderControl) { - QObject::disconnect(_renderControl); - } - + QObject::disconnect(_renderControl); QObject::disconnect(qApp); { QMutexLocker lock(&_mutex); _quit = true; - if (_renderObject) { - QCoreApplication::postEvent(_renderObject, new OffscreenEvent(OffscreenEvent::Quit), Qt::HighEventPriority); - } + QCoreApplication::postEvent(_renderObject, new OffscreenEvent(OffscreenEvent::Quit), Qt::HighEventPriority); } // Block until the rendering thread has stopped // FIXME this is undesirable because this is blocking the main thread, // but I haven't found a reliable way to do this only at application // shutdown - if (_renderThread) { - _renderThread->wait(); - delete _renderThread; - _renderThread = nullptr; - } + _renderThread->wait(); } diff --git a/libraries/render-utils/src/AbstractViewStateInterface.h b/libraries/render-utils/src/AbstractViewStateInterface.h index 54fdc903ca..96e9f4d222 100644 --- a/libraries/render-utils/src/AbstractViewStateInterface.h +++ b/libraries/render-utils/src/AbstractViewStateInterface.h @@ -37,25 +37,15 @@ public: virtual glm::vec3 getAvatarPosition() const = 0; - // Unfortunately, having this here is a bad idea. Lots of objects connect to - // the aboutToQuit signal, and it's impossible to know the order in which - // the receivers will be called, so this might return false negatives - //virtual bool isAboutToQuit() const = 0; - - // Queue code to execute on the main thread. - // If called from the main thread, the lambda will execute synchronously - virtual void postLambdaEvent(const std::function& f) = 0; - // Synchronously execute code on the main thread. This function will - // not return until the code is executed, regardles of which thread it - // is called from - virtual void sendLambdaEvent(const std::function& f) = 0; + virtual bool isAboutToQuit() const = 0; + virtual void postLambdaEvent(std::function f) = 0; virtual qreal getDevicePixelRatio() = 0; virtual render::ScenePointer getMain3DScene() = 0; virtual render::EnginePointer getRenderEngine() = 0; - virtual void pushPostUpdateLambda(void* key, const std::function& func) = 0; + virtual void pushPostUpdateLambda(void* key, std::function func) = 0; virtual bool isHMDMode() const = 0; @@ -64,4 +54,5 @@ public: static void setInstance(AbstractViewStateInterface* instance); }; -#endif // hifi_AbstractViewStateInterface_h + +#endif // hifi_AbstractViewStateInterface_h diff --git a/tests/render-perf/src/main.cpp b/tests/render-perf/src/main.cpp index 9249b3d957..93672cc5a2 100644 --- a/tests/render-perf/src/main.cpp +++ b/tests/render-perf/src/main.cpp @@ -453,8 +453,8 @@ protected: return vec3(); } - void postLambdaEvent(const std::function& f) override {} - void sendLambdaEvent(const std::function& f) override {} + bool isAboutToQuit() const override { return false; } + void postLambdaEvent(std::function f) override {} qreal getDevicePixelRatio() override { return 1.0f; @@ -469,7 +469,7 @@ protected: } std::map> _postUpdateLambdas; - void pushPostUpdateLambda(void* key, const std::function& func) override { + void pushPostUpdateLambda(void* key, std::function func) override { _postUpdateLambdas[key] = func; } From db5e5ba99beab1a45c1aa6b21794414f5eff93ce Mon Sep 17 00:00:00 2001 From: David Back Date: Fri, 27 Apr 2018 11:30:45 -0700 Subject: [PATCH 25/47] ignore right clicks --- scripts/system/controllers/controllerModules/equipEntity.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/system/controllers/controllerModules/equipEntity.js b/scripts/system/controllers/controllerModules/equipEntity.js index 0dca6f11f4..6de62eef85 100644 --- a/scripts/system/controllers/controllerModules/equipEntity.js +++ b/scripts/system/controllers/controllerModules/equipEntity.js @@ -790,7 +790,7 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa }; var onMousePress = function(event) { - if (isInEditMode()) { // don't consider any mouse clicks on the entity while in edit + if (isInEditMode() || event.isRightButton) { // don't consider any mouse clicks on the entity while in edit return; } var pickRay = Camera.computePickRay(event.x, event.y); From 684d2b08ba6206ff1e1f49f925a7e651004a36d9 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Fri, 27 Apr 2018 11:04:46 -0700 Subject: [PATCH 26/47] Finalize work on MS14295 and MS14559 --- .../qml/hifi/commerce/common/sendAsset/SendAsset.qml | 9 +++++---- scripts/system/commerce/wallet.js | 12 +++++++----- scripts/system/marketplaces/marketplaces.js | 12 +++++++----- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/interface/resources/qml/hifi/commerce/common/sendAsset/SendAsset.qml b/interface/resources/qml/hifi/commerce/common/sendAsset/SendAsset.qml index 8411a24bf5..c7c72e5f7c 100644 --- a/interface/resources/qml/hifi/commerce/common/sendAsset/SendAsset.qml +++ b/interface/resources/qml/hifi/commerce/common/sendAsset/SendAsset.qml @@ -1308,7 +1308,7 @@ Item { anchors.right: parent.right; anchors.rightMargin: root.assetName === "" ? 15 : 50; anchors.bottom: parent.bottom; - anchors.bottomMargin: root.assetName === "" ? 15 : 300; + anchors.bottomMargin: root.assetName === "" ? 15 : 240; color: "#FFFFFF"; RalewaySemiBold { @@ -1403,12 +1403,12 @@ Item { id: giftContainer_paymentSuccess; visible: root.assetName !== ""; anchors.top: sendToContainer_paymentSuccess.bottom; - anchors.topMargin: 16; + anchors.topMargin: 8; anchors.left: parent.left; anchors.leftMargin: 20; anchors.right: parent.right; anchors.rightMargin: 20; - height: 80; + height: 30; RalewaySemiBold { id: gift_paymentSuccess; @@ -1431,6 +1431,7 @@ Item { anchors.top: parent.top; anchors.left: gift_paymentSuccess.right; anchors.right: parent.right; + height: parent.height; // Text size size: 18; // Style @@ -1522,7 +1523,7 @@ Item { colorScheme: root.assetName === "" ? hifi.colorSchemes.dark : hifi.colorSchemes.light; anchors.horizontalCenter: parent.horizontalCenter; anchors.bottom: parent.bottom; - anchors.bottomMargin: 80; + anchors.bottomMargin: root.assetName === "" ? 80 : 30; height: 50; width: 120; text: "Close"; diff --git a/scripts/system/commerce/wallet.js b/scripts/system/commerce/wallet.js index 2fbeaec317..aea752c565 100644 --- a/scripts/system/commerce/wallet.js +++ b/scripts/system/commerce/wallet.js @@ -557,12 +557,14 @@ } if (onWalletScreen) { + if (!isWired) { + Users.usernameFromIDReply.connect(usernameFromIDReply); + Controller.mousePressEvent.connect(handleMouseEvent); + Controller.mouseMoveEvent.connect(handleMouseMoveEvent); + triggerMapping.enable(); + triggerPressMapping.enable(); + } isWired = true; - Users.usernameFromIDReply.connect(usernameFromIDReply); - Controller.mousePressEvent.connect(handleMouseEvent); - Controller.mouseMoveEvent.connect(handleMouseMoveEvent); - triggerMapping.enable(); - triggerPressMapping.enable(); } else { off(); } diff --git a/scripts/system/marketplaces/marketplaces.js b/scripts/system/marketplaces/marketplaces.js index b128e100a1..a05778e2dd 100644 --- a/scripts/system/marketplaces/marketplaces.js +++ b/scripts/system/marketplaces/marketplaces.js @@ -1055,12 +1055,14 @@ var selectionDisplay = null; // for gridTool.js to ignore } if (onCommerceScreen) { + if (!isWired) { + Users.usernameFromIDReply.connect(usernameFromIDReply); + Controller.mousePressEvent.connect(handleMouseEvent); + Controller.mouseMoveEvent.connect(handleMouseMoveEvent); + triggerMapping.enable(); + triggerPressMapping.enable(); + } isWired = true; - Users.usernameFromIDReply.connect(usernameFromIDReply); - Controller.mousePressEvent.connect(handleMouseEvent); - Controller.mouseMoveEvent.connect(handleMouseMoveEvent); - triggerMapping.enable(); - triggerPressMapping.enable(); Wallet.refreshWalletStatus(); } else { off(); From f731a2d08eed6eaede593d3fe17b23a1256b7038 Mon Sep 17 00:00:00 2001 From: David Back Date: Fri, 27 Apr 2018 13:32:05 -0700 Subject: [PATCH 27/47] change to not left button --- scripts/system/controllers/controllerModules/equipEntity.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/system/controllers/controllerModules/equipEntity.js b/scripts/system/controllers/controllerModules/equipEntity.js index 6de62eef85..d6e18376bf 100644 --- a/scripts/system/controllers/controllerModules/equipEntity.js +++ b/scripts/system/controllers/controllerModules/equipEntity.js @@ -790,7 +790,7 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa }; var onMousePress = function(event) { - if (isInEditMode() || event.isRightButton) { // don't consider any mouse clicks on the entity while in edit + if (isInEditMode() || !event.isLeftButton) { // don't consider any left clicks on the entity while in edit return; } var pickRay = Camera.computePickRay(event.x, event.y); From 5f8149fd6873ca6ad1163927826283f1809a10c8 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Fri, 27 Apr 2018 13:57:22 -0700 Subject: [PATCH 28/47] Remove debugging pragma --- tests/qml/src/main.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/qml/src/main.cpp b/tests/qml/src/main.cpp index c23a958da7..cd5a567e95 100644 --- a/tests/qml/src/main.cpp +++ b/tests/qml/src/main.cpp @@ -44,8 +44,6 @@ #include #include -#pragma optimize("", off) - namespace gl { extern void initModuleGl(); } From 575520fe3063f3d6deac5e8266b3e886ac25d88a Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Fri, 27 Apr 2018 15:08:10 -0700 Subject: [PATCH 29/47] Ensure we actually delete the QML scenegraph! --- libraries/qml/src/qml/impl/SharedObject.cpp | 7 +- tests/qml/qml/controls/WebEntityView.qml | 15 ++++ tests/qml/src/main.cpp | 84 ++++++++++++++------- 3 files changed, 78 insertions(+), 28 deletions(-) diff --git a/libraries/qml/src/qml/impl/SharedObject.cpp b/libraries/qml/src/qml/impl/SharedObject.cpp index 7bcb216ba8..2fde057ca8 100644 --- a/libraries/qml/src/qml/impl/SharedObject.cpp +++ b/libraries/qml/src/qml/impl/SharedObject.cpp @@ -90,14 +90,17 @@ SharedObject::~SharedObject() { _renderControl = nullptr; } + if (_rootItem) { + delete _rootItem; + _rootItem = nullptr; + } + if (_quickWindow) { _quickWindow->destroy(); delete _quickWindow; _quickWindow = nullptr; } - // _rootItem is parented to the quickWindow, so needs no explicit destruction - if (_qmlContext) { auto engine = _qmlContext->engine(); delete _qmlContext; diff --git a/tests/qml/qml/controls/WebEntityView.qml b/tests/qml/qml/controls/WebEntityView.qml index a6c38f4abd..5bd29ef457 100644 --- a/tests/qml/qml/controls/WebEntityView.qml +++ b/tests/qml/qml/controls/WebEntityView.qml @@ -10,6 +10,21 @@ import QtQuick 2.5 import QtWebEngine 1.5 +import Hifi 1.0 + +/* +TestItem { + Rectangle { + anchors.fill: parent + anchors.margins: 10 + color: "blue" + property string url: "" + ColorAnimation on color { + loops: Animation.Infinite; from: "blue"; to: "yellow"; duration: 1000 + } + } +} +*/ WebEngineView { id: webViewCore diff --git a/tests/qml/src/main.cpp b/tests/qml/src/main.cpp index cd5a567e95..ec4012898f 100644 --- a/tests/qml/src/main.cpp +++ b/tests/qml/src/main.cpp @@ -48,6 +48,17 @@ namespace gl { extern void initModuleGl(); } +class QTestItem : public QQuickItem { + Q_OBJECT +public: + QTestItem(QQuickItem* parent = nullptr) : QQuickItem(parent) { + qDebug() << __FUNCTION__; + } + + ~QTestItem() { + qDebug() << __FUNCTION__; + } +}; QUrl getTestResource(const QString& relativePath) { static QString dir; @@ -89,9 +100,10 @@ private: GLuint _fbo{ 0 }; const QSize _qmlSize{ 640, 480 }; bool _aboutToQuit{ false }; + uint64_t _createStopTime; void initGl(); void updateSurfaces(); - void buildSurface(QmlInfo& qmlInfo); + void buildSurface(QmlInfo& qmlInfo, bool allowVideo); void destroySurface(QmlInfo& qmlInfo); void resizeWindow(const QSize& size); void draw(); @@ -111,8 +123,10 @@ TestWindow::TestWindow() { QSurfaceFormat::setDefaultFormat(format); setFormat(format); - show(); + qmlRegisterType("Hifi", 1, 0, "TestItem"); + show(); + _createStopTime = usecTimestampNow() + (3000u * USECS_PER_SECOND); resize(QSize(800, 600)); @@ -167,40 +181,56 @@ QString getSourceUrl() { return SOURCE_URLS[index]; } +#define CACHE_WEBVIEWS 0 +#if CACHE_WEBVIEWS +static std::list _cache; +#endif -void TestWindow::buildSurface(QmlInfo& qmlInfo) { +hifi::qml::QmlContextObjectCallback callback = [](QQmlContext* context, QQuickItem* item) { + item->setProperty(URL_PROPERTY, getSourceUrl()); +}; + +void TestWindow::buildSurface(QmlInfo& qmlInfo, bool allowVideo) { ++_surfaceCount; - auto lifetimeSecs = (uint32_t)(2.0f + (randFloat() * 10.0f)); + auto lifetimeSecs = (uint32_t)(5.0f + (randFloat() * 10.0f)); auto lifetimeUsecs = (USECS_PER_SECOND * lifetimeSecs); qmlInfo.lifetime = lifetimeUsecs + usecTimestampNow(); qmlInfo.texture = 0; - auto& surface = qmlInfo.surface; - surface.reset(new hifi::qml::OffscreenSurface()); - surface->setMaxFps(DEFAULT_MAX_FPS); - surface->resize(_qmlSize); - surface->setMaxFps(DEFAULT_MAX_FPS); - hifi::qml::QmlContextObjectCallback callback = [](QQmlContext* context, QQuickItem* item) { - item->setProperty(URL_PROPERTY, getSourceUrl()); - }; - surface->load(getTestResource(CONTROL_URL), callback); - surface->resume(); +#if CACHE_WEBVIEWS + if (_cache.empty()) { + _cache.emplace_back(new hifi::qml::OffscreenSurface()); + auto& surface = _cache.back(); + surface->load(getTestResource(CONTROL_URL)); + surface->setMaxFps(DEFAULT_MAX_FPS); + } + qmlInfo.surface = _cache.front(); + _cache.pop_front(); +#else + qmlInfo.surface.reset(new hifi::qml::OffscreenSurface()); + qmlInfo.surface->load(getTestResource(CONTROL_URL)); + qmlInfo.surface->setMaxFps(DEFAULT_MAX_FPS); +#endif + + qmlInfo.surface->resize(_qmlSize); + auto url = allowVideo ? "https://www.youtube.com/watch?v=gDXwhHm4GhM" : getSourceUrl(); + qmlInfo.surface->getRootItem()->setProperty(URL_PROPERTY, url); + qmlInfo.surface->resume(); } + void TestWindow::destroySurface(QmlInfo& qmlInfo) { auto& surface = qmlInfo.surface; - QQuickItem* rootItem = surface->getRootItem(); - if (rootItem) { - QObject* obj = rootItem->findChild("webEngineView"); - if (!obj && rootItem->objectName() == "webEngineView") { - obj = rootItem; - } - if (obj) { - // stop loading - QMetaObject::invokeMethod(obj, "stop"); - } + auto webView = surface->getRootItem(); + if (webView) { + // stop loading + QMetaObject::invokeMethod(webView, "stop"); + webView->setProperty(URL_PROPERTY, "about:blank"); } surface->pause(); +#if CACHE_WEBVIEWS + _cache.push_back(surface); +#endif surface.reset(); } @@ -211,8 +241,8 @@ void TestWindow::updateSurfaces() { for (size_t y = 0; y < DIVISIONS_Y; ++y) { auto& qmlInfo = _surfaces[x][y]; if (!qmlInfo.surface) { - if (randFloat() > 0.99f) { - buildSurface(qmlInfo); + if (now < _createStopTime && randFloat() > 0.99f) { + buildSurface(qmlInfo, x == 0 && y == 0); } else { continue; } @@ -298,3 +328,5 @@ int main(int argc, char** argv) { app.exec(); return 0; } + +#include "main.moc" \ No newline at end of file From 0962e1fc1658f25f44ded0128d93ed802305ee25 Mon Sep 17 00:00:00 2001 From: David Back Date: Fri, 27 Apr 2018 11:30:45 -0700 Subject: [PATCH 30/47] ignore right clicks --- scripts/system/controllers/controllerModules/equipEntity.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/system/controllers/controllerModules/equipEntity.js b/scripts/system/controllers/controllerModules/equipEntity.js index 53dbee829d..1588a17538 100644 --- a/scripts/system/controllers/controllerModules/equipEntity.js +++ b/scripts/system/controllers/controllerModules/equipEntity.js @@ -796,7 +796,7 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa }; var onMousePress = function(event) { - if (isInEditMode()) { // don't consider any mouse clicks on the entity while in edit + if (isInEditMode() || event.isRightButton) { // don't consider any mouse clicks on the entity while in edit return; } var pickRay = Camera.computePickRay(event.x, event.y); From 61afd12eeb91539a7fa50362aeda9276ac3fd718 Mon Sep 17 00:00:00 2001 From: David Back Date: Fri, 27 Apr 2018 13:32:05 -0700 Subject: [PATCH 31/47] change to not left button --- scripts/system/controllers/controllerModules/equipEntity.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/system/controllers/controllerModules/equipEntity.js b/scripts/system/controllers/controllerModules/equipEntity.js index 1588a17538..1fce772ec8 100644 --- a/scripts/system/controllers/controllerModules/equipEntity.js +++ b/scripts/system/controllers/controllerModules/equipEntity.js @@ -796,7 +796,7 @@ EquipHotspotBuddy.prototype.update = function(deltaTime, timestamp, controllerDa }; var onMousePress = function(event) { - if (isInEditMode() || event.isRightButton) { // don't consider any mouse clicks on the entity while in edit + if (isInEditMode() || !event.isLeftButton) { // don't consider any left clicks on the entity while in edit return; } var pickRay = Camera.computePickRay(event.x, event.y); From 8e42bb8c877e693cdf29bdcb2e206316a2bc8a85 Mon Sep 17 00:00:00 2001 From: Brad Davis Date: Fri, 27 Apr 2018 16:04:07 -0700 Subject: [PATCH 32/47] Restore the stop functionality for a browser view when it's being destroyed --- .../qml/controls/FlickableWebViewCore.qml | 5 ++ interface/resources/qml/controls/WebView.qml | 4 ++ .../src/RenderableWebEntityItem.cpp | 11 ++- tests/qml/src/main.cpp | 68 +++++-------------- 4 files changed, 32 insertions(+), 56 deletions(-) diff --git a/interface/resources/qml/controls/FlickableWebViewCore.qml b/interface/resources/qml/controls/FlickableWebViewCore.qml index 8e7db44b7d..943f15e1de 100644 --- a/interface/resources/qml/controls/FlickableWebViewCore.qml +++ b/interface/resources/qml/controls/FlickableWebViewCore.qml @@ -21,6 +21,7 @@ Item { signal newViewRequestedCallback(var request) signal loadingChangedCallback(var loadRequest) + width: parent.width property bool interactive: false @@ -29,6 +30,10 @@ Item { id: hifi } + function stop() { + webViewCore.stop(); + } + function unfocus() { webViewCore.runJavaScript("if (document.activeElement) document.activeElement.blur();", function(result) { console.log('unfocus completed: ', result); diff --git a/interface/resources/qml/controls/WebView.qml b/interface/resources/qml/controls/WebView.qml index 931c64e1ef..71bf69fdc8 100644 --- a/interface/resources/qml/controls/WebView.qml +++ b/interface/resources/qml/controls/WebView.qml @@ -21,6 +21,10 @@ Item { property bool passwordField: false property alias flickable: webroot.interactive + function stop() { + webroot.stop(); + } + // FIXME - Keyboard HMD only: Make Interface either set keyboardRaised property directly in OffscreenQmlSurface // or provide HMDinfo object to QML in RenderableWebEntityItem and do the following. /* diff --git a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp index 7ad74d1eee..693e3d0cf4 100644 --- a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp @@ -318,8 +318,10 @@ bool WebEntityRenderer::buildWebSurface(const TypedEntityPointer& entity) { void WebEntityRenderer::destroyWebSurface() { QSharedPointer webSurface; + ContentType contentType{ ContentType::NoContent }; withWriteLock([&] { webSurface.swap(_webSurface); + std::swap(contentType, _contentType); }); if (webSurface) { @@ -328,12 +330,9 @@ void WebEntityRenderer::destroyWebSurface() { // Fix for crash in QtWebEngineCore when rapidly switching domains // Call stop on the QWebEngineView before destroying OffscreenQMLSurface. - if (rootItem) { - QObject* obj = rootItem->findChild("webEngineView"); - if (obj) { - // stop loading - QMetaObject::invokeMethod(obj, "stop"); - } + if (rootItem && contentType == ContentType::HtmlContent) { + // stop loading + QMetaObject::invokeMethod(rootItem, "stop"); } webSurface->pause(); diff --git a/tests/qml/src/main.cpp b/tests/qml/src/main.cpp index ec4012898f..349ac55d88 100644 --- a/tests/qml/src/main.cpp +++ b/tests/qml/src/main.cpp @@ -45,19 +45,15 @@ #include namespace gl { - extern void initModuleGl(); +extern void initModuleGl(); } class QTestItem : public QQuickItem { Q_OBJECT public: - QTestItem(QQuickItem* parent = nullptr) : QQuickItem(parent) { - qDebug() << __FUNCTION__; - } + QTestItem(QQuickItem* parent = nullptr) : QQuickItem(parent) { qDebug() << __FUNCTION__; } - ~QTestItem() { - qDebug() << __FUNCTION__; - } + ~QTestItem() { qDebug() << __FUNCTION__; } }; QUrl getTestResource(const QString& relativePath) { @@ -71,7 +67,6 @@ QUrl getTestResource(const QString& relativePath) { return QUrl::fromLocalFile(dir + relativePath); } - #define DIVISIONS_X 5 #define DIVISIONS_Y 5 @@ -164,61 +159,41 @@ void TestWindow::resizeWindow(const QSize& size) { } static const int DEFAULT_MAX_FPS = 10; -static const int YOUTUBE_MAX_FPS = 30; static const QString CONTROL_URL{ "/qml/controls/WebEntityView.qml" }; static const char* URL_PROPERTY{ "url" }; -QString getSourceUrl() { +QString getSourceUrl(bool video) { static const std::vector SOURCE_URLS{ "https://www.reddit.com/wiki/random", "https://en.wikipedia.org/wiki/Wikipedia:Random", "https://slashdot.org/", - //"https://www.youtube.com/watch?v=gDXwhHm4GhM", - //"https://www.youtube.com/watch?v=Ch_hoYPPeGc", }; - auto index = rand() % SOURCE_URLS.size(); - return SOURCE_URLS[index]; + static const std::vector VIDEO_SOURCE_URLS{ + "https://www.youtube.com/watch?v=gDXwhHm4GhM", + "https://www.youtube.com/watch?v=Ch_hoYPPeGc", + }; + + const auto& sourceUrls = video ? VIDEO_SOURCE_URLS : SOURCE_URLS; + auto index = rand() % sourceUrls.size(); + return sourceUrls[index]; } -#define CACHE_WEBVIEWS 0 - -#if CACHE_WEBVIEWS -static std::list _cache; -#endif - -hifi::qml::QmlContextObjectCallback callback = [](QQmlContext* context, QQuickItem* item) { - item->setProperty(URL_PROPERTY, getSourceUrl()); -}; - -void TestWindow::buildSurface(QmlInfo& qmlInfo, bool allowVideo) { +void TestWindow::buildSurface(QmlInfo& qmlInfo, bool video) { ++_surfaceCount; auto lifetimeSecs = (uint32_t)(5.0f + (randFloat() * 10.0f)); auto lifetimeUsecs = (USECS_PER_SECOND * lifetimeSecs); qmlInfo.lifetime = lifetimeUsecs + usecTimestampNow(); qmlInfo.texture = 0; -#if CACHE_WEBVIEWS - if (_cache.empty()) { - _cache.emplace_back(new hifi::qml::OffscreenSurface()); - auto& surface = _cache.back(); - surface->load(getTestResource(CONTROL_URL)); - surface->setMaxFps(DEFAULT_MAX_FPS); - } - qmlInfo.surface = _cache.front(); - _cache.pop_front(); -#else qmlInfo.surface.reset(new hifi::qml::OffscreenSurface()); - qmlInfo.surface->load(getTestResource(CONTROL_URL)); + qmlInfo.surface->load(getTestResource(CONTROL_URL), [video](QQmlContext* context, QQuickItem* item) { + item->setProperty(URL_PROPERTY, getSourceUrl(video)); + }); qmlInfo.surface->setMaxFps(DEFAULT_MAX_FPS); -#endif - qmlInfo.surface->resize(_qmlSize); - auto url = allowVideo ? "https://www.youtube.com/watch?v=gDXwhHm4GhM" : getSourceUrl(); - qmlInfo.surface->getRootItem()->setProperty(URL_PROPERTY, url); qmlInfo.surface->resume(); } - void TestWindow::destroySurface(QmlInfo& qmlInfo) { auto& surface = qmlInfo.surface; auto webView = surface->getRootItem(); @@ -228,9 +203,6 @@ void TestWindow::destroySurface(QmlInfo& qmlInfo) { webView->setProperty(URL_PROPERTY, "about:blank"); } surface->pause(); -#if CACHE_WEBVIEWS - _cache.push_back(surface); -#endif surface.reset(); } @@ -296,7 +268,6 @@ void TestWindow::draw() { _glf.glViewport(0, 0, size.width(), size.height()); _glf.glClearColor(1, 0, 0, 1); _glf.glClear(GL_COLOR_BUFFER_BIT); - for (uint32_t x = 0; x < DIVISIONS_X; ++x) { for (uint32_t y = 0; y < DIVISIONS_Y; ++y) { auto& qmlInfo = _surfaces[x][y]; @@ -313,8 +284,6 @@ void TestWindow::draw() { GL_COLOR_BUFFER_BIT, GL_NEAREST); } } - _glf.glFlush(); - _glContext.swapBuffers(this); } @@ -325,8 +294,7 @@ void TestWindow::resizeEvent(QResizeEvent* ev) { int main(int argc, char** argv) { QGuiApplication app(argc, argv); TestWindow window; - app.exec(); - return 0; + return app.exec(); } -#include "main.moc" \ No newline at end of file +#include "main.moc" From a245bb2174de3e4509c97d6de37ab483c4e1f89a Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Sat, 28 Apr 2018 13:30:02 -0700 Subject: [PATCH 33/47] fix libquazip debug filename. link libatomic where needed --- cmake/externals/quazip/CMakeLists.txt | 3 +++ tests/controllers/CMakeLists.txt | 6 +++++- tests/render-perf/CMakeLists.txt | 4 ++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/cmake/externals/quazip/CMakeLists.txt b/cmake/externals/quazip/CMakeLists.txt index f2690e0a7d..7bf6f05d9f 100644 --- a/cmake/externals/quazip/CMakeLists.txt +++ b/cmake/externals/quazip/CMakeLists.txt @@ -41,6 +41,9 @@ if (APPLE) elseif (WIN32) set(${EXTERNAL_NAME_UPPER}_LIBRARY_RELEASE ${INSTALL_DIR}/lib/quazip5.lib CACHE FILEPATH "Location of QuaZip release library") set(${EXTERNAL_NAME_UPPER}_LIBRARY_DEBUG ${INSTALL_DIR}/lib/quazip5d.lib CACHE FILEPATH "Location of QuaZip release library") +elseif (CMAKE_SYSTEM_NAME MATCHES "Linux") + set(${EXTERNAL_NAME_UPPER}_LIBRARY_RELEASE ${INSTALL_DIR}/lib/libquazip5.so CACHE FILEPATH "Location of QuaZip release library") + set(${EXTERNAL_NAME_UPPER}_LIBRARY_DEBUG ${INSTALL_DIR}/lib/libquazip5d.so CACHE FILEPATH "Location of QuaZip release library") else () set(${EXTERNAL_NAME_UPPER}_LIBRARY_RELEASE ${INSTALL_DIR}/lib/libquazip5.so CACHE FILEPATH "Location of QuaZip release library") set(${EXTERNAL_NAME_UPPER}_LIBRARY_DEBUG ${INSTALL_DIR}/lib/libquazip5.so CACHE FILEPATH "Location of QuaZip release library") diff --git a/tests/controllers/CMakeLists.txt b/tests/controllers/CMakeLists.txt index b5e866ccce..ce1c150ed4 100644 --- a/tests/controllers/CMakeLists.txt +++ b/tests/controllers/CMakeLists.txt @@ -23,5 +23,9 @@ if (WIN32) add_dependency_external_projects(wasapi) endif() +if (CMAKE_SYSTEM_NAME MATCHES "Linux") + target_link_libraries(${TARGET_NAME} atomic) +endif() + package_libraries_for_deployment() -endif() \ No newline at end of file +endif() diff --git a/tests/render-perf/CMakeLists.txt b/tests/render-perf/CMakeLists.txt index fd4d8d88dd..d688474379 100644 --- a/tests/render-perf/CMakeLists.txt +++ b/tests/render-perf/CMakeLists.txt @@ -27,6 +27,10 @@ if (WIN32) add_dependency_external_projects(wasapi) endif() +if (CMAKE_SYSTEM_NAME MATCHES "Linux") + target_link_libraries(${TARGET_NAME} atomic) +endif() + package_libraries_for_deployment() From a226790295c8ef4c5e21179993bb5cc0d62995e2 Mon Sep 17 00:00:00 2001 From: Oren Hurvitz Date: Mon, 30 Apr 2018 15:05:27 +0300 Subject: [PATCH 34/47] Eliminated warnings in gcc Note that the call to encodeNode() in FBXWriter.cpp can never be reached, so it's safe to remove it. --- assignment-client/src/assets/AssetServer.h | 2 +- libraries/audio/src/InboundAudioStream.cpp | 3 +++ libraries/entities/src/EntityTree.cpp | 3 +++ libraries/fbx/src/FBXWriter.cpp | 1 - libraries/networking/src/ResourceCache.cpp | 6 ++++++ libraries/shared/src/Gzip.cpp | 3 +++ 6 files changed, 16 insertions(+), 2 deletions(-) diff --git a/assignment-client/src/assets/AssetServer.h b/assignment-client/src/assets/AssetServer.h index fb88df0171..f83545c25c 100644 --- a/assignment-client/src/assets/AssetServer.h +++ b/assignment-client/src/assets/AssetServer.h @@ -63,7 +63,7 @@ struct AssetMeta { AssetMeta() { } - BakeVersion bakeVersion; + BakeVersion bakeVersion { INITIAL_BAKE_VERSION }; bool failedLastBake { false }; QString lastBakeErrors; }; diff --git a/libraries/audio/src/InboundAudioStream.cpp b/libraries/audio/src/InboundAudioStream.cpp index 172ec9411a..bbaedc003b 100644 --- a/libraries/audio/src/InboundAudioStream.cpp +++ b/libraries/audio/src/InboundAudioStream.cpp @@ -149,6 +149,9 @@ int InboundAudioStream::parseData(ReceivedMessage& message) { lostAudioData(packetsDropped); // fall through to OnTime case +#if defined(__GNUC__) + [[gnu::fallthrough]]; +#endif } case SequenceNumberStats::OnTime: { // Packet is on time; parse its data to the ringbuffer diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index b56f367e0a..e9e4f2631a 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1402,6 +1402,9 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c case PacketType::EntityAdd: isAdd = true; // fall through to next case +#if defined(__GNUC__) + [[gnu::fallthrough]]; +#endif case PacketType::EntityPhysics: case PacketType::EntityEdit: { quint64 startDecode = 0, endDecode = 0; diff --git a/libraries/fbx/src/FBXWriter.cpp b/libraries/fbx/src/FBXWriter.cpp index 511f253193..e6adff0df9 100644 --- a/libraries/fbx/src/FBXWriter.cpp +++ b/libraries/fbx/src/FBXWriter.cpp @@ -142,7 +142,6 @@ void FBXWriter::encodeFBXProperty(QDataStream& out, const QVariant& prop) { out << prop.toInt(); break; - encodeNode(out, FBXNode()); case QMetaType::Float: out.device()->write("F", 1); out << prop.toFloat(); diff --git a/libraries/networking/src/ResourceCache.cpp b/libraries/networking/src/ResourceCache.cpp index 7ba7cca96d..3b8c9fdac0 100644 --- a/libraries/networking/src/ResourceCache.cpp +++ b/libraries/networking/src/ResourceCache.cpp @@ -749,6 +749,9 @@ bool Resource::handleFailedRequest(ResourceRequest::Result result) { case ResourceRequest::Result::Timeout: { qCDebug(networking) << "Timed out loading" << _url << "received" << _bytesReceived << "total" << _bytesTotal; // Fall through to other cases +#if defined(__GNUC__) + [[gnu::fallthrough]]; +#endif } case ResourceRequest::Result::ServerUnavailable: { _attempts++; @@ -767,6 +770,9 @@ bool Resource::handleFailedRequest(ResourceRequest::Result result) { break; } // fall through to final failure +#if defined(__GNUC__) + [[gnu::fallthrough]]; +#endif } default: { _attemptsRemaining = 0; diff --git a/libraries/shared/src/Gzip.cpp b/libraries/shared/src/Gzip.cpp index a77f459fc9..44fe13f439 100644 --- a/libraries/shared/src/Gzip.cpp +++ b/libraries/shared/src/Gzip.cpp @@ -60,6 +60,9 @@ bool gunzip(QByteArray source, QByteArray &destination) { switch (status) { case Z_NEED_DICT: status = Z_DATA_ERROR; +#if defined(__GNUC__) + [[gnu::fallthrough]]; +#endif case Z_DATA_ERROR: case Z_MEM_ERROR: case Z_STREAM_ERROR: From ed47cd84581d062dc5127a9f09b6a8d655a1f93b Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 30 Apr 2018 12:35:48 -0700 Subject: [PATCH 35/47] add a missing unsafe_erase for local node ID map --- libraries/networking/src/LimitedNodeList.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 31500be682..2bdd688a73 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -302,6 +302,7 @@ bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packe } } else { NLPacket::LocalID sourceLocalID = Node::NULL_LOCAL_ID; + // check if we were passed a sourceNode hint or if we need to look it up if (!sourceNode) { // figure out which node this is from @@ -608,6 +609,7 @@ bool LimitedNodeList::killNodeWithUUID(const QUuid& nodeUUID, ConnectionID newCo { QWriteLocker writeLocker(&_nodeMutex); + _localIDMap.unsafe_erase(matchingNode->getLocalID()); _nodeHash.unsafe_erase(it); } From 7db07db9e2bdfd4bb92544a951e8e94b38f6ac97 Mon Sep 17 00:00:00 2001 From: Liv Erickson Date: Mon, 30 Apr 2018 13:55:01 -0700 Subject: [PATCH 36/47] remove menu item at cleanup --- scripts/system/edit.js | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/system/edit.js b/scripts/system/edit.js index 6d2b1f129b..c99c8d401a 100644 --- a/scripts/system/edit.js +++ b/scripts/system/edit.js @@ -1291,6 +1291,7 @@ function cleanupModelMenus() { Menu.removeMenuItem("Edit", MENU_EASE_ON_FOCUS); Menu.removeMenuItem("Edit", MENU_SHOW_LIGHTS_AND_PARTICLES_IN_EDIT_MODE); Menu.removeMenuItem("Edit", MENU_SHOW_ZONES_IN_EDIT_MODE); + Menu.removeMenuItem("Edit", MENU_CREATE_ENTITIES_GRABBABLE); } Script.scriptEnding.connect(function () { From f9bda6df15f125b2352dc19a1f6a70b38cea10a2 Mon Sep 17 00:00:00 2001 From: David Rowe Date: Tue, 1 May 2018 11:36:37 +1200 Subject: [PATCH 37/47] Fix LODManager JSDoc --- interface/src/LODManager.h | 49 +++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 27 deletions(-) diff --git a/interface/src/LODManager.h b/interface/src/LODManager.h index e8737d92ae..96d92e91e9 100644 --- a/interface/src/LODManager.h +++ b/interface/src/LODManager.h @@ -9,11 +9,6 @@ // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // -/**jsdoc - * The LOD class manages your Level of Detail functions within interface - * @namespace LODManager - */ - #ifndef hifi_LODManager_h #define hifi_LODManager_h @@ -39,10 +34,32 @@ const float ADJUST_LOD_MIN_SIZE_SCALE = DEFAULT_OCTREE_SIZE_SCALE * 0.04f; class AABox; +/**jsdoc + * The LOD class manages your Level of Detail functions within Interface. + * @namespace LODManager + * @property {number} presentTime Read-only. + * @property {number} engineRunTime Read-only. + * @property {number} gpuTime Read-only. + * @property {number} avgRenderTime Read-only. + * @property {number} fps Read-only. + * @property {number} lodLevel Read-only. + * @property {number} lodDecreaseFPS Read-only. + * @property {number} lodIncreaseFPS Read-only. + */ + class LODManager : public QObject, public Dependency { Q_OBJECT SINGLETON_DEPENDENCY + Q_PROPERTY(float presentTime READ getPresentTime) + Q_PROPERTY(float engineRunTime READ getEngineRunTime) + Q_PROPERTY(float gpuTime READ getGPUTime) + Q_PROPERTY(float avgRenderTime READ getAverageRenderTime) + Q_PROPERTY(float fps READ getMaxTheoreticalFPS) + Q_PROPERTY(float lodLevel READ getLODLevel) + Q_PROPERTY(float lodDecreaseFPS READ getLODDecreaseFPS) + Q_PROPERTY(float lodIncreaseFPS READ getLODIncreaseFPS) + public: /**jsdoc @@ -138,28 +155,6 @@ public: */ Q_INVOKABLE float getLODIncreaseFPS() const; - /**jsdoc - * @namespace LODManager - * @property {number} presentTime Read-only. - * @property {number} engineRunTime Read-only. - * @property {number} gpuTime Read-only. - * @property {number} avgRenderTime Read-only. - * @property {number} fps Read-only. - * @property {number} lodLevel Read-only. - * @property {number} lodDecreaseFPS Read-only. - * @property {number} lodIncreaseFPS Read-only. - */ - - Q_PROPERTY(float presentTime READ getPresentTime) - Q_PROPERTY(float engineRunTime READ getEngineRunTime) - Q_PROPERTY(float gpuTime READ getGPUTime) - Q_PROPERTY(float avgRenderTime READ getAverageRenderTime) - Q_PROPERTY(float fps READ getMaxTheoreticalFPS) - Q_PROPERTY(float lodLevel READ getLODLevel) - - Q_PROPERTY(float lodDecreaseFPS READ getLODDecreaseFPS) - Q_PROPERTY(float lodIncreaseFPS READ getLODIncreaseFPS) - float getPresentTime() const { return _presentTime; } float getEngineRunTime() const { return _engineRunTime; } float getGPUTime() const { return _gpuTime; } From f69ee410748700e547c7fffb5e5a5ee2021b20f6 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Mon, 30 Apr 2018 17:13:44 -0700 Subject: [PATCH 38/47] Prevent crash in ImageProvider when tablet isn't yet initialized --- interface/src/commerce/Wallet.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/interface/src/commerce/Wallet.cpp b/interface/src/commerce/Wallet.cpp index 3e0e4adf18..42c8b9973d 100644 --- a/interface/src/commerce/Wallet.cpp +++ b/interface/src/commerce/Wallet.cpp @@ -615,9 +615,12 @@ void Wallet::updateImageProvider() { securityImageProvider->setSecurityImage(_securityImage); // inform tablet security image provider - QQmlEngine* tabletEngine = DependencyManager::get()->getTablet("com.highfidelity.interface.tablet.system")->getTabletSurface()->getSurfaceContext()->engine(); - securityImageProvider = reinterpret_cast(tabletEngine->imageProvider(SecurityImageProvider::PROVIDER_NAME)); - securityImageProvider->setSecurityImage(_securityImage); + auto tablet = DependencyManager::get()->getTablet("com.highfidelity.interface.tablet.system"); + if (tablet) { + QQmlEngine* tabletEngine = tablet->getTabletSurface()->getSurfaceContext()->engine(); + securityImageProvider = reinterpret_cast(tabletEngine->imageProvider(SecurityImageProvider::PROVIDER_NAME)); + securityImageProvider->setSecurityImage(_securityImage); + } } void Wallet::chooseSecurityImage(const QString& filename) { From a1fc1c4810df8eec32b52969b45fd2e0d5c7448b Mon Sep 17 00:00:00 2001 From: David Rowe Date: Tue, 1 May 2018 21:55:14 +1200 Subject: [PATCH 39/47] Add JSDoc for Entity preload and unload signals --- libraries/script-engine/src/ScriptEngine.cpp | 33 ++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/libraries/script-engine/src/ScriptEngine.cpp b/libraries/script-engine/src/ScriptEngine.cpp index c79ffffec7..f0a13cc62b 100644 --- a/libraries/script-engine/src/ScriptEngine.cpp +++ b/libraries/script-engine/src/ScriptEngine.cpp @@ -2161,6 +2161,32 @@ void ScriptEngine::loadEntityScript(const EntityItemID& entityID, const QString& }, forceRedownload); } +/**jsdoc + * Triggered when the script starts for a user. + *

Note: Can only be connected to via this.preload = function (...) { ... } in the entity script.

+ *
Available in:Client Entity ScriptsServer Entity Scripts
+ * @function Entities.preload + * @param {Uuid} entityID - The ID of the entity that the script is running in. + * @returns {Signal} + * @example Get the ID of the entity that a client entity script is running in. + * var entityScript = (function () { + * this.entityID = Uuid.NULL; + * + * this.preload = function (entityID) { + * this.entityID = entityID; + * print("Entity ID: " + this.entityID); + * }; + * ); + * + * var entityID = Entities.addEntity({ + * type: "Box", + * position: Vec3.sum(MyAvatar.position, Vec3.multiplyQbyV(MyAvatar.orientation, { x: 0, y: 0, z: -5 })), + * dimensions: { x: 0.5, y: 0.5, z: 0.5 }, + * color: { red: 255, green: 0, blue: 0 }, + * script: "(" + entityScript + ")", // Could host the script on a Web server instead. + * lifetime: 300 // Delete after 5 minutes. + * }); + */ // since all of these operations can be asynch we will always do the actual work in the response handler // for the download void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, const QString& scriptOrURL, const QString& contents, bool isURL, bool success , const QString& status) { @@ -2345,6 +2371,13 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co processDeferredEntityLoads(entityScript, entityID); } +/**jsdoc + * Triggered when the script terminates for a user. + *

Note: Can only be connected to via this.unoad = function () { ... } in the entity script.

+ *
Available in:Client Entity ScriptsServer Entity Scripts
+ * @function Entities.unload + * @returns {Signal} + */ void ScriptEngine::unloadEntityScript(const EntityItemID& entityID, bool shouldRemoveFromMap) { if (QThread::currentThread() != thread()) { #ifdef THREAD_DEBUGGING From 65f5915372912077c51319937415f7396df5d923 Mon Sep 17 00:00:00 2001 From: David Rowe Date: Tue, 1 May 2018 21:55:27 +1200 Subject: [PATCH 40/47] Add JSDoc for startFarTrigger etc. calls on entity scripts --- .../scripting/ControllerScriptingInterface.h | 57 ++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/interface/src/scripting/ControllerScriptingInterface.h b/interface/src/scripting/ControllerScriptingInterface.h index f19caa8478..4fceda3b04 100644 --- a/interface/src/scripting/ControllerScriptingInterface.h +++ b/interface/src/scripting/ControllerScriptingInterface.h @@ -28,7 +28,7 @@ class ScriptEngine; /**jsdoc * The Controller API provides facilities to interact with computer and controller hardware. * - *
Functions:
+ *
Functions
* *

Properties

*
    @@ -143,6 +143,61 @@ class ScriptEngine; *
  • {@link Controller.stopInputPlayback|stopInputPlayback}
    • *
* + *
Entity Methods:
+ * + *

The default scripts implement hand controller actions that use {@link Entities.callEntityMethod} to call entity script + * methods, if present in the entity being interacted with.

+ * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + * + *
Method NameDescriptionExample
startFarTrigger
continueFarTrigger
stopFarTrigger		These methods are called when a user is more than 0.3m away from the entity, the entity is triggerable, and the + * user starts, continues, or stops squeezing the trigger.A light switch that can be toggled on and off from a distance.
startNearTrigger
continueNearTrigger
stopNearTrigger		These methods are called when a user is less than 0.3m away from the entity, the entity is triggerable, and the + * user starts, continues, or stops squeezing the trigger.A doorbell that can be rung when a user is near.
startDistanceGrab
continueDistanceGrab
These methods are called when a user is more than 0.3m away from the entity, the entity is either cloneable, or + * grabbable and not locked, and the user starts or continues to squeeze the trigger.A comet that emits icy particle trails when a user is dragging it through the sky.
startNearGrab
continueNearGrab
These methods are called when a user is less than 0.3m away from the entity, the entity is either cloneable, or + * grabbable and not locked, and the user starts or continues to squeeze the trigger.A ball that glows when it's being held close.
releaseGrabThis method is called when a user releases the trigger when having been either distance or near grabbing an + * entity.Turn off the ball glow or comet trail with the user finishes grabbing it.
startEquip
continueEquip
releaseEquip		These methods are called when a user starts, continues, or stops equipping an entity.A glass that stays in the user's hand after the trigger is clicked.
+ *

All the entity methods are called with the following two arguments:

+ *
    + *
  • The entity ID.
    • + *
  • A string, "hand,userID" — where "hand" is "left" or "right", and "userID" + * is the user's {@link MyAvatar|MyAvatar.sessionUUID}.
    • + *
+ * * @namespace Controller * * @property {Controller.Actions} Actions - Predefined actions on Interface and the user's avatar. These can be used as end From 2482da3c259752b670fc4f2acbf3c087073b80ad Mon Sep 17 00:00:00 2001 From: David Rowe Date: Tue, 1 May 2018 21:55:45 +1200 Subject: [PATCH 41/47] Miscellaneous JSDoc fixes noticed in passing --- .../controllers/src/controllers/impl/MappingBuilderProxy.h | 6 ++++-- .../controllers/src/controllers/impl/RouteBuilderProxy.h | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/libraries/controllers/src/controllers/impl/MappingBuilderProxy.h b/libraries/controllers/src/controllers/impl/MappingBuilderProxy.h index 86a43c0c13..3c3858e2ba 100644 --- a/libraries/controllers/src/controllers/impl/MappingBuilderProxy.h +++ b/libraries/controllers/src/controllers/impl/MappingBuilderProxy.h @@ -39,7 +39,9 @@ class UserInputMapper; * methods. *
  • Use {@link Controller.parseMapping} or {@link Controller.loadMapping} to load a {@link Controller.MappingJSON}.
    • * - *

    Enable the mapping using {@link MappingObject#enable|enable} or {@link Controller.enableMapping} for it to take effect. + * + *

    Enable the mapping using {@link MappingObject#enable|enable} or {@link Controller.enableMapping} for it to take + * effect.

    * *

    Mappings and their routes are applied according to the following rules:

    *
      @@ -49,7 +51,7 @@ class UserInputMapper; * output that already has a route the new route is ignored. *
    • New mappings override previous mappings: each output is processed using the route in the most recently enabled * mapping that contains that output.
      • - *

      + *
    * * @class MappingObject */ diff --git a/libraries/controllers/src/controllers/impl/RouteBuilderProxy.h b/libraries/controllers/src/controllers/impl/RouteBuilderProxy.h index 0336638068..d33f3e3383 100644 --- a/libraries/controllers/src/controllers/impl/RouteBuilderProxy.h +++ b/libraries/controllers/src/controllers/impl/RouteBuilderProxy.h @@ -29,7 +29,8 @@ class ScriptingInterface; *

    A route in a {@link MappingObject} used by the {@link Controller} API.

    * *

    Create a route using {@link MappingObject} methods and apply this object's methods to process it, terminating with - * {@link RouteObject#to} to apply it to a Standard control, action, or script function.

    + * {@link RouteObject#to} to apply it to a Standard control, action, or script function. Note: Loops are not + * permitted.

    * *

    Some methods apply to routes with number data, some apply routes with {@link Pose} data, and some apply to both route * types.

    From f1e1c6a34817563ac0a9da876a968561361b9ee6 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Tue, 1 May 2018 09:15:44 -0700 Subject: [PATCH 42/47] fix physics related crash-on-exit --- libraries/physics/src/PhysicalEntitySimulation.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index ab7c2ec252..3ea3616b15 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -153,6 +153,8 @@ void PhysicalEntitySimulation::clearEntitiesInternal() { // remove the objects (aka MotionStates) from physics _physicsEngine->removeSetOfObjects(_physicalObjects); + clearOwnershipData(); + // delete the MotionStates for (auto stateItr : _physicalObjects) { EntityMotionState* motionState = static_cast(&(*stateItr)); @@ -165,7 +167,6 @@ void PhysicalEntitySimulation::clearEntitiesInternal() { _physicalObjects.clear(); // clear all other lists specific to this derived class - clearOwnershipData(); _entitiesToRemoveFromPhysics.clear(); _entitiesToAddToPhysics.clear(); _incomingChanges.clear(); From cd76336e6a937128a5b8f227ae57a3e856431914 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Tue, 1 May 2018 09:54:23 -0700 Subject: [PATCH 43/47] Actually fix --- interface/src/commerce/Wallet.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/interface/src/commerce/Wallet.cpp b/interface/src/commerce/Wallet.cpp index 42c8b9973d..35e6ca1c92 100644 --- a/interface/src/commerce/Wallet.cpp +++ b/interface/src/commerce/Wallet.cpp @@ -615,11 +615,14 @@ void Wallet::updateImageProvider() { securityImageProvider->setSecurityImage(_securityImage); // inform tablet security image provider - auto tablet = DependencyManager::get()->getTablet("com.highfidelity.interface.tablet.system"); + TabletProxy* tablet = DependencyManager::get()->getTablet("com.highfidelity.interface.tablet.system"); if (tablet) { - QQmlEngine* tabletEngine = tablet->getTabletSurface()->getSurfaceContext()->engine(); - securityImageProvider = reinterpret_cast(tabletEngine->imageProvider(SecurityImageProvider::PROVIDER_NAME)); - securityImageProvider->setSecurityImage(_securityImage); + OffscreenQmlSurface* tabletSurface = tablet->getTabletSurface(); + if (tabletSurface) { + QQmlEngine* tabletEngine = tabletSurface->getSurfaceContext()->engine(); + securityImageProvider = reinterpret_cast(tabletEngine->imageProvider(SecurityImageProvider::PROVIDER_NAME)); + securityImageProvider->setSecurityImage(_securityImage); + } } } From cad63ad1071e9a3eb7f742ac7543d9107c089682 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 1 May 2018 14:15:12 -0700 Subject: [PATCH 44/47] Revert "Revert "HOTFIX version of PR 12969: Attempt to shutdown web surfaces more consistently"" This reverts commit acb21cc96a459721bf51638bba15855e36892cd2. --- interface/src/Application.cpp | 13 ++++- interface/src/Application.h | 7 ++- interface/src/ui/overlays/Web3DOverlay.cpp | 12 ++-- .../src/RenderableWebEntityItem.cpp | 39 +++++++------ libraries/qml/src/qml/OffscreenSurface.cpp | 6 +- .../qml/src/qml/impl/RenderEventHandler.cpp | 11 ++-- libraries/qml/src/qml/impl/SharedObject.cpp | 56 +++++++++++++------ .../src/AbstractViewStateInterface.h | 19 +++++-- tests/render-perf/src/main.cpp | 6 +- 9 files changed, 101 insertions(+), 68 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 789f60bdb1..c38caca090 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -4750,7 +4750,7 @@ void Application::updateLOD(float deltaTime) const { } } -void Application::pushPostUpdateLambda(void* key, std::function func) { +void Application::pushPostUpdateLambda(void* key, const std::function& func) { std::unique_lock guard(_postUpdateLambdasLock); _postUpdateLambdas[key] = func; } @@ -7360,7 +7360,7 @@ void Application::windowMinimizedChanged(bool minimized) { } } -void Application::postLambdaEvent(std::function f) { +void Application::postLambdaEvent(const std::function& f) { if (this->thread() == QThread::currentThread()) { f(); } else { @@ -7368,6 +7368,15 @@ void Application::postLambdaEvent(std::function f) { } } +void Application::sendLambdaEvent(const std::function& f) { + if (this->thread() == QThread::currentThread()) { + f(); + } else { + LambdaEvent event(f); + QCoreApplication::sendEvent(this, &event); + } +} + void Application::initPlugins(const QStringList& arguments) { QCommandLineOption display("display", "Preferred displays", "displays"); QCommandLineOption disableDisplays("disable-displays", "Displays to disable", "displays"); diff --git a/interface/src/Application.h b/interface/src/Application.h index 769658b0d6..74b0e5a110 100644 --- a/interface/src/Application.h +++ b/interface/src/Application.h @@ -136,7 +136,8 @@ public: Application(int& argc, char** argv, QElapsedTimer& startup_time, bool runningMarkerExisted); ~Application(); - void postLambdaEvent(std::function f) override; + void postLambdaEvent(const std::function& f) override; + void sendLambdaEvent(const std::function& f) override; QString getPreviousScriptLocation(); void setPreviousScriptLocation(const QString& previousScriptLocation); @@ -240,7 +241,7 @@ public: qint64 getCurrentSessionRuntime() const { return _sessionRunTimer.elapsed(); } - bool isAboutToQuit() const override { return _aboutToQuit; } + bool isAboutToQuit() const { return _aboutToQuit; } bool isPhysicsEnabled() const { return _physicsEnabled; } // the isHMDMode is true whenever we use the interface from an HMD and not a standard flat display @@ -264,7 +265,7 @@ public: render::EnginePointer getRenderEngine() override { return _renderEngine; } gpu::ContextPointer getGPUContext() const { return _gpuContext; } - virtual void pushPostUpdateLambda(void* key, std::function func) override; + virtual void pushPostUpdateLambda(void* key, const std::function& func) override; void updateMyAvatarLookAtPosition(); diff --git a/interface/src/ui/overlays/Web3DOverlay.cpp b/interface/src/ui/overlays/Web3DOverlay.cpp index 1e0b50f091..fcdac8c00c 100644 --- a/interface/src/ui/overlays/Web3DOverlay.cpp +++ b/interface/src/ui/overlays/Web3DOverlay.cpp @@ -65,14 +65,10 @@ const QString Web3DOverlay::TYPE = "web3d"; const QString Web3DOverlay::QML = "Web3DOverlay.qml"; static auto qmlSurfaceDeleter = [](OffscreenQmlSurface* surface) { - AbstractViewStateInterface::instance()->postLambdaEvent([surface] { - if (AbstractViewStateInterface::instance()->isAboutToQuit()) { - // WebEngineView may run other threads (wasapi), so they must be deleted for a clean shutdown - // if the application has already stopped its event loop, delete must be explicit - delete surface; - } else { - surface->deleteLater(); - } + AbstractViewStateInterface::instance()->sendLambdaEvent([surface] { + // WebEngineView may run other threads (wasapi), so they must be deleted for a clean shutdown + // if the application has already stopped its event loop, delete must be explicit + delete surface; }); }; diff --git a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp index 3b46c530cc..f333e805ce 100644 --- a/libraries/entities-renderer/src/RenderableWebEntityItem.cpp +++ b/libraries/entities-renderer/src/RenderableWebEntityItem.cpp @@ -25,7 +25,7 @@ #include #include "EntitiesRendererLogging.h" - +#include using namespace render; using namespace render::entities; @@ -45,6 +45,7 @@ static int DEFAULT_MAX_FPS = 10; static int YOUTUBE_MAX_FPS = 30; static QTouchDevice _touchDevice; +static const char* URL_PROPERTY = "url"; WebEntityRenderer::ContentType WebEntityRenderer::getContentType(const QString& urlString) { if (urlString.isEmpty()) { @@ -52,7 +53,7 @@ WebEntityRenderer::ContentType WebEntityRenderer::getContentType(const QString& } const QUrl url(urlString); - if (url.scheme() == "http" || url.scheme() == "https" || + if (url.scheme() == URL_SCHEME_HTTP || url.scheme() == URL_SCHEME_HTTPS || urlString.toLower().endsWith(".htm") || urlString.toLower().endsWith(".html")) { return ContentType::HtmlContent; } @@ -164,6 +165,8 @@ void WebEntityRenderer::doRenderUpdateSynchronousTyped(const ScenePointer& scene if (urlChanged) { if (newContentType != ContentType::HtmlContent || currentContentType != ContentType::HtmlContent) { destroyWebSurface(); + // If we destroyed the surface, the URL change will be implicitly handled by the re-creation + urlChanged = false; } withWriteLock([&] { @@ -185,8 +188,8 @@ void WebEntityRenderer::doRenderUpdateSynchronousTyped(const ScenePointer& scene return; } - if (urlChanged) { - _webSurface->getRootItem()->setProperty("url", _lastSourceUrl); + if (urlChanged && _contentType == ContentType::HtmlContent) { + _webSurface->getRootItem()->setProperty(URL_PROPERTY, _lastSourceUrl); } if (_contextPosition != entity->getWorldPosition()) { @@ -257,6 +260,14 @@ bool WebEntityRenderer::hasWebSurface() { return (bool)_webSurface && _webSurface->getRootItem(); } +static const auto WebSurfaceDeleter = [](OffscreenQmlSurface* webSurface) { + AbstractViewStateInterface::instance()->sendLambdaEvent([webSurface] { + // WebEngineView may run other threads (wasapi), so they must be deleted for a clean shutdown + // if the application has already stopped its event loop, delete must be explicit + delete webSurface; + }); +}; + bool WebEntityRenderer::buildWebSurface(const TypedEntityPointer& entity) { if (_currentWebCount >= MAX_CONCURRENT_WEB_VIEWS) { qWarning() << "Too many concurrent web views to create new view"; @@ -264,20 +275,9 @@ bool WebEntityRenderer::buildWebSurface(const TypedEntityPointer& entity) { } ++_currentWebCount; - auto deleter = [](OffscreenQmlSurface* webSurface) { - AbstractViewStateInterface::instance()->postLambdaEvent([webSurface] { - if (AbstractViewStateInterface::instance()->isAboutToQuit()) { - // WebEngineView may run other threads (wasapi), so they must be deleted for a clean shutdown - // if the application has already stopped its event loop, delete must be explicit - delete webSurface; - } else { - webSurface->deleteLater(); - } - }); - }; // FIXME use the surface cache instead of explicit creation - _webSurface = QSharedPointer(new OffscreenQmlSurface(), deleter); + _webSurface = QSharedPointer(new OffscreenQmlSurface(), WebSurfaceDeleter); // FIXME, the max FPS could be better managed by being dynamic (based on the number of current surfaces // and the current rendering load) _webSurface->setMaxFps(DEFAULT_MAX_FPS); @@ -305,7 +305,7 @@ bool WebEntityRenderer::buildWebSurface(const TypedEntityPointer& entity) { _webSurface->setMaxFps(DEFAULT_MAX_FPS); } _webSurface->load("controls/WebEntityView.qml", [this](QQmlContext* context, QObject* item) { - item->setProperty("url", _lastSourceUrl); + item->setProperty(URL_PROPERTY, _lastSourceUrl); }); } else if (_contentType == ContentType::QmlContent) { _webSurface->load(_lastSourceUrl, [this](QQmlContext* context, QObject* item) { @@ -330,6 +330,11 @@ void WebEntityRenderer::destroyWebSurface() { if (webSurface) { --_currentWebCount; QQuickItem* rootItem = webSurface->getRootItem(); + // Explicitly set the web URL to an empty string, in an effort to get a + // faster shutdown of any chromium processes interacting with audio + if (rootItem && _contentType == ContentType::HtmlContent) { + rootItem->setProperty(URL_PROPERTY, ""); + } if (rootItem && rootItem->objectName() == "tabletRoot") { auto tabletScriptingInterface = DependencyManager::get(); diff --git a/libraries/qml/src/qml/OffscreenSurface.cpp b/libraries/qml/src/qml/OffscreenSurface.cpp index 2da1c41340..688f3fdb5f 100644 --- a/libraries/qml/src/qml/OffscreenSurface.cpp +++ b/libraries/qml/src/qml/OffscreenSurface.cpp @@ -66,14 +66,10 @@ OffscreenSurface::OffscreenSurface() } OffscreenSurface::~OffscreenSurface() { - disconnect(qApp); - _sharedObject->destroy(); + delete _sharedObject; } bool OffscreenSurface::fetchTexture(TextureAndFence& textureAndFence) { - if (!_sharedObject) { - return false; - } hifi::qml::impl::TextureAndFence typedTextureAndFence; bool result = _sharedObject->fetchTexture(typedTextureAndFence); textureAndFence = typedTextureAndFence; diff --git a/libraries/qml/src/qml/impl/RenderEventHandler.cpp b/libraries/qml/src/qml/impl/RenderEventHandler.cpp index 6b66ce9314..945a469611 100644 --- a/libraries/qml/src/qml/impl/RenderEventHandler.cpp +++ b/libraries/qml/src/qml/impl/RenderEventHandler.cpp @@ -49,8 +49,8 @@ RenderEventHandler::RenderEventHandler(SharedObject* shared, QThread* targetThre qFatal("Unable to create new offscreen GL context"); } - moveToThread(targetThread); _canvas.moveToThreadWithContext(targetThread); + moveToThread(targetThread); } void RenderEventHandler::onInitalize() { @@ -160,11 +160,8 @@ void RenderEventHandler::onQuit() { } _shared->shutdownRendering(_canvas, _currentSize); - // Release the reference to the shared object. This will allow it to - // be destroyed (should happen on it's own thread). - _shared->deleteLater(); - - deleteLater(); - + _canvas.doneCurrent(); + _canvas.moveToThreadWithContext(qApp->thread()); + moveToThread(qApp->thread()); QThread::currentThread()->quit(); } diff --git a/libraries/qml/src/qml/impl/SharedObject.cpp b/libraries/qml/src/qml/impl/SharedObject.cpp index b2057117f6..9253c41b39 100644 --- a/libraries/qml/src/qml/impl/SharedObject.cpp +++ b/libraries/qml/src/qml/impl/SharedObject.cpp @@ -72,26 +72,35 @@ SharedObject::SharedObject() { QObject::connect(qApp, &QCoreApplication::aboutToQuit, this, &SharedObject::onAboutToQuit); } + SharedObject::~SharedObject() { - if (_quickWindow) { - _quickWindow->destroy(); - _quickWindow = nullptr; + // After destroy returns, the rendering thread should be gone + destroy(); + + // _renderTimer is created with `this` as the parent, so need no explicit destruction + + // Destroy the event hand + if (_renderObject) { + delete _renderObject; + _renderObject = nullptr; } if (_renderControl) { - _renderControl->deleteLater(); + delete _renderControl; _renderControl = nullptr; } - if (_renderThread) { - _renderThread->quit(); - _renderThread->deleteLater(); + if (_quickWindow) { + _quickWindow->destroy(); + delete _quickWindow; + _quickWindow = nullptr; } - if (_rootItem) { - _rootItem->deleteLater(); - _rootItem = nullptr; - } + // _rootItem is parented to the quickWindow, so needs no explicit destruction + //if (_rootItem) { + // delete _rootItem; + // _rootItem = nullptr; + //} releaseEngine(_qmlContext->engine()); } @@ -119,6 +128,10 @@ void SharedObject::create(OffscreenSurface* surface) { } void SharedObject::setRootItem(QQuickItem* rootItem) { + if (_quit) { + return; + } + _rootItem = rootItem; _rootItem->setSize(_quickWindow->size()); @@ -127,7 +140,6 @@ void SharedObject::setRootItem(QQuickItem* rootItem) { _renderThread->setObjectName(objectName()); _renderThread->start(); - // Create event handler for the render thread _renderObject = new RenderEventHandler(this, _renderThread); QCoreApplication::postEvent(this, new OffscreenEvent(OffscreenEvent::Initialize)); @@ -137,35 +149,43 @@ void SharedObject::setRootItem(QQuickItem* rootItem) { } void SharedObject::destroy() { + // `destroy` is idempotent, it can be called multiple times without issues if (_quit) { return; } if (!_rootItem) { - deleteLater(); return; } - _paused = true; if (_renderTimer) { + _renderTimer->stop(); QObject::disconnect(_renderTimer); - _renderTimer->deleteLater(); } - QObject::disconnect(_renderControl); + if (_renderControl) { + QObject::disconnect(_renderControl); + } + QObject::disconnect(qApp); { QMutexLocker lock(&_mutex); _quit = true; - QCoreApplication::postEvent(_renderObject, new OffscreenEvent(OffscreenEvent::Quit), Qt::HighEventPriority); + if (_renderObject) { + QCoreApplication::postEvent(_renderObject, new OffscreenEvent(OffscreenEvent::Quit), Qt::HighEventPriority); + } } // Block until the rendering thread has stopped // FIXME this is undesirable because this is blocking the main thread, // but I haven't found a reliable way to do this only at application // shutdown - _renderThread->wait(); + if (_renderThread) { + _renderThread->wait(); + delete _renderThread; + _renderThread = nullptr; + } } diff --git a/libraries/render-utils/src/AbstractViewStateInterface.h b/libraries/render-utils/src/AbstractViewStateInterface.h index 96e9f4d222..54fdc903ca 100644 --- a/libraries/render-utils/src/AbstractViewStateInterface.h +++ b/libraries/render-utils/src/AbstractViewStateInterface.h @@ -37,15 +37,25 @@ public: virtual glm::vec3 getAvatarPosition() const = 0; - virtual bool isAboutToQuit() const = 0; - virtual void postLambdaEvent(std::function f) = 0; + // Unfortunately, having this here is a bad idea. Lots of objects connect to + // the aboutToQuit signal, and it's impossible to know the order in which + // the receivers will be called, so this might return false negatives + //virtual bool isAboutToQuit() const = 0; + + // Queue code to execute on the main thread. + // If called from the main thread, the lambda will execute synchronously + virtual void postLambdaEvent(const std::function& f) = 0; + // Synchronously execute code on the main thread. This function will + // not return until the code is executed, regardles of which thread it + // is called from + virtual void sendLambdaEvent(const std::function& f) = 0; virtual qreal getDevicePixelRatio() = 0; virtual render::ScenePointer getMain3DScene() = 0; virtual render::EnginePointer getRenderEngine() = 0; - virtual void pushPostUpdateLambda(void* key, std::function func) = 0; + virtual void pushPostUpdateLambda(void* key, const std::function& func) = 0; virtual bool isHMDMode() const = 0; @@ -54,5 +64,4 @@ public: static void setInstance(AbstractViewStateInterface* instance); }; - -#endif // hifi_AbstractViewStateInterface_h +#endif // hifi_AbstractViewStateInterface_h diff --git a/tests/render-perf/src/main.cpp b/tests/render-perf/src/main.cpp index 93672cc5a2..9249b3d957 100644 --- a/tests/render-perf/src/main.cpp +++ b/tests/render-perf/src/main.cpp @@ -453,8 +453,8 @@ protected: return vec3(); } - bool isAboutToQuit() const override { return false; } - void postLambdaEvent(std::function f) override {} + void postLambdaEvent(const std::function& f) override {} + void sendLambdaEvent(const std::function& f) override {} qreal getDevicePixelRatio() override { return 1.0f; @@ -469,7 +469,7 @@ protected: } std::map> _postUpdateLambdas; - void pushPostUpdateLambda(void* key, std::function func) override { + void pushPostUpdateLambda(void* key, const std::function& func) override { _postUpdateLambdas[key] = func; } From 0627099667378534ca182f3874e4a2e6df70fe79 Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 1 May 2018 14:33:48 -0700 Subject: [PATCH 45/47] In packetSourceAndHashMatchAndTrackBandwidth() check LocalID after check for DS source Makes check more robust in case domain server itself reaches this point, since LimitedNodeList::getDomainLocalID() debug-asserts. --- libraries/networking/src/LimitedNodeList.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 31500be682..af26a2ea72 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -314,9 +314,9 @@ bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packe QUuid sourceID = sourceNode ? sourceNode->getUUID() : QUuid(); if (!sourceNode && - sourceLocalID == getDomainLocalID() && packet.getSenderSockAddr() == getDomainSockAddr() && - PacketTypeEnum::getDomainSourcedPackets().contains(headerType)) { + PacketTypeEnum::getDomainSourcedPackets().contains(headerType) && + sourceLocalID == getDomainLocalID()) { // This is a packet sourced by the domain server emit dataReceived(NodeType::Unassigned, packet.getPayloadSize()); From bbcb07d9ab7aaea8b88bd1e79c00d2604bdfd91c Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 1 May 2018 15:07:44 -0700 Subject: [PATCH 46/47] Brute force it with isDomainServer() --- libraries/networking/src/LimitedNodeList.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index af26a2ea72..e897fd13e0 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -314,9 +314,10 @@ bool LimitedNodeList::packetSourceAndHashMatchAndTrackBandwidth(const udt::Packe QUuid sourceID = sourceNode ? sourceNode->getUUID() : QUuid(); if (!sourceNode && + !isDomainServer() && + sourceLocalID == getDomainLocalID() && packet.getSenderSockAddr() == getDomainSockAddr() && - PacketTypeEnum::getDomainSourcedPackets().contains(headerType) && - sourceLocalID == getDomainLocalID()) { + PacketTypeEnum::getDomainSourcedPackets().contains(headerType)) { // This is a packet sourced by the domain server emit dataReceived(NodeType::Unassigned, packet.getPayloadSize()); From 26ffb3213cce98f585c979268afc381527395173 Mon Sep 17 00:00:00 2001 From: Oren Hurvitz Date: Wed, 2 May 2018 09:01:11 +0300 Subject: [PATCH 47/47] Changed "[[gnu::fallthrough]]" to "// FALLTHRU" --- libraries/audio/src/InboundAudioStream.cpp | 4 +--- libraries/entities/src/EntityTree.cpp | 4 +--- libraries/networking/src/ResourceCache.cpp | 8 ++------ libraries/shared/src/Gzip.cpp | 4 +--- 4 files changed, 5 insertions(+), 15 deletions(-) diff --git a/libraries/audio/src/InboundAudioStream.cpp b/libraries/audio/src/InboundAudioStream.cpp index bbaedc003b..d60c5ba4ab 100644 --- a/libraries/audio/src/InboundAudioStream.cpp +++ b/libraries/audio/src/InboundAudioStream.cpp @@ -149,10 +149,8 @@ int InboundAudioStream::parseData(ReceivedMessage& message) { lostAudioData(packetsDropped); // fall through to OnTime case -#if defined(__GNUC__) - [[gnu::fallthrough]]; -#endif } + // FALLTHRU case SequenceNumberStats::OnTime: { // Packet is on time; parse its data to the ringbuffer if (message.getType() == PacketType::SilentAudioFrame diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index e9e4f2631a..3149527216 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1402,9 +1402,7 @@ int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned c case PacketType::EntityAdd: isAdd = true; // fall through to next case -#if defined(__GNUC__) - [[gnu::fallthrough]]; -#endif + // FALLTHRU case PacketType::EntityPhysics: case PacketType::EntityEdit: { quint64 startDecode = 0, endDecode = 0; diff --git a/libraries/networking/src/ResourceCache.cpp b/libraries/networking/src/ResourceCache.cpp index 3b8c9fdac0..d3583687e8 100644 --- a/libraries/networking/src/ResourceCache.cpp +++ b/libraries/networking/src/ResourceCache.cpp @@ -749,10 +749,8 @@ bool Resource::handleFailedRequest(ResourceRequest::Result result) { case ResourceRequest::Result::Timeout: { qCDebug(networking) << "Timed out loading" << _url << "received" << _bytesReceived << "total" << _bytesTotal; // Fall through to other cases -#if defined(__GNUC__) - [[gnu::fallthrough]]; -#endif } + // FALLTHRU case ResourceRequest::Result::ServerUnavailable: { _attempts++; _attemptsRemaining--; @@ -770,10 +768,8 @@ bool Resource::handleFailedRequest(ResourceRequest::Result result) { break; } // fall through to final failure -#if defined(__GNUC__) - [[gnu::fallthrough]]; -#endif } + // FALLTHRU default: { _attemptsRemaining = 0; qCDebug(networking) << "Error loading " << _url << "attempt:" << _attempts << "attemptsRemaining:" << _attemptsRemaining; diff --git a/libraries/shared/src/Gzip.cpp b/libraries/shared/src/Gzip.cpp index 44fe13f439..25e214fffb 100644 --- a/libraries/shared/src/Gzip.cpp +++ b/libraries/shared/src/Gzip.cpp @@ -60,9 +60,7 @@ bool gunzip(QByteArray source, QByteArray &destination) { switch (status) { case Z_NEED_DICT: status = Z_DATA_ERROR; -#if defined(__GNUC__) - [[gnu::fallthrough]]; -#endif + // FALLTHRU case Z_DATA_ERROR: case Z_MEM_ERROR: case Z_STREAM_ERROR: