Skip to content

Commit

Permalink
Add a reason property to help the users track why the progress event …
Browse files Browse the repository at this point in the history
…is being fired (#4647)
  • Loading branch information
alexdaube authored Jan 17, 2024
1 parent f290b91 commit 2da8180
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 18 deletions.
2 changes: 1 addition & 1 deletion packages/model-viewer/src/features/ar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ configuration or device capabilities');
}

async prepareUSDZ(): Promise<string> {
const updateSourceProgress = this[$progressTracker].beginActivity();
const updateSourceProgress = this[$progressTracker].beginActivity('usdz-conversion');

await this[$triggerLoad]();

Expand Down
2 changes: 1 addition & 1 deletion packages/model-viewer/src/features/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export const EnvironmentMixin = <T extends Constructor<ModelViewerElementBase>>(
return;
}

const updateEnvProgress = this[$progressTracker].beginActivity();
const updateEnvProgress = this[$progressTracker].beginActivity('environment-update');

try {
const {environmentMap, skybox} =
Expand Down
2 changes: 1 addition & 1 deletion packages/model-viewer/src/features/scene-graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export const SceneGraphMixin = <T extends Constructor<ModelViewerElementBase>>(
super.updated(changedProperties);

if (changedProperties.has('variantName')) {
const updateVariantProgress = this[$progressTracker].beginActivity();
const updateVariantProgress = this[$progressTracker].beginActivity('variant-update');
updateVariantProgress(0.1);
const model = this[$model];
const {variantName} = this;
Expand Down
2 changes: 1 addition & 1 deletion packages/model-viewer/src/model-viewer-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,7 @@ export default class ModelViewerElementBase extends ReactiveElement {
// throw exceptions and/or behave in unexpected ways:
scene.stopAnimation();

const updateSourceProgress = this[$progressTracker].beginActivity();
const updateSourceProgress = this[$progressTracker].beginActivity('model-load');
const source = this.src;
try {
const srcUpdated = scene.setSource(
Expand Down
24 changes: 19 additions & 5 deletions packages/model-viewer/src/test/utilities/progress-tracker-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {waitForEvent} from '../../utilities.js';
import {Activity, ProgressDetails, ProgressTracker} from '../../utilities/progress-tracker.js';

suite('ProgressTracker', () => {
const progressReason = 'progress-test';

let progressTracker: ProgressTracker;
setup(() => {
progressTracker = new ProgressTracker();
Expand All @@ -28,11 +30,23 @@ suite('ProgressTracker', () => {
expect(progressTracker.ongoingActivityCount).to.be.equal(0);
});

test('event includes the reason for the progress', async () => {
const activity = progressTracker.beginActivity(progressReason);

const progressEventDispatches =
waitForEvent<CustomEvent<ProgressDetails>>(
progressTracker, 'progress');
activity(0.5);
const event = await progressEventDispatches;

expect(event.detail.reason).to.be.equal(progressReason);
});

suite('an activity', () => {
let firstActivity: Activity;

setup(() => {
firstActivity = progressTracker.beginActivity();
firstActivity = progressTracker.beginActivity(progressReason);
});

test('increases the ongoing activities count', () => {
Expand Down Expand Up @@ -73,12 +87,12 @@ suite('ProgressTracker', () => {
});

test('is added to the current stack of activities', () => {
progressTracker.beginActivity();
progressTracker.beginActivity(progressReason);
expect(progressTracker.ongoingActivityCount).to.be.equal(2);
});

test('defers marking all activities completed', () => {
progressTracker.beginActivity();
progressTracker.beginActivity(progressReason);
firstActivity(1.0);
expect(progressTracker.ongoingActivityCount).to.be.equal(2);
});
Expand Down Expand Up @@ -107,7 +121,7 @@ suite('ProgressTracker', () => {
let secondActivity: Activity;

setup(() => {
secondActivity = progressTracker.beginActivity();
secondActivity = progressTracker.beginActivity(progressReason);
});

test('increases the ongoing activity count', () => {
Expand Down Expand Up @@ -151,4 +165,4 @@ suite('ProgressTracker', () => {
});
});
});
});
});
15 changes: 8 additions & 7 deletions packages/model-viewer/src/utilities/progress-tracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export type Activity = (progress: number) => number;
*/
export interface ProgressDetails {
totalProgress: number;
reason: string;
}

/**
Expand All @@ -50,7 +51,7 @@ export interface ProgressDetails {
*
* The value of totalProgress is a number that progresses from 0 to 1. The
* ProgressTracker allows for the lazy accumulation of tracked actions, so the
* total progress represents a abstract, non-absolute progress towards the
* total progress represents an abstract, non-absolute progress towards the
* completion of all currently tracked events.
*
* When all currently tracked activities are finished, the ProgressTracker
Expand Down Expand Up @@ -86,15 +87,15 @@ export class ProgressTracker extends EventTarget {
* progress is reported than the previously reported progress, it will be
* ignored.
*/
beginActivity(): Activity {
beginActivity(reason: string): Activity {
const activity: OngoingActivity = {progress: 0, completed: false};

this.ongoingActivities.add(activity);

if (this.ongoingActivityCount === 1) {
// Announce the first progress event (which should always be 0 / 1
// total progress):
this.announceTotalProgress(activity, 0);
this.announceTotalProgress(activity, 0, reason);
}

return (progress: number): number => {
Expand All @@ -103,15 +104,15 @@ export class ProgressTracker extends EventTarget {
nextProgress = Math.max(clamp(progress, 0, 1), activity.progress);

if (nextProgress !== activity.progress) {
this.announceTotalProgress(activity, nextProgress);
this.announceTotalProgress(activity, nextProgress, reason);
}

return activity.progress;
};
}

private announceTotalProgress(
updatedActivity: OngoingActivity, nextProgress: number) {
updatedActivity: OngoingActivity, nextProgress: number, reason: string) {
let progressLeft = 0;
let completedActivities = 0;

Expand All @@ -122,7 +123,7 @@ export class ProgressTracker extends EventTarget {
const {progress} = activity;
progressLeft += 1.0 - progress;

if (activity.completed === true) {
if (activity.completed) {
completedActivities++;
}
}
Expand All @@ -140,7 +141,7 @@ export class ProgressTracker extends EventTarget {
this.totalProgress;

this.dispatchEvent(new CustomEvent<ProgressDetails>(
'progress', {detail: {totalProgress}}));
'progress', {detail: {totalProgress, reason }}));

if (completedActivities === this.ongoingActivityCount) {
this.totalProgress = 0.0;
Expand Down
4 changes: 2 additions & 2 deletions packages/modelviewer.dev/data/docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@
{
"name": "progress",
"htmlName": "progress",
"description": "This event fires while a model, environment, and/or skybox is loading, during both download and processing. The progress of all of these concurrent tasks will be given by a value between 0 and 1: event.detail.totalProgress."
"description": "This event fires while a model, environment, skybox, variant, and/or a usdz conversion is loading, during both download and processing. The progress of all of these concurrent tasks will be given by a value between 0 and 1: event.detail.totalProgress. It also provides the reason why the event is fired ('model-load', 'environment-update', 'variant-update', 'usdz-conversion'): event.detail.reason."
},
{
"name": "render-scale",
Expand Down Expand Up @@ -1147,4 +1147,4 @@
}
]
}
]
]

0 comments on commit 2da8180

Please sign in to comment.