Summary: | [CSS Regions] Fix painting when the composited region has overflow:hidden | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mihai Maerean <mmaerean> | ||||||||||||||||||||
Component: | WebCore Misc. | Assignee: | Mihai Maerean <mmaerean> | ||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||
Severity: | Normal | CC: | achicu, buildbot, commit-queue, esprehn+autocc, glenn, kondapallykalyan, mihnea, rniwa, simon.fraser, WebkitBugTracker | ||||||||||||||||||||
Priority: | P2 | Keywords: | AdobeTracked | ||||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||
Bug Depends on: | |||||||||||||||||||||||
Bug Blocks: | 84900 | ||||||||||||||||||||||
Attachments: |
|
Description
Mihai Maerean
2013-11-26 06:16:34 PST
Created attachment 217875 [details]
layout test
Created attachment 217876 [details]
layout test expected result
Created attachment 218410 [details]
patch
Comment on attachment 218410 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=218410&action=review > LayoutTests/compositing/regions/paint-inside-composited-region-overflow-hidden-versus-div-expected.html:30 > + <div class="content">a b c 1. a b c 2. a b c 3. a b c 4. a b c 5. a b c 6. a b c 7. a b c 8. a b c 9. a b c A. a b c B. a b c C. a b c D. a b c E. a b c F. a b c 0. a b c 1. a b c 2. a b c 3. </div> Is there any reason for using the "a b c" sequences? I think it makes it hard to follow. Just use plain text. > Source/WebCore/rendering/RenderLayer.cpp:6768 > + regionClipRect = backgroundClipRect(clipRectsContext); // Region coordinates. I think we need to be more explicit about explaining the difference between region / fragment / fragment container. > Source/WebCore/rendering/RenderLayer.cpp:6778 > regionClipRect = regionContentBox; I think that this whole function needs some rework. For example, this extra step will overwrite this line from above "regionClipRect = dirtyRect". > Source/WebCore/rendering/RenderLayer.cpp:6782 > + // When the layer of the region is composited, the region receives a GraphicsLayer of its own > + // so the clipping coordinates (caused by overflow:hidden) must be relative to the > + // GraphicsLayer coordinates in which the region gets painted. > + if (!backing()) { The comment is confusing, especially because you have a !backing() just after it. I think the term "region" is now confusing as you have a "fragmentContainer" and a "fragment". Which one are you refering to as a "region". Also, there is a phase where all the layers are flatten even though they have a backing. It's used to draw the "drag" image when you drag a portion of the page. That's why I don't think it's a good idea to check just for !backing(). > Source/WebCore/rendering/RenderLayer.cpp:6785 > + regionClipRect.moveBy(regionOffsetFromRoot); // Screen coordinates. I don't understand how this could be "screen" coordinates. Comment on attachment 218410 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=218410&action=review >> Source/WebCore/rendering/RenderLayer.cpp:6782 >> + if (!backing()) { > Also, there is a phase where all the layers are flatten even though they have a backing. It's used to draw the "drag" image when you drag a portion of the page. That's why I don't think it's a good idea to check just for !backing(). What else to check for? Shouldn't this issue (about dragging of a page portion) be tracked as a different bug? Created attachment 218516 [details]
patch. incorporates the feedback.
Created attachment 220848 [details] patch. incorporates the feedback. The dragged selected text that is composited in a region is not being rendered in Nightly at the moment. I have added https://bugs.webkit.org/show_bug.cgi?id=126755 to track this bug. Created attachment 220849 [details] patch. incorporates the feedback. The dragged selected text that is composited in a region is not being rendered in Nightly at the moment. I have added https://bugs.webkit.org/show_bug.cgi?id=126755 to track this bug. Comment on attachment 220849 [details] patch. incorporates the feedback. Attachment 220849 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6505188202381312 New failing tests: fast/regions/overflow-first-and-last-regions-in-container-hidden.html fast/regions/overflow-scrollable-1.html fast/regions/overflow-scrollable-3.html fast/regions/shape-inside/shape-inside-on-first-region-inline-content.html fast/regions/overflow-scrollable-fit.html fast/regions/shape-inside/shape-inside-on-first-region-block-content.html fast/regions/overflow-scrollable-varying-width-1.html Created attachment 220887 [details]
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-11 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Created attachment 221145 [details]
patch. incorporates the feedback.
please review Comment on attachment 221145 [details] patch. incorporates the feedback. View in context: https://bugs.webkit.org/attachment.cgi?id=221145&action=review > LayoutTests/compositing/regions/paint-inside-composited-region-overflow-hidden-versus-div-expected.html:4 > + <title>Expected Result - Bug #124887: [CSS Regions] Fix painting when the composited region has overflow:hidden</title> nit: Use 4 sapces instead of tabs. r=me Created attachment 221332 [details]
patch for landing
Comment on attachment 221332 [details] patch for landing Clearing flags on attachment: 221332 Committed r162115: <http://trac.webkit.org/changeset/162115> |