Skip to content

Commit

Permalink
Refactor DPR (google#1179)
Browse files Browse the repository at this point in the history 
* no longer changing three.js dpr

* refactored DPR into renderer

* cleanup and fix tests

* fixed AR viewport bug

* pulling the rest of the resizing code into Renderer to fix some bugs

* cleanup

* fixing AR viewport

* fixed hotspot race

* update hotspot-spec

* using the threeRenderer size and dpr

* tiny cleanup
  • Loading branch information
@elalish
elalish authored May 15, 2020
1 parent dd657bf commit 93d81d2
Show file tree
Hide file tree
Showing 10 changed files with 133 additions and 156 deletions.
7 changes: 6 additions & 1 deletion packages/model-viewer/src/features/annotation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,12 @@ export const AnnotationMixin = <T extends Constructor<ModelViewerElementBase>>(
});
this[$hotspotMap].set(node.slot, hotspot);
this[$scene].model.addHotspot(hotspot);
// This happens automatically in render(), but we do it early so that
// the slots appear in the shadow DOM and the elements get attached,
// allowing us to dispatch events on them.
this[$annotationRenderer].domElement.appendChild(hotspot.element);
}
this[$scene].isDirty = true;
}

private[$removeHotspot](node: Node) {
Expand All @@ -202,8 +207,8 @@ export const AnnotationMixin = <T extends Constructor<ModelViewerElementBase>>(
if (hotspot.decrement()) {
this[$scene].model.removeHotspot(hotspot);
this[$hotspotMap].delete(node.slot);
hotspot.dispose();
}
this[$scene].isDirty = true;
}
}

Expand Down
34 changes: 6 additions & 28 deletions packages/model-viewer/src/model-viewer-base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {makeTemplate} from './template.js';
import {$evictionPolicy, CachingGLTFLoader} from './three-components/CachingGLTFLoader.js';
import {ModelScene} from './three-components/ModelScene.js';
import {ContextLostEvent, Renderer} from './three-components/Renderer.js';
import {debounce, deserializeUrl, resolveDpr} from './utilities.js';
import {debounce, deserializeUrl} from './utilities.js';
import {dataUrlToBlob} from './utilities/data-conversion.js';
import {ProgressTracker} from './utilities/progress-tracker.js';

Expand All @@ -35,18 +35,17 @@ const UNSIZED_MEDIA_HEIGHT = 150;
const blobCanvas = document.createElement('canvas');
let blobContext: CanvasRenderingContext2D|null = null;

const $updateSize = Symbol('updateSize');
const $loaded = Symbol('loaded');
const $template = Symbol('template');
const $fallbackResizeHandler = Symbol('fallbackResizeHandler');
const $defaultAriaLabel = Symbol('defaultAriaLabel');
const $resizeObserver = Symbol('resizeObserver');
const $intersectionObserver = Symbol('intersectionObserver');
const $lastDpr = Symbol('lastDpr');
const $clearModelTimeout = Symbol('clearModelTimeout');
const $onContextLost = Symbol('onContextLost');
const $contextLostHandler = Symbol('contextLostHandler');

export const $updateSize = Symbol('updateSize');
export const $isElementInViewport = Symbol('isElementInViewport');
export const $announceModelVisibility = Symbol('announceModelVisibility');
export const $ariaLabel = Symbol('ariaLabel');
Expand Down Expand Up @@ -131,7 +130,6 @@ export default class ModelViewerElementBase extends UpdatingElement {
protected[$userInputElement]: HTMLDivElement;
protected[$canvas]: HTMLCanvasElement;
protected[$defaultAriaLabel]: string;
protected[$lastDpr]: number = resolveDpr();
protected[$clearModelTimeout]: number|null = null;

protected[$fallbackResizeHandler] = debounce(() => {
Expand Down Expand Up @@ -227,7 +225,7 @@ export default class ModelViewerElementBase extends UpdatingElement {
// Update initial size on microtask timing so that subclasses have a
// chance to initialize
Promise.resolve().then(() => {
this[$updateSize](this.getBoundingClientRect(), true);
this[$updateSize](this.getBoundingClientRect());
});

if (HAS_RESIZE_OBSERVER) {
Expand Down Expand Up @@ -369,7 +367,7 @@ export default class ModelViewerElementBase extends UpdatingElement {
const idealAspect = options ? options.idealAspect : undefined;

const {width, height, model, aspect} = this[$scene];
const dpr = resolveDpr();
const {dpr} = this[$renderer];
let outputWidth = width * dpr;
let outputHeight = height * dpr;
let offsetX = 0;
Expand Down Expand Up @@ -455,33 +453,14 @@ export default class ModelViewerElementBase extends UpdatingElement {
/**
* Called on initialization and when the resize observer fires.
*/
[$updateSize](
{width, height}: {width: any, height: any}, forceApply = false) {
const {width: prevWidth, height: prevHeight} = this[$scene].getSize();
// Round off the pixel size
const intWidth = parseInt(width, 10);
const intHeight = parseInt(height, 10);

[$updateSize]({width, height}: {width: any, height: any}) {
this[$container].style.width = `${width}px`;
this[$container].style.height = `${height}px`;

if (forceApply || (prevWidth !== intWidth || prevHeight !== intHeight)) {
this[$onResize]({width: intWidth, height: intHeight});
}
this[$onResize]({width: parseFloat(width), height: parseFloat(height)});
}

[$tick](_time: number, _delta: number) {
const dpr = resolveDpr();
// There is no standard way to detect when DPR changes on account of zoom.
// Here we keep a local copy of DPR updated, and when it changes we invoke
// the fallback resize handler. It might be better to invoke the resize
// handler directly in this case, but the fallback is debounced which will
// save us from doing too much work when DPR and window size changes at the
// same time.
if (dpr !== this[$lastDpr]) {
this[$lastDpr] = dpr;
this[$fallbackResizeHandler]();
}
}

[$markLoaded]() {
Expand All @@ -505,7 +484,6 @@ export default class ModelViewerElementBase extends UpdatingElement {

[$onResize](e: {width: number, height: number}) {
this[$scene].setSize(e.width, e.height);
this[$needsRender]();
}

[$onContextLost](event: ContextLostEvent) {
Expand Down
4 changes: 2 additions & 2 deletions packages/model-viewer/src/test/model-viewer-base-spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import {IS_IE11} from '../constants.js';
import ModelViewerElementBase, {$scene, $userInputElement} from '../model-viewer-base.js';
import {Renderer} from '../three-components/Renderer.js';
import {Constructor, resolveDpr} from '../utilities.js';
import {Constructor} from '../utilities.js';

import {assetPath, spy, timePasses, until, waitForEvent} from './helpers.js';
import {BasicSpecTemplate} from './templates.js';
Expand All @@ -35,7 +35,7 @@ const expectBlobDimensions =
img.src = url;
});

const dpr = resolveDpr();
const dpr = window.devicePixelRatio;
expect(img.width).to.be.equal(width * dpr);
expect(img.height).to.be.equal(height * dpr);
};
Expand Down
16 changes: 10 additions & 6 deletions packages/model-viewer/src/test/three-components/Hotspot-spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ suite('Hotspot', () => {
});

suite('when shown', () => {
setup(() => {
hotspot.show();
});

test('adds a corresponding data-* attribute to assigned nodes', () => {
expect(assigned.hasAttribute('data-bar')).to.be.true;
});
Expand All @@ -70,7 +74,7 @@ suite('Hotspot', () => {
});
});

suite('when hidden', () => {
suite('when shown', () => {
let event: CustomEvent<HotspotVisibilityDetails>;
let hotspot: Hotspot;

Expand All @@ -82,30 +86,30 @@ suite('Hotspot', () => {

const assignedNodeHides = waitForEvent(assigned, 'hotspot-visibility');

hotspot.hide();
hotspot.show();

event = await (
assignedNodeHides as
Promise<CustomEvent<HotspotVisibilityDetails>>);
});

test('dispatches a "visibility-change" on assigned nodes', async () => {
expect(event.detail.visible).to.be.false;
expect(event.detail.visible).to.be.true;
});


suite('and then shown', () => {
suite('and then hidden', () => {
test('dispatches a "visibility-change" on assigned nodes', async () => {
const assignedNodeHides =
waitForEvent(assigned, 'hotspot-visibility');

hotspot.show();
hotspot.hide();

const event = await (
assignedNodeHides as
Promise<CustomEvent<HotspotVisibilityDetails>>);

expect(event.detail.visible).to.be.true;
expect(event.detail.visible).to.be.false;
});
});
});
Expand Down
4 changes: 2 additions & 2 deletions packages/model-viewer/src/test/three-components/Renderer-spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ suite('Renderer', () => {

test('updates effective DPR', async () => {
const {element} = scene;
const initialDpr = renderer.threeRenderer.getPixelRatio();
const initialDpr = renderer.dpr;
const {width, height} = scene.getSize();

element[$onResize]({width, height});
Expand All @@ -170,7 +170,7 @@ suite('Renderer', () => {

await new Promise(resolve => requestAnimationFrame(resolve));

const newDpr = renderer.threeRenderer.getPixelRatio();
const newDpr = renderer.dpr;

expect(newDpr).to.be.equal(initialDpr + 1);
});
Expand Down
13 changes: 7 additions & 6 deletions packages/model-viewer/src/three-components/ARRenderer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,15 @@ export class ARRenderer extends EventDispatcher {
});
await waitForXRAnimationFrame;

scene.element[$onResize](window.screen);

const {framebuffer, framebufferWidth, framebufferHeight} =
session.renderState.baseLayer!;
// Redirect rendering to the WebXR offscreen framebuffer.
// TODO: this method should be added to three.js's exported interface.
(this.threeRenderer as any)
.setFramebuffer(session.renderState.baseLayer!.framebuffer);
scene.element[$onResize](window.screen);
(this.threeRenderer as any).setFramebuffer(framebuffer);
this.threeRenderer.setPixelRatio(1);
this.threeRenderer.setSize(framebufferWidth, framebufferHeight, false);

return session;
}
Expand Down Expand Up @@ -258,7 +262,6 @@ export class ARRenderer extends EventDispatcher {
});

this[$currentSession] = currentSession;
this[$presentedScene] = scene;
this[$placementBox] = placementBox;
this[$lastTick] = performance.now();

Expand Down Expand Up @@ -317,8 +320,6 @@ export class ARRenderer extends EventDispatcher {
model.orientHotspots(0);
element.requestUpdate('cameraTarget');
element[$onResize](element.getBoundingClientRect());

this.renderer.expandTo(scene.width, scene.height);
}

if (this[$placementBox] != null) {
Expand Down
36 changes: 8 additions & 28 deletions packages/model-viewer/src/three-components/Hotspot.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ const $referenceCount = Symbol('referenceCount');
const $updateVisibility = Symbol('updateVisibility');
const $visible = Symbol('visible');

const $onSlotchange = Symbol('onSlotchange');
const $slotchangeHandler = Symbol('slotchangeHandler');

/**
* The Hotspot object is a reference-counted slot. If decrement() returns true,
* it should be removed from the tree so it can be garbage-collected.
Expand All @@ -53,23 +50,19 @@ export class Hotspot extends CSS2DObject {
private[$referenceCount] = 1;
private[$pivot] = document.createElement('div');
private[$slot]: HTMLSlotElement = document.createElement('slot');
private[$slotchangeHandler] = () => this[$onSlotchange]();

constructor(config: HotspotConfiguration) {
super(document.createElement('div'));

this.element.classList.add('annotation-wrapper');

this[$slot].name = config.name;
this[$slot].addEventListener('slotchange', this[$slotchangeHandler]);

this.element.appendChild(this[$pivot]);
this[$pivot].appendChild(this[$slot]);

this.updatePosition(config.position);
this.updateNormal(config.normal);

this.show();
}

/**
Expand All @@ -78,7 +71,7 @@ export class Hotspot extends CSS2DObject {
show() {
if (!this[$visible]) {
this[$visible] = true;
this[$updateVisibility]({notify: true});
this[$updateVisibility]();
}
}

Expand All @@ -88,17 +81,10 @@ export class Hotspot extends CSS2DObject {
hide() {
if (this[$visible]) {
this[$visible] = false;
this[$updateVisibility]({notify: true});
this[$updateVisibility]();
}
}

/**
* Cleans up the held references of this Hotspot when it is done being used.
*/
dispose() {
this[$slot].removeEventListener('slotchange', this[$slotchangeHandler]);
}

/**
* Call this when adding elements to the same slot to keep track.
*/
Expand Down Expand Up @@ -149,7 +135,7 @@ export class Hotspot extends CSS2DObject {
this[$pivot].style.transform = `rotate(${radians}rad)`;
}

protected[$updateVisibility]({notify}: {notify: boolean}) {
protected[$updateVisibility]() {
// NOTE: IE11 doesn't support a second arg for classList.toggle
if (this[$visible]) {
this.element.classList.remove('hide');
Expand Down Expand Up @@ -179,17 +165,11 @@ export class Hotspot extends CSS2DObject {
}
}

if (notify) {
element.dispatchEvent(new CustomEvent('hotspot-visibility', {
detail: {
visible: this[$visible],
},
}));
}
element.dispatchEvent(new CustomEvent('hotspot-visibility', {
detail: {
visible: this[$visible],
},
}));
});
}

protected[$onSlotchange]() {
this[$updateVisibility]({notify: false});
}
}
Loading

0 comments on commit 93d81d2

Please sign in to comment.