Skip to content

Conversation

sebastienlagarde
Copy link
Contributor

@sebastienlagarde sebastienlagarde commented May 12, 2020

Purpose of this PR

This PR is just moving code to their "correct" location.

Testing status

Manual Tests

What have you tested?

Automated Tests

What did you setup? (Add a screenshot or the reference image of the test please)

Links

Yamato: (Select your branch) https://yamato.prd.cds.internal.unity3d.com/jobs/902-Graphics

Any test projects or documents to go with this to help reviewers?

Comments to reviewers

Notes for the reviewers you have assigned.

@github-actions github-actions bot added the SRP label May 12, 2020
@sebastienlagarde sebastienlagarde changed the title Hdrp Move code to right location Hdrp: Move packing and dots instancing code to right location May 12, 2020
@sebastienlagarde
Copy link
Contributor Author

Yamato is green for the code move (the red part are due to different issue).

@JussiKnuuttila
Copy link
Contributor

Code changes look good to me, I didn't find the Hybrid CI in Yamato so I queued that to run.

Copy link
Contributor

@JussiKnuuttila JussiKnuuttila left a comment

Choose a reason for hiding this comment

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

I'll quickly test this locally as well to make sure there are no problems with ObjectToWorld. I remember there were some problems when this code was originally written, which is why the macros were originally in a weird place in the code.

@sebastienlagarde
Copy link
Contributor Author

sebastienlagarde commented May 13, 2020

I'll quickly test this locally as well to make sure there are no problems with ObjectToWorld. I remember there were some problems when this code was originally written, which is why the macros were originally in a weird place in the code.

Thanks,

The ObjectToWorld was not working due to the macro definition that I have now move to after the include of instancing.
We need to keep this macro (which were disable in DOTS before this PR fix it), otherwise users can use the wrong variable (in particular when they migrate from builtin project)

you can find the Yamato for Hybrid here: https://yamato.prd.cds.internal.unity3d.com/jobs/902-Graphics/tree/master/.yamato%252Fupm-ci-abv.yml%2523all_project_ci_nightly_2020.1/2167305/job/pipeline

Copy link
Contributor

@JussiKnuuttila JussiKnuuttila left a comment

Choose a reason for hiding this comment

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

Unfortunately, I get some errors in local testing, and this Yamato job seems to have failed as well: https://yamato.prd.cds.internal.unity3d.com/jobs/902-Graphics/tree/HDRP%252Fmove-code-to-right-location/.yamato%252Fupm-ci-hdrp_hybrid.yml%2523All_HDRP_Hybrid_2020.1/2175942/job

The error message I get locally is this: Shader error in 'Shader Graphs/RGBShader3': undeclared identifier 'unity_DOTSInstancing_F64_Metadata_Use_Macro_UNITY_MATRIX_M_instead_of_unity_ObjectToWorld' 'LoadDOTSInstancedData_float4x4': no matching 1 parameter function at /Coding/Graphics/com.unity.render-pipelines.core/ShaderLibrary/SpaceTransforms.hlsl(7) (on d3d11)

This suggests that the changed code does not interact well with the DOTS instancing macros. I will start investigating this immediately.

…_MACRO versions to avoid problems with other macros.
…d for Hybrid V2 to clean up code and reduce corner cases.
@sebastienlagarde
Copy link
Contributor Author

Replace by this PR: #442

@sebastienlagarde sebastienlagarde deleted the HDRP/move-code-to-right-location branch September 25, 2020 15:18
@sebastienlagarde sebastienlagarde restored the HDRP/move-code-to-right-location branch September 25, 2020 15:18
@sebastienlagarde sebastienlagarde deleted the HDRP/move-code-to-right-location branch November 12, 2020 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants