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-placement ignored on quick look #3989

Open
3 of 13 tasks
pixlhero opened this issue Dec 6, 2022 · 15 comments · May be fixed by #4030
Open
3 of 13 tasks

ar-placement ignored on quick look #3989

pixlhero opened this issue Dec 6, 2022 · 15 comments · May be fixed by #4030

Comments

@pixlhero
Copy link

pixlhero commented Dec 6, 2022

Description

It seems that the ar-placement option gets ignored on AR Quick Look. Is this a bug or intended behaviour? If it's intended, the documentation should mention that imo. If it's not implemented yet, it should at least be possible to make it available when converting the .glb to .usdz file, because the placement info is embedded into the .usdz file for Quick Look.

Live Demo

on iOS doesn't show the vertical wall, but the horizontal floor at the onboarding animation:
https://modelviewer.dev/examples/augmentedreality/index.html#wall

Version

  • model-viewer: v2.1.1

Browser Affected

  • Chrome, version: xx.x.xxxx.xx
  • Edge
  • Firefox
  • IE
  • Safari

OS

  • Android
  • iOS 16.0 iPhone XR
  • Linux
  • MacOS
  • Windows

AR

  • WebXR
  • SceneViewer
  • QuickLook
@elalish
Copy link
Collaborator

elalish commented Dec 12, 2022

Indeed, it's not implemented yet. Any idea if there's a way to do this with the three.js USDZExporter? A PR would be most welcome.

@pixlhero
Copy link
Author

I checked the code over at threejs and it looks like it should be a small change to expose the placement options for the USDZExporter. I'll keep this issue updated on the progress there.

@pixlhero
Copy link
Author

Ok, so there is some good and bad news.

threejs supports supplying an optional options object to USDZExporter.parse in version 0.147.0 (it's also mentioned in their release notes).

Unfortunately @types/three is still on 0.146.0 currently, so to test I had to ignore typing for now.

But the main problem is how USDZ interprets "placing a 3D model on a wall". Specifically it will rotate the model automatically, such that the "feet" of the model are placed on the wall. This is different from how it works in Scene Viewer, where it will not rotate the model. For clarification see following pictures:

Scene Viewer Quick Look

This also seems to be the case when using Reality Composer, where the model is rotated depending on the anchor type as well.

Horizontal Vertical
Screenshot 2022-12-13 at 13 22 02 Screenshot 2022-12-13 at 13 22 09

One ugly solution would be to rotate the model before converting it to usdz. That would make it consistent across platforms, but also kind of "lie" to Quick Look, which could become a problem in the future.

The other solution I could think of is to leave the logic as it is. Maybe mention in the docs that ar-placement doesn't have any effect on Quick Look (converted usdz or given as ios-src).

Any thoughts on this?

@elalish
Copy link
Collaborator

elalish commented Dec 13, 2022

Ugh, good catch! I think I actually like the rotate-before-usdz-export option. The reason is that glTF specifies that +Y is up, hence the model-viewer/SceneViewer behavior. QuickLook's behavior implies USDZ has a different convention, so I think making this change as part of the conversion is appropriate. Likewise, we anchor at the center of the anchored bounding box face, so we may need to account for that too as part of this transform.

I have no problem with as any casting while we wait for the three types to update. I just updated us to r147, so this should be ready for a PR. Thanks for working on this!

@pixlhero
Copy link
Author

Hmm, well I am not too sure if it's easy to implement rotating the model before converting. Any pointers? I would be happy to fix this, but is there anywhere where model-viewer already transforms .glb models like this?

@elalish
Copy link
Collaborator

elalish commented Dec 13, 2022

Sure, search for the implementation of orientation. Basically you just need to apply some rotations/translations to the Object3D that's passed to the USDZExporter, then reset them after.

@pixlhero pixlhero linked a pull request Jan 3, 2023 that will close this issue
@vincentfretin
Copy link

From https://developer.apple.com/documentation/realitykit/uniform-token-preliminary-planeanchoring-alignment we can set aligment to "any" to snap on floor or wall. I tested the "any" in the USDZExporter options, unfortunately the model also rotate automatically, like "vertical".
The alignment "vertical" or "any" produce the same behavior in QuickLook for me, the model is still snapped on the floor prior to be snapped on wall.

I noticed that if we remove the two lines https://github.com/mrdoob/three.js/blob/aaafb4ffe8830932e31aa44003376457265d126a/examples/jsm/exporters/USDZExporter.js#L209-L210 it actually works fine, the model is not rotated and it snaps on both floor and wall. I found out this because producing a usdz with another tool didn't add those properties and using ios-src with this usdz worked fine on the wall.

@elalish
Copy link
Collaborator

elalish commented Apr 1, 2024

Interesting; presumably that means we can also control this from the MV side by changing the options we specify. Do you have a recommendation on what changes would be useful?

@vincentfretin
Copy link

We need a way in threejs USDZExporter to not add those lines. We could probably use the following:

const usdzOptions = {
  ar: undefined
};
const exporter = new USDZExporter() as any;
const arraybuffer = await exporter.parse(model, usdzOptions);

and in https://github.com/mrdoob/three.js/blob/aaafb4ffe8830932e31aa44003376457265d126a/examples/jsm/exporters/USDZExporter.js#L209-L210
check if options.ar is defined before adding those two lines.

@pixlhero
Copy link
Author

pixlhero commented Apr 2, 2024

I also have some updates. I tried for two days to get the rotation and position resetting right, with the malformed test-model. But I just can't get it to work. What I tried was moving it to the origin, rotating, and then moving it again to the previous offset. I tried a lot of other things, but I just can't get it to work.
So I'm giving up on this approach... Also not sure if I want to continue this PR.

@vincentfretin
Copy link

Thanks @pixlhero for trying.
I'll do first a PR in threejs so we are able to remove the two lines if placement is wall like I explained in my above comment.

Somewhat related to this, I wrote in the WebXR UX improvements thread about ar-placement="any" and ar-placement="both" if anyone have any feedback on this. #2413 (comment)

@yoavniran
Copy link

hi there,
Just want to make sure i'm not missing anything - there's currently no way to tell iOS which placement i'm interested in using for Quick Look, right?
There's no equivalent enable_vertical_placement=true (like in Scene Viewer), right?

thanks!

@pixlhero
Copy link
Author

hi there, Just want to make sure i'm not missing anything - there's currently no way to tell iOS which placement i'm interested in using for Quick Look, right? There's no equivalent enable_vertical_placement=true (like in Scene Viewer), right?

thanks!

There is technically. But it works different on iOS/QuickLook. It's actually embedded in the .usdz file. At the moment the best way seems to be to provide the usdz file yourself with ios-src. When creating the usdz file you can specify what orientation you want.

@vincentfretin
Copy link

I just pushed a PR for three.js mrdoob/three.js#28363 to allow removing the alignment properties in the generated usdz.
If this is merged, next step would be to create a PR to update the types in https://github.com/three-types/DefinitelyTyped/blob/master/types/three/examples/jsm/exporters/USDZExporter.d.ts see the new type I used in b6d4cbb
on my branch https://github.com/vincentfretin/model-viewer/tree/ios-placement-wall against model-viewer 3.5.0 three r163.
Please note that three r164 renamed USDZExporter.parse to USDZExporter.parseAsync so you will need to be careful to that when you update three in the repo @elalish
Once the changes are in three r165 and @types/three and model-viewer master are on this version, then I could do a PR against model-viewer.

If you want to test my branch ios-placement-wall:
I really struggled with the types and getting my three fork to install
when I do npm install at the root,
node_modules/@types/three/ is 0.163.0 but packages/model-viewer/node_modules/@types/three/ is 0.164.0 giving an error with npm run build, and we currently use three 0.163.0 in model-viewer 3.5.0
what I did:

npm install
cd packages/model-viewer
rm -rf node_modules/@types/three/
cp -rf ../../node_modules/@types/three/ node_modules/@types/
cp -rf ~/workspace/three.js/ node_modules/three/
npm run build

~/workspace/three.js/ is a clone of my https://github.com/vincentfretin/three.js/tree/usdzexporter-wall-and-floor-r163 branch, same as the three PR but for r163.

@vincentfretin
Copy link

FYI mrdoob/three.js#28363 is merged and types will also be up to date three-types/three-ts-types#938 for r165.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants