fix: Treat regions uniquely (shaka-project#4841)

Captions split over stream segment boundaries are repeated over more
than one segment.
Regions in which they sit maybe described the same but allocated
different ids and vice versa.
Proposal to treat internal region ids uniqely by encoding the dimensions
in the id.

fixes shaka-project#4839

lib/text/ui_text_displayer.js

@@ -371,6 +371,25 @@ shaka.text.UITextDisplayer = class {
     }
   }
 
+  /**
+   * Compute a unique internal id:
+   * Regions can reuse the id but have different dimensions, we need to
+   * consider those differences
+   * @param {shaka.extern.CueRegion} region
+   * @private
+   */
+  generateRegionId_(region) {
+    const percentageUnit = shaka.text.CueRegion.units.PERCENTAGE;
+    const heightUnit = region.heightUnits == percentageUnit ? '%' : 'px';
+    const viewportAnchorUnit =
+        region.viewportAnchorUnits == percentageUnit ? '%' : 'px';
+    const uniqueRegionId = `${region.id}_${
+      region.width}x${region.height}${heightUnit}-${
+      region.viewportAnchorX}x${region.viewportAnchorY}${viewportAnchorUnit}`;
+
+    return uniqueRegionId;
+  }
+
   /**
    * Get or create a region element corresponding to the cue region.  These are
    * cached by ID.
@@ -382,8 +401,9 @@ shaka.text.UITextDisplayer = class {
   getRegionElement_(cue) {
     const region = cue.region;
 
-    if (this.regionElements_.has(region.id)) {
-      return this.regionElements_.get(region.id);
+    const regionId = this.generateRegionId_(region);
+    if (this.regionElements_.has(regionId)) {
+      return this.regionElements_.get(regionId);
     }
 
     const regionElement = shaka.util.Dom.createHTMLElement('span');
@@ -394,7 +414,7 @@ shaka.text.UITextDisplayer = class {
     const viewportAnchorUnit =
         region.viewportAnchorUnits == percentageUnit ? '%' : 'px';
 
-    regionElement.id = 'shaka-text-region---' + region.id;
+    regionElement.id = 'shaka-text-region---' + regionId;
     regionElement.classList.add('shaka-text-region');
 
     regionElement.style.height = region.height + heightUnit;
@@ -415,7 +435,7 @@ shaka.text.UITextDisplayer = class {
       regionElement.style.justifyContent = 'flex-end';
     }
 
-    this.regionElements_.set(region.id, regionElement);
+    this.regionElements_.set(regionId, regionElement);
     return regionElement;
   }
 

test/text/ui_text_displayer_unit.js

@@ -537,4 +537,74 @@ describe('UITextDisplayer', () => {
         '.shaka-text-region');
     expect(allRegionElements.length).toBe(1);
   });
+
+  it('creates separate regions when dimensions differ but id same', () => {
+    const identicalRegionId = 'regionId';
+
+    const cueRegion1 = new shaka.text.CueRegion();
+    const cueRegion2 = new shaka.text.CueRegion();
+    cueRegion1.id = identicalRegionId;
+    cueRegion2.id = identicalRegionId;
+
+    cueRegion1.height = 80;
+    cueRegion1.heightUnits = shaka.text.CueRegion.units.PERCENTAGE;
+    cueRegion1.width = 80;
+    cueRegion1.widthUnits = shaka.text.CueRegion.units.PERCENTAGE;
+
+    cueRegion2.height = 160; // the only difference!
+    cueRegion2.heightUnits = shaka.text.CueRegion.units.PERCENTAGE;
+    cueRegion2.width = 80;
+    cueRegion2.widthUnits = shaka.text.CueRegion.units.PERCENTAGE;
+
+    cueRegion1.viewportAnchorX = 10;
+    cueRegion1.viewportAnchorY = 10;
+    cueRegion1.viewportAnchorUnits = shaka.text.CueRegion.units.PERCENTAGE;
+
+    cueRegion2.viewportAnchorX = 10;
+    cueRegion2.viewportAnchorY = 10;
+    cueRegion2.viewportAnchorUnits = shaka.text.CueRegion.units.PERCENTAGE;
+
+    // These all attach to the same region, but only one region element should
+    // be created.
+    const firstBatchOfCues = [
+      new shaka.text.Cue(0, 100, ''),
+      new shaka.text.Cue(0, 100, ''),
+      new shaka.text.Cue(0, 100, ''),
+    ];
+    for (const cue of firstBatchOfCues) {
+      cue.displayAlign = shaka.text.Cue.displayAlign.CENTER;
+      cue.region = cueRegion1;
+    }
+
+    // Another batch for the other region
+    const secondBatchOfCues = [
+      new shaka.text.Cue(0, 100, ''),
+      new shaka.text.Cue(0, 100, ''),
+      new shaka.text.Cue(0, 100, ''),
+    ];
+    for (const cue of secondBatchOfCues) {
+      cue.displayAlign = shaka.text.Cue.displayAlign.CENTER;
+      cue.region = cueRegion2;
+    }
+
+    textDisplayer.setTextVisibility(true);
+    textDisplayer.append(firstBatchOfCues);
+    textDisplayer.append(secondBatchOfCues);
+    updateCaptions();
+
+    const textContainer = videoContainer.querySelector('.shaka-text-container');
+    const allRegionElements = textContainer.querySelectorAll(
+        '.shaka-text-region');
+
+    // Verify that the nested cues are attached to respective region element.
+    expect(allRegionElements.length).toBe(2);
+
+    const childrenOfOne = Array.from(allRegionElements[0].childNodes).filter(
+        (e) => e.nodeType == Node.ELEMENT_NODE);
+    expect(childrenOfOne.length).toBe(3);
+
+    const childrenOfTwo = Array.from(allRegionElements[1].childNodes).filter(
+        (e) => e.nodeType == Node.ELEMENT_NODE);
+    expect(childrenOfTwo.length).toBe(3);
+  });
 });