icosa-foundation / open-brush

Open Brush is the open source, community led evolution of Tilt Brush! Forked from https://github.com/googlevr/tilt-brush
https://openbrush.app
Apache License 2.0
777 stars 166 forks source link

Unity 2022 LTS #708

Closed mikeskydev closed 1 week ago

mikeskydev commented 2 months ago

Upgrade to 2022 LTS (again). Noting I had to change the tracking origin from Floor to Unspecified in the main scene, as it wants to set this itself and you end up in the floor if you don't. This may break other platforms though, so we'll need a good test. This was compared to the official VR template that also seemed to have this "bug".

Test Platforms:

mikeskydev commented 2 months ago

Yes and yes, that color space thing has been pending forever now! The other was a half hearted attempt to fix the input sensitivity, but I'm going to need to dive deeper.

andybak commented 2 months ago

Noting I had to change the tracking origin from Floor to Unspecified in the main scene, as it wants to set this itself and you end up in the floor if you don't. This may break other platforms though, so we'll need a good test.

How does this affect the "locked to world" mode we use with passthrough?

mikeskydev commented 2 months ago

It doesn't, just the runtime picks if its device or floor instead of us setting directly, or in quest's case overrides to stage.

mikeage commented 2 months ago

First results on Q1 seem reasonable. No major improvements or degradations seen, but a few small observations:

  1. Language works correctly on Q1 (see the discord bug where it didn't before)!
  2. The initial splash screen background changed from black to a sort of pale dark brownish color.

I couldn't get the profiling tool to work properly (see discord again), so I'm not sure how the performance is.

andybak commented 2 months ago

This is weird. I merged this into a different branch just to test if it fixed an issue on that branch and I found I couldn't build.

The error was:

Shader error in 'TiltBrush/Standard (Specular setup)': 'vertShadowCaster': no matching 2 parameter function at line 189 (on d3d11)

Which relates to Assets/ThirdParty/UnityODS/TiltBrushStandardSpecular.shader

This is basically a copy of Unity's built-in Standard Specular, with various hooks added to support ODS rendering.

The file has never been modified (going right back to the first open source code dump) and it's drifted a long way from the current built-in Unity code. I think it's failing because of this:

    vertShadowCaster(v, opos
#ifdef UNITY_STANDARD_USE_SHADOW_OUTPUT_STRUCT
                                    ,o
#endif
#ifdef UNITY_STANDARD_USE_STEREO_SHADOW_OUTPUT_STRUCT
                                    ,os
#endif
    );

I presume neither of those compiler flags exists any more so the result is an invalid function call.

But the mystery is - how comes it builds for you @mikeskydev ?

andybak commented 2 months ago

Here's a clue as to the way to fix it:

https://github.com/KhronosGroup/UnityGLTF/blob/99c96d9029d4ce02e5f729a5e069a44aef6faa99/Runtime/Shaders/UnityStandardShadow.cginc#L26

Still doesn't explain why it works ok for the CI build on this PR.

andybak commented 2 months ago

This commit might be in the right ballpark: https://github.com/icosa-foundation/open-brush/commit/df36787e5a01fbdd3c4f51675fa0ffc90318b911

mikeskydev commented 1 month ago

This is weird. I merged this into a different branch just to test if it fixed an issue on that branch and I found I couldn't build.

The error was:

Shader error in 'TiltBrush/Standard (Specular setup)': 'vertShadowCaster': no matching 2 parameter function at line 189 (on d3d11)

Which relates to Assets/ThirdParty/UnityODS/TiltBrushStandardSpecular.shader

This is basically a copy of Unity's built-in Standard Specular, with various hooks added to support ODS rendering.

The file has never been modified (going right back to the first open source code dump) and it's drifted a long way from the current built-in Unity code. I think it's failing because of this:

    vertShadowCaster(v, opos
#ifdef UNITY_STANDARD_USE_SHADOW_OUTPUT_STRUCT
                                    ,o
#endif
#ifdef UNITY_STANDARD_USE_STEREO_SHADOW_OUTPUT_STRUCT
                                    ,os
#endif
    );

I presume neither of those compiler flags exists any more so the result is an invalid function call.

But the mystery is - how comes it builds for you @mikeskydev ?

I didn't notice this error in 2022, but I did when i played around with unity 6? 🤔 Not sure why it was giving only you issues - but did your borrowed code from UnityGLTF fix it?

mikeage commented 1 month ago

Can anyone confirm if the development builds work for them?

mikeskydev commented 1 month ago

I'll add a list in the first post we can tick off to check

mikeage commented 1 month ago

Thanks. It's probably important on its own, but more than that, I wanted to use it for benchmarking

mikeskydev commented 3 weeks ago

head offset issues in both pico and quest versions

mikeskydev commented 3 weeks ago

Quest 1 and Android OpenXR builds aren't working on a Quest 3 either, when theoretically they should.

mikeskydev commented 2 weeks ago

Quest 1 and Android OpenXR builds aren't working on a Quest 3 either, when theoretically they should.

This might have been Meta that broke this, theoretically the newest version of the android openxr loader in unity's openxr should work

mikeage commented 2 weeks ago

Reminder that when merging this, the existing caches should be deleted (and once it builds, open PRs should rebase/merge with it)

mikeage commented 1 week ago

Followup PRs: #732 #733