Refactor timing function solving code
Created attachment 330804 [details] Patch
Created attachment 330821 [details] Patch
Created attachment 330822 [details] Patch
Comment on attachment 330822 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=330822&action=review > Source/WebCore/platform/animation/TimingFunction.cpp:71 > + return std::min(1.0, (floor(m_steps * inputTime) + 1) / m_steps); Should use std::floor. > Source/WebCore/platform/animation/TimingFunction.cpp:72 > + return floor(m_steps * inputTime) / m_steps; Should use std::floor.
Comment on attachment 330822 [details] Patch Attachment 330822 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/5998231 New failing tests: imported/w3c/web-platform-tests/css/css-ui-3/caret-color-021.html transitions/steps-timing-function.html animations/animation-direction-reverse-timing-functions.html animations/keyframe-multiple-timing-functions-transform.html animations/fill-mode-iteration-count-non-integer.html fast/css-generated-content/pseudo-animation.html animations/multiple-animations-timing-function.html animations/missing-keyframe-properties-timing-function.html fast/css-generated-content/pseudo-transition.html animations/spring-function.html transitions/default-timing-function.html transitions/transition-display-property-2.html animations/trigger-container-scroll-simple.html animations/animation-direction-reverse-timing-functions-hardware.html animations/timing-functions.html
Created attachment 330825 [details] Archive of layout-test-results from ews103 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 330822 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=330822&action=review > Source/WebCore/platform/animation/TimingFunction.h:53 > + double transformTime(double inputTime, double) const { return inputTime; } Need to mark this virtual. > Source/WebCore/platform/animation/TimingFunction.h:160 > + double transformTime(double, double) const; Need to mark this final. > Source/WebCore/platform/animation/TimingFunction.h:210 > + double transformTime(double, double) const; Need to mark this final. > Source/WebCore/platform/animation/TimingFunction.h:264 > + double transformTime(double, double) const; Need to mark this final.
Comment on attachment 330822 [details] Patch Attachment 330822 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5998401 New failing tests: animations/multiple-animations-timing-function.html transitions/steps-timing-function.html fast/css-generated-content/pseudo-animation.html animations/keyframe-multiple-timing-functions-transform.html animations/fill-mode-iteration-count-non-integer.html animations/animation-direction-reverse-timing-functions.html imported/w3c/web-platform-tests/css/css-ui-3/caret-color-021.html fast/css-generated-content/pseudo-transition.html animations/spring-function.html transitions/transition-display-property-2.html animations/animation-direction-reverse-timing-functions-hardware.html animations/trigger-container-scroll-simple.html animations/animation-initial-inheritance.html animations/missing-keyframe-properties-timing-function.html animations/timing-functions.html
Created attachment 330826 [details] Archive of layout-test-results from ews123 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
(In reply to Darin Adler from comment #7) > Comment on attachment 330822 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=330822&action=review > > > Source/WebCore/platform/animation/TimingFunction.h:53 > > + double transformTime(double inputTime, double) const { return inputTime; } > > Need to mark this virtual. There was a linking issue with a virtual method here that I couldn't fix, even with the help of some fine folks on #webkit. I'm preparing a new patch where we use a single method on TimingFunction that downcasts based on the type.
Created attachment 330827 [details] Patch
(In reply to Darin Adler from comment #4) > Comment on attachment 330822 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=330822&action=review > > > Source/WebCore/platform/animation/TimingFunction.cpp:71 > > + return std::min(1.0, (floor(m_steps * inputTime) + 1) / m_steps); > > Should use std::floor. > > > Source/WebCore/platform/animation/TimingFunction.cpp:72 > > + return floor(m_steps * inputTime) / m_steps; > > Should use std::floor. The new patch uses std::floor.
Comment on attachment 330827 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=330827&action=review > Source/WebCore/platform/animation/TimingFunction.cpp:64 > + auto& function = *static_cast<const CubicBezierTimingFunction*>(this); Maybe we should have is<> and downcast<> for these? > Source/WebCore/platform/animation/TimingFunction.cpp:66 > + auto epsilon = 1.0 / (200.0 * duration); > + return UnitBezier(function.x1(), function.y1(), function.x2(), function.y2()).solve(inputTime, epsilon); Any reason you moved the epsilon to a variable? This means we no longer have the comment on why we change the accuracy for longer durations.
Committed r226645: <https://trac.webkit.org/changeset/226645>
(In reply to Dean Jackson from comment #13) > Comment on attachment 330827 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=330827&action=review > > > Source/WebCore/platform/animation/TimingFunction.cpp:66 > > + auto epsilon = 1.0 / (200.0 * duration); > > + return UnitBezier(function.x1(), function.y1(), function.x2(), function.y2()).solve(inputTime, epsilon); > > Any reason you moved the epsilon to a variable? This means we no longer have > the comment on why we change the accuracy for longer durations. I added it back in the commit.
<rdar://problem/36378011>