Skip to content

Commit

Permalink
fix(text): Fix cue region rendering in UI (#4412)
Browse files Browse the repository at this point in the history 
Both TTML and VTT regions should be thought of as separate elements
from the cue structures they contain.  To this end, the UI text
displayer now creates separate region elements to represent CueRegion
objects, and the Cues attached to them nest inside those region
elements in the DOM.

Closes #4381
  • Loading branch information
@joeyparrish
joeyparrish authored Aug 16, 2022
1 parent 324dcb8 commit b1f46db
Show file tree
Hide file tree
Showing 2 changed files with 148 additions and 19 deletions.
102 changes: 83 additions & 19 deletions lib/text/ui_text_displayer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ shaka.text.UITextDisplayer = class {
* the cue (e.g. the HTML element to put nested cues inside).
* @private {Map.<!shaka.extern.Cue, !{
* cueElement: !HTMLElement,
* regionElement: HTMLElement,
* wrapper: !HTMLElement
* }>}
*/
Expand All @@ -97,6 +98,9 @@ shaka.text.UITextDisplayer = class {
});
this.resizeObserver_.observe(this.textContainer_);
}

/** @private {Map.<string, !HTMLElement>} */
this.regionElements_ = new Map();
}


Expand Down Expand Up @@ -234,6 +238,11 @@ shaka.text.UITextDisplayer = class {
// even ones which are going to be planted again.
toUproot.push(cueRegistry.cueElement);

// Also uproot all displayed region elements.
if (cueRegistry.regionElement) {
toUproot.push(cueRegistry.regionElement);
}

// If the cue should not be displayed, remove it entirely.
if (!shouldBeDisplayed) {
// Since something has to be removed, we will need to update the DOM.
Expand Down Expand Up @@ -268,8 +277,12 @@ shaka.text.UITextDisplayer = class {
goog.asserts.assert(topCue == cue, 'Parent cues should be kept in order');
}
if (updateDOM) {
for (const cueElement of toUproot) {
container.removeChild(cueElement);
for (const element of toUproot) {
// NOTE: Because we uproot shared region elements, too, we might hit an
// element here that has no parent because we've already processed it.
if (element.parentElement) {
element.parentElement.removeChild(element);
}
}
toPlant.sort((a, b) => {
if (a.startTime != b.startTime) {
Expand All @@ -281,7 +294,12 @@ shaka.text.UITextDisplayer = class {
for (const cue of toPlant) {
const cueRegistry = this.currentCuesMap_.get(cue);
goog.asserts.assert(cueRegistry, 'cueRegistry should exist.');
container.appendChild(cueRegistry.cueElement);
if (cueRegistry.regionElement) {
container.appendChild(cueRegistry.regionElement);
cueRegistry.regionElement.appendChild(cueRegistry.cueElement);
} else {
container.appendChild(cueRegistry.cueElement);
}
}
}
}
Expand All @@ -298,11 +316,15 @@ shaka.text.UITextDisplayer = class {

const currentTime = this.video_.currentTime;
if (!this.isTextVisible_ || forceUpdate) {
if (this.currentCuesMap_.size > 0) {
// Clear away any existing cues.
shaka.util.Dom.removeAllChildren(this.textContainer_);
this.currentCuesMap_.clear();
// Remove child elements from all regions.
for (const regionElement of this.regionElements_.values()) {
shaka.util.Dom.removeAllChildren(regionElement);
}
// Remove all top-level elements in the text container.
shaka.util.Dom.removeAllChildren(this.textContainer_);
// Clear the element maps.
this.currentCuesMap_.clear();
this.regionElements_.clear();
}
if (this.isTextVisible_) {
// Log currently attached cue elements for verification, later.
Expand Down Expand Up @@ -332,6 +354,54 @@ shaka.text.UITextDisplayer = class {
}
}

/**
* Get or create a region element corresponding to the cue region. These are
* cached by ID.
*
* @param {!shaka.extern.Cue} cue
* @return {!HTMLElement}
* @private
*/
getRegionElement_(cue) {
const region = cue.region;

if (this.regionElements_.has(region.id)) {
return this.regionElements_.get(region.id);
}

const regionElement = shaka.util.Dom.createHTMLElement('span');

const percentageUnit = shaka.text.CueRegion.units.PERCENTAGE;
const heightUnit = region.heightUnits == percentageUnit ? '%' : 'px';
const widthUnit = region.widthUnits == percentageUnit ? '%' : 'px';
const viewportAnchorUnit =
region.viewportAnchorUnits == percentageUnit ? '%' : 'px';

regionElement.id = 'shaka-text-region---' + region.id;
regionElement.classList.add('shaka-text-region');

regionElement.style.height = region.height + heightUnit;
regionElement.style.width = region.width + widthUnit;
regionElement.style.position = 'absolute';
regionElement.style.top = region.viewportAnchorY + viewportAnchorUnit;
regionElement.style.left = region.viewportAnchorX + viewportAnchorUnit;

regionElement.style.display = 'flex';
regionElement.style.flexDirection = 'column';
regionElement.style.alignItems = 'center';

if (cue.displayAlign == shaka.text.Cue.displayAlign.BEFORE) {
regionElement.style.justifyContent = 'flex-start';
} else if (cue.displayAlign == shaka.text.Cue.displayAlign.CENTER) {
regionElement.style.justifyContent = 'center';
} else {
regionElement.style.justifyContent = 'flex-end';
}

this.regionElements_.set(region.id, regionElement);
return regionElement;
}

/**
* Creates the object for a cue.
*
Expand All @@ -354,6 +424,11 @@ shaka.text.UITextDisplayer = class {
this.setCaptionStyles_(cueElement, cue, parents, needWrapper);
}

let regionElement = null;
if (cue.region && cue.region.id) {
regionElement = this.getRegionElement_(cue);
}

let wrapper = cueElement;
if (needWrapper) {
// Create a wrapper element which will serve to contain all children into
Expand All @@ -365,7 +440,7 @@ shaka.text.UITextDisplayer = class {
cueElement.appendChild(wrapper);
}

this.currentCuesMap_.set(cue, {cueElement, wrapper});
this.currentCuesMap_.set(cue, {cueElement, wrapper, regionElement});
}

/**
Expand Down Expand Up @@ -526,17 +601,6 @@ shaka.text.UITextDisplayer = class {
}
}
}
} else if (cue.region && cue.region.id) {
const percentageUnit = shaka.text.CueRegion.units.PERCENTAGE;
const heightUnit = cue.region.heightUnits == percentageUnit ? '%' : 'px';
const widthUnit = cue.region.widthUnits == percentageUnit ? '%' : 'px';
const viewportAnchorUnit =
cue.region.viewportAnchorUnits == percentageUnit ? '%' : 'px';
style.height = cue.region.height + heightUnit;
style.width = cue.region.width + widthUnit;
style.position = 'absolute';
style.top = cue.region.viewportAnchorY + viewportAnchorUnit;
style.left = cue.region.viewportAnchorX + viewportAnchorUnit;
}

style.lineHeight = cue.lineHeight;
Expand Down
65 changes: 65 additions & 0 deletions test/text/ui_text_displayer_unit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ describe('UITextDisplayer', () => {
});

beforeEach(() => {
video.currentTime = 0;
textDisplayer = new shaka.text.UITextDisplayer(video, videoContainer);
});

Expand Down Expand Up @@ -396,4 +397,68 @@ describe('UITextDisplayer', () => {
expect(parentCueElements.length).toBe(1);
expect(parentCueElements[0].textContent).toBe('');
});

it('creates separate elements for cue regions', () => {
const cueRegion = new shaka.text.CueRegion();
cueRegion.id = 'regionId';
cueRegion.height = 80;
cueRegion.heightUnits = shaka.text.CueRegion.units.PERCENTAGE;
cueRegion.width = 80;
cueRegion.widthUnits = shaka.text.CueRegion.units.PERCENTAGE;
cueRegion.viewportAnchorX = 10;
cueRegion.viewportAnchorY = 10;
cueRegion.viewportAnchorUnits = shaka.text.CueRegion.units.PERCENTAGE;

// These all attach to the same region, but only one region element should
// be created.
const cues = [
new shaka.text.Cue(0, 100, ''),
new shaka.text.Cue(0, 100, ''),
new shaka.text.Cue(0, 100, ''),
];
for (const cue of cues) {
cue.displayAlign = shaka.text.Cue.displayAlign.CENTER;
cue.region = cueRegion;
}

textDisplayer.setTextVisibility(true);
textDisplayer.append(cues);
updateCaptions();

const textContainer = videoContainer.querySelector('.shaka-text-container');
const allRegionElements = textContainer.querySelectorAll(
'.shaka-text-region');

// Verify that the nested cues are all attached to a single region element.
expect(allRegionElements.length).toBe(1);
const regionElement = allRegionElements[0];
const children = Array.from(regionElement.childNodes).filter(
(e) => e.nodeType == Node.ELEMENT_NODE);
expect(children.length).toBe(3);

// Verify styles applied to the region element.
const regionCssObj = parseCssText(regionElement.style.cssText);
const expectRegionCssObj = {
'position': 'absolute',
'height': '80%',
'width': '80%',
'top': '10%',
'left': '10%',
'display': 'flex',
'flex-direction': 'column',
'align-items': 'center',
'justify-content': 'center',
};
expect(regionCssObj).toEqual(jasmine.objectContaining(expectRegionCssObj));

for (const caption of children) {
// Verify that styles applied to the nested cues _DO NOT_ include region
// placement.
const cueCssObj = parseCssText(caption.style.cssText);
expect(Object.keys(cueCssObj)).not.toContain('height');
expect(Object.keys(cueCssObj)).not.toContain('width');
expect(Object.keys(cueCssObj)).not.toContain('top');
expect(Object.keys(cueCssObj)).not.toContain('left');
}
});
});

0 comments on commit b1f46db

Please sign in to comment.