[usdview] Added currentFrame command line argument #832
Conversation
…gs to startFrame and lastFrame.
--ff FIRSTFRAME Set the first frame of the viewer
--lf LASTFRAME Set the last frame of the viewer
--cf CURRENTFRAME Set the current frame of the viewer
* Refactored setFrame to _setFrameIndex
* Removed the closestFrame lookup - as far as I'm aware this does nothing of value as closestFrame always equals frame.
* This has given us a speed increase on playback - the closestFrame lookup iterated over the entire _timeSamples list every time the frame changed.
* Removed the need for the forceUpdate variable.
* Re-purposed setFrame to be a simple wrapper that finds a frameIndex and calls _setFrameIndex.
* Found bug where resetting of the timeline would also not update the dataModel - so the value on the timeline and what was being shown in the viewer would not match up.
* This was fixed by adding a call to setFrame in _UpdateTimeSamples
|
Filed as internal issue #USD-5233 |
There was a problem hiding this comment.
Nice, @ianlawson ! Just a few minor things, and then we can pull this down for testing!
| cf = self._parserData.currentframe | ||
| if cf: | ||
| if cf >= self.realStartTimeCode and cf <= self.realEndTimeCode: | ||
| self.setFrame(cf) |
There was a problem hiding this comment.
_UpdateTimeSamples() is going to call setFrame() unless resetStageDataOnly is True. So, to avoid calling setFrame() twice, should we figure out here whether we're going to call setFrame before we call _UpdateTimeSamples, and if we are, force resetStageDataOnly to True?
There was a problem hiding this comment.
I've removed the setFrame call from _UpdateTimeSamples. It isn't needed as this frame is set correctly when _self.ui.frameSlider.setValue(0) is called!
|
|
||
| if closestFrame is None: | ||
| return | ||
| frameIndex = int(frameIndex) |
There was a problem hiding this comment.
Why is this cast needed if the documented input type is int?
There was a problem hiding this comment.
Just being safe - but shouldn't need it! Removed.
|
|
||
| self._dataModel.currentFrame = Usd.TimeCode(closestFrame) | ||
| if self._dataModel.currentFrame != frame: | ||
| self._dataModel.currentFrame = Usd.TimeCode(frame) |
There was a problem hiding this comment.
Just to ensure that the comparison is happening inthe right space, should we convert frame to a TimeCode before doing the comparison?
| @@ -732,7 +732,7 @@ def __init__(self, parserData, resolverContextFn): | |||
|
|
|||
| self._ui.primView.expanded.connect(self._primViewExpanded) | |||
|
|
|||
| self._ui.frameSlider.valueChanged.connect(self.setFrame) | |||
| self._ui.frameSlider.valueChanged.connect(self._setFrameIndex) | |||
There was a problem hiding this comment.
Yay! Happy speed boost on long shots!!
|
@spiffmon - anything left for me to do here? |
|
Hi @ianlawson - sorry, was meaning to post an update. We are working through several test failures in the testusdview suite due to the change. We're going to try fixing them locally so we don't need to go through the whole PR/sync-local/merge/resolve process again. Out of curiosity, are you able to run the testusdview tests locally? |
|
Ah sorry about this! I've found the executable but I'm unsure how to run it! What does --testScript expect? |
|
Got it all worked out on my end - I'm happy to take a look into these failures! |
|
Give me a day or two more to poke at it - I've already got the NavigationKeys one worked out (it was a real problem with the new factoring), and am digging into the Skinning one, which I think may scratch at some deeper issues that aren't really due to your code! I'll let you know how it's going soon - got bogged down in meetings today! |
|
OK - two of the failures necessitated small tweaks to the new logic, and the third, testusdviewSkinning, was breaking (modulo removal of the forceUpdate parameter) because of the bug your change fixes, so needed some baseline updates. I'm passing that change by the test's author, but hope to get this checked in soon. Thanks, @ianlawson ! |
|
Thank you @spiffmon! |
[usdview] Added currentFrame command line argument (Internal change: 1972807)
Description of Change(s)
Added help strings to startFrame and lastFrame.
--ff FIRSTFRAME Set the first frame of the viewer
--lf LASTFRAME Set the last frame of the viewer
--cf CURRENTFRAME Set the current frame of the viewer
Refactored setFrame to _setFrameIndex
Re-purposed setFrame to be a simple wrapper that finds a frameIndex and calls _setFrameIndex.
Found bug where resetting of the timeline would also not update the dataModel - so the value on the timeline and what was being shown in the viewer would not match up.
Fixes Issue(s)