Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AR autogenerate USDZ #2374

Merged
merged 9 commits into from
May 11, 2021
Merged
63 changes: 59 additions & 4 deletions packages/model-viewer/src/features/ar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@

import {property} from 'lit-element';
import {Event as ThreeEvent} from 'three';
import {USDZExporter} from 'three/examples/jsm/exporters/USDZExporter';

import {IS_AR_QUICKLOOK_CANDIDATE, IS_SCENEVIEWER_CANDIDATE, IS_WEBXR_AR_CANDIDATE} from '../constants.js';
import ModelViewerElementBase, {$needsRender, $renderer, $scene, $shouldAttemptPreload, $updateSource} from '../model-viewer-base.js';
import ModelViewerElementBase, {$needsRender, $onModelLoad, $progressTracker, $renderer, $scene, $shouldAttemptPreload, $updateSource} from '../model-viewer-base.js';
import {enumerationDeserializer} from '../styles/deserializers.js';
import {ARStatus} from '../three-components/ARRenderer.js';
import {Constructor, waitForEvent} from '../utilities.js';
Expand Down Expand Up @@ -53,6 +54,7 @@ const $arMode = Symbol('arMode');
const $arModes = Symbol('arModes');
const $arAnchor = Symbol('arAnchor');
const $preload = Symbol('preload');
const $generatedIosUrl = Symbol('generatedIosUrl');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is vestigial now.


const $onARButtonContainerClick = Symbol('onARButtonContainerClick');
const $onARStatus = Symbol('onARStatus');
Expand All @@ -72,6 +74,8 @@ export const ARMixin = <T extends Constructor<ModelViewerElementBase>>(
ModelViewerElement: T): Constructor<ARInterface>&T => {
class ARModelViewerElement extends ModelViewerElement {
@property({type: Boolean, attribute: 'ar'}) ar: boolean = false;
@property({type: Boolean, attribute: 'ar-autogenerate-usdz'})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set this property to true if you want to enable on the fly USDZ generation

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you see my comment about doing this without introducing a new attribute? If you look at my PR, I changed the logic a bit so that the presence of ios-src automatically enables Quick Look, while explicitly adding the quick-look ar-mode opts in to USDZ generation. I think I prefer that approach; what do you think? Would you mind merging in that part of my PR here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, quick-look presence can be enough. I also added blob clean up before creating a new one (see the commit)

arAutogenerateUsdz: boolean = false;

@property({type: String, attribute: 'ar-scale'}) arScale: string = 'auto';

Expand Down Expand Up @@ -100,6 +104,8 @@ export const ARMixin = <T extends Constructor<ModelViewerElementBase>>(
protected[$arMode]: ARMode = ARMode.NONE;
protected[$preload] = false;

private[$generatedIosUrl]: string|null = null;

private[$onARButtonContainerClick] = (event: Event) => {
event.preventDefault();
this.activateAR();
Expand Down Expand Up @@ -206,7 +212,8 @@ configuration or device capabilities');
this[$arMode] = ARMode.SCENE_VIEWER;
break;
} else if (
value === 'quick-look' && !!this.iosSrc &&
value === 'quick-look' &&
(this.arAutogenerateUsdz || !!this.iosSrc) &&
IS_AR_QUICKLOOK_CANDIDATE) {
this[$arMode] = ARMode.QUICK_LOOK;
break;
Expand Down Expand Up @@ -327,8 +334,15 @@ configuration or device capabilities');
* Takes a URL to a USDZ file and sets the appropriate fields so that Safari
* iOS can intent to their AR Quick Look.
*/
[$openIOSARQuickLook]() {
const modelUrl = new URL(this.iosSrc!, self.location.toString());
async[$openIOSARQuickLook]() {
if (this.arAutogenerateUsdz && !this[$generatedIosUrl]) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I just noticed a problematic aspect of your feature 2: if the file changes due to the scene-graph API (switched variant, swapped texture/color etc) and you jump into Quick Look again, you won't see the changes due to caching the USDZ. We could either skip the cache entirely (I don't think the perf change is a huge deal, especially since going into Quick Look twice with the same model is probably pretty rare), or else we'd have to add some kind of isDirty for the scene-graph functions that would tell us to regenerate the USDZ. I think simpler is better, especially for the first version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the scene-graph I can see that this is used when changing something:

[$needsRender]() {
  this[$scene].isDirty = true;
}

We can override this method in AR mixin to revoke our URL

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but the chance of breaking things is high. Again, I don't think the perf gain here is enough to justify the complexity. Let's just skip the USDZ caching and call it a win on memory footprint. 😁

Copy link
Contributor Author

@kolodi kolodi May 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alright, for now, let's make those bionic chips do their job :). I really hope they will finally let us use WebXR on iOS directly so we can avoid using USDZ for good :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here here! Someday...

this[$arButtonContainer].classList.remove('enabled');
kolodi marked this conversation as resolved.
Show resolved Hide resolved
await this.prepareUSDZ();
this[$arButtonContainer].classList.add('enabled');
}
const modelUrl = this.arAutogenerateUsdz ?
new URL(this[$generatedIosUrl]!, self.location.toString()) :
new URL(this.iosSrc!, self.location.toString());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can put the ternary op inside of the new URL() to simplify this a bit.

if (this.arScale === 'fixed') {
if (modelUrl.hash) {
modelUrl.hash += '&';
Expand All @@ -340,9 +354,50 @@ configuration or device capabilities');
const img = document.createElement('img');
anchor.appendChild(img);
anchor.setAttribute('href', modelUrl.toString());
if (this.arAutogenerateUsdz) {
anchor.setAttribute('download', 'model.usdz');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do? Is it necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I prepare the Blob, I use simply "application/octet-stream"for the type. So Safari need a "hint" to treat the link as a valid source to open Quick Look, that's why there is a download attribute. As a side effect you can also test the export in desktop browser as it will download the usdz file for you.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, interesting. I think it worked for me because I used a more specific mime type; you can see in my PR. The button still doesn't appear on desktop Safari though, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you have to manually override IS_AR_QUICKLOOK_CANDIDATE constant :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this tricks Safari to open the Quick Look. the file name has been chosen the same as it is in USDZExporter.js, although it could be anything.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my testing it wasn't necessary on my iPhone SE. But it also probably doesn't hurt anything.

}
anchor.click();
anchor.removeChild(img);
}

[$onModelLoad]() {
super[$onModelLoad]();
if (this.arAutogenerateUsdz && this[$generatedIosUrl] != null) {
URL.revokeObjectURL(this[$generatedIosUrl]!);
this[$generatedIosUrl] = null;
}
}

async prepareUSDZ() {
const updateSourceProgress = this[$progressTracker].beginActivity();
const scene = this[$scene];

const shadow = scene.shadow;
let visible = false;

// Remove shadow from export
if (shadow != null) {
visible = shadow.visible;
shadow.visible = false;
kolodi marked this conversation as resolved.
Show resolved Hide resolved
}

updateSourceProgress(0.2);
kolodi marked this conversation as resolved.
Show resolved Hide resolved

const exporter = new USDZExporter();
const arraybuffer = await exporter.parse(scene.modelContainer);
const blob = new Blob([arraybuffer], {
type: 'application/octet-stream',
});

this[$generatedIosUrl] = URL.createObjectURL(blob);

updateSourceProgress(1);

if (shadow != null) {
shadow.visible = visible;
}
}
}

return ARModelViewerElement;
Expand Down