Merge pull request #12958 from Beilinson/fix-billboard-subregion

fix billboard subregion not displayed

Author: mzschwartz5

CHANGES.md

@@ -1,5 +1,13 @@
 # Change Log
 
+## 1.136
+
+### @cesium/engine
+
+#### Fixes :wrench:
+
+- Billboards using `imageSubRegion` now render as expected. [#12585](https://github.com/CesiumGS/cesium/issues/12585)
+
 ## 1.135 - 2025-11-03
 
 ### @cesium/engine

packages/engine/Source/Renderer/TextureAtlas.js

@@ -59,12 +59,16 @@ function TextureAtlas(options) {
   this._initialSize = initialSize;
 
   this._texturePacker = undefined;
+  /** @type {BoundingRectangle[]} */
   this._rectangles = [];
+  /** @type {Map<number, number>} */
   this._subRegions = new Map();
   this._guid = createGuid();
 
   this._imagesToAddQueue = [];
+  /** @type {Map<string, number>} */
   this._indexById = new Map();
+  /** @type {Map<string, Promise<number>>} */
   this._indexPromiseById = new Map();
   this._nextIndex = 0;
 }
@@ -644,7 +648,7 @@ async function resolveImage(image, id) {
  * @param {string} id An identifier to detect whether the image already exists in the atlas.
  * @param {HTMLImageElement|HTMLCanvasElement|string|Resource|Promise|TextureAtlas.CreateImageCallback} image An image or canvas to add to the texture atlas,
  *        or a URL to an Image, or a Promise for an image, or a function that creates an image.
- * @returns {Promise<number>} A Promise that resolves to the image region index. -1 is returned if resouces are in the process of being destroyed.
+ * @returns {Promise<number>} A Promise that resolves to the image region index, or -1 if resources are in the process of being destroyed.
  */
 TextureAtlas.prototype.addImage = function (id, image) {
   //>>includeStart('debug', pragmas.debug);
@@ -653,12 +657,17 @@ TextureAtlas.prototype.addImage = function (id, image) {
   //>>includeEnd('debug');
 
   let promise = this._indexPromiseById.get(id);
+  let index = this._indexById.get(id);
   if (defined(promise)) {
-    // This image has already been added
+    // This image is already being added
     return promise;
   }
+  if (defined(index)) {
+    // This image has already been added and resolved
+    return Promise.resolve(index);
+  }
 
-  const index = this._nextIndex++;
+  index = this._nextIndex++;
   this._indexById.set(id, index);
 
   const resolveAndAddImage = async () => {
@@ -668,57 +677,79 @@ TextureAtlas.prototype.addImage = function (id, image) {
     //>>includeEnd('debug');
 
     if (this.isDestroyed() || !defined(image)) {
+      this._indexPromiseById.delete(id);
       return -1;
     }
 
-    return this._addImage(index, image);
+    const imageIndex = await this._addImage(index, image);
+    this._indexPromiseById.delete(id);
+    return imageIndex;
   };
 
   promise = resolveAndAddImage();
   this._indexPromiseById.set(id, promise);
   return promise;
 };
 
+/**
+ * Get an existing sub-region of an existing atlas image as additional image indices.
+ * @private
+ * @param {string} id The identifier of the existing image.
+ * @param {BoundingRectangle} subRegion An {@link BoundingRectangle} defining a region of an existing image, measured in pixels from the bottom-left of the image.
+ * @param {number} imageIndex The index of the image.
+ * @returns {Promise<number> | number | undefined} The existing subRegion index, or undefined if not yet added.
+ */
+TextureAtlas.prototype.getCachedImageSubRegion = function (
+  id,
+  subRegion,
+  imageIndex,
+) {
+  const imagePromise = this._indexPromiseById.get(id);
+  for (const [index, parentIndex] of this._subRegions.entries()) {
+    if (imageIndex === parentIndex) {
+      const boundingRegion = this._rectangles[index];
+      if (boundingRegion.equals(subRegion)) {
+        // The subregion is already being tracked
+        if (imagePromise) {
+          return imagePromise.then((resolvedImageIndex) =>
+            resolvedImageIndex === -1 ? -1 : index,
+          );
+        }
+        return index;
+      }
+    }
+  }
+};
+
 /**
  * Add a sub-region of an existing atlas image as additional image indices.
  * @private
  * @param {string} id The identifier of the existing image.
  * @param {BoundingRectangle} subRegion An {@link BoundingRectangle} defining a region of an existing image, measured in pixels from the bottom-left of the image.
- * @returns {Promise<number>} A Promise that resolves to the image region index. -1 is returned if resouces are in the process of being destroyed.
+ * @returns {number | Promise<number>} The resolved image region index, or a Promise that resolves to it. -1 is returned if resources are in the process of being destroyed.
  */
 TextureAtlas.prototype.addImageSubRegion = function (id, subRegion) {
   //>>includeStart('debug', pragmas.debug);
   Check.typeOf.string("id", id);
   Check.defined("subRegion", subRegion);
   //>>includeEnd('debug');
-
   const imageIndex = this._indexById.get(id);
   if (!defined(imageIndex)) {
     throw new RuntimeError(`image with id "${id}" not found in the atlas.`);
   }
 
-  const indexPromise = this._indexPromiseById.get(id);
-  for (const [index, parentIndex] of this._subRegions.entries()) {
-    if (imageIndex === parentIndex) {
-      const boundingRegion = this._rectangles[index];
-      if (boundingRegion.equals(subRegion)) {
-        // The subregion is already being tracked
-        return indexPromise.then((resolvedImageIndex) => {
-          if (resolvedImageIndex === -1) {
-            // The atlas has been destroyed
-            return -1;
-          }
-
-          return index;
-        });
-      }
-    }
+  let index = this.getCachedImageSubRegion(id, subRegion, imageIndex);
+  if (defined(index)) {
+    return index;
   }
 
-  const index = this._nextIndex++;
+  index = this._nextIndex++;
   this._subRegions.set(index, imageIndex);
   this._rectangles[index] = subRegion.clone();
 
+  const indexPromise =
+    this._indexPromiseById.get(id) ?? Promise.resolve(imageIndex);
+
   return indexPromise.then((imageIndex) => {
     if (imageIndex === -1) {
       // The atlas has been destroyed

packages/engine/Source/Scene/Billboard.js

@@ -189,8 +189,6 @@ function Billboard(options, billboardCollection) {
   this._batchIndex = undefined; // Used only by Vector3DTilePoints and BillboardCollection
 
   this._imageTexture = new BillboardTexture(billboardCollection);
-  this._imageWidth = undefined;
-  this._imageHeight = undefined;
 
   this._labelDimensions = undefined;
   this._labelHorizontalOrigin = undefined;

packages/engine/Source/Scene/BillboardTexture.js

@@ -251,23 +251,66 @@ BillboardTexture.prototype.loadImage = async function (id, image) {
  * @param {string} id An identifier to detect whether the image already exists in the atlas.
  * @param {BoundingRectangle} subRegion An {@link BoundingRectangle} defining a region of an existing image, measured in pixels from the bottom-left of the image.
  */
-BillboardTexture.prototype.addImageSubRegion = async function (id, subRegion) {
+BillboardTexture.prototype.addImageSubRegion = function (id, subRegion) {
   this._id = id;
-  this._loadState = BillboardLoadState.LOADING;
   this._loadError = undefined;
   this._hasSubregion = true;
 
-  let index;
   const atlas = this._billboardCollection.textureAtlas;
+  const indexOrPromise = atlas.addImageSubRegion(id, subRegion);
+
+  if (typeof indexOrPromise === "number") {
+    this.setImageSubRegion(indexOrPromise, subRegion);
+    return;
+  }
+
+  this.loadImageSubRegion(id, subRegion, indexOrPromise);
+};
+
+/**
+ * @see {TextureAtlas#addImageSubRegion}
+ * @private
+ * @param {string} id An identifier to detect whether the image already exists in the atlas.
+ * @param {BoundingRectangle} subRegion An {@link BoundingRectangle} defining a region of an existing image, measured in pixels from the bottom-left of the image.
+ * @param {Promise<number>} indexPromise A promise that resolves to the image region index.
+ */
+BillboardTexture.prototype.loadImageSubRegion = async function (
+  id,
+  subRegion,
+  indexPromise,
+) {
+  let index;
   try {
-    index = await atlas.addImageSubRegion(id, subRegion);
+    this._loadState = BillboardLoadState.LOADING;
+    index = await indexPromise;
   } catch (error) {
     // There was an error loading the referenced image
     this._loadState = BillboardLoadState.ERROR;
     this._loadError = error;
     return;
   }
 
+  if (this._id !== id) {
+    // Another load was initiated and resolved resolved before this one. This operation is cancelled.
+    return;
+  }
+
+  this._loadState = BillboardLoadState.LOADED;
+
+  this.setImageSubRegion(index, subRegion);
+};
+
+/**
+ * @see {TextureAtlas#addImageSubRegion}
+ * @private
+ * @param {number} index The resolved index in the {@link TextureAtlas}
+ * @param {BoundingRectangle} subRegion An {@link BoundingRectangle} defining a region of an existing image, measured in pixels from the bottom-left of the image.
+ */
+BillboardTexture.prototype.setImageSubRegion = function (index, subRegion) {
+  if (this._index === index) {
+    return;
+  }
+
   if (!defined(index) || index === -1) {
     this._loadState = BillboardLoadState.FAILED;
     this._index = -1;
@@ -280,7 +323,6 @@ BillboardTexture.prototype.addImageSubRegion = async function (id, subRegion) {
   this._height = subRegion.height;
 
   this._index = index;
-  this._loadState = BillboardLoadState.LOADED;
 
   this.dirty = true;
 };

packages/engine/Specs/DataSources/BillboardVisualizerSpec.js

@@ -178,9 +178,10 @@ describe(
       const bb = billboardCollection.get(0);
 
       await pollToPromise(function () {
+        entityCluster.update(scene.frameState);
         scene.renderForSpecs();
         visualizer.update(time);
-        return bb.show;
+        return bb.ready;
       });
 
       expect(bb.position).toEqual(testObject.position.getValue(time));