[URP][Converter] anim clip converter #4773

martint-unity · 2021-06-03T11:10:05Z

Purpose of this PR

Added an animation Clip Converter.
In menu Window -> Rendering -> Render Pipeline Converter

We now have a new converter called animation clip converter.
This will convert animation clip properties if needed.

This can be tested in UniversalUpgradeTest project
for example changing
attribute: material._Color.a to attribute: material._BaseColor.a

If opening the test project and initialize the converters you will have the following

After running convert you will have something that looks similar to this

- Add status messages - Also clean up to use global object identifiers

Beautified some code

Ensures we do not skip clips that have a mix of upgradable and non-upgradable properties

github-actions · 2021-06-03T11:10:36Z

It appears that you made a non-draft PR!
Please convert your PR to draft (button on the right side of the page)
and cancel any jobs that started on Yamato.
See the PR template for more information.
Thank you!

sim-bz · 2021-06-04T17:51:52Z

I'm hitting an error after going through domain reload. I'm running the converter and then I git reset --hard to reset the assets. The converter window then opens up with the following errors:

System.Reflection.RuntimeFieldInfo.SetValue (System.Object obj, System.Object val, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Globalization.CultureInfo culture) (at <56a09cea74224b4cafcbda9f814f7d23>:0)
System.Reflection.FieldInfo.SetValue (System.Object obj, System.Object value) (at <56a09cea74224b4cafcbda9f814f7d23>:0)
UnityEditor.Rendering.Universal.Converters.RenderPipelineConvertersEditor.DontSaveToLayout (UnityEditor.EditorWindow wnd) (at /Users/sbz/work/Graphics/com.unity.render-pipelines.universal/Editor/Converter/RenderPipelineConvertersEditor.cs:115)
UnityEditor.Rendering.Universal.Converters.RenderPipelineConvertersEditor.ShowWindow () (at /Users/sbz/work/Graphics/com.unity.render-pipelines.universal/Editor/Converter/RenderPipelineConvertersEditor.cs:94)

UnityEditor.EditorStyles.get_largeLabel () (at /Users/sbz/work/unity-clone-1/Editor/Mono/GUI/EditorStyles.cs:26)
UnityEditor.Rendering.CoreEditorStyles..cctor () (at /Users/sbz/work/Graphics/com.unity.render-pipelines.core/Editor/CoreEditorStyles.cs:75)
Rethrow as TypeInitializationException: The type initializer for 'UnityEditor.Rendering.CoreEditorStyles' threw an exception.
UnityEditor.Rendering.Universal.Converters.RenderPipelineConvertersEditor.CreateGUI () (at /Users/sbz/work/Graphics/com.unity.render-pipelines.universal/Editor/Converter/RenderPipelineConvertersEditor.cs:168)
System.Reflection.RuntimeMethodInfo.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) (at <56a09cea74224b4cafcbda9f814f7d23>:0)
Rethrow as TargetInvocationException: Exception has been thrown by the target of an invocation.
System.Reflection.RuntimeMethodInfo.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) (at <56a09cea74224b4cafcbda9f814f7d23>:0)
System.Reflection.MethodBase.Invoke (System.Object obj, System.Object[] parameters) (at <56a09cea74224b4cafcbda9f814f7d23>:0)
UnityEditor.UIElements.DefaultEditorWindowBackend.Invoke (System.String methodName) (at /Users/sbz/work/unity-clone-1/ModuleOverrides/com.unity.ui/Editor/WindowBackends/DefaultEditorWindowBackend.cs:358)
UnityEditor.UIElements.DefaultEditorWindowBackend.SendInitializeIfNecessary () (at /Users/sbz/work/unity-clone-1/ModuleOverrides/com.unity.ui/Editor/WindowBackends/DefaultEditorWindowBackend.cs:351)
UnityEditor.UIElements.DefaultEditorWindowBackend.RegisterWindow () (at /Users/sbz/work/unity-clone-1/ModuleOverrides/com.unity.ui/Editor/WindowBackends/DefaultEditorWindowBackend.cs:140)
UnityEditor.UIElements.DefaultEditorWindowBackend.UnityEditor.IEditorWindowBackend.OnRegisterWindow () (at /Users/sbz/work/unity-clone-1/ModuleOverrides/com.unity.ui/Editor/WindowBackends/DefaultEditorWindowBackend.cs:99)
UnityEditor.HostView.RegisterSelectedPane (System.Boolean sendEvents) (at /Users/sbz/work/unity-clone-1/Editor/Mono/HostView.cs:504)
UnityEditor.HostView.SetActualViewInternal (UnityEditor.EditorWindow value, System.Boolean sendEvents) (at /Users/sbz/work/unity-clone-1/Editor/Mono/HostView.cs:83)
UnityEditor.DockArea.SetSelectedPrivate (System.Int32 value, System.Boolean sendEvents) (at /Users/sbz/work/unity-clone-1/Editor/Mono/GUI/DockArea.cs:99)
UnityEditor.DockArea.AddTab (System.Int32 idx, UnityEditor.EditorWindow pane, System.Boolean sendPaneEvents) (at /Users/sbz/work/unity-clone-1/Editor/Mono/GUI/DockArea.cs:195)
UnityEditor.DockArea.AddTab (UnityEditor.EditorWindow pane, System.Boolean sendPaneEvents) (at /Users/sbz/work/unity-clone-1/Editor/Mono/GUI/DockArea.cs:188)
UnityEditor.EditorWindow.CreateNewWindowForEditorWindow (UnityEditor.EditorWindow window, System.Boolean loadPosition, System.Boolean showImmediately, System.Boolean setFocus) (at /Users/sbz/work/unity-clone-1/Editor/Mono/EditorWindow.cs:1227)
UnityEditor.EditorWindow.Show (System.Boolean immediateDisplay) (at /Users/sbz/work/unity-clone-1/Editor/Mono/EditorWindow.cs:675)
UnityEditor.EditorWindow.Show () (at /Users/sbz/work/unity-clone-1/Editor/Mono/EditorWindow.cs:667)
UnityEditor.EditorWindow.GetWindowPrivate (System.Type t, System.Boolean utility, System.String title, System.Boolean focus) (at /Users/sbz/work/unity-clone-1/Editor/Mono/EditorWindow.cs:719)
UnityEditor.EditorWindow.GetWindow (System.Type t, System.Boolean utility, System.String title, System.Boolean focus) (at /Users/sbz/work/unity-clone-1/Editor/Mono/EditorWindow.cs:733)
UnityEditor.EditorWindow.GetWindow[T] (System.Boolean utility, System.String title, System.Boolean focus) (at /Users/sbz/work/unity-clone-1/Editor/Mono/EditorWindow.cs:799)
UnityEditor.EditorWindow.GetWindow[T] () (at /Users/sbz/work/unity-clone-1/Editor/Mono/EditorWindow.cs:773)
UnityEditor.Rendering.Universal.Converters.RenderPipelineConvertersEditor.ShowWindow () (at /Users/sbz/work/Graphics/com.unity.render-pipelines.universal/Editor/Converter/RenderPipelineConvertersEditor.cs:92)```

sim-bz · 2021-06-04T18:40:40Z

I spent some time playing with the animation clip converter trying to make sense of the assets that were shown in the UI after having successfully updated them. When saving the assets to disk and reopening the converter, I'm retrieving the exact same list of animation clips to upgrade, but this time the operation mostly ends in failure.

Some things to consider:

The clips that are green are the "WithMaterialProperties_OnlyUsedByNotUpgradable" clips that are showing up in the upgraded list. Since our filter is set to "UsedByUpgraded | UsedByNonUpgraded", these show up as upgraded while we haven't done any work on them. Although not an error per se, these should probably be in the nonUpgraded list still.
Clips that were already upgraded are now added to the nonUpgraded list with a usage set to "Unknown". Should these clips be in the list to begin with? Would it be possible to remove clips from the list that either have a usage set to "UsedByNonUpgraded" or "Unknown"?

martint-unity · 2021-06-07T14:38:30Z

@SimonBZ Domain reload is an issue and there are other PR's that will fix that. On is in the default Material Converter and on is in A Lazy load GUI states.
@amechtley Could you maybe sync with @SimonBZ and figure out what we would / should do for the
"Some things to consider:

The clips that are green are the "WithMaterialProperties_OnlyUsedByNotUpgradable" clips that are showing up in the upgraded list. Since our filter is set to "UsedByUpgraded | UsedByNonUpgraded", these show up as upgraded while we haven't done any work on them. Although not an error per se, these should probably be in the nonUpgraded list still.
Clips that were already upgraded are now added to the nonUpgraded list with a usage set to "Unknown". Should these clips be in the list to begin with? Would it be possible to remove clips from the list that either have a usage set to "UsedByNonUpgraded" or "Unknown"?"
And maybe we can address it?

- Remove 0 case (same as Unknown) and make it clear that it is the usage that is unknown, rather than the error being unknown. - Change unknown message to specify the problem may also be that the clip is used, but by objects with no renderers - Report errors for used by upgraded and ambiguous cases, whether or not they pass through the filter. - Changed messages from "failed to upgrade" to "was not modified" to make the outcome a little clearer

material/clip upgrade.

amechtley

lgtm

erikabar

Tested if Animation clips appear in the converter window and are successfully converted to URP.

martint-unity and others added 7 commits May 31, 2021 11:46

added init step to anim clip converter

22fde2b

Added OnClicked method pinging right assets

643babe

Implement clip conversion in new window

0b729f2

- Add status messages - Also clean up to use global object identifiers

Added OnPreRun method

a09e369

cleared out unnened code in the init method

676cc89

removed unused code

ef6c447

Beautified some code

Update filter flags

079952b

Ensures we do not skip clips that have a mix of upgradable and non-upgradable properties

martint-unity requested review from a team as code owners June 3, 2021 11:10

github-actions bot added the universal label Jun 3, 2021

martint-unity requested review from amechtley and sim-bz June 3, 2021 11:10

Added suggested changes to Animation clip converter messaging.

01f48b6

amechtley and others added 4 commits June 8, 2021 14:09

Adjusted UpgradeUtility to account for clips that were already upgraded.

ecabfe4

Reverted to only using UsedByUpgraded to avoid confusion surrounding

573e2ca

material/clip upgrade.

Added tests to cover already upgraded clip properties.

f0d8439

amechtley approved these changes Jun 8, 2021

View reviewed changes

sim-bz approved these changes Jun 8, 2021

View reviewed changes

erikabar self-requested a review June 14, 2021 06:45

erikabar approved these changes Jun 14, 2021

View reviewed changes

phi-lira approved these changes Jun 14, 2021

View reviewed changes

phi-lira merged commit 54d4d75 into master Jun 14, 2021

phi-lira deleted the universal/upgrader/anim-clip-converter branch June 14, 2021 11:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[URP][Converter] anim clip converter #4773

[URP][Converter] anim clip converter #4773

Uh oh!

martint-unity commented Jun 3, 2021

Uh oh!

github-actions bot commented Jun 3, 2021

Uh oh!

sim-bz commented Jun 4, 2021 •

edited

Loading

Uh oh!

sim-bz commented Jun 4, 2021

Uh oh!

martint-unity commented Jun 7, 2021

Uh oh!

amechtley left a comment

Uh oh!

erikabar left a comment

Uh oh!

Uh oh!

[URP][Converter] anim clip converter #4773

[URP][Converter] anim clip converter #4773

Uh oh!

Conversation

martint-unity commented Jun 3, 2021

Purpose of this PR

Uh oh!

github-actions bot commented Jun 3, 2021

Uh oh!

sim-bz commented Jun 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sim-bz commented Jun 4, 2021

Uh oh!

martint-unity commented Jun 7, 2021

Uh oh!

amechtley left a comment

Choose a reason for hiding this comment

Uh oh!

erikabar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sim-bz commented Jun 4, 2021 •

edited

Loading