Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash in FrameStore #176

Merged
merged 1 commit into from
Jun 7, 2021
Merged

Fix crash in FrameStore #176

merged 1 commit into from
Jun 7, 2021

Conversation

kean
Copy link
Collaborator

@kean kean commented Jun 2, 2021

I don't know if that's an ideal design, but it resolves #173. I tested it and I no longer see Thread Sanitizer warnings.

By the way, does this work need to be performed in a background?

@kean
Copy link
Collaborator Author

kean commented Jun 2, 2021

This should also resolve #132

@kaishin
Copy link
Owner

kaishin commented Jun 4, 2021

@kean Thank you for the PR! I didn't manage to look into #173 and I appreciate you taking a look.

I will go over the changes but as you mentioned it might be an unnecessary optimization to perform this in a background thread. Some performance benchmarks might be necessary.

Copy link
Owner

@kaishin kaishin left a comment

Choose a reason for hiding this comment

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

Locks are indeed a good solution for the time being. Might need to rethink a bit how the animator and frame store are designed with concurrency in mind.

Preparing frames on the main queue didn't seem to affect performance in the demo app but there is a need for a more demanding example before making that call.

Let's get this in for now and revisit later!

@kean kean merged commit 3623a05 into kaishin:master Jun 7, 2021
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.

Index out of range
2 participants