Specialized observables for transitions #5953
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
cc: @maxkatz6 Since you were changing that code lately as well. |
|
By getting rid of refcounting in |
maxkatz6
reviewed
May 19, 2021
MarchingCube
commented
May 19, 2021
|
|
||
| private TimeSpan? _previousTime; | ||
| private TimeSpan _internalTime; | ||
|
|
||
| protected ClockBase() | ||
| { | ||
| _observable = new ClockObservable(); | ||
| _connectedObservable = _observable.Publish().RefCount(); |
There was a problem hiding this comment.
Apart from adding 6 layers of indirection this was not doing anything useful. This observable does not initialize any resources when subscribed. I guess it was remnant of times where subscribing to clock was creating an actual timer.
Updated benchmarks after fixing invalid time easing. |
MarchingCube
changed the title
maxkatz6
reviewed
May 30, 2021
maxkatz6
approved these changes
May 30, 2021
|
@MarchingCube I think, it can be backported to 0.10.x ? |
|
@maxkatz6 Yeah, in the current form it breaks no APIs and improves perf/readability of transition code. |
danwalmsley
pushed a commit
that referenced
this pull request
Jun 4, 2021
Specialized observables for transitions
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does the pull request do?
Reworked transitions to utilize custom observables, this eliminates several allocations and speeds up transitions,
Stack traces also look more sane (left = old, right = new). First one is for initial application of the transition, second one is for timer tick. I think timer tick can be cleaned up even further by replacing Rx setup with our specialized lightweight observable (since there are no threading concerns).
What is the current behavior?
What is the updated/expected behavior with this PR?
How was the solution implemented (if it's not obvious)?
Checklist
Breaking changes
Obsoletions / Deprecations
Fixed issues