SSTU - Shadow Space Technologies Unlimited

SSTU - Shadow Space Technologies Unlimited

98.5k Downloads

Fairing Opacity Partially Broken in Editor

shadowmage45 opened this issue ยท 16 comments

commented

As per title; the opacity on fairings appears to be working inconsistently. For fresh parts added to the craft, the opacity is broken. For parts that were on the craft when it was loaded into the editor, the opacity works.

Likely cause is either KSP overriding some shaders or shader settings during part initialization, or the 'inactive part' opacity code is conflicting with the module-driven setup of the same property. (More of KSP's blatantly overwriting material property blocks without bothering to query for the existing property block first).

Potential solution is to delay the shader/material property updating code until the 'last moment' (likely the Start() method), or to run the opacity updating code a second time after the part is attached in the editor.

commented

I've seen some oddities in my own modding, and it did seem like moving the initialization later helped (either to OnStartFinished or Start).

Oddly enough it was the opposite case - something freshly placed worked fine but loading a saved craft caused it to break (well sort of, it looked fine when you first loaded the craft but after picking up and re-placing the part it would get messed up)

Overall though, transparency issues have been cleaned up since previous versions of KSP (I think this mainly happened in 1.2). At least you don't need to re-do the setup every time a part highlights.

commented

Interesting.... I haven't yet started digging into this problem, but that gives me a bit more information to help figure out what is going on.

Is 'OnStartFinished' a new stock method? Did they actually implement multi-pass initialization? (digging into the API now, and the answers appear to be 'yes' and 'yes' respectively). -- This might allow me to replace a ton of uses of the Unity 'Start()' method for cross-module interaction.

commented

Looks like it was added in 1.2, but it wasn't documented anywhere. And yeah, it definitely helps. There are scenarios where Start will still be necessary, for instance if you have to remove a module from the part, but otherwise yeah.

commented

Apparently stock now uses a MaterialPropertyBlock to set/overwrite the opacity values on the materials.

I think the only option is per-tick overwriting of that opacity value; investigating now if this is actually workable.

commented

Thanks, I'll definitely take another look at it this evening. What you have posted is almost exactly what my code was doing: https://github.com/shadowmage45/SSTULabs/blob/master/Plugin/SSTUTools/SSTUTools/Util/SSTUUtils.cs#L490-L499

Even my attempts at setting the opacity every tick through MPB in LateUpdate wasn't working. If I used material.getFloat or MPB.getFloat, they returned the proper 0.25f opacity value, but rendering was still full opaque.

I'll see if changing from sharedMaterial to material makes any changes; though it shouldn't as the material in use is non-shared (created at run-time specifically for these procedural meshes).

One thing to note on your code; anytime you call renderer.material -- it copies and returns a new instance of the material and assigns that returned reference to the renderer.sharedMaterial variable. So make sure you do not use renderer.material on a per-tick basis (and/or make sure to clean up/unload unused assets, as materials can cause memory leaks as they are not GC'd normally).

Sadly, I'm unaware of any way to tell if renderer.sharedMaterial returns a unique (non-shared) material instance. I think the only way to get it working cleanly is on any mesh you intend to do material-manipulation to query the renderer.material once after the model is initialized (you don't even need to do anything with the call, just pulling it to a local-scope variable will cause the copying). From there any further access to that particular renderer should be able to use renderer.sharedMaterial without problems.

commented

Yeah this is definitely not code I would want to run every frame - I would need to optimize it a lot more if that were the case. But it seems to work without running every frame.

That is an interesting tidbit about renderer.material though. I may have to look into that.

commented

Yeah, the details about the copy/replace/new-instance functionality can be found in the Unity docs:

https://docs.unity3d.com/ScriptReference/Renderer-material.html

That quirk actually bit me a few times causing memory leaks from per-frame material access. Not too hard to work around once you know what is going on, but is not exactly the behavior I would expect either.

commented

Based on the docs, it sounds like the material will only get cloned the first time you access it for a particular renderer. Any following calls will use the unique material for that renderer and not clone it again. Am I reading that wrong?

commented

It clones it -every- time that renderer.material is accessed, even if it was already accessed and assigned to renderer.sharedMaterial.

That was the cause of my memory leaks -- subsequent access. The memory-safe way is to call renderer.material and store the returned references for as long as you need it. The alternative is to call renderer.material a single time, and then access it through renderer.sharedMaterial for subsequent calls without storing a local reference.

commented

So is the documentation wrong then? It seems to imply that it will only clone the material once, if it is shared by any other renderers, and then return that instance going forward.

commented

Hmm, I tried this and was not able to reproduce the leak you described:

Material m1 = meshRenderer.material;
Material m2 = meshRenderer.material;

Debug.Log(object.ReferenceEquals(m1, m2)); // true

Do the calls have to be some amount of time apart to see this?

commented

Yeah, the docs are wrong, or at least misleading.

Use renderer.material with great caution wrt leaking memory / material instances.

commented

Very strange.... for some reason if I switch the shader to use KSP/Diffuse, the opacity in-editor works just fine with my existing / previous code. However the SSTU masked shader doesn't. Neither are 'transparent' shaders, and both appear to have the same shader-level settings (render queue, etc).

Doing some additional debugging now.

Even stranger -- in the Unity Editor, the opacity doesn't even work on the KSP/Diffuse shader. No clue what is going on there. Making it really hard to debug/test/fix though.

commented

Holy cow, what a PITA it is dealing with Unity shaders....

But I think I have made some progress, even if I had to go down many dead-end roads first...

screenshot8

In the end I had to add a 'keepalpha' line to the #pragma surface directive in the shader. Really no clue how the stock shaders allow alpha, as they do not have that line (at least in the shader source I have, no clue if that is the actual in-use shader source or not).

Now to do a bunch more debugging to figure out why/how the KSP shader works, and to clean up the mess I made of my plugin code while tracking down what was going on....

commented

Huh, perhaps they fixed it in some interim Unity version since I did testing (which was back in the... KSP-1.0 days). A reference equals would point to it having the same backing memory location.

Have you tried checking memory allocation before/after the calls? When I was testing I was getting like 3kb+ allocation per call to renderer.material; a simple allocation of a new reference should have been only ~64 bytes (+/- a bit for the allocations caused by the debug print statements)?

I'll have to do a bit more testing on this as well. If it now works reliably like you are seeing... well... I can clean up quite a bit of convoluted code.