Summary: | [UI-side compositing] Proxy animations to the UI process | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||||||||||
Component: | New Bugs | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | bfulgham, buildbot, commit-queue, dino, rniwa, sam, simon.fraser, thorton | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2014-03-30 13:29:16 PDT
Created attachment 228130 [details]
Patch
Attachment 228130 [details] did not pass style-queue:
ERROR: Source/WebCore/platform/animation/TimingFunction.h:81: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/WebCore/platform/animation/TimingFunction.h:176: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/WebCore/platform/animation/TimingFunction.h:224: This { should be at the end of the previous line [whitespace/braces] [4]
Total errors found: 3 in 43 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 228130 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=228130&action=review > Source/WebCore/ChangeLog:48 > + * platform/graphics/ca/win/PlatformCAAnimationWin.cpp: > + * platform/graphics/ca/win/PlatformCAAnimationWin.h: Added. Not added to the project file? > Source/WebKit2/ChangeLog:16 > + Add support the "animationDidStart" callback and sending this back to the add support the? > Source/WebKit2/ChangeLog:24 > + and that the RemoteLayerTreeHost have a reference to the drawing area. This is slightly unfortunate. > Source/WebCore/platform/animation/TimingFunction.h:193 > + static PassRefPtr<StepsTimingFunction> create() newline above. > Source/WebCore/platform/animation/TimingFunction.h:213 > + void setStepAtStart(bool stepsAtStart) { m_stepAtStart = stepsAtStart; } step or steps? > Source/WebCore/platform/graphics/ca/mac/PlatformCAAnimationMac.h:30 > +#include <wtf/RetainPtr.h> newline above > Source/WebCore/platform/graphics/ca/mac/PlatformCAAnimationMac.h:34 > +OBJC_CLASS CAPropertyAnimation; > +OBJC_CLASS NSString; > +OBJC_CLASS CAMediaTimingFunction; sort > Source/WebCore/platform/graphics/ca/mac/PlatformCAAnimationMac.mm:2 > - * Copyright (C) 2010 Apple Inc. All rights reserved. > + * Copyright (C) 2014 Apple Inc. All rights reserved. ... > Source/WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.h:32 > +#include <wtf/RetainPtr.h> newline above > Source/WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.h:42 > +// static PassRefPtr<PlatformCAAnimation> create(PlatformAnimationRef); ??? (commented out) > Source/WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.h:48 > + virtual PassRefPtr<PlatformCAAnimation> copy() const override; in some of the other classes (the timing functions), this was called clone. here, copy. should we pick one? > Source/WebKit2/Platform/IPC/ArgumentCoders.h:203 > + for (typename HashSetType::const_iterator it = hashSet.begin(), end = hashSet.end(); it != end; ++it) isn't this a good candidate for range-based for? > Source/WebKit2/WebProcess/WebPage/mac/GraphicsLayerCARemote.cpp:61 > +// FIXME: reject filter animations if we don't support them yet, shouldn't we reject them now? :D > Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.mm:62 > + if ((self = [super init])) { I think we tend to use an early return here but I admit it doesn't gain us much. > Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.mm:159 > + no need for this newline > Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.mm:162 > + encoder << *static_cast<LinearTimingFunction*>(timingFunction.get()); I think encoder << static_cast<LinearTimingFunction>(*timingFunction); might have the same effect with less wackiness? > Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.mm:243 > + if (!decoder.decode(*static_cast<LinearTimingFunction*>(timingFunction.get()))) ditto > Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.mm:267 > +#pragma mark - ? > Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.mm:306 > + m_properties.hasNonZeroBeginTime = value; Is this a "had", then, according to the comment? > Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.mm:615 > + [NSNumber numberWithDouble:color.red()], @[ @(color.red()), @(color.green()), @(color.blue()), @(color.alpha()) ];? > Source/WebKit2/WebProcess/WebPage/mac/PlatformCAAnimationRemote.mm:625 > + [NSNumber numberWithDouble:point.x()], ditto > Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemote.cpp:281 > + // FIXME: remove from m_properties.addedAnimations ? probably? Created attachment 228151 [details]
For Win EWS
Attachment 228151 [details] did not pass style-queue:
ERROR: Source/WebCore/platform/animation/TimingFunction.h:81: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/WebCore/platform/animation/TimingFunction.h:176: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/WebCore/platform/animation/TimingFunction.h:224: This { should be at the end of the previous line [whitespace/braces] [4]
Total errors found: 3 in 44 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 228182 [details]
For Win EWS
Attachment 228182 [details] did not pass style-queue:
ERROR: Source/WebCore/platform/animation/TimingFunction.h:81: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/WebCore/platform/animation/TimingFunction.h:176: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/WebCore/platform/animation/TimingFunction.h:224: This { should be at the end of the previous line [whitespace/braces] [4]
Total errors found: 3 in 44 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 228182 [details] For Win EWS View in context: https://bugs.webkit.org/attachment.cgi?id=228182&action=review I also needed to: 1. Add a `#include "GraphicsContext.h"' to PlatformCALayerWinInternal.cpp. I guess the header includes are a little different now. 2. Continue to have a friend relationship between PlatformCAAnimation and PlatformCALayerWin, so that PlatformCALayerWin could call "setActualStartTimeIfNeeded" from animationStarted. > Source/WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:147 > return adoptRef(new PlatformCAAnimation(type, keyPath)); return adoptRef(new PlatformCAAnimationWin(type, keyPath)); ^^^ > Source/WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:152 > return adoptRef(new PlatformCAAnimation(animation)); return adoptRef(new PlatformCAAnimationWin(type, keyPath)); ^^^ > Source/WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:156 > : m_type(type) : PlatformCAAnimation(type) > Source/WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:169 > m_type = Basic; This needs to be setType(Basic); (and later, setType(Keyframe);) > Source/WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:302 > CACFTimingFunctionRef timingFunc = CACFAnimationGetTimingFunction(value->m_animation.get()); Need to use "toPlatformCAAnimationWin(value)->m_animation.get()" > Source/WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:389 > CACFAnimationSetFromValue(m_animation.get(), CACFAnimationGetFromValue(value->platformAnimation())); toPlatformCAAnimationWin(value)->platformAnimation() > Source/WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:440 > return; Need to use toPlatformCAAnimationWin(value)->platformAnimation() for next line. > Source/WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:515 > return; toPlatformCAAnimationWin on next line. > Source/WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:537 > return; toPlatformCAAnimationWin on next line. > Source/WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:559 > CACFAnimationSetTimingFunctions(m_animation.get(), CACFAnimationGetTimingFunctions(value->platformAnimation())); Need to use toPlatformCAAnimationWin(value)->platformAnimation() This line continues to fail on Windows (32-bit) build: COMPILE_ASSERT(sizeof(RenderStyle) == sizeof(SameSizeAsRenderStyle), RenderStyle_should_stay_small); Created attachment 228195 [details]
For Win EWS
Attachment 228195 [details] did not pass style-queue:
ERROR: Source/WebCore/platform/animation/TimingFunction.h:81: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/WebCore/platform/animation/TimingFunction.h:176: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/WebCore/platform/animation/TimingFunction.h:224: This { should be at the end of the previous line [whitespace/braces] [4]
Total errors found: 3 in 45 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 228195 [details] For Win EWS Attachment 228195 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5233147704770560 New failing tests: media/W3C/audio/canPlayType/canPlayType_application_octet_stream_with_codecs_1.html Created attachment 228200 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Created attachment 228202 [details]
For Win EWS
Attachment 228202 [details] did not pass style-queue:
ERROR: Source/WebCore/platform/animation/TimingFunction.h:81: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/WebCore/platform/animation/TimingFunction.h:176: This { should be at the end of the previous line [whitespace/braces] [4]
ERROR: Source/WebCore/platform/animation/TimingFunction.h:224: This { should be at the end of the previous line [whitespace/braces] [4]
Total errors found: 3 in 46 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 228202 [details] For Win EWS Cleared review? from attachment 228202 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again). |