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

Unity 2022 LTS #708

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Unity 2022 LTS #708

wants to merge 15 commits into from

Conversation

mikeskydev
Copy link
Member

@mikeskydev mikeskydev commented May 3, 2024

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:

  • Quest 1
  • Quest 2
  • PCVR
  • Oculus PCVR
  • Pico
  • Monoscopic Windows
  • Monoscopic Mac
  • Monoscopic Linux
  • Zapbox

@mikeskydev mikeskydev added the maintenance Upgrades etc. label May 3, 2024
@mikeskydev mikeskydev requested review from andybak and mikeage May 3, 2024 14:49
Copy link
Member

@mikeage mikeage left a comment

Choose a reason for hiding this comment

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

I highlighted two lines that I didn't expect to change; were they deliberate?

I can test on Q1 on Saturday night, assuming it builds

Assets/Resources/OculusRuntimeSettings.asset Show resolved Hide resolved
Assets/InputSystem.inputsettings.asset Show resolved Hide resolved
@mikeskydev
Copy link
Member Author

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
Copy link
Contributor

andybak commented May 3, 2024

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
Copy link
Member Author

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
Copy link
Member

mikeage commented May 5, 2024

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
Copy link
Contributor

andybak commented May 5, 2024

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
Copy link
Contributor

andybak commented May 5, 2024

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
Copy link
Contributor

andybak commented May 5, 2024

This commit might be in the right ballpark: df36787

@mikeskydev
Copy link
Member Author

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
Copy link
Member

mikeage commented May 22, 2024

Can anyone confirm if the development builds work for them?

@mikeskydev
Copy link
Member Author

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

@mikeage
Copy link
Member

mikeage commented May 22, 2024

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

@mikeskydev
Copy link
Member Author

head offset issues in both pico and quest versions

@mikeskydev
Copy link
Member Author

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

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

Successfully merging this pull request may close these issues.

None yet

3 participants