From 4b3bfbf303eee17a6d5dd5172ade8f814e1e5d1f Mon Sep 17 00:00:00 2001 From: Sean Lilley Date: Fri, 19 Oct 2018 15:45:29 -0400 Subject: [PATCH 1/4] Cleanup and fixes from #7115 --- CHANGES.md | 2 + Source/Core/Ray.js | 35 ++-- Source/Scene/Cesium3DTile.js | 69 ++++++++ Source/Scene/Cesium3DTileset.js | 82 ++++----- Source/Scene/Cesium3DTilesetTraversal.js | 80 +-------- Source/Scene/PointCloudEyeDomeLighting.js | 4 +- Source/Scene/Scene.js | 163 +++++++++++------- .../Cesium3DTilesInspectorViewModel.js | 2 +- Specs/Core/RaySpec.js | 21 +++ Specs/Scene/Cesium3DTilesetSpec.js | 4 +- Specs/Scene/PickSpec.js | 71 +++++++- 11 files changed, 348 insertions(+), 185 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 40ce73e6a6d2..34d76c63947a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,6 +4,7 @@ Change Log ### 1.51 - 2018-11-01 ##### Additions :tada: +* Added `Ray.clone`. [#7115](https://github.com/AnalyticalGraphicsInc/cesium/pull/7115) * Shrink minified and gzipped Cesium.js by 27 KB (~3.7%) by delay loading seldom-used third-party dependencies. [#7140](https://github.com/AnalyticalGraphicsInc/cesium/pull/7140) * Added WMS-T (time) support in WebMapServiceImageryProvider [#2581](https://github.com/AnalyticalGraphicsInc/cesium/issues/2581) * Added `Transforms.fixedFrameToHeadingPitchRoll`, a helper function for extracting a `HeadingPitchRoll` from a fixed frame transform [#7164](https://github.com/AnalyticalGraphicsInc/cesium/pull/7164) @@ -13,6 +14,7 @@ Change Log ##### Fixes :wrench: * Fixed an issue where `pickPosition` would return incorrect results when called after `sampleHeight` or `clampToHeight`. [#7113](https://github.com/AnalyticalGraphicsInc/cesium/pull/7113) +* Fixed an issue where `sampleHeight` and `clampToHeight` would crash if picking a primitive that doesn't write depth. [#7120](https://github.com/AnalyticalGraphicsInc/cesium/issues/7120) * Fixed crash when updating polyline attributes twice in one frame [#7155](https://github.com/AnalyticalGraphicsInc/cesium/pull/7155) * Fixed a crash when using `BingMapsGeocoderService` [#7143](https://github.com/AnalyticalGraphicsInc/cesium/issues/7143) diff --git a/Source/Core/Ray.js b/Source/Core/Ray.js index e8e202f3bbeb..3aab72cb5625 100644 --- a/Source/Core/Ray.js +++ b/Source/Core/Ray.js @@ -1,13 +1,13 @@ define([ './Cartesian3', + './Check', './defaultValue', - './defined', - './DeveloperError' + './defined' ], function( Cartesian3, + Check, defaultValue, - defined, - DeveloperError) { + defined) { 'use strict'; /** @@ -38,6 +38,25 @@ define([ this.direction = direction; } + /** + * Duplicates a Ray instance. + * + * @param {Ray} ray The ray to duplicate. + * @param {Ray} [result] The object onto which to store the result. + * @returns {Ray} The modified result parameter or a new Ray instance if one was not provided. (Returns undefined if ray is undefined) + */ + Ray.clone = function(ray, result) { + if (!defined(ray)) { + return undefined; + } + if (!defined(result)) { + return new Ray(ray.origin, ray.direction); + } + result.origin = ray.origin; + result.direction = ray.direction; + return result; + }; + /** * Computes the point along the ray given by r(t) = o + t*d, * where o is the origin of the ray and d is the direction. @@ -54,12 +73,8 @@ define([ */ Ray.getPoint = function(ray, t, result) { //>>includeStart('debug', pragmas.debug); - if (!defined(ray)){ - throw new DeveloperError('ray is requred'); - } - if (typeof t !== 'number') { - throw new DeveloperError('t is a required number'); - } + Check.typeOf.object('ray', ray); + Check.typeOf.number('t', t); //>>includeEnd('debug'); if (!defined(result)) { diff --git a/Source/Scene/Cesium3DTile.js b/Source/Scene/Cesium3DTile.js index 113fc0d7acc2..1dd025dc5e65 100644 --- a/Source/Scene/Cesium3DTile.js +++ b/Source/Scene/Cesium3DTile.js @@ -17,6 +17,7 @@ define([ '../Core/Matrix3', '../Core/Matrix4', '../Core/OrientedBoundingBox', + '../Core/OrthographicFrustum', '../Core/Rectangle', '../Core/Request', '../Core/RequestScheduler', @@ -54,6 +55,7 @@ define([ Matrix3, Matrix4, OrientedBoundingBox, + OrthographicFrustum, Rectangle, Request, RequestScheduler, @@ -607,6 +609,63 @@ define([ var scratchJulianDate = new JulianDate(); + /** + * Get the tile's screen space error. + * + * @private + */ + Cesium3DTile.prototype.getScreenSpaceError = function(frameState, useParentGeometricError) { + var tileset = this._tileset; + var parentGeometricError = defined(this.parent) ? this.parent.geometricError : tileset._geometricError; + var geometricError = useParentGeometricError ? parentGeometricError : this.geometricError; + if (geometricError === 0.0) { + // Leaf tiles do not have any error so save the computation + return 0.0; + } + var camera = frameState.camera; + var frustum = camera.frustum; + var context = frameState.context; + var width = context.drawingBufferWidth; + var height = context.drawingBufferHeight; + var error; + if (frameState.mode === SceneMode.SCENE2D || frustum instanceof OrthographicFrustum) { + if (defined(frustum._offCenterFrustum)) { + frustum = frustum._offCenterFrustum; + } + var pixelSize = Math.max(frustum.top - frustum.bottom, frustum.right - frustum.left) / Math.max(width, height); + error = geometricError / pixelSize; + } else { + // Avoid divide by zero when viewer is inside the tile + var distance = Math.max(this._distanceToCamera, CesiumMath.EPSILON7); + var sseDenominator = camera.frustum.sseDenominator; + error = (geometricError * height) / (distance * sseDenominator); + if (tileset.dynamicScreenSpaceError) { + var density = tileset._dynamicScreenSpaceErrorComputedDensity; + var factor = tileset.dynamicScreenSpaceErrorFactor; + var dynamicError = CesiumMath.fog(distance, density) * factor; + error -= dynamicError; + } + } + return error; + }; + /** + * Update the tile's visibility. + * + * @private + */ + Cesium3DTile.prototype.updateVisibility = function(frameState) { + var parent = this.parent; + var parentTransform = defined(parent) ? parent.computedTransform : this._tileset.modelMatrix; + var parentVisibilityPlaneMask = defined(parent) ? parent._visibilityPlaneMask : CullingVolume.MASK_INDETERMINATE; + this.updateTransform(parentTransform); + this._distanceToCamera = this.distanceToTile(frameState); + this._centerZDepth = this.distanceToTileCenter(frameState); + this._screenSpaceError = this.getScreenSpaceError(frameState); + this._visibilityPlaneMask = this.visibility(frameState, parentVisibilityPlaneMask); // Use parent's plane mask to speed up visibility test + this._visible = this._visibilityPlaneMask !== CullingVolume.MASK_OUTSIDE; + this._inRequestVolume = this.insideViewerRequestVolume(frameState); + }; + /** * Update whether the tile has expired. * @@ -846,6 +905,12 @@ define([ return Intersect.INSIDE; } + if (this._visibilityPlaneMask === CullingVolume.MASK_INSIDE) { + // The tile's bounding volume is completely inside the culling volume so + // the content bounding volume must also be inside. + return Intersect.INSIDE; + } + // PERFORMANCE_IDEA: is it possible to burn less CPU on this test since we know the // tile's (not the content's) bounding volume intersects the culling volume? var cullingVolume = frameState.cullingVolume; @@ -1055,6 +1120,10 @@ define([ }; function applyDebugSettings(tile, tileset, frameState) { + if (!frameState.passes.render) { + return; + } + var hasContentBoundingVolume = defined(tile._header.content) && defined(tile._header.content.boundingVolume); var empty = tile.hasEmptyContent || tile.hasTilesetContent; diff --git a/Source/Scene/Cesium3DTileset.js b/Source/Scene/Cesium3DTileset.js index 154d24a78c45..cb9839b98190 100644 --- a/Source/Scene/Cesium3DTileset.js +++ b/Source/Scene/Cesium3DTileset.js @@ -200,7 +200,7 @@ define([ this._modelMatrix = defined(options.modelMatrix) ? Matrix4.clone(options.modelMatrix) : Matrix4.clone(Matrix4.IDENTITY); this._statistics = new Cesium3DTilesetStatistics(); - this._statisticsLastColor = new Cesium3DTilesetStatistics(); + this._statisticsLastRender = new Cesium3DTilesetStatistics(); this._statisticsLastPick = new Cesium3DTilesetStatistics(); this._tilesLoaded = false; @@ -1461,7 +1461,7 @@ define([ destroySubtree(tileset, tile); } else { statistics.decrementLoadCounts(tile.content); - --tileset._statistics.numberOfTilesWithContentReady; + --statistics.numberOfTilesWithContentReady; } } @@ -1689,6 +1689,8 @@ define([ function updateTiles(tileset, frameState) { tileset._styleEngine.applyStyle(tileset, frameState); + var passes = frameState.passes; + var isRender = passes.render; var statistics = tileset._statistics; var commandList = frameState.commandList; var numberOfInitialCommands = commandList.length; @@ -1713,7 +1715,9 @@ define([ tile = selectedTiles[i]; // Raise the tileVisible event before update in case the tileVisible event // handler makes changes that update needs to apply to WebGL resources - tileVisible.raiseEvent(tile); + if (isRender) { + tileVisible.raiseEvent(tile); + } tile.update(tileset, frameState); statistics.incrementSelectionCounts(tile.content); ++statistics.selected; @@ -1723,8 +1727,7 @@ define([ tile.update(tileset, frameState); } - var lengthAfterUpdate = commandList.length; - var addedCommandsLength = lengthAfterUpdate - lengthBeforeUpdate; + var addedCommandsLength = commandList.length - lengthBeforeUpdate; tileset._backfaceCommands.trim(); @@ -1770,22 +1773,26 @@ define([ } // Number of commands added by each update above - statistics.numberOfCommands = (commandList.length - numberOfInitialCommands); + addedCommandsLength = commandList.length - numberOfInitialCommands; + statistics.numberOfCommands = addedCommandsLength; // Only run EDL if simple attenuation is on - if (tileset.pointCloudShading.attenuation && + if (isRender && + tileset.pointCloudShading.attenuation && tileset.pointCloudShading.eyeDomeLighting && (addedCommandsLength > 0)) { tileset._pointCloudEyeDomeLighting.update(frameState, numberOfInitialCommands, tileset.pointCloudShading); } - if (tileset.debugShowGeometricError || tileset.debugShowRenderingStatistics || tileset.debugShowMemoryUsage || tileset.debugShowUrl) { - if (!defined(tileset._tileDebugLabels)) { - tileset._tileDebugLabels = new LabelCollection(); + if (isRender) { + if (tileset.debugShowGeometricError || tileset.debugShowRenderingStatistics || tileset.debugShowMemoryUsage || tileset.debugShowUrl) { + if (!defined(tileset._tileDebugLabels)) { + tileset._tileDebugLabels = new LabelCollection(); + } + updateTileDebugLabels(tileset, frameState); + } else { + tileset._tileDebugLabels = tileset._tileDebugLabels && tileset._tileDebugLabels.destroy(); } - updateTileDebugLabels(tileset, frameState); - } else { - tileset._tileDebugLabels = tileset._tileDebugLabels && tileset._tileDebugLabels.destroy(); } } @@ -1793,7 +1800,6 @@ define([ function destroySubtree(tileset, tile) { var root = tile; - var statistics = tileset._statistics; var stack = scratchStack; stack.push(tile); while (stack.length > 0) { @@ -1805,7 +1811,7 @@ define([ } if (tile !== root) { destroyTile(tileset, tile); - --statistics.numberOfTilesTotal; + --tileset._statistics.numberOfTilesTotal; } } root.children = []; @@ -1844,7 +1850,7 @@ define([ function raiseLoadProgressEvent(tileset, frameState) { var statistics = tileset._statistics; - var statisticsLast = tileset._statisticsLastColor; + var statisticsLast = tileset._statisticsLastRender; var numberOfPendingRequests = statistics.numberOfPendingRequests; var numberOfTilesProcessing = statistics.numberOfTilesProcessing; var lastNumberOfPendingRequest = statisticsLast.numberOfPendingRequests; @@ -1904,12 +1910,11 @@ define([ this._skipLevelOfDetail = this.skipLevelOfDetail && !defined(this._classificationType) && !this._disableSkipLevelOfDetail && !this._allTilesAdditive; - // Do not do out-of-core operations (new content requests, cache removal, - // process new tiles) during the pick pass. + // Do out-of-core operations (new content requests, cache removal, + // process new tiles) only during the render pass. var passes = frameState.passes; var isRender = passes.render; var isPick = passes.pick; - var outOfCore = isRender; var statistics = this._statistics; statistics.clear(); @@ -1918,42 +1923,41 @@ define([ updateDynamicScreenSpaceError(this, frameState); } - if (outOfCore) { + if (isRender) { this._cache.reset(); } - this._requestedTiles.length = 0; Cesium3DTilesetTraversal.selectTiles(this, frameState); - if (outOfCore) { + if (isRender) { requestTiles(this); processTiles(this, frameState); } updateTiles(this, frameState); - if (outOfCore) { + if (isRender) { unloadTiles(this); - } - // Events are raised (added to the afterRender queue) here since promises - // may resolve outside of the update loop that then raise events, e.g., - // model's readyPromise. - raiseLoadProgressEvent(this, frameState); - - // Update last statistics - var statisticsLast = isPick ? this._statisticsLastPick : this._statisticsLastColor; - Cesium3DTilesetStatistics.clone(statistics, statisticsLast); - - if (statistics.selected !== 0) { - var credits = this._credits; - if (defined(credits)) { - var length = credits.length; - for (var i = 0; i < length; i++) { - frameState.creditDisplay.addCredit(credits[i]); + // Events are raised (added to the afterRender queue) here since promises + // may resolve outside of the update loop that then raise events, e.g., + // model's readyPromise. + raiseLoadProgressEvent(this, frameState); + + if (statistics.selected !== 0) { + var credits = this._credits; + if (defined(credits)) { + var length = credits.length; + for (var i = 0; i < length; i++) { + frameState.creditDisplay.addCredit(credits[i]); + } } } } + + // Update last statistics + var statisticsLast = isPick ? this._statisticsLastPick : this._statisticsLastRender; + Cesium3DTilesetStatistics.clone(statistics, statisticsLast); }; /** diff --git a/Source/Scene/Cesium3DTilesetTraversal.js b/Source/Scene/Cesium3DTilesetTraversal.js index 332a729a684a..23c4e52a018f 100644 --- a/Source/Scene/Cesium3DTilesetTraversal.js +++ b/Source/Scene/Cesium3DTilesetTraversal.js @@ -1,27 +1,15 @@ define([ - '../Core/CullingVolume', - '../Core/defaultValue', '../Core/defined', - '../Core/freezeObject', '../Core/Intersect', '../Core/ManagedArray', - '../Core/Math', - '../Core/OrthographicFrustum', './Cesium3DTileOptimizationHint', - './Cesium3DTileRefine', - './SceneMode' + './Cesium3DTileRefine' ], function( - CullingVolume, - defaultValue, defined, - freezeObject, Intersect, ManagedArray, - CesiumMath, - OrthographicFrustum, Cesium3DTileOptimizationHint, - Cesium3DTileRefine, - SceneMode) { + Cesium3DTileRefine) { 'use strict'; /** @@ -59,6 +47,8 @@ define([ var descendantSelectionDepth = 2; Cesium3DTilesetTraversal.selectTiles = function(tileset, frameState) { + tileset._requestedTiles.length = 0; + if (tileset.debugFreezeFrame) { return; } @@ -77,7 +67,7 @@ define([ } // The tileset doesn't meet the SSE requirement, therefore the tree does not need to be rendered - if (getScreenSpaceError(tileset, tileset._geometricError, root, frameState) <= tileset._maximumScreenSpaceError) { + if (root.getScreenSpaceError(frameState, true) <= tileset._maximumScreenSpaceError) { return; } @@ -124,13 +114,8 @@ define([ tileset._emptyTiles.push(tile); } - function contentVisible(tile, frameState) { - return (tile._visibilityPlaneMask === CullingVolume.MASK_INSIDE) || - (tile.contentVisibility(frameState) !== Intersect.OUTSIDE); - } - function selectTile(tileset, tile, frameState) { - if (contentVisible(tile, frameState)) { + if (tile.contentVisibility(frameState) !== Intersect.OUTSIDE) { var tileContent = tile.content; if (tileContent.featurePropertiesDirty) { // A feature's property in this tile changed, the tile needs to be re-styled. @@ -229,42 +214,6 @@ define([ } } - function getScreenSpaceError(tileset, geometricError, tile, frameState) { - if (geometricError === 0.0) { - // Leaf tiles do not have any error so save the computation - return 0.0; - } - - var camera = frameState.camera; - var frustum = camera.frustum; - var context = frameState.context; - var height = context.drawingBufferHeight; - - var error; - if (frameState.mode === SceneMode.SCENE2D || frustum instanceof OrthographicFrustum) { - if (defined(frustum._offCenterFrustum)) { - frustum = frustum._offCenterFrustum; - } - var width = context.drawingBufferWidth; - var pixelSize = Math.max(frustum.top - frustum.bottom, frustum.right - frustum.left) / Math.max(width, height); - error = geometricError / pixelSize; - } else { - // Avoid divide by zero when viewer is inside the tile - var distance = Math.max(tile._distanceToCamera, CesiumMath.EPSILON7); - var sseDenominator = camera.frustum.sseDenominator; - error = (geometricError * height) / (distance * sseDenominator); - - if (tileset.dynamicScreenSpaceError) { - var density = tileset._dynamicScreenSpaceErrorComputedDensity; - var factor = tileset.dynamicScreenSpaceErrorFactor; - var dynamicError = CesiumMath.fog(distance, density) * factor; - error -= dynamicError; - } - } - - return error; - } - function updateVisibility(tileset, tile, frameState) { if (tile._updatedVisibilityFrame === frameState.frameNumber) { // Return early if visibility has already been checked during the traversal. @@ -272,17 +221,7 @@ define([ return; } - var parent = tile.parent; - var parentTransform = defined(parent) ? parent.computedTransform : tileset._modelMatrix; - var parentVisibilityPlaneMask = defined(parent) ? parent._visibilityPlaneMask : CullingVolume.MASK_INDETERMINATE; - - tile.updateTransform(parentTransform); - tile._distanceToCamera = tile.distanceToTile(frameState); - tile._centerZDepth = tile.distanceToTileCenter(frameState); - tile._screenSpaceError = getScreenSpaceError(tileset, tile.geometricError, tile, frameState); - tile._visibilityPlaneMask = tile.visibility(frameState, parentVisibilityPlaneMask); // Use parent's plane mask to speed up visibility test - tile._visible = tile._visibilityPlaneMask !== CullingVolume.MASK_OUTSIDE; - tile._inRequestVolume = tile.insideViewerRequestVolume(frameState); + tile.updateVisibility(frameState); tile._updatedVisibilityFrame = frameState.frameNumber; } @@ -305,8 +244,7 @@ define([ } // Use parent's geometric error with child's box to see if the tile already meet the SSE - var sse = getScreenSpaceError(tileset, parent.geometricError, tile, frameState); - return sse <= tileset._maximumScreenSpaceError; + return tile.getScreenSpaceError(frameState, true) <= tileset._maximumScreenSpaceError; } function updateTileVisibility(tileset, tile, frameState) { @@ -530,7 +468,7 @@ define([ visitTile(tileset, tile, frameState); touchTile(tileset, tile, frameState); tile._refines = refines; - tile._updatedVisibilityFrame = 0; // Reset so visibility is checked during the next pass + tile._updatedVisibilityFrame = 0; // Reset so visibility is checked during the next pass which may use a different camera } } diff --git a/Source/Scene/PointCloudEyeDomeLighting.js b/Source/Scene/PointCloudEyeDomeLighting.js index a2a8de574de5..16d5daa5d46b 100644 --- a/Source/Scene/PointCloudEyeDomeLighting.js +++ b/Source/Scene/PointCloudEyeDomeLighting.js @@ -236,9 +236,7 @@ define([ } PointCloudEyeDomeLighting.prototype.update = function(frameState, commandStart, pointCloudShading) { - var passes = frameState.passes; - var isPick = (passes.pick && !passes.render); - if (!isSupported(frameState.context) || isPick) { + if (!isSupported(frameState.context)) { return; } diff --git a/Source/Scene/Scene.js b/Source/Scene/Scene.js index d094ef78216f..8aa28e34e48c 100644 --- a/Source/Scene/Scene.js +++ b/Source/Scene/Scene.js @@ -776,7 +776,8 @@ define([ this._view = this._defaultView; // Give frameState, camera, and screen space camera controller initial state before rendering - updateFrameState(this, 0.0, JulianDate.now()); + updateFrameNumber(this, 0.0, JulianDate.now()); + updateFrameState(this); this.initializeFrame(); } @@ -869,7 +870,7 @@ define([ }, /** - * Returns true if the pickPosition function is supported. + * Returns true if the {@link Scene#pickPosition} function is supported. * @memberof Scene.prototype * * @type {Boolean} @@ -884,7 +885,7 @@ define([ }, /** - * Returns true if the sampleHeight function is supported. + * Returns true if the {@link Scene#sampleHeight} function is supported. * @memberof Scene.prototype * * @type {Boolean} @@ -899,7 +900,7 @@ define([ }, /** - * Returns true if the clampToHeight function is supported. + * Returns true if the {@link Scene#clampToHeight} function is supported. * @memberof Scene.prototype * * @type {Boolean} @@ -1572,7 +1573,13 @@ define([ passes.offscreen = false; } - function updateFrameState(scene, frameNumber, time) { + function updateFrameNumber(scene, frameNumber, time) { + var frameState = scene._frameState; + frameState.frameNumber = frameNumber; + frameState.time = JulianDate.clone(time, frameState.time); + } + + function updateFrameState(scene) { var camera = scene.camera; var frameState = scene._frameState; @@ -1583,8 +1590,6 @@ define([ frameState.mode = scene._mode; frameState.morphTime = scene.morphTime; frameState.mapProjection = scene.mapProjection; - frameState.frameNumber = frameNumber; - frameState.time = JulianDate.clone(time, frameState.time); frameState.camera = camera; frameState.cullingVolume = camera.frustum.computeCullingVolume(camera.positionWC, camera.directionWC, camera.upWC); frameState.occluder = getOccluder(scene); @@ -3002,7 +3007,7 @@ define([ frameState.creditDisplay.update(); } - function render(scene, time) { + function render(scene) { scene._pickPositionCacheDirty = true; var context = scene.context; @@ -3012,8 +3017,7 @@ define([ var view = scene._defaultView; scene._view = view; - var frameNumber = CesiumMath.incrementWrap(frameState.frameNumber, 15000000.0, 1.0); - updateFrameState(scene, frameNumber, time); + updateFrameState(scene); frameState.passes.render = true; frameState.passes.postProcess = scene.postProcessStages.hasSelected; @@ -3071,9 +3075,9 @@ define([ context.endFrame(); } - function tryAndCatchError(scene, time, functionToExecute) { + function tryAndCatchError(scene, functionToExecute) { try { - functionToExecute(scene, time); + functionToExecute(scene); } catch (error) { scene._renderError.raiseEvent(scene, error); @@ -3094,13 +3098,9 @@ define([ time = JulianDate.now(); } + var frameState = this._frameState; this._jobScheduler.resetBudgets(); - // Update - this._preUpdate.raiseEvent(this, time); - tryAndCatchError(this, time, update); - this._postUpdate.raiseEvent(this, time); - var cameraChanged = this._view.checkForCameraUpdates(this); var shouldRender = !this.requestRenderMode || this._renderRequested || cameraChanged || this._logDepthBufferDirty || (this.mode === SceneMode.MORPHING); if (!shouldRender && defined(this.maximumRenderTimeChange) && defined(this._lastRenderTime)) { @@ -3112,10 +3112,19 @@ define([ this._lastRenderTime = JulianDate.clone(time, this._lastRenderTime); this._renderRequested = false; this._logDepthBufferDirty = false; + var frameNumber = CesiumMath.incrementWrap(frameState.frameNumber, 15000000.0, 1.0); + updateFrameNumber(this, frameNumber, time); + } + + // Update + this._preUpdate.raiseEvent(this, time); + tryAndCatchError(this, update); + this._postUpdate.raiseEvent(this, time); + if (shouldRender) { // Render this._preRender.raiseEvent(this, time); - tryAndCatchError(this, time, render); + tryAndCatchError(this, render); RequestScheduler.update(); } @@ -3302,8 +3311,7 @@ define([ this._jobScheduler.disableThisFrame(); - // Update with previous frame's number and time, assuming that render is called before picking. - updateFrameState(this, frameState.frameNumber, frameState.time); + updateFrameState(this); frameState.cullingVolume = getPickCullingVolume(this, drawingBufferPosition, rectangleWidth, rectangleHeight, viewport); frameState.invertClassification = false; frameState.passes.pick = true; @@ -3323,7 +3331,6 @@ define([ var object = view.pickFramebuffer.end(scratchRectangle); context.endFrame(); - callAfterRenderFunctions(this); return object; }; @@ -3503,16 +3510,7 @@ define([ return result; }; - function isExcluded(object, objectsToExclude) { - if (!defined(objectsToExclude) || objectsToExclude.length === 0) { - return false; - } - return (objectsToExclude.indexOf(object) > -1) || - (objectsToExclude.indexOf(object.primitive) > -1) || - (objectsToExclude.indexOf(object.id) > -1); - } - - function drillPick(limit, pickCallback, objectsToExclude) { + function drillPick(limit, pickCallback) { // PERFORMANCE_IDEA: This function calls each primitive's update for each pass. Instead // we could update the primitive once, and then just execute their commands for each pass, // and cull commands for picked primitives. e.g., base on the command's owner. @@ -3530,6 +3528,7 @@ define([ while (defined(pickedResult)) { var object = pickedResult.object; var position = pickedResult.position; + var exclude = pickedResult.exclude; if (defined(position) && !defined(object)) { result.push(pickedResult); @@ -3540,7 +3539,7 @@ define([ break; } - if (!isExcluded(object, objectsToExclude)) { + if (!exclude) { result.push(pickedResult); if (0 >= --limit) { break; @@ -3619,7 +3618,9 @@ define([ var object = that.pick(windowPosition, width, height); if (defined(object)) { return { - object : object + object : object, + position : undefined, + exclude : false }; } }; @@ -3637,13 +3638,23 @@ define([ var orthogonalAxis = Cartesian3.mostOrthogonalAxis(direction, scratchRight); var right = Cartesian3.cross(direction, orthogonalAxis, scratchRight); var up = Cartesian3.cross(direction, right, scratchUp); + camera.position = ray.origin; camera.direction = direction; camera.up = up; camera.right = right; } - function getRayIntersection(scene, ray) { + function isExcluded(object, objectsToExclude) { + if (!defined(object) || !defined(objectsToExclude) || objectsToExclude.length === 0) { + return false; + } + return (objectsToExclude.indexOf(object) > -1) || + (objectsToExclude.indexOf(object.primitive) > -1) || + (objectsToExclude.indexOf(object.id) > -1); + } + + function getRayIntersection(scene, ray, objectsToExclude, requirePosition) { var context = scene._context; var uniformState = context.uniformState; var frameState = scene._frameState; @@ -3659,8 +3670,7 @@ define([ scene._jobScheduler.disableThisFrame(); - // Update with previous frame's number and time, assuming that render is called before picking. - updateFrameState(scene, frameState.frameNumber, frameState.time); + updateFrameState(scene); frameState.invertClassification = false; frameState.passes.pick = true; frameState.passes.offscreen = true; @@ -3692,17 +3702,17 @@ define([ scene._view = scene._defaultView; context.endFrame(); - callAfterRenderFunctions(scene); if (defined(object) || defined(position)) { return { object : object, - position : position + position : position, + exclude : (!defined(position) && requirePosition) || isExcluded(object, objectsToExclude) }; } } - function getRayIntersections(scene, ray, limit, objectsToExclude) { + function getRayIntersections(scene, ray, limit, objectsToExclude, requirePosition) { //>>includeStart('debug', pragmas.debug); Check.defined('ray', ray); if (scene._mode !== SceneMode.SCENE3D) { @@ -3710,9 +3720,20 @@ define([ } //>>includeEnd('debug'); var pickCallback = function() { - return getRayIntersection(scene, ray); + return getRayIntersection(scene, ray, objectsToExclude, requirePosition); }; - return drillPick(limit, pickCallback, objectsToExclude); + return drillPick(limit, pickCallback); + } + + function pickFromRay(scene, ray, objectsToExclude, requirePosition) { + var results = getRayIntersections(scene, ray, 1, objectsToExclude, requirePosition); + if (results.length > 0) { + return results[0]; + } + } + + function drillPickFromRay(scene, ray, limit, objectsToExclude, requirePosition) { + return getRayIntersections(scene, ray, limit, objectsToExclude, requirePosition); } /** @@ -3720,6 +3741,10 @@ define([ * or undefined if there were no intersections. The intersected object has a primitive * property that contains the intersected primitive. Other properties may be set depending on the type of primitive * and may be used to further identify the picked object. The ray must be given in world coordinates. + *

+ * This function only picks globe tiles and 3D Tiles that are rendered in the current view. Picks all other + * primitives regardless of their visibility. + *

* * @private * @@ -3730,10 +3755,13 @@ define([ * @exception {DeveloperError} Ray intersections are only supported in 3D mode. */ Scene.prototype.pickFromRay = function(ray, objectsToExclude) { - var results = getRayIntersections(this, ray, 1, objectsToExclude); - if (results.length > 0) { - return results[0]; + //>>includeStart('debug', pragmas.debug); + Check.defined('ray', ray); + if (this._mode !== SceneMode.SCENE3D) { + throw new DeveloperError('Ray intersections are only supported in 3D mode.'); } + //>>includeEnd('debug'); + return pickFromRay(this, ray, objectsToExclude, false); }; /** @@ -3742,6 +3770,10 @@ define([ * properties may also be set depending on the type of primitive and may be used to further identify the picked object. * The primitives in the list are ordered by first intersection to last intersection. The ray must be given in * world coordinates. + *

+ * This function only picks globe tiles and 3D Tiles that are rendered in the current view. Picks all other + * primitives regardless of their visibility. + *

* * @private * @@ -3753,7 +3785,13 @@ define([ * @exception {DeveloperError} Ray intersections are only supported in 3D mode. */ Scene.prototype.drillPickFromRay = function(ray, limit, objectsToExclude) { - return getRayIntersections(this, ray, limit, objectsToExclude); + //>>includeStart('debug', pragmas.debug); + Check.defined('ray', ray); + if (this._mode !== SceneMode.SCENE3D) { + throw new DeveloperError('Ray intersections are only supported in 3D mode.'); + } + //>>includeEnd('debug'); + return drillPickFromRay(this, ray, limit, objectsToExclude, false); }; var scratchSurfacePosition = new Cartesian3(); @@ -3783,6 +3821,13 @@ define([ return getRayForSampleHeight(scene, cartographic); } + function getHeightFromCartesian(scene, cartesian) { + var globe = scene.globe; + var ellipsoid = defined(globe) ? globe.ellipsoid : scene.mapProjection.ellipsoid; + var cartographic = Cartographic.fromCartesian(cartesian, ellipsoid, scratchCartographic); + return cartographic.height; + } + /** * Returns the height of scene geometry at the given cartographic position or undefined if there was no * scene geometry to sample height from. May be used to clamp objects to the globe, 3D Tiles, or primitives in the scene. @@ -3797,24 +3842,23 @@ define([ * * @see Scene#clampToHeight * - * @exception {DeveloperError} Ray intersections are only supported in 3D mode. - * @exception {DeveloperError} sampleHeight required depth texture support. Check sampleHeightSupported. + * @exception {DeveloperError} sampleHeight is only supported in 3D mode. + * @exception {DeveloperError} sampleHeight requires depth texture support. Check sampleHeightSupported. */ Scene.prototype.sampleHeight = function(position, objectsToExclude) { //>>includeStart('debug', pragmas.debug); Check.defined('position', position); + if (this._mode !== SceneMode.SCENE3D) { + throw new DeveloperError('sampleHeight is only supported in 3D mode.'); + } if (!this.sampleHeightSupported) { - throw new DeveloperError('sampleHeight required depth texture support. Check sampleHeightSupported.'); + throw new DeveloperError('sampleHeight requires depth texture support. Check sampleHeightSupported.'); } //>>includeEnd('debug'); var ray = getRayForSampleHeight(this, position); - var pickResult = this.pickFromRay(ray, objectsToExclude); + var pickResult = pickFromRay(this, ray, objectsToExclude, true); if (defined(pickResult)) { - var cartesian = pickResult.position; - var globe = this.globe; - var ellipsoid = defined(globe) ? globe.ellipsoid : this.mapProjection.ellipsoid; - var cartographic = Cartographic.fromCartesian(cartesian, ellipsoid, scratchCartographic); - return cartographic.height; + return getHeightFromCartesian(this, pickResult.position); } }; @@ -3834,18 +3878,21 @@ define([ * * @see Scene#sampleHeight * - * @exception {DeveloperError} Ray intersections are only supported in 3D mode. - * @exception {DeveloperError} clampToHeight required depth texture support. Check clampToHeightSupported. + * @exception {DeveloperError} clampToHeight is only supported in 3D mode. + * @exception {DeveloperError} clampToHeight requires depth texture support. Check clampToHeightSupported. */ Scene.prototype.clampToHeight = function(cartesian, objectsToExclude, result) { //>>includeStart('debug', pragmas.debug); Check.defined('cartesian', cartesian); + if (this._mode !== SceneMode.SCENE3D) { + throw new DeveloperError('sampleHeight is only supported in 3D mode.'); + } if (!this.clampToHeightSupported) { - throw new DeveloperError('clampToHeight required depth texture support. Check clampToHeightSupported.'); + throw new DeveloperError('clampToHeight requires depth texture support. Check clampToHeightSupported.'); } //>>includeEnd('debug'); var ray = getRayForClampToHeight(this, cartesian); - var pickResult = this.pickFromRay(ray, objectsToExclude); + var pickResult = pickFromRay(this, ray, objectsToExclude, true); if (defined(pickResult)) { return Cartesian3.clone(pickResult.position, result); } diff --git a/Source/Widgets/Cesium3DTilesInspector/Cesium3DTilesInspectorViewModel.js b/Source/Widgets/Cesium3DTilesInspector/Cesium3DTilesInspectorViewModel.js index d80ba01ddb0e..bca09a0834cb 100644 --- a/Source/Widgets/Cesium3DTilesInspector/Cesium3DTilesInspectorViewModel.js +++ b/Source/Widgets/Cesium3DTilesInspector/Cesium3DTilesInspectorViewModel.js @@ -71,7 +71,7 @@ define([ return ''; } - var statistics = isPick ? tileset._statisticsLastPick : tileset._statisticsLastColor; + var statistics = isPick ? tileset._statisticsLastPick : tileset._statisticsLastRender; // Since the pick pass uses a smaller frustum around the pixel of interest, // the statistics will be different than the normal render pass. diff --git a/Specs/Core/RaySpec.js b/Specs/Core/RaySpec.js index 91ea5084b816..815f6f24070f 100644 --- a/Specs/Core/RaySpec.js +++ b/Specs/Core/RaySpec.js @@ -28,6 +28,27 @@ defineSuite([ expect(ray.direction).toEqual(Cartesian3.UNIT_X); }); + it('clone with a result parameter', function() { + var direction = Cartesian3.normalize(new Cartesian3(1, 2, 3), new Cartesian3()); + var ray = new Ray(Cartesian3.UNIT_X, direction); + var result = new Ray(); + var returnedResult = Ray.clone(ray, result); + expect(ray).not.toBe(result); + expect(result).toBe(returnedResult); + expect(ray).toEqual(result); + }); + + it('clone works with a result parameter that is an input parameter', function() { + var direction = Cartesian3.normalize(new Cartesian3(1, 2, 3), new Cartesian3()); + var ray = new Ray(Cartesian3.UNIT_X, direction); + var returnedResult = Ray.clone(ray, ray); + expect(ray).toBe(returnedResult); + }); + + it('clone returns undefined if ray is undefined', function() { + expect(Ray.clone()).toBeUndefined(); + }); + it('getPoint along ray works without a result parameter', function() { var direction = Cartesian3.normalize(new Cartesian3(1, 2, 3), new Cartesian3()); var ray = new Ray(Cartesian3.UNIT_X, direction); diff --git a/Specs/Scene/Cesium3DTilesetSpec.js b/Specs/Scene/Cesium3DTilesetSpec.js index e68374ff6c5e..f14180e0bc06 100644 --- a/Specs/Scene/Cesium3DTilesetSpec.js +++ b/Specs/Scene/Cesium3DTilesetSpec.js @@ -2095,9 +2095,9 @@ defineSuite([ return Cesium3DTilesTester.loadTileset(scene, withBatchTableUrl).then(function(tileset) { tileset.style = new Cesium3DTileStyle({color: 'color("red")'}); scene.renderForSpecs(); - expect(tileset.statistics.numberOfTilesStyled).toBe(1); + expect(tileset._statisticsLastRender.numberOfTilesStyled).toBe(1); scene.pickForSpecs(); - expect(tileset.statistics.numberOfTilesStyled).toBe(0); + expect(tileset._statisticsLastPick.numberOfTilesStyled).toBe(0); }); }); diff --git a/Specs/Scene/PickSpec.js b/Specs/Scene/PickSpec.js index 34021fbc10ad..9c7b9825ffc5 100644 --- a/Specs/Scene/PickSpec.js +++ b/Specs/Scene/PickSpec.js @@ -18,6 +18,7 @@ defineSuite([ 'Scene/Cesium3DTileStyle', 'Scene/EllipsoidSurfaceAppearance', 'Scene/Globe', + 'Scene/PointPrimitiveCollection', 'Scene/Primitive', 'Scene/Scene', 'Scene/SceneMode', @@ -45,6 +46,7 @@ defineSuite([ Cesium3DTileStyle, EllipsoidSurfaceAppearance, Globe, + PointPrimitiveCollection, Primitive, Scene, SceneMode, @@ -546,6 +548,18 @@ defineSuite([ }, primitiveRay); }); + it('picks primitive that doesn\'t write depth', function() { + var collection = scene.primitives.add(new PointPrimitiveCollection()); + var point = collection.add({ + position : Cartographic.fromRadians(0.0, 0.0, 100.0), + disableDepthTestDistance : Number.POSITIVE_INFINITY + }); + expect(scene).toPickFromRayAndCall(function(result) { + expect(result.object.primitive).toBe(point); + expect(result.position).toBeUndefined(); + }, primitiveRay); + }); + it('throws if ray is undefined', function() { expect(function() { scene.pickFromRay(undefined); @@ -894,6 +908,34 @@ defineSuite([ }, cartographic, [rectangle2, rectangle3]); }); + it('excludes primitive that doesn\'t write depth', function() { + if (!scene.sampleHeightSupported) { + return; + } + + var rectangle = createSmallRectangle(0.0); + var height = 100.0; + var cartographic = new Cartographic(0.0, 0.0, height); + var collection = scene.primitives.add(new PointPrimitiveCollection()); + var point = collection.add({ + position : Cartographic.toCartesian(cartographic) + }); + + expect(scene).toSampleHeightAndCall(function(height) { + expect(height).toEqualEpsilon(height, CesiumMath.EPSILON3); + }, cartographic); + point.disableDepthTestDistance = Number.POSITIVE_INFINITY; + + expect(scene).toSampleHeightAndCall(function(height) { + expect(height).toEqualEpsilon(0.0, CesiumMath.EPSILON3); + }, cartographic); + rectangle.show = false; + + expect(scene).toSampleHeightAndCall(function(height) { + expect(height).toBeUndefined(); + }, cartographic); + }); + it('throws if position is undefined', function() { if (!scene.sampleHeightSupported) { return; @@ -966,7 +1008,7 @@ defineSuite([ }); it('clamps to the globe', function() { - if (!scene.sampleHeightSupported) { + if (!scene.clampToHeightSupported) { return; } @@ -1032,6 +1074,33 @@ defineSuite([ }, cartesian, [rectangle2, rectangle3]); }); + it('excludes primitive that doesn\'t write depth', function() { + if (!scene.clampToHeightSupported) { + return; + } + + var rectangle = createSmallRectangle(0.0); + var cartesian = Cartesian3.fromRadians(0.0, 0.0, 100.0); + var collection = scene.primitives.add(new PointPrimitiveCollection()); + var point = collection.add({ + position : cartesian + }); + + expect(scene).toClampToHeightAndCall(function(clampedCartesian) { + expect(clampedCartesian).toEqualEpsilon(cartesian, CesiumMath.EPSILON3); + }, cartesian); + + point.disableDepthTestDistance = Number.POSITIVE_INFINITY; + expect(scene).toClampToHeightAndCall(function(clampedCartesian) { + expect(clampedCartesian).toEqualEpsilon(cartesian, CesiumMath.EPSILON3); + }, cartesian); + + rectangle.show = false; + expect(scene).toClampToHeightAndCall(function(clampedCartesian) { + expect(clampedCartesian).toBeUndefined(); + }, cartesian); + }); + it('throws if cartesian is undefined', function() { if (!scene.clampToHeightSupported) { return; From 9d69b5a512aac2af3313465d9b7b44a6801facf0 Mon Sep 17 00:00:00 2001 From: Sean Lilley Date: Fri, 19 Oct 2018 18:14:48 -0400 Subject: [PATCH 2/4] Remove pragma from internal function --- Source/Scene/Scene.js | 6 ------ Specs/Scene/PickSpec.js | 4 ++-- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/Source/Scene/Scene.js b/Source/Scene/Scene.js index 8aa28e34e48c..a1bdd0acbef4 100644 --- a/Source/Scene/Scene.js +++ b/Source/Scene/Scene.js @@ -3713,12 +3713,6 @@ define([ } function getRayIntersections(scene, ray, limit, objectsToExclude, requirePosition) { - //>>includeStart('debug', pragmas.debug); - Check.defined('ray', ray); - if (scene._mode !== SceneMode.SCENE3D) { - throw new DeveloperError('Ray intersections are only supported in 3D mode.'); - } - //>>includeEnd('debug'); var pickCallback = function() { return getRayIntersection(scene, ray, objectsToExclude, requirePosition); }; diff --git a/Specs/Scene/PickSpec.js b/Specs/Scene/PickSpec.js index 9c7b9825ffc5..33de07c814fd 100644 --- a/Specs/Scene/PickSpec.js +++ b/Specs/Scene/PickSpec.js @@ -924,13 +924,13 @@ defineSuite([ expect(scene).toSampleHeightAndCall(function(height) { expect(height).toEqualEpsilon(height, CesiumMath.EPSILON3); }, cartographic); - point.disableDepthTestDistance = Number.POSITIVE_INFINITY; + point.disableDepthTestDistance = Number.POSITIVE_INFINITY; expect(scene).toSampleHeightAndCall(function(height) { expect(height).toEqualEpsilon(0.0, CesiumMath.EPSILON3); }, cartographic); - rectangle.show = false; + rectangle.show = false; expect(scene).toSampleHeightAndCall(function(height) { expect(height).toBeUndefined(); }, cartographic); From 9e77d1fd7f8c26c921105c9bdb8437bac9a99ad9 Mon Sep 17 00:00:00 2001 From: Sean Lilley Date: Mon, 22 Oct 2018 16:30:06 -0400 Subject: [PATCH 3/4] Updates --- Source/Core/Ray.js | 4 ++-- Source/Scene/Cesium3DTile.js | 3 ++- Specs/Core/RaySpec.js | 12 ++++++++++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/Source/Core/Ray.js b/Source/Core/Ray.js index 3aab72cb5625..4df665ebceeb 100644 --- a/Source/Core/Ray.js +++ b/Source/Core/Ray.js @@ -52,8 +52,8 @@ define([ if (!defined(result)) { return new Ray(ray.origin, ray.direction); } - result.origin = ray.origin; - result.direction = ray.direction; + result.origin = Cartesian3.clone(ray.origin); + result.direction = Cartesian3.clone(ray.direction); return result; }; diff --git a/Source/Scene/Cesium3DTile.js b/Source/Scene/Cesium3DTile.js index 1dd025dc5e65..03270229eff5 100644 --- a/Source/Scene/Cesium3DTile.js +++ b/Source/Scene/Cesium3DTile.js @@ -648,6 +648,7 @@ define([ } return error; }; + /** * Update the tile's visibility. * @@ -660,7 +661,7 @@ define([ this.updateTransform(parentTransform); this._distanceToCamera = this.distanceToTile(frameState); this._centerZDepth = this.distanceToTileCenter(frameState); - this._screenSpaceError = this.getScreenSpaceError(frameState); + this._screenSpaceError = this.getScreenSpaceError(frameState, false); this._visibilityPlaneMask = this.visibility(frameState, parentVisibilityPlaneMask); // Use parent's plane mask to speed up visibility test this._visible = this._visibilityPlaneMask !== CullingVolume.MASK_OUTSIDE; this._inRequestVolume = this.insideViewerRequestVolume(frameState); diff --git a/Specs/Core/RaySpec.js b/Specs/Core/RaySpec.js index 815f6f24070f..9e861c6d8a11 100644 --- a/Specs/Core/RaySpec.js +++ b/Specs/Core/RaySpec.js @@ -28,12 +28,24 @@ defineSuite([ expect(ray.direction).toEqual(Cartesian3.UNIT_X); }); + it('clone without a result parameter', function() { + var direction = Cartesian3.normalize(new Cartesian3(1, 2, 3), new Cartesian3()); + var ray = new Ray(Cartesian3.UNIT_X, direction); + var returnedResult = Ray.clone(ray); + expect(ray).not.toBe(returnedResult); + expect(ray.origin).not.toBe(returnedResult.origin); + expect(ray.direction).not.toBe(returnedResult.direction); + expect(ray).toEqual(returnedResult); + }); + it('clone with a result parameter', function() { var direction = Cartesian3.normalize(new Cartesian3(1, 2, 3), new Cartesian3()); var ray = new Ray(Cartesian3.UNIT_X, direction); var result = new Ray(); var returnedResult = Ray.clone(ray, result); expect(ray).not.toBe(result); + expect(ray.origin).not.toBe(returnedResult.origin); + expect(ray.direction).not.toBe(returnedResult.direction); expect(result).toBe(returnedResult); expect(ray).toEqual(result); }); From 27763ff98e45b80c81056769d3bd3f3a1b7556e6 Mon Sep 17 00:00:00 2001 From: Sean Lilley Date: Mon, 22 Oct 2018 16:32:19 -0400 Subject: [PATCH 4/4] Update CHANGES.md number --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 34d76c63947a..e91f75e8b551 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,7 +4,7 @@ Change Log ### 1.51 - 2018-11-01 ##### Additions :tada: -* Added `Ray.clone`. [#7115](https://github.com/AnalyticalGraphicsInc/cesium/pull/7115) +* Added `Ray.clone`. [#7174](https://github.com/AnalyticalGraphicsInc/cesium/pull/7174) * Shrink minified and gzipped Cesium.js by 27 KB (~3.7%) by delay loading seldom-used third-party dependencies. [#7140](https://github.com/AnalyticalGraphicsInc/cesium/pull/7140) * Added WMS-T (time) support in WebMapServiceImageryProvider [#2581](https://github.com/AnalyticalGraphicsInc/cesium/issues/2581) * Added `Transforms.fixedFrameToHeadingPitchRoll`, a helper function for extracting a `HeadingPitchRoll` from a fixed frame transform [#7164](https://github.com/AnalyticalGraphicsInc/cesium/pull/7164)