| Summary: | Share code between Mac (CA) and Windows (CACF) GraphicsLayer implementations | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Chris Marrin <cmarrin> | ||||||||||||||||||||||||||||||||||||||||
| Component: | Layout and Rendering | Assignee: | Chris Marrin <cmarrin> | ||||||||||||||||||||||||||||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||||||||||||||||
| Severity: | Normal | CC: | abarth, aroben, buildbot, darin, dglazkov, eric, simon.fraser, vangelis, webkit-ews, webkit.review.bot | ||||||||||||||||||||||||||||||||||||||||
| Priority: | P2 | ||||||||||||||||||||||||||||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||||||||||||||||||||||||||||
| Hardware: | All | ||||||||||||||||||||||||||||||||||||||||||
| OS: | OS X 10.5 | ||||||||||||||||||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||||||||||||||||||
|
Description
Chris Marrin
2010-11-11 10:02:52 PST
Maybe it's a good time to think about how to factor the code such that common pieces can be used by non-CA platforms. Right now platform/graphics/chromium duplicates a lot of the CACF code as well. Created attachment 73632 [details]
Patch changing name of GraphicsLayerCA.* to GraphicsLayerMac.*
Comment on attachment 73632 [details]
Patch changing name of GraphicsLayerCA.* to GraphicsLayerMac.*
The GraphicsLayerCA class needs to renamed too (hint: Edit All in Scope).
Comment on attachment 73632 [details]
Patch changing name of GraphicsLayerCA.* to GraphicsLayerMac.*
I was confused by SVN.
Landed rename part in http://trac.webkit.org/changeset/71848 (In reply to comment #1) > Maybe it's a good time to think about how to factor the code such that common pieces can be used by non-CA platforms. Right now platform/graphics/chromium duplicates a lot of the CACF code as well. I think it's reasonable to make the work I'm doing have a wider scope than "CA based implementations". But I'd like to get the convergence between CA and CACF finished first. Then I think it will be simpler for you to understand how your implementation fits in and how we can change the new "ca" based API to have that wider scope. http://trac.webkit.org/changeset/71848 might have broken Leopard Intel Release (Tests) The following tests are not passing: fast/workers/storage/interrupt-database-sync.html Comment on attachment 73632 [details] Patch changing name of GraphicsLayerCA.* to GraphicsLayerMac.* Cleared Simon Fraser's review+ from obsolete attachment 73632 [details] so that this bug does not appear in http://webkit.org/pending-commit. Created attachment 75382 [details]
Patch with Mac Implementation
Created attachment 75394 [details]
Fix merge issue with last patch
Attachment 75394 [details] did not build on qt: Build output: http://queues.webkit.org/results/6801015 Attachment 75394 [details] did not build on chromium: Build output: http://queues.webkit.org/results/6826009 Comment on attachment 75394 [details] Fix merge issue with last patch > WebCore/platform/graphics/GraphicsLayer.h:51 > typedef CALayer PlatformLayer; I think we should do this as: typedef CALayer* PlatformLayerRef to avoid confusion about whether it's a pointer or not. > WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:258 > +PassOwnPtr<GraphicsLayer> GraphicsLayer::create(GraphicsLayerClient* client) > +{ > + return new GraphicsLayerCA(client); > +} > + This could be in the header. > WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:291 > + // Release the clone layers inside the exception-handling block. > + removeCloneLayers(); The comment is no longer applicable. > WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:1459 > +static void copyAnimationProperties(const PlatformCAAnimation* from, PlatformCAAnimation* to) > +{ > + to->setBeginTime(from->beginTime()); > + to->setDuration(from->duration()); > + to->setRepeatCount(from->repeatCount()); > + to->setAutoreverses(from->autoreverses()); > + to->setFillMode(from->fillMode()); > + to->setRemovedOnCompletion(from->isRemovedOnCompletion()); > + to->setAdditive(from->isAdditive()); > + to->copyTimingFunctionFrom(from); > + > +#if HAVE_MODERN_QUARTZCORE I think we could have a copy constructor on PlatformCAAnimation instead, here. > WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:1524 > +void GraphicsLayerCA::setContentsToCanvas(PlatformLayer* canvasLayer) > +{ > + if (m_contentsLayer && canvasLayer == m_contentsLayer->platformLayer()) > + return; > + > + // Create the PlatformCALayer to wrap the incoming layer > + m_contentsLayer = canvasLayer ? PlatformCALayer::create(canvasLayer, this) : 0; > + > + m_contentsLayerPurpose = canvasLayer ? ContentsLayerForCanvas : NoContentsLayer; > + > + noteSublayersChanged(); > + noteLayerPropertyChanged(ContentsCanvasLayerChanged); > +} Please move this so it's next to the other "setContentsTo" methods. > WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:1762 > + > + > + > + > + if (isTransformTypeNumber(transformOpType)) { Extra blank lines here. > WebCore/platform/graphics/ca/PlatformCAAnimation.h:63 > + static PassRefPtr<PlatformCAAnimation> create(void* animation); CAPropertyAnimation rather than void*? Or maybe you have to have a typedef that maps to CAPropertyAnimation on Mac, and CACFAnimationRef on Windows? > WebCore/platform/graphics/ca/PlatformCAAnimation.h:69 > + void* platformAnimation() const; Why not return a CAPropertyAnimation? > WebCore/platform/graphics/ca/PlatformCALayer.h:50 > + // FIXME: This object represents all Layer types used in GraphicsLayer, and those used by video and canvas. Why is that a FIXME? > WebCore/platform/graphics/ca/PlatformCALayer.h:65 > + // FIXME: turn off implicit animation in ctor Either do that, or remove the FIXME. r=me with those comments addressed. Attachment 75394 [details] did not build on win: Build output: http://queues.webkit.org/results/6839009 Attachment 75394 [details] did not build on mac: Build output: http://queues.webkit.org/results/6754009 Created attachment 75422 [details]
Patch fixing issues from the review and add 2 files
Comment on attachment 75422 [details]
Patch fixing issues from the review and add 2 files
The last patch was missing the PlatformCALayerMac.mm and PlatformCAAnimationMac.mm implementation files. I also added some initial settings to in TiledLayer creation. Then I implemented all of Simon's recommendations.
This latest patch also fixes build errors in Qt, Win, and Chrome builds Comment on attachment 75422 [details] Patch fixing issues from the review and add 2 files View in context: https://bugs.webkit.org/attachment.cgi?id=75422&action=review > WebCore/platform/graphics/ca/mac/PlatformCAAnimationMac.mm:234 > +bool PlatformCAAnimation::nonZeroBeginTimeFlag() const I think this should be hasNonZeroBeginTimeFlag() > WebCore/platform/graphics/ca/mac/PlatformCAAnimationMac.mm:242 > + [m_animation.get() setValue:[NSNumber numberWithBool:value] forKey:WKNonZeroBeginTimeFlag]; > + Extra space. > WebCore/platform/graphics/ca/mac/PlatformCALayerMac.mm:182 > + layerClass = NSClassFromString(@"CALayer"); [CALayer class] would be fine here, and faster. This was rolled out because of leopard build issues. I tried to fix them, but fixes were non-obvious. Attachment 75422 [details] did not build on mac: Build output: http://queues.webkit.org/results/6851007 Created attachment 75522 [details]
Please do not review. Just submitting to see the errors on the bots
Created attachment 75530 [details]
Fixed merge conflicts
Created attachment 75575 [details]
Patch
Comment on attachment 75575 [details] Patch This patch landed with http://trac.webkit.org/changeset/73380 Created attachment 75842 [details]
Patch
Comment on attachment 75842 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=75842&action=review New files are missing from the patch. > JavaScriptCore/wtf/Platform.h:535 > +#if PLATFORM(MAC) || PLATFORM(IOS) || PLATFORM(WIN) > +#define WTF_PLATFORM_CA 1 > +#endif Are you sure this won't affect other ports on Windows? Created attachment 75844 [details]
Patch
Created attachment 75846 [details]
Patch with some housekeeping changes
Comment on attachment 75846 [details] Patch with some housekeeping changes View in context: https://bugs.webkit.org/attachment.cgi?id=75846&action=review Don't forget to add the new files to the vcproj. > WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:-45 > -#include <wtf/UnusedParam.h> Are you sure it's unused on all platforms? (In reply to comment #30) > (From update of attachment 75846 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=75846&action=review > > Don't forget to add the new files to the vcproj. I will do that when I do the Windows side, which I haven't yet. > > > WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:-45 > > -#include <wtf/UnusedParam.h> > > Are you sure it's unused on all platforms? The macro isn't used anywhere in GraphicsLayerCA.cpp and there aren't any relevant ifdefs Attachment 75842 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/update-webkit']" exit_code: 2
Updating OpenSource
Incomplete data: Delta source ended unexpectedly at /usr/lib/git-core/git-svn line 5061
Died at WebKitTools/Scripts/update-webkit line 132.
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 75844 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/update-webkit']" exit_code: 2
Updating OpenSource
Incomplete data: Delta source ended unexpectedly at /usr/lib/git-core/git-svn line 5061
Died at WebKitTools/Scripts/update-webkit line 132.
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 75846 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/update-webkit']" exit_code: 2
Updating OpenSource
Incomplete data: Delta source ended unexpectedly at /usr/lib/git-core/git-svn line 5061
Died at WebKitTools/Scripts/update-webkit line 132.
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 75943 [details]
Resubmission of last patch with build fixes
relanded housekeeping patch in http://trac.webkit.org/changeset/73540 Created attachment 76144 [details]
Patch getting rid of NonZeroBeginTimeFlag
Comment on attachment 76144 [details]
Patch getting rid of NonZeroBeginTimeFlag
Windows doesn't need this flag, so it is just an implementation detail. This buries it in the Mac implementation.
Comment on attachment 76144 [details] Patch getting rid of NonZeroBeginTimeFlag View in context: https://bugs.webkit.org/attachment.cgi?id=76144&action=review > WebCore/platform/graphics/ca/mac/PlatformCAAnimationMac.mm:252 > + // The flag is needed because later beginTIme will get changed beginTime. Comment on attachment 76144 [details] Patch getting rid of NonZeroBeginTimeFlag Landed in http://trac.webkit.org/changeset/73731 Created attachment 76223 [details]
Patch adding CACFContextGetLastCommitTime to WKSI
Created attachment 76225 [details]
Patch adding CACFContextGetLastCommitTime to WKSI (gets rid of bogus .lib file)
Comment on attachment 76225 [details] Patch adding CACFContextGetLastCommitTime to WKSI (gets rid of bogus .lib file) View in context: https://bugs.webkit.org/attachment.cgi?id=76225&action=review > WebKitLibraries/ChangeLog:7 > + Also got rid of WebKitSystemInterface_debug.lib as recommended by > + Adam Roben. You should mention that this hasn't been used since r72327. Comment on attachment 76225 [details] Patch adding CACFContextGetLastCommitTime to WKSI (gets rid of bogus .lib file) Landed in http://trac.webkit.org/changeset/73790 Comment on attachment 75575 [details] Patch Cleared Simon Fraser's review+ from obsolete attachment 75575 [details] so that this bug does not appear in http://webkit.org/pending-commit. Comment on attachment 75846 [details] Patch with some housekeeping changes Cleared Simon Fraser's review+ from obsolete attachment 75846 [details] so that this bug does not appear in http://webkit.org/pending-commit. Comment on attachment 76144 [details] Patch getting rid of NonZeroBeginTimeFlag Cleared Simon Fraser's review+ from obsolete attachment 76144 [details] so that this bug does not appear in http://webkit.org/pending-commit. Comment on attachment 76225 [details] Patch adding CACFContextGetLastCommitTime to WKSI (gets rid of bogus .lib file) Cleared Adam Roben's review+ from obsolete attachment 76225 [details] so that this bug does not appear in http://webkit.org/pending-commit. Created attachment 78154 [details]
Patch
Comment on attachment 78154 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=78154&action=review This all looks fine, but reflections aren't working because layerDidDisplay isn't being called on Windows, as we discussed. > WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:916 > - for (size_t i = 0; i < newSublayers.size(); ++i) > + for (int i = newSublayers.size() - 1; i >= 0; --i) > newSublayers[i]->removeFromSuperlayer(); Why the order reversal? > WebCore/platform/graphics/ca/GraphicsLayerCA.h:67 > + // PlatformCALayerClient overrides > + virtual void platformCALayerLayoutSublayersOfLayer(PlatformCALayer*) { } > + virtual bool platformCALayerRespondsToLayoutChanges() const { return false; } > + > + virtual void platformCALayerAnimationStarted(CFTimeInterval beginTime); > + virtual CompositingCoordinatesOrientation platformCALayerContentsOrientation() const { return contentsOrientation(); } > + virtual void platformCALayerPaintContents(GraphicsContext& context, const IntRect& clip) { paintGraphicsLayerContents(context, clip); } > + virtual bool platformCALayerShowDebugBorders() const { return showDebugBorders(); } > + virtual bool platformCALayerShowRepaintCounter() const { return showRepaintCounter(); } > + virtual int platformCALayerIncrementRepaintCount() { return incrementRepaintCount(); } > + > + virtual bool platformCALayerContentsOpaque() const { return contentsOpaque(); } > + virtual bool platformCALayerDrawsContent() const { return drawsContent(); } > + virtual void platformCALayerLayerDidDisplay(PlatformLayer* layer) { return layerDidDisplay(layer); } Please make these private. > WebCore/platform/graphics/ca/PlatformCALayerClient.h:2 > + * Copyright (C) 2010 Apple Inc. All rights reserved. 2011! Comment on attachment 78154 [details]
Patch
Ah, this is Mac code. r=me
Attachment 78154 [details] did not build on win: Build output: http://queues.webkit.org/results/7402011 Committed r75199: <http://trac.webkit.org/changeset/75199> Not quite done yet. Hopefully the next patch will be the last. It will have the Windows side of all this http://trac.webkit.org/changeset/75199 might have broken Windows Release (Build) and Windows Debug (Build) Created attachment 78196 [details]
Patch
Created attachment 78197 [details]
Patch
Created attachment 78200 [details]
Final Patch
Comment on attachment 78196 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=78196&action=review r=me with comments, and questions about the vcproj change. > WebCore/WebCore.vcproj/WebCore.vcproj:22 > <Configuration > Name="Debug|Win32" > ConfigurationType="4" > - InheritedPropertySheets="$(WebKitVSPropsRedirectionDir)..\..\WebKitLibraries\win\tools\vsprops\FeatureDefines.vsprops;$(WebKitVSPropsRedirectionDir)..\..\WebKitLibraries\win\tools\vsprops\common.vsprops;$(WebKitVSPropsRedirectionDir)..\..\WebKitLibraries\win\tools\vsprops\debug.vsprops;.\WebCoreCommon.vsprops;.\WebCoreCG.vsprops;.\WebCoreCFNetwork.vsprops;.\WebCorePthreads.vsprops;.\WebCoreMediaQT.vsprops;.\WebCoreQuartzCore.vsprops" > + InheritedPropertySheets="..\..\WebKitLibraries\win\tools\vsprops\FeatureDefines.vsprops;..\..\WebKitLibraries\win\tools\vsprops\common.vsprops;..\..\WebKitLibraries\win\tools\vsprops\debug.vsprops;.\WebCoreCommon.vsprops;.\WebCoreCG.vsprops;.\WebCoreCFNetwork.vsprops;.\WebCorePthreads.vsprops;.\WebCoreMediaQT.vsprops;.\WebCoreQuartzCore.vsprops" > CharacterSet="1" Many of these changes look bad. You might want to hand-edit the project file. > WebCore/WebCore.vcproj/WebCoreCommon.vsprops:11 > - AdditionalIncludeDirectories=""$(ProjectDir)..";"$(ProjectDir)..\accessibility";"$(ProjectDir)..\accessibility\win";"$(ProjectDir)..\bridge";"$(ProjectDir)..\bridge\c";"$(ProjectDir)..\bridge\jsc";"$(ProjectDir)..\css";"$(ProjectDir)..\editing";"$(ProjectDir)..\fileapi";"$(ProjectDir)..\rendering";"$(ProjectDir)..\rendering\style";"$(ProjectDir)..\rendering\svg";"$(ProjectDir)..\bindings";"$(ProjectDir)..\bindings\generic";"$(ProjectDir)..\bindings\js";"$(ProjectDir)..\bindings\js\specialization";"$(ProjectDir)..\dom";"$(ProjectDir)..\dom\default";"$(ProjectDir)..\history";"$(ProjectDir)..\html";"$(ProjectDir)..\html\canvas";"$(ProjectDir)..\html\parser";"$(ProjectDir)..\html\shadow";"$(ProjectDir)..\inspector";"$(ProjectDir)..\loader";"$(ProjectDir)..\loader\appcache";"$(ProjectDir)..\loader\archive";"$(ProjectDir)..\loader\archive\cf";"$(ProjectDir)..\loader\cache";"$(ProjectDir)..\loader\icon";"$(ProjectDir)..\mathml";"$(ProjectDir)..\notifications";"$(ProjectDir)..\page";"$(ProjectDir)..\page\animation";"$(ProjectDir)..\page\win";"$(ProjectDir)..\platform";"$(ProjectDir)..\platform\animation";"$(ProjectDir)..\platform\mock";"$(ProjectDir)..\platform\sql";"$(ProjectDir)..\platform\win";"$(ProjectDir)..\platform\network";"$(ProjectDir)..\platform\network\win";"$(ProjectDir)..\platform\cf";"$(ProjectDir)..\platform\graphics";"$(ProjectDir)..\platform\graphics\filters";"$(ProjectDir)..\platform\graphics\opentype";"$(ProjectDir)..\platform\graphics\transforms";"$(ProjectDir)..\platform\text";"$(ProjectDir)..\platform\text\transcoder";"$(ProjectDir)..\platform\graphics\win";"$(ProjectDir)..\xml";"$(ConfigurationBuildDir)\obj\WebCore\DerivedSources";"$(ProjectDir)..\plugins";"$(ProjectDir)..\plugins\win";"$(ProjectDir)..\svg\animation";"$(ProjectDir)..\svg\graphics";"$(ProjectDir)..\svg\properties";"$(ProjectDir)..\svg\graphics\filters";"$(ProjectDir)..\svg";"$(ProjectDir)..\wml";"$(ProjectDir)..\storage";"$(ProjectDir)..\websockets";"$(ProjectDir)..\workers";"$(ConfigurationBuildDir)\include";"$(ConfigurationBuildDir)\include\private";"$(ConfigurationBuildDir)\include\JavaScriptCore";"$(ConfigurationBuildDir)\include\private\JavaScriptCore";"$(ProjectDir)..\ForwardingHeaders";"$(WebKitLibrariesDir)\include";"$(WebKitLibrariesDir)\include\private";"$(WebKitLibrariesDir)\include\private\JavaScriptCore";"$(WebKitLibrariesDir)\include\pthreads";"$(WebKitLibrariesDir)\include\sqlite";"$(WebKitLibrariesDir)\include\JavaScriptCore";"$(WebKitLibrariesDir)\include\zlib"" > + AdditionalIncludeDirectories=""$(ProjectDir)..";"$(ProjectDir)..\accessibility";"$(ProjectDir)..\accessibility\win";"$(ProjectDir)..\bridge";"$(ProjectDir)..\bridge\c";"$(ProjectDir)..\bridge\jsc";"$(ProjectDir)..\css";"$(ProjectDir)..\editing";"$(ProjectDir)..\fileapi";"$(ProjectDir)..\rendering";"$(ProjectDir)..\rendering\style";"$(ProjectDir)..\rendering\svg";"$(ProjectDir)..\bindings";"$(ProjectDir)..\bindings\generic";"$(ProjectDir)..\bindings\js";"$(ProjectDir)..\bindings\js\specialization";"$(ProjectDir)..\dom";"$(ProjectDir)..\dom\default";"$(ProjectDir)..\history";"$(ProjectDir)..\html";"$(ProjectDir)..\html\canvas";"$(ProjectDir)..\html\parser";"$(ProjectDir)..\html\shadow";"$(ProjectDir)..\inspector";"$(ProjectDir)..\loader";"$(ProjectDir)..\loader\appcache";"$(ProjectDir)..\loader\archive";"$(ProjectDir)..\loader\archive\cf";"$(ProjectDir)..\loader\cache";"$(ProjectDir)..\loader\icon";"$(ProjectDir)..\mathml";"$(ProjectDir)..\notifications";"$(ProjectDir)..\page";"$(ProjectDir)..\page\animation";"$(ProjectDir)..\page\win";"$(ProjectDir)..\platform";"$(ProjectDir)..\platform\animation";"$(ProjectDir)..\platform\mock";"$(ProjectDir)..\platform\sql";"$(ProjectDir)..\platform\win";"$(ProjectDir)..\platform\network";"$(ProjectDir)..\platform\network\win";"$(ProjectDir)..\platform\cf";"$(ProjectDir)..\platform\graphics";"$(ProjectDir)..\platform\graphics\ca";"$(ProjectDir)..\platform\graphics\filters";"$(ProjectDir)..\platform\graphics\opentype";"$(ProjectDir)..\platform\graphics\transforms";"$(ProjectDir)..\platform\text";"$(ProjectDir)..\platform\text\transcoder";"$(ProjectDir)..\platform\graphics\win";"$(ProjectDir)..\xml";"$(ConfigurationBuildDir)\obj\WebCore\DerivedSources";"$(ProjectDir)..\plugins";"$(ProjectDir)..\plugins\win";"$(ProjectDir)..\svg\animation";"$(ProjectDir)..\svg\graphics";"$(ProjectDir)..\svg\properties";"$(ProjectDir)..\svg\graphics\filters";"$(ProjectDir)..\svg";"$(ProjectDir)..\wml";"$(ProjectDir)..\storage";"$(ProjectDir)..\websockets";"$(ProjectDir)..\workers";"$(ConfigurationBuildDir)\include";"$(ConfigurationBuildDir)\include\private";"$(ConfigurationBuildDir)\include\JavaScriptCore";"$(ConfigurationBuildDir)\include\private\JavaScriptCore";"$(ProjectDir)..\ForwardingHeaders";"$(WebKitLibrariesDir)\include";"$(WebKitLibrariesDir)\include\private";"$(WebKitLibrariesDir)\include\private\JavaScriptCore";"$(WebKitLibrariesDir)\include\pthreads";"$(WebKitLibrariesDir)\include\sqlite";"$(WebKitLibrariesDir)\include\JavaScriptCore";"$(WebKitLibrariesDir)\include\zlib"" > PreprocessorDefinitions="__WIN32__;DISABLE_3D_RENDERING;WEBCORE_CONTEXT_MENUS" Do you really want this change? > WebCore/html/canvas/CanvasRenderingContext.h:63 > - virtual PlatformLayer* platformLayer() const { return 0; } > +// virtual PlatformLayer* platformLayer() const { return 0; } No commenting out please! > WebCore/platform/graphics/ca/PlatformCAAnimation.h:47 > +typedef struct _CACFAnimation *CACFAnimationRef; The * should be on the other side here. > WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:2 > + * Copyright (C) 2010 Apple Inc. All rights reserved. 2011! > WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:153 > +static void initialize(PlatformCAAnimation* animation) > +{ > +} Unused? > WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:188 > + RetainPtr<CFStringRef> s(AdoptCF, animation->keyPath().createCFString()); > + CACFAnimationSetKeyPath(m_animation.get(), s.get()); Please rename 's' to keyPath or similar. > WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:220 > +bool PlatformCAAnimation::supportsValueFunction() > +{ > + return true; > +} Method should be const, could be inline. > WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:300 > + RetainPtr<CFStringRef> s(AdoptCF, toCACFFillModeType(value).createCFString()); > + CACFAnimationSetFillMode(m_animation.get(), s.get()); Please rename 's'. > WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:341 > + RetainPtr<CFStringRef> s(AdoptCF, toCACFValueFunctionType(value).createCFString()); > + CACFAnimationSetValueFunction(m_animation.get(), CACFValueFunctionGetFunctionWithName(s.get())); ditto > WebCore/platform/graphics/ca/win/PlatformCALayerWin.cpp:2 > + * Copyright (C) 2010 Apple Inc. All rights reserved. 2011! > WebCore/platform/graphics/ca/win/PlatformCALayerWin.cpp:47 > +bool PlatformCALayer::isValueFunctionSupported() > +{ > + return true; > +} Should be const and inline. > WebCore/platform/graphics/ca/win/PlatformCALayerWin.cpp:303 > +PassRefPtr<PlatformCAAnimation> PlatformCALayer::animationForKey(const String& key) Method should be const. > WebCore/platform/graphics/ca/win/PlatformCALayerWin.cpp:460 > + return const_cast<void*>(CACFLayerGetContents(m_layer.get())); Is that the right cast? > WebCore/platform/graphics/ca/win/PlatformCALayerWinInternal.cpp:2 > + * Copyright (C) 2010 Apple Inc. All rights reserved. 2011! > WebCore/platform/graphics/ca/win/PlatformCALayerWinInternal.cpp:63 > + if (!owner() || !owner()->owner()) owner()->owner() is weird. > WebCore/platform/graphics/ca/win/PlatformCALayerWinInternal.h:2 > + * Copyright (C) 2010 Apple Inc. All rights reserved. 2011 > WebCore/platform/graphics/win/GraphicsContextCGWin.cpp:48 > + if (info.bmBitsPixel != 32) > + return 0; Why this change? > WebCore/platform/graphics/win/MediaPlayerPrivateQuickTimeVisualContext.cpp:104 > + virtual void platformCALayerLayoutSublayersOfLayer(PlatformCALayer*); > + virtual bool platformCALayerRespondsToLayoutChanges() const { return true; } > + > + virtual void platformCALayerAnimationStarted(CFTimeInterval beginTime) { } > + virtual GraphicsLayer::CompositingCoordinatesOrientation platformCALayerContentsOrientation() const { return GraphicsLayer::CompositingCoordinatesBottomUp; } > + virtual void platformCALayerPaintContents(GraphicsContext&, const IntRect& inClip) { } > + virtual bool platformCALayerShowDebugBorders() const { return false; } > + virtual bool platformCALayerShowRepaintCounter() const { return false; } > + virtual int platformCALayerIncrementRepaintCount() { return 0; } > + > + virtual bool platformCALayerContentsOpaque() const { return false; } > + virtual bool platformCALayerDrawsContent() const { return false; } > + virtual void platformCALayerLayerDidDisplay(PlatformLayer*) { } Make these private if possible. > WebCore/platform/graphics/win/WKCACFLayerRenderer.cpp:481 > + if (!wkCACFContextBeginUpdate(m_context, space, sizeof(space), CACurrentMediaTime(), bounds, windowDirtyRects.data(), windowDirtyRects.size())) Share the same CACurrentMediaTime you used above? > WebKit/win/FullscreenVideoController.cpp:193 > + virtual void platformCALayerLayoutSublayersOfLayer(PlatformCALayer*); > + virtual bool platformCALayerRespondsToLayoutChanges() const { return true; } > + > + virtual void platformCALayerAnimationStarted(CFTimeInterval beginTime) { } > + virtual GraphicsLayer::CompositingCoordinatesOrientation platformCALayerContentsOrientation() const { return GraphicsLayer::CompositingCoordinatesBottomUp; } > + virtual void platformCALayerPaintContents(GraphicsContext&, const IntRect& inClip) { } > + virtual bool platformCALayerShowDebugBorders() const { return false; } > + virtual bool platformCALayerShowRepaintCounter() const { return false; } > + virtual int platformCALayerIncrementRepaintCount() { return 0; } > + > + virtual bool platformCALayerContentsOpaque() const { return false; } > + virtual bool platformCALayerDrawsContent() const { return false; } > + virtual void platformCALayerLayerDidDisplay(PlatformLayer*) { } Make private? Comment on attachment 78200 [details] Final Patch r=me with comments, and questions about the vcproj change. > WebCore/WebCore.vcproj/WebCore.vcproj:22 > <Configuration > Name="Debug|Win32" > ConfigurationType="4" > - InheritedPropertySheets="$(WebKitVSPropsRedirectionDir)..\..\WebKitLibraries\win\tools\vsprops\FeatureDefines.vsprops;$(WebKitVSPropsRedirectionDir)..\..\WebKitLibraries\win\tools\vsprops\common.vsprops;$(WebKitVSPropsRedirectionDir)..\..\WebKitLibraries\win\tools\vsprops\debug.vsprops;.\WebCoreCommon.vsprops;.\WebCoreCG.vsprops;.\WebCoreCFNetwork.vsprops;.\WebCorePthreads.vsprops;.\WebCoreMediaQT.vsprops;.\WebCoreQuartzCore.vsprops" > + InheritedPropertySheets="..\..\WebKitLibraries\win\tools\vsprops\FeatureDefines.vsprops;..\..\WebKitLibraries\win\tools\vsprops\common.vsprops;..\..\WebKitLibraries\win\tools\vsprops\debug.vsprops;.\WebCoreCommon.vsprops;.\WebCoreCG.vsprops;.\WebCoreCFNetwork.vsprops;.\WebCorePthreads.vsprops;.\WebCoreMediaQT.vsprops;.\WebCoreQuartzCore.vsprops" > CharacterSet="1" Many of these changes look bad. You might want to hand-edit the project file. > WebCore/WebCore.vcproj/WebCoreCommon.vsprops:11 > - AdditionalIncludeDirectories=""$(ProjectDir)..";"$(ProjectDir)..\accessibility";"$(ProjectDir)..\accessibility\win";"$(ProjectDir)..\bridge";"$(ProjectDir)..\bridge\c";"$(ProjectDir)..\bridge\jsc";"$(ProjectDir)..\css";"$(ProjectDir)..\editing";"$(ProjectDir)..\fileapi";"$(ProjectDir)..\rendering";"$(ProjectDir)..\rendering\style";"$(ProjectDir)..\rendering\svg";"$(ProjectDir)..\bindings";"$(ProjectDir)..\bindings\generic";"$(ProjectDir)..\bindings\js";"$(ProjectDir)..\bindings\js\specialization";"$(ProjectDir)..\dom";"$(ProjectDir)..\dom\default";"$(ProjectDir)..\history";"$(ProjectDir)..\html";"$(ProjectDir)..\html\canvas";"$(ProjectDir)..\html\parser";"$(ProjectDir)..\html\shadow";"$(ProjectDir)..\inspector";"$(ProjectDir)..\loader";"$(ProjectDir)..\loader\appcache";"$(ProjectDir)..\loader\archive";"$(ProjectDir)..\loader\archive\cf";"$(ProjectDir)..\loader\cache";"$(ProjectDir)..\loader\icon";"$(ProjectDir)..\mathml";"$(ProjectDir)..\notifications";"$(ProjectDir)..\page";"$(ProjectDir)..\page\animation";"$(ProjectDir)..\page\win";"$(ProjectDir)..\platform";"$(ProjectDir)..\platform\animation";"$(ProjectDir)..\platform\mock";"$(ProjectDir)..\platform\sql";"$(ProjectDir)..\platform\win";"$(ProjectDir)..\platform\network";"$(ProjectDir)..\platform\network\win";"$(ProjectDir)..\platform\cf";"$(ProjectDir)..\platform\graphics";"$(ProjectDir)..\platform\graphics\filters";"$(ProjectDir)..\platform\graphics\opentype";"$(ProjectDir)..\platform\graphics\transforms";"$(ProjectDir)..\platform\text";"$(ProjectDir)..\platform\text\transcoder";"$(ProjectDir)..\platform\graphics\win";"$(ProjectDir)..\xml";"$(ConfigurationBuildDir)\obj\WebCore\DerivedSources";"$(ProjectDir)..\plugins";"$(ProjectDir)..\plugins\win";"$(ProjectDir)..\svg\animation";"$(ProjectDir)..\svg\graphics";"$(ProjectDir)..\svg\properties";"$(ProjectDir)..\svg\graphics\filters";"$(ProjectDir)..\svg";"$(ProjectDir)..\wml";"$(ProjectDir)..\storage";"$(ProjectDir)..\websockets";"$(ProjectDir)..\workers";"$(ConfigurationBuildDir)\include";"$(ConfigurationBuildDir)\include\private";"$(ConfigurationBuildDir)\include\JavaScriptCore";"$(ConfigurationBuildDir)\include\private\JavaScriptCore";"$(ProjectDir)..\ForwardingHeaders";"$(WebKitLibrariesDir)\include";"$(WebKitLibrariesDir)\include\private";"$(WebKitLibrariesDir)\include\private\JavaScriptCore";"$(WebKitLibrariesDir)\include\pthreads";"$(WebKitLibrariesDir)\include\sqlite";"$(WebKitLibrariesDir)\include\JavaScriptCore";"$(WebKitLibrariesDir)\include\zlib"" > + AdditionalIncludeDirectories=""$(ProjectDir)..";"$(ProjectDir)..\accessibility";"$(ProjectDir)..\accessibility\win";"$(ProjectDir)..\bridge";"$(ProjectDir)..\bridge\c";"$(ProjectDir)..\bridge\jsc";"$(ProjectDir)..\css";"$(ProjectDir)..\editing";"$(ProjectDir)..\fileapi";"$(ProjectDir)..\rendering";"$(ProjectDir)..\rendering\style";"$(ProjectDir)..\rendering\svg";"$(ProjectDir)..\bindings";"$(ProjectDir)..\bindings\generic";"$(ProjectDir)..\bindings\js";"$(ProjectDir)..\bindings\js\specialization";"$(ProjectDir)..\dom";"$(ProjectDir)..\dom\default";"$(ProjectDir)..\history";"$(ProjectDir)..\html";"$(ProjectDir)..\html\canvas";"$(ProjectDir)..\html\parser";"$(ProjectDir)..\html\shadow";"$(ProjectDir)..\inspector";"$(ProjectDir)..\loader";"$(ProjectDir)..\loader\appcache";"$(ProjectDir)..\loader\archive";"$(ProjectDir)..\loader\archive\cf";"$(ProjectDir)..\loader\cache";"$(ProjectDir)..\loader\icon";"$(ProjectDir)..\mathml";"$(ProjectDir)..\notifications";"$(ProjectDir)..\page";"$(ProjectDir)..\page\animation";"$(ProjectDir)..\page\win";"$(ProjectDir)..\platform";"$(ProjectDir)..\platform\animation";"$(ProjectDir)..\platform\mock";"$(ProjectDir)..\platform\sql";"$(ProjectDir)..\platform\win";"$(ProjectDir)..\platform\network";"$(ProjectDir)..\platform\network\win";"$(ProjectDir)..\platform\cf";"$(ProjectDir)..\platform\graphics";"$(ProjectDir)..\platform\graphics\ca";"$(ProjectDir)..\platform\graphics\filters";"$(ProjectDir)..\platform\graphics\opentype";"$(ProjectDir)..\platform\graphics\transforms";"$(ProjectDir)..\platform\text";"$(ProjectDir)..\platform\text\transcoder";"$(ProjectDir)..\platform\graphics\win";"$(ProjectDir)..\xml";"$(ConfigurationBuildDir)\obj\WebCore\DerivedSources";"$(ProjectDir)..\plugins";"$(ProjectDir)..\plugins\win";"$(ProjectDir)..\svg\animation";"$(ProjectDir)..\svg\graphics";"$(ProjectDir)..\svg\properties";"$(ProjectDir)..\svg\graphics\filters";"$(ProjectDir)..\svg";"$(ProjectDir)..\wml";"$(ProjectDir)..\storage";"$(ProjectDir)..\websockets";"$(ProjectDir)..\workers";"$(ConfigurationBuildDir)\include";"$(ConfigurationBuildDir)\include\private";"$(ConfigurationBuildDir)\include\JavaScriptCore";"$(ConfigurationBuildDir)\include\private\JavaScriptCore";"$(ProjectDir)..\ForwardingHeaders";"$(WebKitLibrariesDir)\include";"$(WebKitLibrariesDir)\include\private";"$(WebKitLibrariesDir)\include\private\JavaScriptCore";"$(WebKitLibrariesDir)\include\pthreads";"$(WebKitLibrariesDir)\include\sqlite";"$(WebKitLibrariesDir)\include\JavaScriptCore";"$(WebKitLibrariesDir)\include\zlib"" > PreprocessorDefinitions="__WIN32__;DISABLE_3D_RENDERING;WEBCORE_CONTEXT_MENUS" Do you really want this change? > WebCore/html/canvas/CanvasRenderingContext.h:63 > - virtual PlatformLayer* platformLayer() const { return 0; } > +// virtual PlatformLayer* platformLayer() const { return 0; } No commenting out please! > WebCore/platform/graphics/ca/PlatformCAAnimation.h:47 > +typedef struct _CACFAnimation *CACFAnimationRef; The * should be on the other side here. > WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:2 > + * Copyright (C) 2010 Apple Inc. All rights reserved. 2011! > WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:153 > +static void initialize(PlatformCAAnimation* animation) > +{ > +} Unused? > WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:188 > + RetainPtr<CFStringRef> s(AdoptCF, animation->keyPath().createCFString()); > + CACFAnimationSetKeyPath(m_animation.get(), s.get()); Please rename 's' to keyPath or similar. > WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:220 > +bool PlatformCAAnimation::supportsValueFunction() > +{ > + return true; > +} Method should be const, could be inline. > WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:300 > + RetainPtr<CFStringRef> s(AdoptCF, toCACFFillModeType(value).createCFString()); > + CACFAnimationSetFillMode(m_animation.get(), s.get()); Please rename 's'. > WebCore/platform/graphics/ca/win/PlatformCAAnimationWin.cpp:341 > + RetainPtr<CFStringRef> s(AdoptCF, toCACFValueFunctionType(value).createCFString()); > + CACFAnimationSetValueFunction(m_animation.get(), CACFValueFunctionGetFunctionWithName(s.get())); ditto > WebCore/platform/graphics/ca/win/PlatformCALayerWin.cpp:2 > + * Copyright (C) 2010 Apple Inc. All rights reserved. 2011! > WebCore/platform/graphics/ca/win/PlatformCALayerWin.cpp:47 > +bool PlatformCALayer::isValueFunctionSupported() > +{ > + return true; > +} Should be const and inline. > WebCore/platform/graphics/ca/win/PlatformCALayerWin.cpp:303 > +PassRefPtr<PlatformCAAnimation> PlatformCALayer::animationForKey(const String& key) Method should be const. > WebCore/platform/graphics/ca/win/PlatformCALayerWin.cpp:460 > + return const_cast<void*>(CACFLayerGetContents(m_layer.get())); Is that the right cast? > WebCore/platform/graphics/ca/win/PlatformCALayerWinInternal.cpp:2 > + * Copyright (C) 2010 Apple Inc. All rights reserved. 2011! > WebCore/platform/graphics/ca/win/PlatformCALayerWinInternal.cpp:63 > + if (!owner() || !owner()->owner()) owner()->owner() is weird. > WebCore/platform/graphics/ca/win/PlatformCALayerWinInternal.h:2 > + * Copyright (C) 2010 Apple Inc. All rights reserved. 2011 > WebCore/platform/graphics/win/GraphicsContextCGWin.cpp:48 > + if (info.bmBitsPixel != 32) > + return 0; Why this change? > WebCore/platform/graphics/win/MediaPlayerPrivateQuickTimeVisualContext.cpp:104 > + virtual void platformCALayerLayoutSublayersOfLayer(PlatformCALayer*); > + virtual bool platformCALayerRespondsToLayoutChanges() const { return true; } > + > + virtual void platformCALayerAnimationStarted(CFTimeInterval beginTime) { } > + virtual GraphicsLayer::CompositingCoordinatesOrientation platformCALayerContentsOrientation() const { return GraphicsLayer::CompositingCoordinatesBottomUp; } > + virtual void platformCALayerPaintContents(GraphicsContext&, const IntRect& inClip) { } > + virtual bool platformCALayerShowDebugBorders() const { return false; } > + virtual bool platformCALayerShowRepaintCounter() const { return false; } > + virtual int platformCALayerIncrementRepaintCount() { return 0; } > + > + virtual bool platformCALayerContentsOpaque() const { return false; } > + virtual bool platformCALayerDrawsContent() const { return false; } > + virtual void platformCALayerLayerDidDisplay(PlatformLayer*) { } Make these private if possible. > WebCore/platform/graphics/win/WKCACFLayerRenderer.cpp:481 > + if (!wkCACFContextBeginUpdate(m_context, space, sizeof(space), CACurrentMediaTime(), bounds, windowDirtyRects.data(), windowDirtyRects.size())) Share the same CACurrentMediaTime you used above? > WebKit/win/FullscreenVideoController.cpp:193 > + virtual void platformCALayerLayoutSublayersOfLayer(PlatformCALayer*); > + virtual bool platformCALayerRespondsToLayoutChanges() const { return true; } > + > + virtual void platformCALayerAnimationStarted(CFTimeInterval beginTime) { } > + virtual GraphicsLayer::CompositingCoordinatesOrientation platformCALayerContentsOrientation() const { return GraphicsLayer::CompositingCoordinatesBottomUp; } > + virtual void platformCALayerPaintContents(GraphicsContext&, const IntRect& inClip) { } > + virtual bool platformCALayerShowDebugBorders() const { return false; } > + virtual bool platformCALayerShowRepaintCounter() const { return false; } > + virtual int platformCALayerIncrementRepaintCount() { return 0; } > + > + virtual bool platformCALayerContentsOpaque() const { return false; } > + virtual bool platformCALayerDrawsContent() const { return false; } > + virtual void platformCALayerLayerDidDisplay(PlatformLayer*) { } Make private? The Windows build has been failing since this was checked in. On example <http://build.webkit.org/builders/Windows%20Release%20%28Build%29/builds/10209/steps/compile-webkit/logs/stdio>. MediaPlayerPrivateQuickTimeVisualContext.cpp ..\platform\graphics\win\MediaPlayerPrivateQuickTimeVisualContext.cpp(1193) : error C2027: use of undefined type '_CACFLayer' c:\cygwin\home\buildbot\slave\win-release\build\WebCore\platform\graphics\GraphicsLayer.h(56) : see declaration of '_CACFLayer' ..\platform\graphics\win\MediaPlayerPrivateQuickTimeVisualContext.cpp(1193) : error C2227: left of '->setLayoutClient' must point to class/struct/union/generic type ..\platform\graphics\win\MediaPlayerPrivateQuickTimeVisualContext.cpp(1212) : error C2027: use of undefined type '_CACFLayer' c:\cygwin\home\buildbot\slave\win-release\build\WebCore\platform\graphics\GraphicsLayer.h(56) : see declaration of '_CACFLayer' ..\platform\graphics\win\MediaPlayerPrivateQuickTimeVisualContext.cpp(1212) : error C2227: left of '->addSublayer' must point to class/struct/union/generic type ..\platform\graphics\win\MediaPlayerPrivateQuickTimeVisualContext.cpp(1213) : error C2027: use of undefined type '_CACFLayer' c:\cygwin\home\buildbot\slave\win-release\build\WebCore\platform\graphics\GraphicsLayer.h(56) : see declaration of '_CACFLayer' ..\platform\graphics\win\MediaPlayerPrivateQuickTimeVisualContext.cpp(1213) : error C2227: left of '->setNeedsLayout' must point to class/struct/union/generic type MediaPlayerPrivateQuickTimeWin.cpp ..\platform\graphics\win\MediaPlayerPrivateQuickTimeWin.cpp(763) : error C2027: use of undefined type '_CACFLayer' c:\cygwin\home\buildbot\slave\win-release\build\WebCore\platform\graphics\GraphicsLayer.h(56) : see declaration of '_CACFLayer' ..\platform\graphics\win\MediaPlayerPrivateQuickTimeWin.cpp(763) : error C2227: left of '->setNeedsDisplay' must point to class/struct/union/generic type GraphicsLayerCACF.cpp ..\platform\graphics\win\GraphicsLayerCACF.cpp(375) : error C2679: binary '=' : no operator found which takes a right-hand operand of type 'PlatformLayer *' (or there is no acceptable conversion) c:\cygwin\home\buildbot\slave\win-release\build\webkitbuild\release\include\private\javascriptcore\RefPtr.h(73): could be 'WTF::RefPtr<T> &WTF::RefPtr<T>::operator =(const WTF::RefPtr<T> &)' with [ T=WebCore::WKCACFLayer ] c:\cygwin\home\buildbot\slave\win-release\build\webkitbuild\release\include\private\javascriptcore\RefPtr.h(74): or 'WTF::RefPtr<T> &WTF::RefPtr<T>::operator =(T *)' with [ T=WebCore::WKCACFLayer ] c:\cygwin\home\buildbot\slave\win-release\build\webkitbuild\release\include\private\javascriptcore\RefPtr.h(75): or 'WTF::RefPtr<T> &WTF::RefPtr<T>::operator =(const WTF::PassRefPtr<T> &)' with [ T=WebCore::WKCACFLayer ] c:\cygwin\home\buildbot\slave\win-release\build\webkitbuild\release\include\private\javascriptcore\RefPtr.h(76): or 'WTF::RefPtr<T> &WTF::RefPtr<T>::operator =(const WTF::NonNullPassRefPtr<T> &)' with [ T=WebCore::WKCACFLayer ] c:\cygwin\home\buildbot\slave\win-release\build\webkitbuild\release\include\private\javascriptcore\RefPtr.h(78): or 'WTF::RefPtr<T> &WTF::RefPtr<T>::operator =(std::nullptr_t)' with [ T=WebCore::WKCACFLayer ] while trying to match the argument list '(WTF::RefPtr<T>, PlatformLayer *)' with [ T=WebCore::WKCACFLayer ] ..\platform\graphics\win\GraphicsLayerCACF.cpp(385) : error C2556: 'PlatformLayer *WebCore::GraphicsLayerCACF::hostLayerForSublayers(void) const' : overloaded function differs only by return type from 'WebCore::WKCACFLayer *WebCore::GraphicsLayerCACF::hostLayerForSublayers(void) const' c:\cygwin\home\buildbot\slave\win-release\build\webcore\platform\graphics\win\GraphicsLayerCACF.h(93) : see declaration of 'WebCore::GraphicsLayerCACF::hostLayerForSublayers' ..\platform\graphics\win\GraphicsLayerCACF.cpp(385) : error C2371: 'WebCore::GraphicsLayerCACF::hostLayerForSublayers' : redefinition; different basic types c:\cygwin\home\buildbot\slave\win-release\build\webcore\platform\graphics\win\GraphicsLayerCACF.h(93) : see declaration of 'WebCore::GraphicsLayerCACF::hostLayerForSublayers' ..\platform\graphics\win\GraphicsLayerCACF.cpp(390) : error C2556: 'PlatformLayer *WebCore::GraphicsLayerCACF::layerForSuperlayer(void) const' : overloaded function differs only by return type from 'WebCore::WKCACFLayer *WebCore::GraphicsLayerCACF::layerForSuperlayer(void) const' c:\cygwin\home\buildbot\slave\win-release\build\webcore\platform\graphics\win\GraphicsLayerCACF.h(94) : see declaration of 'WebCore::GraphicsLayerCACF::layerForSuperlayer' ..\platform\graphics\win\GraphicsLayerCACF.cpp(390) : error C2371: 'WebCore::GraphicsLayerCACF::layerForSuperlayer' : redefinition; different basic types c:\cygwin\home\buildbot\slave\win-release\build\webcore\platform\graphics\win\GraphicsLayerCACF.h(94) : see declaration of 'WebCore::GraphicsLayerCACF::layerForSuperlayer' ..\platform\graphics\win\GraphicsLayerCACF.cpp(396) : error C2440: 'return' : cannot convert from 'WebCore::WKCACFLayer *' to 'PlatformLayer *' Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast ..\platform\graphics\win\GraphicsLayerCACF.cpp(493) : error C2264: 'WebCore::GraphicsLayerCACF::layerForSuperlayer' : error in function definition or declaration; function not called I wanted to fix this, but I could not figure out exactly how. Thanks for trying Darin. I've been trying to get the Windows side of this checked in for the last several hours. I am almost there, but am now stuck because DRT is crashing on every test, even those having nothing to do with compositing. I don't understand the errors being spewed so I will try again tomorrow. It probably just needs a clean rebuild. For now I am back the change out. Sorry for the delay... Backed out latest change in http://trac.webkit.org/changeset/75227. Will fix the Windows side and then resubmit. Comment on attachment 78200 [details] Final Patch View in context: https://bugs.webkit.org/attachment.cgi?id=78200&action=review You should make sure that the new CACFAnimation stuff you use is all present in WebKitSupportLibrary. > WebCore/platform/graphics/win/WKCACFLayerRenderer.cpp:538 > + // If timeToNextRender is not infinity, it means animations are running, so queue up to render again > + if (timeToNextRender != numeric_limits<CFTimeInterval>::infinity()) > + renderSoon(); It seems wasteful always to render again immediately rather than waiting until timeToNextRender has elapsed. Created attachment 78251 [details]
Final Patch
Comment on attachment 78251 [details]
Final Patch
rs=me
Final patch landed in http://trac.webkit.org/changeset/75262 (In reply to comment #68) > Final patch landed in http://trac.webkit.org/changeset/75262 Broke Chromium compile (forgot an include?): http://build.webkit.org/builders/Chromium%20Linux%20Release/builds/20309/steps/compile-webkit/logs/stdio http://trac.webkit.org/changeset/75262 might have broken Windows Release (Build), Windows Debug (Build), and Chromium Linux Release I no longer see any build breakage related to these patches. Closing |