Suggested CR changes

Author: Beilinson

packages/engine/Source/DataSources/BillboardVisualizer.js

@@ -41,7 +41,6 @@ function EntityData(entity) {
   this.entity = entity;
   this.billboard = undefined;
   this.textureValue = undefined;
-  this.subRegion = undefined;
 }
 
 /**
@@ -129,7 +128,6 @@ BillboardVisualizer.prototype.update = function (time) {
       billboard.id = entity;
       item.billboard = billboard;
       item.textureValue = undefined;
-      item.subRegion = undefined;
     }
 
     billboard.show = show;
@@ -235,14 +233,7 @@ BillboardVisualizer.prototype.update = function (time) {
       time,
       boundingRectangleScratch,
     );
-    const subRegionUpdated = !BoundingRectangle.equals(
-      subRegion,
-      item.subRegion,
-    );
-    if (subRegionUpdated) {
-      item.subRegion = BoundingRectangle.clone(subRegion, item.subRegion);
-    }
-    if (defined(subRegion) && (imageUpdated || subRegionUpdated)) {
+    if (defined(subRegion)) {
       billboard.setImageSubRegion(billboard.image, subRegion);
     }
   }

packages/engine/Source/Renderer/TextureAtlas.js

@@ -680,13 +680,13 @@ TextureAtlas.prototype.addImage = function (id, image) {
 };
 
 /**
- * Add a sub-region of an existing atlas image as additional image indices.
+ * Get 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 | undefined} The existing subRegion index, or undefined if not yet added.
  */
-TextureAtlas.prototype.addImageSubRegion = function (id, subRegion) {
+TextureAtlas.prototype.getImageSubRegion = function (id, subRegion) {
   //>>includeStart('debug', pragmas.debug);
   Check.typeOf.string("id", id);
   Check.defined("subRegion", subRegion);
@@ -697,28 +697,36 @@ TextureAtlas.prototype.addImageSubRegion = function (id, subRegion) {
     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;
-        });
+        return index;
       }
     }
   }
+};
 
-  const index = this._nextIndex++;
+/**
+ * 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.
+ */
+TextureAtlas.prototype.addImageSubRegion = function (id, subRegion) {
+  let index = this.getImageSubRegion(id, subRegion);
+  if (index) {
+    return index;
+  }
+  const imageIndex = this._indexById.get(id);
+
+  index = this._nextIndex++;
   this._subRegions.set(index, imageIndex);
   this._rectangles[index] = subRegion.clone();
 
+  const indexPromise = this._indexPromiseById.get(id);
   return indexPromise.then((imageIndex) => {
     if (imageIndex === -1) {
       // The atlas has been destroyed

packages/engine/Source/Scene/BillboardTexture.js

@@ -253,24 +253,27 @@ BillboardTexture.prototype.loadImage = async function (id, image) {
  */
 BillboardTexture.prototype.addImageSubRegion = async function (id, subRegion) {
   this._id = id;
-  this._loadState = BillboardLoadState.LOADING;
   this._loadError = undefined;
   this._hasSubregion = true;
 
-  let index;
   const atlas = this._billboardCollection.textureAtlas;
-  try {
-    index = await atlas.addImageSubRegion(id, subRegion);
-  } catch (error) {
-    // There was an error loading the referenced image
-    this._loadState = BillboardLoadState.ERROR;
-    this._loadError = error;
-    return;
-  }
+  let index = atlas.getImageSubRegion(id, subRegion);
+  if (!index) {
+    try {
+      this._loadState = BillboardLoadState.LOADING;
+      index = await atlas.addImageSubRegion(id, subRegion);
+    } 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;
+    if (this._id !== id) {
+      // Another load was initiated and resolved resolved before this one. This operation is cancelled.
+      return;
+    }
+    this._loadState = BillboardLoadState.LOADED;
   }
 
   if (!defined(index) || index === -1) {
@@ -285,7 +288,6 @@ BillboardTexture.prototype.addImageSubRegion = async function (id, subRegion) {
   this._height = subRegion.height;
 
   this._index = index;
-  this._loadState = BillboardLoadState.LOADED;
 
   this.dirty = true;
 };