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

Underwater Fog Input #867

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Underwater Fog Input #867

wants to merge 12 commits into from

Conversation

daleeidd
Copy link
Collaborator

@daleeidd daleeidd commented Jul 9, 2021

Status: Experimental

Adds an input for underwater fog which renders whatever it is attached to after the underwater effect and applies the fog in a separate pass. I have named the component RegisterUnderwaterInput for consistency but it could be renamed.

The original idea was to use Blend and BlendOp to apply the fog but I wasn't able to work it out. I think using subtract might work if we somehow bake density fog and scatter colour into the fragment colour. Instead I rendered the object offscreen and provided texture to the fog pass so it samples itself to get the screen colour. This way it works like the underwater effect does.

Steps:

  1. Register object and disable renderer
  2. Sorts objects back to front using distance from camera
  3. Enables SH and set SH values using LightProbeUtility
  4. Setup main texture to get alpha channel
  5. Copies camera target to temporary texture
  6. Renders object into temporary texture
  7. Passes temporary texture to fog and renders fog over the top using alpha blending

It didn't work out as nice as I hoped as it exposed us a little bit to render pipeline (SH and _MainTex). We need _MainTex to get the alpha value for blending; an example is the particles will be rendered as squares without it.

Also, I had to cull back faces as the example transparent objects in the test scene didn't like culling disabled (need to look into that), but some transparent objects want culling disable for them to look right.

I made the shader a separate pass rather than a separate file as it saves us another material and code reuse.

I have provided test data in the main scene.

Let me know what you think.

Renders anything after underwater effect and applies underwater fog.
Useful for some transparent objects and particles.
@daleeidd daleeidd requested a review from huwb July 9, 2021 23:59
Copy link
Contributor

@huwb huwb left a comment

Choose a reason for hiding this comment

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

The sh code is interesting to see. Is it needed here? I'm aware it's used to light the underwater fog, but had thought it was evaluated at the camera position, and am wondering if we already have it evaluated somewhere else (in oceanrenderer maybe)

}

return half4(wt * sceneColour, 1.0);
}
ENDHLSL
}

Pass
Copy link
Contributor

Choose a reason for hiding this comment

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

A comment above each pass might be useful to explain each one

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@huwb
Copy link
Contributor

huwb commented Jul 10, 2021

I wasn't able to add a general comment without submitting the review. Nice one GitHub.

@huwb
Copy link
Contributor

huwb commented Jul 10, 2021

Yeah the blending is worth talking through. I had the following in my head but I don't know if it works

  • User has alpha geo in the scene and unity just draws on top of the underwater i.e. it does not have depth fog applied and looks broken. This uses whatever transparent shader is applied. Unity applies SH and everything else needed to render as normal.
  • We have a component to tag this geo, like you've added, and that renders the geo again but with the depth fog shader which is what I think you have. It alpha blends, with the alpha equal to whatever the depth fog gives (exp(-density*waterRayLength)).

Would that work?

using UnityEngine;

// TODO: Maybe rename.
public class RegisterUnderwaterInput : MonoBehaviour
Copy link
Contributor

@huwb huwb Jul 10, 2021

Choose a reason for hiding this comment

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

Yeah I think it may want to be distinct from the register input naming as it's quite different. Although that might just be my bias from knowing the internals. My brain initially went to something like "ApplyUnderwaterEffectToAlpha"

(Now that I'm commenting on a particular line, its forcing me to submit a new review 😅)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I renamed to ApplyUnderwaterEffectToTransparent as that might be more recognisable.

@daleeidd
Copy link
Collaborator Author

The sh code is interesting to see. Is it needed here? I'm aware it's used to light the underwater fog, but had thought it was evaluated at the camera position, and am wondering if we already have it evaluated somewhere else (in oceanrenderer maybe)

I am using it for rendering the transparent object the first time round. For some reason it isn't bound if I render it myself even though I am using DrawRenderer. I also have to enable the SH keyword manually. I will look into what I can do to remove it. I should pass the one we already have from the underwater effect for the fog pass to keep it consistent at the very least.

It alpha blends, with the alpha equal to whatever the depth fog gives (exp(-density*waterRayLength)).

That was my first attempt but density has three components which doesn't look right when converted to a scalar value (tried averaging it or picking only one).

The is what the underwater effect has:

lerp(sceneColour, scatterCol, saturate(1.0 - exp(-_DepthFogDensity.xyz * sceneZ)));

And this is what I tried:

return (scatterCol, saturate(1.0 - exp(-Average(_DepthFogDensity.xyz) * sceneZ)));

Here is a comparison. Current:
2

Using depth fog as alpha value:
1

It might be tolerable?

@huwb
Copy link
Contributor

huwb commented Jul 11, 2021

Ah interesting! It may be worth trying min or max, but otherwise yeah tolerable I would vote. I don't think we should do anything special to render the original alpha geo. I have expanded my understanding notes above accordingly.

@daleeidd
Copy link
Collaborator Author

Ah interesting! It may be worth trying min or max

BlendOp Min or Max? Neither produced good results.

but otherwise yeah tolerable I would vote

I have added both for now for evaluating. Or maybe permanently if you think technical debt is not too high.

User has alpha geo in the scene and unity just draws on top of the underwater i.e. it does not have depth fog applied and looks broken. This uses whatever transparent shader is applied. Unity applies SH and everything else needed to render as normal.

I have thought of another approach which might be closer to this. We render the fog before the transparent pass. And add the fog calculation to the ocean shader again. Then we just duplicate the "tagged" objects and then dilate them slightly. The duplicate will have our material which adds the fog and alpha blends. It should sort correctly and be similar in quality to the lower quality alpha blending above. And requires minimal intervention from us.

@huwb
Copy link
Contributor

huwb commented Jul 24, 2021

Oh I meant use min or max density (of the three channels). Not confident it would help but just an idea.

Would you be able to let me know where my notes are falling short or not going to work? I'm not following I'm afraid and am hoping we can use it as a tool to sync on the plan

@daleeidd daleeidd force-pushed the feature/underwater-fog-input branch from cca31e1 to b452e34 Compare August 12, 2021 07:43
@daleeidd daleeidd force-pushed the feature/underwater-fog-input branch from b452e34 to 22961c1 Compare August 12, 2021 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants