Skip to content

Conversation

PaulDemeulenaere
Copy link
Contributor

@PaulDemeulenaere PaulDemeulenaere commented Apr 28, 2021

Purpose of this PR


Testing status

  • Already tested by @VladNeykov : See QA report

  • Retested locally
    _fixed_time_step_retested_bis

  • Awaiting for yamato : Mostly green at this revision only a couple of missing image reference, fixed at db413be


Comments to reviewers

This is a copy of the internal PR : https://github.cds.internal.unity3d.com/unity/vfx-graphics/pull/167

PaulDemeulenaere and others added 30 commits September 2, 2020 20:12
* *Prepare a proper way to test different shadow cases

* prepare a shadow graph unlit (modifying alpha)

* Add reference data for shadow test

* Minimal data for reproduce issue 1259511

* Fix shadowpath using common integration of PassDepthOrMV & adding VFX_PASSDEPTH_SHADOW define

* *Update ShadowMaterial

* *Update ShaderGraphShadow.vfx

* *Move shader to common testing asset

* Add equivalent test for URP

* Add graphicTest for URP

* *Update reference images

* Fix display of alphaTreshold slider with shaderGraph : only if transparent using motionVector or shadowCasting

* Fix alphaClipping condition (will require validation of @shader-graph) : use proper boolean synchronized instead of testing slot presence

* Fix incorrect block listing

* *Update changelog.md

* Fix pass selection for lit effect

* Early remove ports which aren't active in VFXTarget : save code compilation & cleaner solution

* Voluntary add disabled branch on shadow unlit to cover alphaTreshold case

* *Minor comment

* Fix return alpha clipping (missing actual alpha test enabled)

* *Restore deleted file (to minimize change in MR, these files can be directly deleted on vfx/staging)

* VFX Graph Bugfix for shader compilation error in scene selection pass: 'Shaderpass should be defined at this stage.'

# Conflicts:
#	com.unity.visualeffectgraph/Shaders/ParticlePlanarPrimitives/PassDepthOrMV.template

* Revert unexpected change iVisualEffectGraph_HDRP\ProjectSettings\EditorBuildSettings.asset

* *Update reference image & Fix treshold for URP scene which was really too low (set to default 5e-4f)

* Fix issue https://github.cds.internal.unity3d.com/unity/vfx-graphics/pull/77#discussion_r69288

It was a bad merge

Co-authored-by: pastasfuture <pastasfuture@gmail.com>
* Fix missing type copy in reduce

Tested locally on a PS4

* Add expection to detect unsupported value type
* Change the way rand expression equality is handled

* Refactor a bit

* Add editor test
* World to Viewport Point & Viewport to World Point operators + Test

First implementation of World to Viewport Point operator and its reciprocal Viewport to World Point operator.
Includes a test scene for these 2 operators.

* Update CHANGELOG.md

Update Changelog

* Added documentation md files.

Co-authored-by: Lewis Jordan <lewis.jordan@hotmail.co.uk>
* Operator creation, does NOT sample an SDF

* First implementation of the sampleSDF operator, might not be optimised

* Delete VFXExpressionSampleSDF.cs

* Updated Test Graph

* Better handling of out of bounds queries for normals and distances

* Delete HDRenderPipelineEditorResources.asset

* Delete XRPackageSettings.asset

* Assumption that the SDF is baked in the world space, in a box matching the orientedBox given as input

* Update 27_SampleSDF.vfx

* Update 27_SampleSDF.vfx

* restored the deleted asset

* Enforces and apply the 4 parents max rule

* Removes the return statements in branch

* Revert "restored the deleted asset"

This reverts commit 57757b5a3ed4c303c34dc7dc920ad6d0328fcfdb.

* Revert "Revert "restored the deleted asset""

This reverts commit 7753b4e85aba5d75e8af659aedc66515a0c56e20.

* Fix compilation error and warnings

* Fix Pascal case

* Fix Pascal. Really.

* Revert "Revert "Revert "restored the deleted asset"""

This reverts commit daf8b79a78f55b47b6bd85afd9008a57d96f50f1.

* Uses the Inverse-Transpose matrix for SDF related normal transformations (for non uniform scaling)

* Delete packages-lock.json

* Revert "Delete packages-lock.json"

This reverts commit 4a10123f3171d6cf981c5effa52128ebb1411735.

* Revert "Revert "Delete packages-lock.json""

This reverts commit 84d700f0f1fde91d4ad5a6ab87f3946216f883f4.

* Restore this mistakenly modified file

* Update 27_SampleSDF.vfx

* Get rid of useless transpose

* Added Graphic Test

* Added reference images

* Fix incorrect out-of-bounds check  + auto recompile

* Added reference images

* Clarify the name of the output

* Remove the use of inverse transpose for the "normals"

* Restore the Inverse Transpose and modify Collision code

* Restore Conform and Collide previous behavior + direction points to the surface

* add documentation

* Consistent distance and normal handling + ref images

* Cleaning + World space stick distance and radius + references

* Max scale through expression evaluated on GPU (if needed)

* Max3 on CPU
* Simpler update to use now available ByteAddressBuffer

* Temp Workaround a GPU hang

We should isolate properly this code.

* Proper fix of OOB FetchBuffer

* *Update changelog.md
* Fix VFXExpressionVector3sToMatrix & VFXExpressionVector4sToMatrix evaluation on GPU

* *Update changelog.md
* Base Commit

* Moved Files down one folder

* Updated Package Configuration, CHANGELOG and documentation

* Fixed Behavior of ExposedProperty by implementing a simple Drawer / Debug Behaviour

* Small fixes and checks

* Updated Documentation / Renamed Attribute handler RigidBody to RigidBodyVeolcity + Safe check

* Small Fixes

* Added the Execute In Editor Capability + Custom Inspectors in order to handle the capability correctly

* Other custom Editors + Helpbox

* Added CMCameraShake Editor + Capabilities / Fixed Possible Circular Reference in  Prefab Spawn

* Removed Rerouting Helpers as CopyValuesFrom is broken at the moment.

* Fixed ASMDEFs / class accessibility

* Updated Documentation

* Fixed Class Accessibility for ExposedPropertyDrawer

* Updated Changelog (missing entry for Exposed Proeprty Custom Property Drawer)

Co-authored-by: Thomas ICHÉ <peeweek@gmail.com>
* Revert "Output Event Helpers (#58)"

This reverts commit eb830bb.

* Revert "Fix changelog"

This reverts commit f6fd8a5.
* Point cache header, wait for new line character only (/n) and skip carriage return(/r)

An extra trailing new line character was causing an invalid offset in binary point cache files.

* Update Changelog

* Remove unnecessary files
…les (#109)

* Change casting to byte

We were trying to cast an object containing a byte to an int.
Also, a better normalization would be dividing by 255.

* Update CHANGELOG.md
* Base Commit

* Moved Files down one folder

* Updated Package Configuration, CHANGELOG and documentation

* Fixed Behavior of ExposedProperty by implementing a simple Drawer / Debug Behaviour

* Small fixes and checks

* Updated Documentation / Renamed Attribute handler RigidBody to RigidBodyVeolcity + Safe check

* Small Fixes

* Added the Execute In Editor Capability + Custom Inspectors in order to handle the capability correctly

* Other custom Editors + Helpbox

* Added CMCameraShake Editor + Capabilities / Fixed Possible Circular Reference in  Prefab Spawn

* Removed Rerouting Helpers as CopyValuesFrom is broken at the moment.

* Fixed ASMDEFs / class accessibility

* Updated Documentation

* Fixed Class Accessibility for ExposedPropertyDrawer

* Updated Changelog (missing entry for Exposed Proeprty Custom Property Drawer)

* Revert "Revert "Output Event Helpers" (#110)"

This reverts commit e72745d.

* Revert "Revert "Output Event Helpers" (#110)"

This reverts commit e72745d.

* First pass of coding rule validation

* Rename VFXOutputEventHandler in VFXOutputEventAbstractHandler

Following coding rule from https://ono.unity3d.com/unity-extra/unity-meta/files/@/ReferenceSource/CSharp/Assets/CSharpReference.cs

* *Missing rename

* Remove useless member

* Rework VFXOutputEventPrefabSpawn :

- Avoid any inconsistency with transform
- Use prefab link when available
- Use a lazy approach and lock editable values to editor

* Fix issue with "Open Prefab" which was resetting the "Hide Flag"

* *Minor cleanup

* *Minor Change

* Missing DisposeInstance when we are disabling "Execute In Editor"

* Fix missing full stop at the end of all tooltips

* *Apply auto formatting

* Fix spamming null reference error reported by @vlad

https://unity.slack.com/archives/GA2Q6JU1X/p1601055259085900?thread_ts=1601044036.085800&cid=GA2Q6JU1X

* Safely fallback on Instanciate when InstantiatePrefab returns silently null

Instance of prefab within the scene are actually copies because an override could have been applied.

* Fix model spawn using NotAPrefab instead of Regular

* Expliciter code in VFXOutputEventPrefabSpawn

Doesn't change the internal behavior

* Minor fix : audio != null isn't equivalent than audio?.

* Fix assignement : prevent setting audioSource or rigidBody from a prefab asset

* Fix issue https://github.cds.internal.unity3d.com/unity/vfx-graphics/pull/111#discussion_r73827

Use explicit DrawOutputEventProperties to avoid multiple serializedObject.Update per Inspector

* Minor : remove unexpected carriage return

Co-authored-by: Thomas ICHÉ <peeweek@gmail.com>
Co-authored-by: Thomas Iché <thomasi@unity3d.com>
PaulDemeulenaere and others added 14 commits March 22, 2021 10:26
* Local test (I'm using git diff to check actual file change)

* test adding a lit mesh output

* *Testing with shaderGraph

* *test bis

* *Add unit test to cover unexpected increasing size

* Properly implement the test

* Remove test data

* *Temp lot of log & save it to independant file

* HotFix : UpdateSubAsset in OnPreprocessAsset

Shouldn't be necessary, the actual problem is maybe in C++ implementation

* Clean test & add comment

* *WIP* Serialization test which seems (tm) to reproduce the same issue

* To revert : experiment adding log while loading VFXSerializableObject

*Without Import Asset after write*
Saving : 100
Loading : 100
Loading : 100
OnBeforeSerialize : 100
Saving : 101
Loading : 101
Loading : 101
OnAfterDeserialize : 100
Loading : 100
=> Test fail !

*With Import Asset after write*
Saving : 100
Loading : 100
Loading : 100
OnBeforeSerialize : 100
OnAfterDeserialize : 100
Saving : 101
Loading : 101
Loading : 101
Loading : 101
OnBeforeSerialize : 101
OnAfterDeserialize : 101
Saving : 102
Loading : 102
Loading : 102
Loading : 102
OnBeforeSerialize : 102
OnAfterDeserialize : 102
Saving : 103
Loading : 103
Loading : 103

* *Revert unwanted change

* Clean & Unify test behavior

* Safety check in newly integrated test (if asset is empty, it isn't expected neither)

* Remove OnCompile : Avoid recompile twice, the WriteAsset is already trigerring an import
* *Add test to cover CopyValuesFrom issue

* *Extend the newly added test

Check every value in copied values
int ia, int ib, ... is actually not needed since "i" remains an local scope value
* Fix missing BuildParameterInfo when we are removing a VFXParameter

* Minor : fix typo in comment
…ime-step

# Conflicts:
#	TestProjects/VisualEffectGraph_HDRP/ProjectSettings/EditorBuildSettings.asset
#	TestProjects/VisualEffectGraph_URP/ProjectSettings/EditorBuildSettings.asset
@PaulDemeulenaere PaulDemeulenaere changed the title Vfx/fix/1289829 add test for exact fixed time step [VFX] Test for exact fixed time step (& restore option) Apr 28, 2021
*Warning* OSX HDRP is actually a copy of OSX URP (I don't expect difference)
Copy link
Contributor

@VladNeykov VladNeykov left a comment

Choose a reason for hiding this comment

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

Already tested by @VladNeykov : See QA report
Retested locally

That's good for me!

…ime-step

# Conflicts:
#	com.unity.visualeffectgraph/Documentation~/VisualEffectGraphAsset.md
@PaulDemeulenaere PaulDemeulenaere marked this pull request as ready for review May 4, 2021 13:24
@PaulDemeulenaere
Copy link
Contributor Author

All test 🟢 at a3644ef
Awaiting for formatting before merge.

@PaulDemeulenaere PaulDemeulenaere merged commit 30e69c2 into master May 5, 2021
@PaulDemeulenaere PaulDemeulenaere deleted the vfx/fix/1289829-add-test-for-exact-fixed-time-step branch May 5, 2021 06:11
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.

7 participants