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.

Sign up for GitHub

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

Reduce allocations #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+22 −8
Open

Reduce allocations #12

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 22 additions & 8 deletions Runtime/Mirror.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ public enum OutputScope
UniversalAdditionalCameraData m_CameraData;
RenderTexture m_RenderTexture;
RenderTextureDescriptor m_PreviousDescriptor;
string m_identifier;
#endregion

#region Constructors
Expand Down Expand Up @@ -197,6 +198,17 @@ RenderTextureDescriptor GetDescriptor(Camera camera)
var renderTextureFormat = hdr ? RenderTextureFormat.DefaultHDR : RenderTextureFormat.Default;
return new RenderTextureDescriptor(width, height, renderTextureFormat, 16) { autoGenerateMips = true, useMipMap = true };
}

bool DescriptorEquality(RenderTextureDescriptor a, RenderTextureDescriptor b)
{
return
a.width == b.width
&& a.height == b.height
&& a.colorFormat == b.colorFormat
&& a.autoGenerateMips == b.autoGenerateMips
&& a.useMipMap == b.useMipMap
&& a.depthBufferBits == b.depthBufferBits;
}
#endregion

#region Rendering
Expand All @@ -206,15 +218,16 @@ void BeginCameraRendering(ScriptableRenderContext context, Camera camera)
if(camera.cameraType == CameraType.Preview || camera.cameraType == CameraType.Reflection)
return;

m_identifier ??= $"Mirror {gameObject.GetInstanceID()}";
// Profiling command
CommandBuffer cmd = CommandBufferPool.Get($"Mirror {gameObject.GetInstanceID()}");
using (new ProfilingSample(cmd, $"Mirror {gameObject.GetInstanceID()}"))
CommandBuffer cmd = CommandBufferPool.Get(m_identifier);
using (new ProfilingSample(cmd, m_identifier))
Comment on lines +221 to +224
Copy link
Author

@Eideren Eideren Jun 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Identifier is a constant, string interpolation is never cached so might as well do it manually.

Something you might want to take a look at is the fact that pooled commandbuffers cannot be reused when profiling, perhaps related to this issue on the unity tracker - so right now the profiler will spit errors.
Deep profile works as expected though, not sure what's up with that.
We could always omit the command from the sample ?

{
ExecuteCommand(context, cmd);

// Test for Descriptor changes
var descriptor = GetDescriptor(camera);
if(!descriptor.Equals(m_PreviousDescriptor))
if(!DescriptorEquality(descriptor, m_PreviousDescriptor))
Copy link
Author

Choose a reason for hiding this comment

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

This relied on default value type equality comparison which generates about 80 bytes per call

{
// Dispose RenderTexture
if(m_RenderTexture != null)
Expand All @@ -233,6 +246,7 @@ void BeginCameraRendering(ScriptableRenderContext context, Camera camera)
SetShaderUniforms(context, m_RenderTexture, cmd);
}
ExecuteCommand(context, cmd);
CommandBufferPool.Release(cmd);
Copy link
Author

Choose a reason for hiding this comment

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

Official logic from unity does release pooled commands, now I'm not very familiar with this so feel free to investigate further if you think that's not right

}

void RenderMirror(ScriptableRenderContext context, Camera camera)
Expand Down Expand Up @@ -305,16 +319,16 @@ Vector4 GetMirrorPlane(Camera camera)
#region Output
void SetShaderUniforms(ScriptableRenderContext context, RenderTexture renderTexture, CommandBuffer cmd)
{
var block = new MaterialPropertyBlock();
var block = renderers.Count == 0 ? null : new MaterialPropertyBlock();
Copy link
Author

Choose a reason for hiding this comment

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

Blocks are not required when no renderers are there to feed them to, 'kind of ugly but I didn't want to significantly change your logic. Feel free to change into something less hacky.

switch(scope)
{
case OutputScope.Global:
// Globals
cmd.SetGlobalTexture("_ReflectionMap", renderTexture);
ExecuteCommand(context, cmd);

// Property Blocm
block.SetFloat("_LocalMirror", 0.0f);
// Property Block
block?.SetFloat("_LocalMirror", 0.0f);
foreach(var renderer in renderers)
{
renderer.SetPropertyBlock(block);
Expand All @@ -325,8 +339,8 @@ void SetShaderUniforms(ScriptableRenderContext context, RenderTexture renderText
Shader.EnableKeyword("_BLEND_MIRRORS");

// Property Block
block.SetTexture("_LocalReflectionMap", renderTexture);
block.SetFloat("_LocalMirror", 1.0f);
block?.SetTexture("_LocalReflectionMap", renderTexture);
block?.SetFloat("_LocalMirror", 1.0f);
foreach(var renderer in renderers)
{
renderer.SetPropertyBlock(block);
Expand Down