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 #2071: cancelScheduledValues with setValueCurveAtTime #2109

Merged
merged 2 commits into from
Mar 19, 2020

Conversation

rtoy
Copy link
Member

@rtoy rtoy commented Dec 6, 2019

If the cancelTime overlaps the setValueCurveAtTime event, remove the
event too.


Preview | Diff

If the cancelTime overlaps the setValueCurveAtTime event, remove the
event too.
@rtoy
Copy link
Member Author

rtoy commented Dec 6, 2019

This is blocked on #2107

@rtoy rtoy requested a review from hoch January 13, 2020 16:18
@rtoy
Copy link
Member Author

rtoy commented Jan 13, 2020

#2107 is fixed now and merging the changes fixes the build error.

@rtoy rtoy changed the title Fix #2095: cancelScheduledValues with setValueCurveAtTime Fix #2071: cancelScheduledValues with setValueCurveAtTime Jan 13, 2020
Copy link
Member

@hoch hoch left a comment

Choose a reason for hiding this comment

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

This makes sense, but what will happen to the computed value after the cancellation?

@rtoy
Copy link
Member Author

rtoy commented Jan 16, 2020

I explicitly left that undefined because we don't say what happens if you cancel events that are currently running. If we want to define that, then that's a different issue.

But a quick test with Chrome shows that if a linear ramp is already running and then cancel events between current time and end of the ramp, then Chrome holds the ramp value at the cancel time. (If the ramp hasn't started, then the output is different.)

@rtoy rtoy added this to In PR Review in V1 via automation Feb 14, 2020
@padenot padenot self-requested a review February 27, 2020 17:12
@padenot
Copy link
Member

padenot commented Mar 19, 2020

I explicitly left that undefined because we don't say what happens if you cancel events that are currently running. If we want to define that, then that's a different issue.

It would be best to fix that.

@rtoy rtoy merged commit 9d4d866 into WebAudio:master Mar 19, 2020
V1 automation moved this from In PR Review to Done Mar 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
V1
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants