Summary: | [chromium] LayerRendererChromium shouldn't know anything about CCLayerImpl | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | James Robinson <jamesr> | ||||||||||||
Component: | New Bugs | Assignee: | James Robinson <jamesr> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | abarth, cc-bugs, dglazkov, enne, fishd, shawnsingh, tkent, webkit.review.bot | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
James Robinson
2012-04-06 18:39:47 PDT
Created attachment 136118 [details]
Patch
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI. (note: this currently fails some webkit_unit_tests. I'm not sure why yet, but I want to check that my EWS changes to run unit tests work so I'ma let this patch fail). Comment on attachment 136118 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136118&action=review > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:-398 > - m_layerRenderer->beginDrawingFrame(); whoops, this call was kind of important! will readd, but i want to see this fail EWS first Comment on attachment 136118 [details] Patch Attachment 136118 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12371156 New failing tests: compositing/checkerboard.html compositing/fixed-position-changed-within-composited-parent-layer.html animations/3d/matrix-transform-type-animation.html animations/3d/state-at-end-event-transform.html compositing/direct-image-compositing.html compositing/video-page-visibility.html animations/additive-transform-animations.html animations/3d/replace-filling-transform.html compositing/scrollbar-painting.html compositing/backface-visibility.html CCLayerTreeHostImplTest.reshapeNotCalledUntilDraw compositing/text-on-large-layer.html compositing/layers-inside-overflow-scroll.html compositing/animation/state-at-end-event-transform-layer.html compositing/sibling-positioning.html compositing/generated-content.html compositing/fixed-position-changed-in-composited-layer.html compositing/self-painting-layers.html compositing/culling/filter-occlusion-alpha-large.html animations/3d/change-transform-in-end-event.html compositing/absolute-position-changed-with-composited-parent-layer.html compositing/absolute-position-changed-in-composited-layer.html compositing/flat-with-transformed-child.html accessibility/aria-disabled.html compositing/backface-visibility-hierarchical-transform.html compositing/culling/filter-occlusion-alpha.html compositing/culling/clear-fixed-iframe.html compositing/compositing-visible-descendant.html compositing/culling/filter-occlusion-blur-large.html CCLayerTreeHostTestAtomicCommitWithPartialUpdate.runMultiThread compositing/color-matching/image-color-matching.html CCLayerTreeHostTestAtomicCommit.runMultiThread compositing/color-matching/pdf-image-match.html Created attachment 136208 [details]
Archive of layout-test-results from ec2-cr-linux-03
The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-03 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Created attachment 136493 [details]
Patch
Comment on attachment 136493 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136493&action=review > Source/Platform/ChangeLog:6 > + Remove unused compositeOffscreen setting. ...how is this change related? > Source/WebCore/ChangeLog:10 > + LayerRendererChromium shouldn't reach back up to CCLayerImpl. This gets rid of all CCLayerImpl references in > + LayerRendererChromium except for one which is being fixed in https://bugs.webkit.org/show_bug.cgi?id=83287. This patch has already landed. > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:402 > + m_layerRenderer->beginDrawingFrame(rootLayer()->renderSurface()); > + > + // FIXME: use the frame begin time from the overall compositor scheduler. > + // This value is currently inaccessible because it is up in Chromium's > + // RenderWidget. > + m_headsUpDisplay->onFrameBegin(currentTime()); I think this should go before beginDrawingFrame. (In reply to comment #8) > (From update of attachment 136493 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=136493&action=review > > > Source/Platform/ChangeLog:6 > > + Remove unused compositeOffscreen setting. > > ...how is this change related? > The compositeOffscreen touches the root layer in a few places in LayerRendererChromium. Keeping it working would require plumbing lots of data down to LRC. Since we've never used this path and don't plan to ever use it, seemed better to just remove it. I could rip this out as a separate prerequisite patch to this one. That's probably a good idea anyway. > > Source/WebCore/ChangeLog:10 > > + LayerRendererChromium shouldn't reach back up to CCLayerImpl. This gets rid of all CCLayerImpl references in > > + LayerRendererChromium except for one which is being fixed in https://bugs.webkit.org/show_bug.cgi?id=83287. > > This patch has already landed. Good point! Will update. > > > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:402 > > + m_layerRenderer->beginDrawingFrame(rootLayer()->renderSurface()); > > + > > + // FIXME: use the frame begin time from the overall compositor scheduler. > > + // This value is currently inaccessible because it is up in Chromium's > > + // RenderWidget. > > + m_headsUpDisplay->onFrameBegin(currentTime()); > > I think this should go before beginDrawingFrame. Yep, probably should. Comment on attachment 136493 [details] Patch I'd missed some of the compositeOffscreen support code, moved that patch (now patch series) out into https://bugs.webkit.org/show_bug.cgi?id=83733. I'll refresh this after that lands. Created attachment 137166 [details]
Patch
Shawn - this has the ownership changes for CCFontAtlas that I was mentioning on the other bug. Comment on attachment 137166 [details] Patch Attachment 137166 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12400204 Oops, I accidentally too many #includes. Will fix. (In reply to comment #12) > Shawn - this has the ownership changes for CCFontAtlas that I was mentioning on the other bug. So I took a look. That's exactly what I had in mind when we discussed before, seems correct to me. Sorry to have made you do this extra work, I should have waited for this to land before landing my other patch. =) (In reply to comment #15) > Sorry to have made you do this extra work, I should have waited for this to land before landing my other patch. =) Don't worry about it - this patch had other dependencies that I had to take care of first, so you waiting wouldn't really have made anything easier. Created attachment 137187 [details]
add PlatformCanvas.h include back to CCHeadsUpDisplay.cpp
Comment on attachment 137187 [details]
add PlatformCanvas.h include back to CCHeadsUpDisplay.cpp
R=me. Nice!
Comment on attachment 137187 [details] add PlatformCanvas.h include back to CCHeadsUpDisplay.cpp Clearing flags on attachment: 137187 Committed r114218: <http://trac.webkit.org/changeset/114218> All reviewed patches have been landed. Closing bug. |