Bug 117074

Summary: [CSS Regions] REGRESSION Incorrect layer clipping inside flow thread
Product: WebKit Reporter: Andrei Bucur <abucur>
Component: WebCore Misc.Assignee: Mihnea Ovidenie <mihnea>
Status: RESOLVED FIXED    
Severity: Normal CC: achicu, cdumez, commit-queue, esprehn+autocc, glenn, gyuyoung.kim, hyatt, mihnea, rakuco, WebkitBugTracker
Priority: P2 Keywords: AdobeTracked
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 117295    
Bug Blocks: 57312, 84900    
Attachments:
Description Flags
Test case
none
Patch
none
Patch for landing none

Description Andrei Bucur 2013-05-31 07:07:00 PDT
Created attachment 203444 [details]
Test case

fast/regions/overflow-size-change-with-stacking-context.html is failing for some time.
Comment 1 Mihnea Ovidenie 2013-06-02 09:57:00 PDT
It fails from https://bugs.webkit.org/show_bug.cgi?id=76486 because the clip rects get transformed in RenderView coordinates - through associated render regions - while the layer bounds are still in flow thread coordinates (the flow thread is the root layer). Because of that, the intersection between the layer bounds and the clip rects fails and the layer is not displayed.
Comment 2 Mihnea Ovidenie 2013-06-04 07:53:40 PDT
Created attachment 203697 [details]
Patch
Comment 3 Dave Hyatt 2013-06-04 12:26:35 PDT
Comment on attachment 203697 [details]
Patch

r=me
Comment 4 Mihnea Ovidenie 2013-06-05 01:02:51 PDT
Created attachment 203765 [details]
Patch for landing
Comment 5 WebKit Commit Bot 2013-06-05 01:19:47 PDT
Comment on attachment 203765 [details]
Patch for landing

Clearing flags on attachment: 203765

Committed r151202: <http://trac.webkit.org/changeset/151202>
Comment 6 WebKit Commit Bot 2013-06-05 01:19:51 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Alexandru Chiculita 2013-06-05 15:32:45 PDT
(In reply to comment #6)
> All reviewed patches have been landed.  Closing bug.

It's better if you just moved this functionality to
RenderFlowThread::mapLocalToContainer. 

That's for two reasons: 
1) the RenderFlowThread check will only be hit when there's a RenderFlowThread object
on the chain. 
2) other non-RenderBox objects will benefit from that change.

void RenderFlowThread::mapLocalToContainer(const RenderLayerModelObject*
repaintContainer, TransformState& transformState, MapCoordinatesFlags mode,
bool* wasFixed) const
{
    if (repaintContainer == this)
        return;

    // Transform from render flow coordinates into region coordinates.
    if (RenderRegion* region = mapFromFlowToRegion(transformState))
       
static_cast<RenderObject*>(region)->mapLocalToContainer(region->containerForRepaint(),
transformState, mode, wasFixed);
}
Comment 8 Mihnea Ovidenie 2013-06-06 06:12:05 PDT
(In reply to comment #7)
> (In reply to comment #6)
> > All reviewed patches have been landed.  Closing bug.
> 
> It's better if you just moved this functionality to
> RenderFlowThread::mapLocalToContainer. 
> 
> That's for two reasons: 
> 1) the RenderFlowThread check will only be hit when there's a RenderFlowThread object
> on the chain. 
> 2) other non-RenderBox objects will benefit from that change.
> 
> void RenderFlowThread::mapLocalToContainer(const RenderLayerModelObject*
> repaintContainer, TransformState& transformState, MapCoordinatesFlags mode,
> bool* wasFixed) const
> {
>     if (repaintContainer == this)
>         return;
> 
>     // Transform from render flow coordinates into region coordinates.
>     if (RenderRegion* region = mapFromFlowToRegion(transformState))
> 
> static_cast<RenderObject*>(region)->mapLocalToContainer(region->containerForRepaint(),
> transformState, mode, wasFixed);
> }

Ok, i will fix in https://bugs.webkit.org/show_bug.cgi?id=117290
Comment 9 Mihai Maerean 2013-06-06 08:20:59 PDT
> void RenderFlowThread::mapLocalToContainer(const RenderLayerModelObject* repaintContainer, TransformState& transformState, MapCoordinatesFlags mode, bool* wasFixed) const
> if (RenderRegion* region = mapFromFlowToRegion(transformState))
> static_cast<RenderObject*>(region)->mapLocalToContainer(region->containerForRepaint(),  transformState, mode, wasFixed);

You need to either make RenderRegion::mapLocalToContainer public or make RenderFlowThread a friend class RenderRegion in order to call mapLocalToContainer from RenderFlowThread (instead of using static_cast to RenderObject).