search

specklesystems / speckle-unity

AEC Interoperability for Unity through Speckle
https://speckle.systems/tag/unity/
Apache License 2.0
55 stars 20 forks source link

Missing Shader on Build #38

Closed psarras closed 3 years ago

psarras commented 3 years ago

It seems that after you build the line shader = Shader.Find("Transparent/Diffuse"); has trouble returning a valid shader. I think this is probably because this shader is not included in the build, unless it is used somewhere in the scenes..

image

A potential fix would be to have a central settings GO or panel to add your own materials.

ln 414 in ConvertUnity.Geometry.cs

JR-Morgan commented 3 years ago

Hi @psarras, currently inorder to build projects using the Speckle Connector, you need to make sure you add Standard and Transparent/Diffuse to project settings. Project Settings → Graphics → Built-in Shader Settings → Always Included shaders

image

@teocomi It might be possible for us to explore to ways of allowing users to specify their own shaders, similar how it now works in the Unreal connector.

psarras commented 3 years ago

Hey @JR-Morgan, aren't those saved on the settings, so I should have them no?

https://github.com/specklesystems/speckle-unity/blob/main/ProjectSettings/GraphicsSettings.asset

JR-Morgan commented 3 years ago

Hi @psarras, sorry for taking a while to get back (see Slow Mode: We’re on a Retreat!)

Hey @JR-Morgan, aren't those saved on the settings, so I should have them no?

That was my expectation aswell @psarras.

Unfortuantly, it looks like Unity doesn't modify GraphicsSettings.asset when you add shaders to the Always Included shaders. (Unity actually saves these changes to Library/SceneVisibilityState.asset which is ignored by git).

Since these changes are not picked up by git, Speckle Unity users (currently) need to manually add the two shaders to Always Included shaders. This extra step is quite annoying, so thanks for highlighting the issue.

I have experimented with modifying the GraphicsSettings.asset to include Standard and Legacy Shaders/Transparent/Diffuse, and it looks like this might solve this issue. 🥳

I'm not entirely sure why Unity doesn't update the GraphicsSettings, there might be a good reason, so I want to do a little more testing and asking around before I merge that fix to main.

JR-Morgan commented 3 years ago

I've tested now with a blank project created in 2020.3.17f, and it looks like changing Always Included shaders does update the GraphicsSettings.asset.

Not sure if I was experiancing some sort of Unity bug or import issue that was causing GraphicsSettings.asset to not be updated, but I've created a Pull request #42 that should now fix the issue for this repo's project.

Users creating their own project, and importing the connector assets will still need to add the shaders manually.

JR-Morgan commented 3 years ago

It might be worth exploring some options to allow users to select what shaders they want to use, rather than allways using Standard and Legacy Shaders/Transparent/Diffuse.

This would be similar to what I've just implemented in the Unreal connector.

Next sprint, I plan on doing some rework of the way Materials are imported, I'll create a seperate issue for the above and hopefully work on it soon.