Skip to content

Conversation

@clusty
Copy link
Contributor

@clusty clusty commented Apr 27, 2021

Purpose of this PR

USDU-162

Implement USD Recorder integration.
This is a repackaging of the timeline used recorder track.

usd_recorder

Testing

Functional Testing status:

Added basic unit test and yamato project test trigger.
Manually exporting Cubes and skinned meshes.

@clusty clusty requested a review from judubu April 27, 2021 13:32
Copy link
Contributor

@judubu judubu left a comment

Choose a reason for hiding this comment

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

Beside the check for opened layer it looks good to me. Great job.
Add path validator and I'm good


context = new ExportContext
{
scene = ExportHelpers.InitForSave(outputFile)
Copy link
Contributor

Choose a reason for hiding this comment

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

This will throw an ApllicationException if the output file is already open.
Repro steps:

  • Export once
  • ImportAsGameobject
  • select the UsdAsset to trigger the creation of a Scene
  • start a recording -> BOOM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will leave it be for now: the error message is not too shabby and because of the tokens in the name this be a pre-validation.
If this occurs, Recording stops, Exists playmode and a console and recorder window error message is logged:
"ApplicationException: USD ERROR: A layer already exists with identifier '/d-unity-sdk/TestProject/Usd-Recorder/Recordings/Usd Clip.usd'"

@clusty clusty requested a review from judubu April 30, 2021 14:54
Copy link
Contributor

@sebastienduverne sebastienduverne left a comment

Choose a reason for hiding this comment

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

LGTM (virtual review over the shoulder)

Copy link
Contributor

@cguer cguer left a comment

Choose a reason for hiding this comment

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

Looks good overall, some tiny improvements and one bug when exporting USDZ

Minor improvement

  • Change default output file name from <Recorder> to <Recorder>_<Take> (to avoid the risks of users overwriting their outputs by default)

Bug
An exception is logged when Recording a USDZ if there is a Timeline in the current scene and the Timeline Window is open.

Reproduction Steps

  1. Create an empty GameObject in the current scene and add a Timeline to it.
  2. Create a USD Recorder and change the source to any GameObject in the current scene
  3. Set the Export Format to USDZ
  4. Make sure that the Timeline window is currently opened in the Editor
  5. Start Recording

The following exception is logged:

System.IO.File.GetAttributes (System.String path) (at <695d1cc93cca45069c528c15c9fdd749>:0)
UnityEditor.FileUtil.IsReadOnly (UnityEngine.Object asset) (at <0ba34cef20a14b1e8b24b1efe992a9d0>:0)
UnityEditor.Timeline.SequenceState.get_isReadOnly () (at Library/PackageCache/com.unity.timeline@1.6.0-pre.3/Editor/State/SequenceState.cs:63)
UnityEditor.Timeline.TimelineWindow.get_currentMode () (at Library/PackageCache/com.unity.timeline@1.6.0-pre.3/Editor/Window/TimelineWindow_Gui.cs:123)
UnityEditor.Timeline.TimelineWindow.DurationGUI (UnityEditor.Timeline.TimelineWindow+TimelineItemArea area, System.Double duration) (at Library/PackageCache/com.unity.timeline@1.6.0-pre.3/Editor/Window/TimelineWindow_Duration.cs:14)
UnityEditor.Timeline.TimelineWindow.SequencerGUI () (at Library/PackageCache/com.unity.timeline@1.6.0-pre.3/Editor/Window/TimelineWindow_Gui.cs:287)
UnityEditor.Timeline.TimelineWindow.DoLayout () (at Library/PackageCache/com.unity.timeline@1.6.0-pre.3/Editor/Window/TimelineWindow_Gui.cs:160)
UnityEditor.Timeline.TimelineWindow.OnGUI () (at Library/PackageCache/com.unity.timeline@1.6.0-pre.3/Editor/Window/TimelineWindow.cs:224)
UnityEditor.HostView.InvokeOnGUI (UnityEngine.Rect onGUIPosition, UnityEngine.Rect viewRect) (at <0ba34cef20a14b1e8b24b1efe992a9d0>:0)
UnityEditor.DockArea.DrawView (UnityEngine.Rect viewRect, UnityEngine.Rect dockAreaRect) (at <0ba34cef20a14b1e8b24b1efe992a9d0>:0)
UnityEditor.DockArea.OldOnGUI () (at <0ba34cef20a14b1e8b24b1efe992a9d0>:0)
UnityEngine.UIElements.IMGUIContainer.DoOnGUI (UnityEngine.Event evt, UnityEngine.Matrix4x4 parentTransform, UnityEngine.Rect clippingRect, System.Boolean isComputingLayout, UnityEngine.Rect layoutSize, System.Action onGUIHandler, System.Boolean canAffectFocus) (at <d98a0f9d3a9a450c9f1ea4983cb665e9>:0)
UnityEngine.UIElements.IMGUIContainer.HandleIMGUIEvent (UnityEngine.Event e, UnityEngine.Matrix4x4 worldTransform, UnityEngine.Rect clippingRect, System.Action onGUIHandler, System.Boolean canAffectFocus) (at <d98a0f9d3a9a450c9f1ea4983cb665e9>:0)
UnityEngine.UIElements.IMGUIContainer.DoIMGUIRepaint () (at <d98a0f9d3a9a450c9f1ea4983cb665e9>:0)
UnityEngine.UIElements.UIR.RenderChainCommand.ExecuteNonDrawMesh (UnityEngine.UIElements.UIR.DrawParams drawParams, System.Single pixelsPerPoint, System.Exception& immediateException) (at <d98a0f9d3a9a450c9f1ea4983cb665e9>:0)
Rethrow as ImmediateModeException
UnityEngine.UIElements.UIR.RenderChain.Render () (at <d98a0f9d3a9a450c9f1ea4983cb665e9>:0)
UnityEngine.UIElements.UIRRepaintUpdater.Update () (at <d98a0f9d3a9a450c9f1ea4983cb665e9>:0)
UnityEngine.UIElements.VisualTreeUpdater.UpdateVisualTreePhase (UnityEngine.UIElements.VisualTreeUpdatePhase phase) (at <d98a0f9d3a9a450c9f1ea4983cb665e9>:0)
UnityEngine.UIElements.Panel.UpdateForRepaint () (at <d98a0f9d3a9a450c9f1ea4983cb665e9>:0)
UnityEngine.UIElements.Panel.Repaint (UnityEngine.Event e) (at <d98a0f9d3a9a450c9f1ea4983cb665e9>:0)
UnityEngine.UIElements.UIElementsUtility.DoDispatch (UnityEngine.UIElements.BaseVisualElementPanel panel) (at <d98a0f9d3a9a450c9f1ea4983cb665e9>:0)
UnityEngine.UIElements.UIElementsUtility.UnityEngine.UIElements.IUIElementsUtility.ProcessEvent (System.Int32 instanceID, System.IntPtr nativeEventPtr, System.Boolean& eventHandled) (at <d98a0f9d3a9a450c9f1ea4983cb665e9>:0)
UnityEngine.UIElements.UIEventRegistration.ProcessEvent (System.Int32 instanceID, System.IntPtr nativeEventPtr) (at <d98a0f9d3a9a450c9f1ea4983cb665e9>:0)
UnityEngine.UIElements.UIEventRegistration+<>c.<.cctor>b__1_2 (System.Int32 i, System.IntPtr ptr) (at <d98a0f9d3a9a450c9f1ea4983cb665e9>:0)
UnityEngine.GUIUtility.ProcessEvent (System.Int32 instanceID, System.IntPtr nativeEventPtr, System.Boolean& result) (at <a58ba13cea6f47f98e8c8c75d33f99e3>:0)


namespace UnityEditor.Formats.USD.Recorder
{
[RecorderSettings(typeof(UsdRecorder), "Usd Clip", "usd_recorder")]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be "USD Clip" (with all caps). I'm not sure if it's the convention but it appears to be capitalized in the official Pixar documentation.

@judubu judubu added this to the 3.0.0 milestone May 31, 2021
@cguer cguer self-requested a review May 31, 2021 20:48
Copy link
Contributor

@cguer cguer left a comment

Choose a reason for hiding this comment

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

QA approved 👍!

Tested on:

  • 2021.2.0a17 (Windows only)

@clusty clusty merged commit f956b1d into dev Jun 1, 2021
@clusty clusty deleted the recorder branch June 1, 2021 12:43
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.

6 participants