search

google / model-viewer

Easily display interactive 3D models on the web and in AR!
https://modelviewer.dev
Apache License 2.0
6.84k stars 807 forks source link

QuickLook broken on visionOS, does not open generated USDZ blobs #4354

Closed hybridherbst closed 6 months ago

hybridherbst commented 1 year ago

Description

Looks like iOS 17 has changed something about how dynamically generated USDZ files are treated, and as a consequence the AR conversion of model-viewer breaks.

Edit: From my tests, it looks like when the file extension is missing (such as when using a blob URL from URL.createObjectUrl) QuickLook immediately closes or, in some cases, crashes Safari.

Live Demo

Bug Reports

Version

latest

Browser Affected

OS

AR

elalish commented 1 year ago

@meshula Looks like QuickLook broke again. Any ideas regarding whether this is intentional or can be worked around or is planning to be fixed before release? @grorg too - hoping we can get a fix in before Safari's branch cut this time?

meshula commented 1 year ago

According to Felix's analysis, the usdz file is able to be loaded by QuickLook, so I don't think there's a problem with the USD side of things. I think the issue title might more accurately be "QuickLook doesn't open files from model-viewer"?I'm not an Apple or Google employee, so I wouldn't have knowledge about how model-viewer and QuickLook are intended to work together.

elalish commented 1 year ago

Gotcha, thanks. This is QuickLook / Safari not handling Blob URLs properly, most likely. More in @grorg's wheelhouse, probably.

meshula commented 1 year ago

An issue with blob url handling sounds quite possible!

milesgreen commented 1 year ago

Confirming I see same issue.

Safari unable to open USDZ blob, but can open USDZ from URL. Also, Chrome IS able to open USDZ from blob. So appears to be a Safari regression rather than QuickLook?

iPhone 12 Pro, iOS 17 Beta 3, (21A5277h)

milesgreen commented 1 year ago

Reported on WebKit Bugzilla too, to cover all bases. https://bugs.webkit.org/show_bug.cgi?id=259141

hybridherbst commented 1 year ago

Updated the issue title to mention blobs and added the WebKit bug link in the post, too. @milesgreen thanks for opening the WebKit issue! You may want to add that the issue also reproduces in iPadOS and visionOS (in the simulator)

milesgreen commented 1 year ago

Looks like Dean is already on it with a fix in the works:

https://bugs.webkit.org/show_bug.cgi?id=258957

lalit-kumar-1994 commented 1 year ago

After updating to IOS 17 beta, having an issue with the Quicklook viewer Any buddy faced the same issue and what would be the solution for it ios17beta

hybridherbst commented 1 year ago

@lalitkumar1994 iOS 17 currently doesn't support QuickLook from blob URLs, thus this issue. Apple will need to fix this in Safari and/or in iOS.

milesgreen commented 1 year ago

Just tested with the very latest iOS 17 Dev Beta 5 (21A5303d), and this issue now appears to be fixed. I have tried a number of USDZ blob scenarios and so far they are all working as expected. So it looks like this fix (https://bugs.webkit.org/show_bug.cgi?id=258957) is in the build and should launch with iOS17 public release.

hybridherbst commented 1 year ago

Can confirm it works on the modelviewer.dev page BUT not on all pages using model-viewer or three's USDZ conversion in general...

hybridherbst commented 1 year ago

@milesgreen does it work for you on https://threejs.org/examples/?q=usdz#misc_exporter_usdz?

milesgreen commented 1 year ago

Zoiks. No. The three.js demo link above does not work for me. Clicking on the AR button opens QuickLook briefly then closes immediately.

milesgreen commented 1 year ago

ModelViewer latest build is using three r154, right? Could this be a three r155 issue? So might the failing ModelViewer examples be using the unbundled MV library and instead using three r155?

hybridherbst commented 1 year ago

Yes, should be three r154. I tested a couple of things today, trying to see if there's something that can be fixed in three in the meantime:

I also went through and compared how three and model-viewer launch QuickLook with blobs and it looks like it's identical (rel="ar" is there, download="model.usdz" is there, the a has an img child, ...). Can't find a meaningful difference. Resulting USDZ files seem to be mostly identical too.

milesgreen commented 1 year ago

@hybridherbst - I just downloaded the very latest iOS 17 Dev Beta 6 (21A5312c) and, for me, the three.js examples usdz_exporter now DOES work (it didn't work with beta 5). So unless something changed in three.js, this might be an improvement in iOS 17. How do your tests now perform with Beta 6?

hybridherbst commented 1 year ago

Thanks @milesgreen, I'm very happy to confirm that iOS 17 Dev Beta 6 fixes the issue. It's still not working on visionOS (in the simulator at least), so I'll keep the issue open until that is hopefully fixed too, but looks like we're on a good way.

lalit-kumar-1994 commented 1 year ago

@hybridherbst I am still facing same issue using reality file, Even after updating to newer 17beta version.

lalit-kumar-1994 commented 11 months ago

is any one facing same issue still? Even after updating to iOS 17 stable version

DouglasLapsley commented 10 months ago

We're seeing the rel="ar" links failing on iOS 17 production release. Previously working links are now failing with the "Object requires a newer version of iOS". A simple <a href.... works.

`

`

But it opens the ugly AR preview before going into the AR itself.

I really wish Apple would stop mucking around with this stuff.

elalish commented 10 months ago

FYI @grorg

vortice3D commented 10 months ago

Good evening, @DouglasLapsley, @lalitkumar1994 and, of course, @elalish:

I'm experiencing this same issue under iOS 17.0.3 running on an iPad (9th generation), trying to visualize .reality files created with Reality Composer app on iOS 16.5.1 running on an iPad (8th generation). Contents are shown flawless in iOS 16 and below devices.

Any success solving this?

Thanks for your time.

P.S. By the way, as Douglas says, the contents are visualized on iOS 17 if the rel="ar" property is deleted from the anchor tag, but then an annoying (not styled) AR preview appears as an additional step, ruining all the UX/UI.

hybridherbst commented 10 months ago

@vortice3D please report a bug to Apple and feel free to post the Feedback item number you get here - there seem to be more issues at hand here so we really need to get them in front of their eyes.

vortice3D commented 10 months ago

Hi there, @hybridherbst:

I did that way and the issue is at the moment waiting for moderation. Anyway you know about that Apple's developer forums, hundreds (or thousands) of views but not answers at all.

Anyway, maybe the following info could bring some light to the issue. Hosting the "CosmonautSuit_en.reality" example from Apple (https://developer.apple.com/augmented-reality/quick-look/) in my server, the same "Object requires a newer version of iOS." (as @DouglasLapsley and @lalitkumar1994 both experiment) is shown on screen. That is an Apple's generated and so perfectly formed .reality file, so it leads me to think that the problem might be related to something in the hosting server.

Perhaps, something related with security specification for the hosting server that has been changed on iOS17 (but not on previous versions of Safari). I'm saying here "extra" because I have at the moment a valid SSL/TLS certificate, so I wonder if HSTS would be also necessary.

In parallel, the direct access to the .reality file (typing its URL in the address box of Safari) do work, and you can access the AR experience by clicking the on-the-fly poster generated by Quick Look AR (QLAR). Also, not specifying the rel="ar" attribute in the anchor tag makes the trick, and, again after showing the on-the-fly generated poster, you are transported to the AR by clicking it down.

Thanks for your time.

milesgreen commented 10 months ago

@vortice3D - do you have a repro link to test?

I just made a Glitch that opens a copy of the Apple Cosmonaut reality file here:

https://arql-reality-test.glitch.me/

I'm testing in iOS 17.2 beta (21C5029g) and this opens fine for me.

Does the above link work for you?

vortice3D commented 10 months ago

Good morning (here is Spain), @milesgreen:

First, let me thank you very much for the interest in this issue of mine. Appreciate a lot!

Your page (and of course Apple's one) are working flawless, so obviously something is going wrong with my web-server, even having it (and the QLAR subdomain I'm using for this tests) with a valid SSL/TLS certificate. Also, the same page is working all right from iOS16 and below.

Anyway, please, check my version of your "glitch" by yourself.

Accessing it from several iPhone and iPad devices, running on iOS 17.X, is showing the infamous "Object requires a newer version of iOS."

Again, thanks for your time!

hybridherbst commented 10 months ago

@vortice3D if it only happens with your webserver I recommend checking the mime type of the served files, QuickLook is picky about those being correct (and maybe got even more picky).

It looks like your server serves the file as application/octet-stream but it needs to be model/vnd.reality as it is on @milesgreen server. image

So make sure your server returns .reality as model/vnd.reality and .usdz as vnd.usdz+zip.

milesgreen commented 10 months ago

I think @hybridherbst is right on the money.

To sanity check though, @vortice3D, I see your sample link breaks in iOS 17.2 beta, but does indeed work in iOS 16.6.1.

Also, both our links work in iOS 16.6.1

So if it is mime types, then those must have become stricter in iOS17

Let us know if updating mime type solves it.

vortice3D commented 10 months ago

Good evening, @hybridherbst and @milesgreen:

It's really curious, but the case is walking back to home, after writing to you my last response at office, I was thinking about all this and suddenly the MIME stuff came to my mind (walking and explaining to your rubber duck are the best for solve software developing problems). It have all the sense.

So, as soon as I get home, I just opened my Plesk and updated Apache/nginx settings, adding the aforementioned types: model/vnd.usdz+zip .usdz model/vnd.reality .reality

Now it's working fine, as you can check.

Again, thank you very much for your help on this issue.

Best regards.

chelbergeac commented 6 months ago

I have discovered a new detail on this. I have a GLB model that I'm using with Model Viewer. It works fine on current iOS when I just load the Model Viewer page and activate AR. However, if I use

{somepart}.pbrMetallicRoughness.setBaseColorFactor([0,1,1])

to modify a component's color, I get the desired effect in the Model Viewer view, but when I try to switch over to AR/QuickLook, I get the error "object requires a newer version of iOS" and I can't see my model in AR.

It seems that the general conversion to USDZ works OK, but if the appearance has been modified (e.g. setBaseColorFactor()), then the conversion fails, or the converted file is somehow incompatible with iOS 17.

I see this problem in both Safari and Chrome on iOS, on both iPhone 12 and a reasonably new iPad pro.

elalish commented 6 months ago

@chelbergeac Thanks, that's very helpful info indeed! @mrdoob this seems to point to a USDZExporter issue. Can you repro? Wild guess is a color value going negative because of a color management conversion?

hybridherbst commented 6 months ago

@chelbergeac would you be able to provide the source GLB and the converted USDZ that you're getting? Thanks

hybridherbst commented 6 months ago

@elalish I think this is actually a bug in or incorrect usage of model-viewer: When using setBaseColorFactor([0,1,1]) it looks like the opacity value is set to undefined here:

I don't think that's allowed / expected to work.

chelbergeac commented 6 months ago

@hybridherbst Ah, yes, you are right. Switching to CSS style color specs, or adding the extra A parameter to the array, both resolved the problem. Thanks!

FWIW, I was working from the example here: https://modelviewer.dev/examples/scenegraph/#pickMaterialExample, which includes the line: material.pbrMetallicRoughness. setBaseColorFactor([Math.random(), Math.random(), Math.random()]);

Also, the 3-parameter version does seem to work the way I expected it to everywhere except in the USDZ QuickLook conversion, so that's why I thought it was a problem with the conversion.

hybridherbst commented 6 months ago

Potentially three.js does better checks in general for an invalid opacity parameter – according to the docs, only number is allowed though, not undefined :) So yes, the conversion could check, but in my experience PRs for "protect me from doing something explicitly not allowed and undefined" are most often rejected.

chelbergeac commented 6 months ago

@hybridherbst Right--I wasn't suggesting that the conversion be modified to try to cover for omitted A channel, but maybe the examples should be updated so they don't include code that makes this omission. There are a few spots in the example code that use this function with only 3 parameters instead of 4.

mrdoob commented 6 months ago

@hybridherbst

Potentially three.js does better checks in general for an invalid opacity parameter – according to the docs, only number is allowed though, not undefined :) So yes, the conversion could check, but in my experience PRs for "protect me from doing something explicitly not allowed and undefined" are most often rejected.

Can we do that check in the USDZExporter?

hybridherbst commented 6 months ago

@mrdoob do you still want that change in USDZExporter? I think since it's invalid data and now fixed in model-viewer it's not needed. Maybe I understood something wrong though in my various PRs regarding invalid data handling in exporters 👀

mrdoob commented 6 months ago

I thought this was an issue with USDZExporter not handling colors properly but maybe I misunderstood?

hybridherbst commented 6 months ago

No, it's an issue with model-viewer setting opacity to undefined on materials. That in turn seems to be handled across parts of the three.js codebase, but according to the docs opacity is a number and USDZExporter / GLTFExporter expect it to be a number.

mrdoob commented 6 months ago

Ah I see... Yeah...

It would be nice if there was a way to tell the JS engine that a variable shouldn't change type.

chelbergeac commented 6 months ago

Strictly speaking, always including opacity could be considered a best practice. OTOH, from my perspective, some "syntactic sugar" that sets it to 1 if it's omitted would be nice (which seems to be what three.js does). Especially since this pattern shows up in some of the example code.

hybridherbst commented 6 months ago

I believe the sample code has now been updated to be correct, plus the API in model-viewer fixed:

I think if you find more issues in the docs/samples using the API wrong, that PR is a good place to add info about it

chelbergeac commented 6 months ago

@hybridherbst excellent, thanks!