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

Reduce allocations #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Eideren
Copy link

@Eideren Eideren commented Jun 21, 2022

No description provided.

(cherry picked from commit 1045a8e)
{
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

Comment on lines +221 to +224
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))
Copy link
Author

@Eideren Eideren Jun 21, 2022

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 ?

@@ -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

@@ -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.

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

Successfully merging this pull request may close these issues.

1 participant