Apple: Enable iOS Imaging support #3160
Apple: Enable iOS Imaging support #3160dgovil wants to merge 4 commits intoPixarAnimationStudios:devfrom
Conversation
|
Filed as internal issue #USD-9841 |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| // to our original, private texture. | ||
|
|
||
| // Modify texture descriptor to describe the temp texture. | ||
| #if defined(ARCH_OS_OSX) |
There was a problem hiding this comment.
could we have a comment here about why the Mode is not the same on desktop and embedded? I realize Managed isn't available on iOS but why aren't both Shared?
There was a problem hiding this comment.
I'll double check with Thor if there's any other reason, but afaik it's because Managed is required for older Macs with a dGPU (AMD/NVIDIA but not Intel).
A better check would probably be to use hasUnifiedMemory, instead of the ifdef.
However, I am torn on whether its best to leave Mac behaviour alone for now and address it in a future PR, so that we can keep the scope of this PR tight? Let me know how you feel, and I can put it in this PR if you prefer
There was a problem hiding this comment.
Checked with Thor, and it is the reason above. Anyway, I'll wait to hear back from you before I add a comment or just swap to the unifiedMemory check which should then be self explanatory.
There was a problem hiding this comment.
tbh I'd prefer we switch to the check because I worry that such a minor detail might just slip away for more months. @davidgyu any thought on this?
There was a problem hiding this comment.
Fair enough. I made the change after discussing with a few folks who seem confident it shouldn't cause issues.
Unfortunately I do not have a Mac with an intel gpu to test on, but it seems to be working fine on Apple Silicon Macs, so I think we should be good.
I don't have a good benchmark scene, but I suspect it'll be slightly faster too with this change now.
There was a problem hiding this comment.
I like that suggestion @meshula. I might be able to check for correct behavior on my stash of old macOS systems with Intel and AMD GPUs.
|
I need to fix up some issues the arose from rebasing. Fixing it up now, and should have it good to go soon. |
…ase earlier today
|
With some of the changes in 24.8, I'm just going to close this PR and re-open it later once I can resolve some things. Seems like its a little more involved than I thought. |
|
Okay that actually was less involved than I thought. I'll resubmit this PR because it won't let me re-open this for some reason. |
|
Re-opened here #3215 |
4 participants
Description of Change(s)
This PR ports Thor's imaging changes against the latest dev. This should enable imaging on all iOS derivatives, including visionOS.
I think I got everything, and it does build/run for me, but as mentioned I haven't done significant testing just yet.
I think we can use this branch to iterate the changes for imaging as needed.
Fixes Issue(s)