Skip to content

Conversation

@alelievr
Copy link
Member

@alelievr alelievr commented Jul 22, 2021

Purpose of this PR

Updated the custom post-processes template to use cmd.Blit instead of HDUtils.DrawFullscreen which is more intuitive for users.
Also, the custom PP shader template was not working with cmd.Blit because of the missing Property scope in the shader and mismatching source texture name.

I also updated the doc about the usage of cmd.Blit and HDUtils.DrawFullscreen
image


Testing status

Tested that the greyscale example in our doc works as expected with cmd.Blit. Also replaced one of the custom post-process in our test project to use Blit() instead of HDUtils.DrawFullscreen.

https://yamato.cds.internal.unity3d.com/jobs/902-Graphics/tree/hd%252Ffix%252Fcustom-post-process-blit/.yamato%252Fall-hdrp.yml%2523PR_HDRP_trunk/7800994/job

@alelievr alelievr added the HDRP label Jul 22, 2021
@alelievr alelievr self-assigned this Jul 22, 2021
@github-actions
Copy link

Hi! This comment will help you figure out which jobs to run before merging your PR. The suggestions are dynamic based on what files you have changed.
Link to Yamato: https://yamato.cds.internal.unity3d.com/jobs/902-Graphics
Search for your PR branch using the sidebar on the left, then add the following segment(s) to the end of the URL (you may need multiple tabs depending on how many packages you change)

HDRP
/.yamato%252Fall-hdrp.yml%2523PR_HDRP_2021.2

Depending on the scope of your PR, you may need to run more jobs than what has been suggested. Please speak to your lead or a Graphics SDET (#devs-graphics-automation) if you are unsure.

@alelievr alelievr marked this pull request as ready for review July 22, 2021 14:54
Copy link
Contributor

@FrancescoC-unity FrancescoC-unity left a comment

Choose a reason for hiding this comment

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

LGTM (note I have not read the doc, leaving that to Vic!)

alelievr and others added 2 commits July 23, 2021 12:38
Reviewed the changes in this PR and made some small improvements to this section.
Copy link
Contributor

@Vic-Cooper Vic-Cooper left a comment

Choose a reason for hiding this comment

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

Docs changes done :)

JMargevics
JMargevics approved these changes Jul 28, 2021
@sebastienlagarde sebastienlagarde merged commit 56bf278 into master Jul 28, 2021
@sebastienlagarde sebastienlagarde deleted the hd/fix/custom-post-process-blit branch July 28, 2021 09:48
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.

6 participants