diff --git a/CHANGES.md b/CHANGES.md index a7497e57c406..23f5e58cbf33 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,10 +4,17 @@ ### @cesium/engine +#### Breaking Changes :mega: + +- `scene.drillPick` now uses a breadth-first search strategy instead of depth-first. This may change which entities are picked when + using large values of `width` and `height` when providing a `limit`, prioritizing entities closer to the camera. + #### Fixes :wrench: - Fixes an event bug following recent changes, where adding a new listener during an event callback caused an infinite loop. [#12955](https://github.com/CesiumGS/cesium/pull/12955) - Fix issues with label background when updating properties while `label.show` is `false`. [#12138](https://github.com/CesiumGS/cesium/issues/12138) +- Improved performance of `scene.drillPick`. [#12916](https://github.com/CesiumGS/cesium/pull/12916) +- Improved performance when removing primitives. [#3018](https://github.com/CesiumGS/cesium/pull/3018) ## 1.134 - 2025-10-01 diff --git a/Specs/TypeScript/tsconfig.json b/Specs/TypeScript/tsconfig.json index 0438045af17b..4aa56c499da9 100644 --- a/Specs/TypeScript/tsconfig.json +++ b/Specs/TypeScript/tsconfig.json @@ -3,7 +3,8 @@ "noEmit": true, "types": [], "strict": true, - "isolatedModules": true + "isolatedModules": true, + "lib": ["ES2020", "DOM"] }, "include": [ "index.ts", diff --git a/Tools/jsdoc/tsconfig.json b/Tools/jsdoc/tsconfig.json index 2cd41ea62053..36d739f37577 100644 --- a/Tools/jsdoc/tsconfig.json +++ b/Tools/jsdoc/tsconfig.json @@ -1,7 +1,8 @@ { "compilerOptions": { - "noEmit": true, - "types": [] + "noEmit": true, + "types": [], + "lib": ["ES2020", "DOM"] }, "files": [ "../../Source/Cesium.d.ts" diff --git a/packages/engine/Source/Core/Color.js b/packages/engine/Source/Core/Color.js index c9a433d1eebd..558ac668e9b9 100644 --- a/packages/engine/Source/Core/Color.js +++ b/packages/engine/Source/Core/Color.js @@ -679,6 +679,23 @@ Color.prototype.toBytes = function (result) { return result; }; +/** + * Converts RGBA values in bytes to a single numeric unsigned 32-bit RGBA value, using the endianness + * of the system. + * + * @returns {number} A single numeric unsigned 32-bit RGBA value. + * + * @see Color.toRgba + */ +Color.bytesToRgba = function (red, green, blue, alpha) { + // scratchUint32Array and scratchUint8Array share an underlying array buffer + scratchUint8Array[0] = red; + scratchUint8Array[1] = green; + scratchUint8Array[2] = blue; + scratchUint8Array[3] = alpha; + return scratchUint32Array[0]; +}; + /** * Converts this color to a single numeric unsigned 32-bit RGBA value, using the endianness * of the system. @@ -692,12 +709,12 @@ Color.prototype.toBytes = function (result) { * @see Color.fromRgba */ Color.prototype.toRgba = function () { - // scratchUint32Array and scratchUint8Array share an underlying array buffer - scratchUint8Array[0] = Color.floatToByte(this.red); - scratchUint8Array[1] = Color.floatToByte(this.green); - scratchUint8Array[2] = Color.floatToByte(this.blue); - scratchUint8Array[3] = Color.floatToByte(this.alpha); - return scratchUint32Array[0]; + return Color.bytesToRgba( + Color.floatToByte(this.red), + Color.floatToByte(this.green), + Color.floatToByte(this.blue), + Color.floatToByte(this.alpha), + ); }; /** diff --git a/packages/engine/Source/Renderer/Context.js b/packages/engine/Source/Renderer/Context.js index c6562f27f6d6..b5d4d83a2a2f 100644 --- a/packages/engine/Source/Renderer/Context.js +++ b/packages/engine/Source/Renderer/Context.js @@ -356,7 +356,7 @@ function Context(canvas, options) { this._vertexAttribDivisors.push(0); } - this._pickObjects = {}; + this._pickObjects = new Map(); this._nextPickColor = new Uint32Array(1); /** @@ -1562,7 +1562,7 @@ Context.prototype.createViewportQuadCommand = function ( /** * Gets the object associated with a pick color. * - * @param {Color} pickColor The pick color. + * @param {number} pickColor The unsigned 32-bit RGBA pick color * @returns {object} The object associated with the pick color, or undefined if no object is associated with that color. * * @example @@ -1575,9 +1575,15 @@ Context.prototype.getObjectByPickColor = function (pickColor) { Check.defined("pickColor", pickColor); //>>includeEnd('debug'); - return this._pickObjects[pickColor.toRgba()]; + return this._pickObjects.get(pickColor); }; +/** + * + * @param {Map} pickObjects + * @param {number} key + * @param {Color} color + */ function PickId(pickObjects, key, color) { this._pickObjects = pickObjects; this.key = key; @@ -1587,16 +1593,16 @@ function PickId(pickObjects, key, color) { Object.defineProperties(PickId.prototype, { object: { get: function () { - return this._pickObjects[this.key]; + return this._pickObjects.get(this.key); }, set: function (value) { - this._pickObjects[this.key] = value; + this._pickObjects.set(this.key, value); }, }, }); PickId.prototype.destroy = function () { - delete this._pickObjects[this.key]; + this._pickObjects.delete(this.key); return undefined; }; @@ -1633,7 +1639,7 @@ Context.prototype.createPickId = function (object) { throw new RuntimeError("Out of unique Pick IDs."); } - this._pickObjects[key] = object; + this._pickObjects.set(key, object); return new PickId(this._pickObjects, key, Color.fromRgba(key)); }; diff --git a/packages/engine/Source/Scene/PickFramebuffer.js b/packages/engine/Source/Scene/PickFramebuffer.js index bb9cb777d5fd..b6884933b0d2 100644 --- a/packages/engine/Source/Scene/PickFramebuffer.js +++ b/packages/engine/Source/Scene/PickFramebuffer.js @@ -48,15 +48,14 @@ PickFramebuffer.prototype.begin = function (screenSpaceRectangle, viewport) { return this._passState; }; -const colorScratchForPickFramebuffer = new Color(); - /** - * Return the picked object rendered within a given rectangle. + * Return the picked objects rendered within a given rectangle. * * @param {BoundingRectangle} screenSpaceRectangle - * @returns {object|undefined} The object rendered in the middle of the rectangle, or undefined if nothing was rendered. + * @param {number} [limit=1] If supplied, stop iterating after collecting this many objects. + * @returns {object[]} A list of rendered objects, ordered by distance to the middle of the rectangle. */ -PickFramebuffer.prototype.end = function (screenSpaceRectangle) { +PickFramebuffer.prototype.end = function (screenSpaceRectangle, limit = 1) { const width = screenSpaceRectangle.width ?? 1.0; const height = screenSpaceRectangle.height ?? 1.0; @@ -84,6 +83,7 @@ PickFramebuffer.prototype.end = function (screenSpaceRectangle) { // The region does not have to square and the dimensions do not have to be odd, but // loop iterations would be wasted. Prefer square regions where the size is odd. + const objects = new Set(); for (let i = 0; i < length; ++i) { if ( -halfWidth <= x && @@ -93,22 +93,19 @@ PickFramebuffer.prototype.end = function (screenSpaceRectangle) { ) { const index = 4 * ((halfHeight - y) * width + x + halfWidth); - colorScratchForPickFramebuffer.red = Color.byteToFloat(pixels[index]); - colorScratchForPickFramebuffer.green = Color.byteToFloat( + const pickColor = Color.bytesToRgba( + pixels[index], pixels[index + 1], - ); - colorScratchForPickFramebuffer.blue = Color.byteToFloat( pixels[index + 2], - ); - colorScratchForPickFramebuffer.alpha = Color.byteToFloat( pixels[index + 3], ); - const object = context.getObjectByPickColor( - colorScratchForPickFramebuffer, - ); + const object = context.getObjectByPickColor(pickColor); if (defined(object)) { - return object; + objects.add(object); + if (objects.size >= limit) { + break; + } } } @@ -123,8 +120,7 @@ PickFramebuffer.prototype.end = function (screenSpaceRectangle) { x += dx; y += dy; } - - return undefined; + return [...objects]; }; /** diff --git a/packages/engine/Source/Scene/Picking.js b/packages/engine/Source/Scene/Picking.js index e2c581abcc7f..02fda9302065 100644 --- a/packages/engine/Source/Scene/Picking.js +++ b/packages/engine/Source/Scene/Picking.js @@ -262,9 +262,9 @@ function computePickingDrawingBufferRectangle( } /** - * Returns an object with a primitive property that contains the first (top) primitive in the scene - * at a particular window coordinate or undefined if nothing is at the location. Other properties may - * potentially be set depending on the type of primitive and may be used to further identify the picked object. + * Returns a list of objects with a primitive property that contains the first (top) primitives + * in the scene at a particular window coordinate. Other properties may potentially be set depending on the + * type of primitive and may be used to further identify the picked object. *

* When a feature of a 3D Tiles tileset is picked, pick returns a {@link Cesium3DTileFeature} object. *

@@ -272,9 +272,16 @@ function computePickingDrawingBufferRectangle( * @param {Cartesian2} windowPosition Window coordinates to perform picking on. * @param {number} [width=3] Width of the pick rectangle. * @param {number} [height=3] Height of the pick rectangle. - * @returns {object | undefined} Object containing the picked primitive. + * @param {number} [limit=1] If supplied, stop iterating after collecting this many objects. + * @returns {object[]} List of objects containing the picked primitives. */ -Picking.prototype.pick = function (scene, windowPosition, width, height) { +Picking.prototype.pick = function ( + scene, + windowPosition, + width, + height, + limit = 1, +) { //>>includeStart('debug', pragmas.debug); Check.defined("windowPosition", windowPosition); //>>includeEnd('debug'); @@ -328,9 +335,9 @@ Picking.prototype.pick = function (scene, windowPosition, width, height) { scene.updateAndExecuteCommands(passState, scratchColorZero); scene.resolveFramebuffers(passState); - const object = pickFramebuffer.end(drawingBufferRectangle); + const pickedObjects = pickFramebuffer.end(drawingBufferRectangle, limit); context.endFrame(); - return object; + return pickedObjects; }; /** @@ -711,39 +718,41 @@ Picking.prototype.pickPosition = function (scene, windowPosition, result) { return result; }; -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. - let i; - let attributes; - const result = []; - const pickedPrimitives = []; - const pickedAttributes = []; - const pickedFeatures = []; - if (!defined(limit)) { - limit = Number.MAX_VALUE; - } - - let pickedResult = pickCallback(); - while (defined(pickedResult)) { +/** + * @param {object[]} pickedResults the results from the pickCallback + * @param {number} limit If supplied, stop drilling after collecting this many picks. + * @param {object[]} results + * @param {object[]} pickedPrimitives + * @param {object[]} pickedAttributes + * @param {object[]} pickedFeatures + * @returns {boolean} whether picking should end + */ +function addDrillPickedResults( + pickedResults, + limit, + results, + pickedPrimitives, + pickedAttributes, + pickedFeatures, +) { + for (const pickedResult of pickedResults) { const object = pickedResult.object; const position = pickedResult.position; const exclude = pickedResult.exclude; if (defined(position) && !defined(object)) { - result.push(pickedResult); - break; + results.push(pickedResult); + return true; } if (!defined(object) || !defined(object.primitive)) { - break; + return true; } if (!exclude) { - result.push(pickedResult); - if (0 >= --limit) { - break; + results.push(pickedResult); + if (results.length >= limit) { + return true; } } @@ -753,7 +762,7 @@ function drillPick(limit, pickCallback) { // If the picked object has a show attribute, use it. if (typeof primitive.getGeometryInstanceAttributes === "function") { if (defined(object.id)) { - attributes = primitive.getGeometryInstanceAttributes(object.id); + const attributes = primitive.getGeometryInstanceAttributes(object.id); if (defined(attributes) && defined(attributes.show)) { hasShowAttribute = true; attributes.show = ShowGeometryInstanceAttribute.toValue( @@ -776,28 +785,61 @@ function drillPick(limit, pickCallback) { primitive.show = false; pickedPrimitives.push(primitive); } + } +} - pickedResult = pickCallback(); +/** + * Drill pick by repeatedly calling a given `pickCallback`, each time stripping away the previously picked objects. + * @param {function(number): object[]} pickCallback Pick callback to execute each iteration + * @param {number} [limit=Number.MAX_VALUE] If supplied, stop drilling after collecting this many picks + * @returns {object[]} List of picked results + */ +function drillPick(pickCallback, limit) { + // 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. + const results = []; + const pickedPrimitives = []; + const pickedAttributes = []; + const pickedFeatures = []; + if (!defined(limit)) { + limit = Number.MAX_VALUE; + } + + let pickedResults = pickCallback(limit); + while (defined(pickedResults) && pickedResults.length > 0) { + const complete = addDrillPickedResults( + pickedResults, + limit, + results, + pickedPrimitives, + pickedAttributes, + pickedFeatures, + ); + if (complete) { + break; + } + pickedResults = pickCallback(limit - results.length); } // Unhide everything we hid while drill picking - for (i = 0; i < pickedPrimitives.length; ++i) { + for (let i = 0; i < pickedPrimitives.length; ++i) { pickedPrimitives[i].show = true; } - for (i = 0; i < pickedAttributes.length; ++i) { - attributes = pickedAttributes[i]; + for (let i = 0; i < pickedAttributes.length; ++i) { + const attributes = pickedAttributes[i]; attributes.show = ShowGeometryInstanceAttribute.toValue( true, attributes.show, ); } - for (i = 0; i < pickedFeatures.length; ++i) { + for (let i = 0; i < pickedFeatures.length; ++i) { pickedFeatures[i].show = true; } - return result; + return results; } Picking.prototype.drillPick = function ( @@ -807,21 +849,22 @@ Picking.prototype.drillPick = function ( width, height, ) { - const that = this; - const pickCallback = function () { - const object = that.pick(scene, windowPosition, width, height); - if (defined(object)) { - return { - object: object, - position: undefined, - exclude: false, - }; - } + const pickCallback = (limit) => { + const pickedObjects = this.pick( + scene, + windowPosition, + width, + height, + limit, + ); + return pickedObjects.map((object) => ({ + object: object, + position: undefined, + exclude: false, + })); }; - const objects = drillPick(limit, pickCallback); - return objects.map(function (element) { - return element.object; - }); + const objects = drillPick(pickCallback, limit); + return objects.map((element) => element.object); }; const scratchRight = new Cartesian3(); @@ -1016,7 +1059,8 @@ function getRayIntersection( scene.resolveFramebuffers(passState); let position; - const object = view.pickFramebuffer.end(drawingBufferRectangle); + // Picking one object, result is either [object] or [] + const object = view.pickFramebuffer.end(drawingBufferRectangle, 1)[0]; if (scene.context.depthTexture) { const { frustumCommandsList } = view; @@ -1054,7 +1098,7 @@ function getRayIntersection( } } -function getRayIntersections( +function drillPickFromRay( picking, scene, ray, @@ -1065,7 +1109,7 @@ function getRayIntersections( mostDetailed, ) { const pickCallback = function () { - return getRayIntersection( + const pickResult = getRayIntersection( picking, scene, ray, @@ -1074,8 +1118,9 @@ function getRayIntersections( requirePosition, mostDetailed, ); + return pickResult ? [pickResult] : undefined; }; - return drillPick(limit, pickCallback); + return drillPick(pickCallback, limit); } function pickFromRay( @@ -1087,7 +1132,8 @@ function pickFromRay( requirePosition, mostDetailed, ) { - const results = getRayIntersections( + // Use drillPickFromRay rather than getRayIntersection directly to select the first non-excluded object + const results = drillPickFromRay( picking, scene, ray, @@ -1102,28 +1148,6 @@ function pickFromRay( } } -function drillPickFromRay( - picking, - scene, - ray, - limit, - objectsToExclude, - width, - requirePosition, - mostDetailed, -) { - return getRayIntersections( - picking, - scene, - ray, - limit, - objectsToExclude, - width, - requirePosition, - mostDetailed, - ); -} - function deferPromiseUntilPostRender(scene, promise) { // Resolve promise after scene's postRender in case entities are created when the promise resolves. // Entities can't be created between viewer._onTick and viewer._postRender. diff --git a/packages/engine/Source/Scene/Scene.js b/packages/engine/Source/Scene/Scene.js index f19d80396c8d..16149ca8b23a 100644 --- a/packages/engine/Source/Scene/Scene.js +++ b/packages/engine/Source/Scene/Scene.js @@ -4402,7 +4402,8 @@ Scene.prototype.clampLineWidth = function (width) { * @returns {Object | undefined} Object containing the picked primitive or undefined if nothing is at the location. */ Scene.prototype.pick = function (windowPosition, width, height) { - return this._picking.pick(this, windowPosition, width, height); + // Picking one object, result is either [object] or [] + return this._picking.pick(this, windowPosition, width, height, 1)[0]; }; /** diff --git a/packages/engine/Specs/Renderer/ContextSpec.js b/packages/engine/Specs/Renderer/ContextSpec.js index 7f24b0de96f4..82526e2ecfdc 100644 --- a/packages/engine/Specs/Renderer/ContextSpec.js +++ b/packages/engine/Specs/Renderer/ContextSpec.js @@ -266,7 +266,7 @@ describe( const pickId = context.createPickId(o); expect(pickId).toBeDefined(); - expect(context.getObjectByPickColor(pickId.color)).toBe(o); + expect(context.getObjectByPickColor(pickId.color.toRgba())).toBe(o); }); it("throws when creating a pick ID without an object", function () { diff --git a/packages/engine/Specs/Scene/Model/PickingPipelineStageSpec.js b/packages/engine/Specs/Scene/Model/PickingPipelineStageSpec.js index e1a0a7235c96..fa22f7894d7e 100644 --- a/packages/engine/Specs/Scene/Model/PickingPipelineStageSpec.js +++ b/packages/engine/Specs/Scene/Model/PickingPipelineStageSpec.js @@ -141,7 +141,7 @@ describe( const frameState = scene.frameState; const context = frameState.context; // Reset pick objects. - context._pickObjects = []; + context._pickObjects = new Map(); PickingPipelineStage.process(renderResources, primitive, frameState); @@ -150,8 +150,7 @@ describe( "uniform vec4 czm_pickColor;", ]); - const pickObject = - context._pickObjects[Object.keys(context._pickObjects)[0]]; + const pickObject = context._pickObjects.values().next().value; expect(pickObject).toBe(customPickObject); const uniformMap = renderResources.uniformMap; @@ -182,7 +181,7 @@ describe( const frameState = scene.frameState; const context = frameState.context; // Reset pick objects. - context._pickObjects = []; + context._pickObjects = new Map(); PickingPipelineStage.process(renderResources, primitive, frameState); @@ -191,8 +190,7 @@ describe( "uniform vec4 czm_pickColor;", ]); - const pickObject = - context._pickObjects[Object.keys(context._pickObjects)[0]]; + const pickObject = context._pickObjects.values().next().value; verifyPickObject(pickObject, renderResources); const uniformMap = renderResources.uniformMap; @@ -216,7 +214,7 @@ describe( const frameState = scene.frameState; const context = frameState.context; // Reset pick objects. - context._pickObjects = []; + context._pickObjects = new Map(); PickingPipelineStage.process(renderResources, primitive, frameState); @@ -225,8 +223,7 @@ describe( "uniform vec4 czm_pickColor;", ]); - const pickObject = - context._pickObjects[Object.keys(context._pickObjects)[0]]; + const pickObject = context._pickObjects.values().next().value; verifyPickObject(pickObject, renderResources); const uniformMap = renderResources.uniformMap; @@ -251,7 +248,7 @@ describe( const frameState = scene.frameState; const context = frameState.context; // Reset pick objects. - context._pickObjects = []; + context._pickObjects = new Map(); PickingPipelineStage.process(renderResources, primitive, frameState); @@ -260,8 +257,7 @@ describe( "uniform vec4 czm_pickColor;", ]); - const pickObject = - context._pickObjects[Object.keys(context._pickObjects)[0]]; + const pickObject = context._pickObjects.values().next().value; verifyPickObject(pickObject, renderResources, undefined); expect(pickObject.id).toBe(mockIdObject); @@ -291,7 +287,7 @@ describe( const frameState = scene.frameState; const context = frameState.context; // Reset pick objects. - context._pickObjects = []; + context._pickObjects = new Map(); PickingPipelineStage.process(renderResources, primitive, frameState); @@ -306,7 +302,7 @@ describe( let i = 0; for (const key in context._pickObjects) { if (context._pickObjects.hasOwnProperty(key)) { - const pickObject = context._pickObjects[key]; + const pickObject = context._pickObjects.get(key); verifyPickObject(pickObject, renderResources, i++); } } @@ -353,7 +349,7 @@ describe( const frameState = scene.frameState; const context = frameState.context; // Reset pick objects. - context._pickObjects = []; + context._pickObjects = new Map(); PickingPipelineStage.process(renderResources, primitive, frameState);