Bug 84900

Summary: [CSS Regions] Enable accelerated layers on elements inside a RenderFlowThread
Product: WebKit Reporter: Alexandru Chiculita <achicu>
Component: Layout and RenderingAssignee: Mihnea Ovidenie <mihnea>
Status: ASSIGNED ---    
Severity: Normal CC: abucur, buildbot, commit-queue, donggwan.kim, dw.im, esprehn+autocc, glenn, heejin.r.chung, hyatt, jchaffraix, mihnea, mmaerean, rniwa, s.choi, simon.fraser, sudarshan.p, WebkitBugTracker
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 117409, 123142, 126755, 116242, 117074, 117270, 117365, 120457, 120760, 123143, 123193, 124042, 124887, 124978, 125144, 128218, 128527, 129371, 131485    
Bug Blocks: 57312    
Attachments:
Description Flags
First attempt
none
Patch V2
none
Patch V3
none
Patch V4
none
Patch V5
none
Patch V6
none
Patch V7
none
Patch V8
none
WIP Patch
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2
none
Patch
buildbot: commit-queue-
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2
none
Archive of layout-test-results from APPLE-EWS-5 for win-future
none
CSSRegionsAcceleratedCompositingTests.zip none

Description Alexandru Chiculita 2012-04-25 15:58:15 PDT
The accelerated layers are not working correctly when enabled on elements that draw inside a RenderFlowThread, so they were disabled. This bug should remove that limitation.
Comment 1 Mihnea Ovidenie 2012-09-22 02:45:42 PDT
I have added a new test for this issue in compositing/regions/webkit-flow-renderer-layer-compositing.html. Right now, the test is skipped but it should be unskipped once the code is fixed.
Comment 2 Andrei Bucur 2012-12-06 05:04:18 PST
The test seems to pass now on some platforms (maybe all?)
http://trac.webkit.org/changeset/136230
http://trac.webkit.org/changeset/136489

I don't think this has been fixed yet so we should investigate what happened.
Comment 3 Dongwoo Joshua Im (dwim) 2012-12-10 05:10:11 PST
(In reply to comment #2)
> The test seems to pass now on some platforms (maybe all?)
> http://trac.webkit.org/changeset/136230
> http://trac.webkit.org/changeset/136489
> 
> I don't think this has been fixed yet so we should investigate what happened.

Dear abucur,

If you get something about this,
please let us know.

Thanks.
Comment 4 Alexandru Chiculita 2012-12-10 10:46:27 PST
(In reply to comment #3)
> (In reply to comment #2)
> > The test seems to pass now on some platforms (maybe all?)
> > http://trac.webkit.org/changeset/136230
> > http://trac.webkit.org/changeset/136489
> > 
> > I don't think this has been fixed yet so we should investigate what happened.
> 
> Dear abucur,
> 
> If you get something about this,
> please let us know.
> 
> Thanks.

It's not fixed yet. We had composited layers disabled for a while for RenderFlowThread children, but there was a different bug that made the test fail before. 

https://bugs.webkit.org/show_bug.cgi?id=102839

I fixed that bug, but I didn't re-enable that test. I think we need a different way to to test for it. For example we could use real 3D transforms that cannot be rendered in software.
Comment 5 Ryosuke Niwa 2013-01-03 13:35:19 PST
Removed the Mac test expectation in http://trac.webkit.org/changeset/138734 since the test has been passing.
Comment 6 Soo-Hyun Choi 2013-01-09 02:16:20 PST
I just wonder how this bug is going on right now. Any update or progress?
Comment 7 Alexandru Chiculita 2013-01-17 09:37:59 PST
(In reply to comment #6)
> I just wonder how this bug is going on right now. Any update or progress?

We are now working on stabilizing CSS Regions and implementing this feature requires a quite a bit of changes in the RenderLayer code. We would have to somehow collect the layers from the RenderFlowThread into the layers of the regions.
Comment 8 Alexandru Chiculita 2013-05-20 13:48:00 PDT
Created attachment 202317 [details]
First attempt
Comment 9 Alexandru Chiculita 2013-05-21 11:13:40 PDT
Created attachment 202452 [details]
Patch V2

Avoided to use region ranges as they are too fragile for now (ie. do not work for inline boxes and most of the non-blocks). Also, RenderBlock::offsetFromLogicalTopOfFirstPage doesn't seem to compute the right position in the flow.

This patch uses the layers to figure out their first region.
Comment 10 Alexandru Chiculita 2013-05-21 13:15:10 PDT
Created attachment 202462 [details]
Patch V3

Disabled viewport clipping on elements inside the flow-thread.
Comment 11 Alexandru Chiculita 2013-05-30 16:26:33 PDT
Created attachment 203394 [details]
Patch V4

Added support for region based multi-columns.
Comment 12 Alexandru Chiculita 2013-05-31 14:47:46 PDT
Created attachment 203469 [details]
Patch V5

Regions are now composited only when they contain a composited layer. The overlap map is most probably still wrong at checking elements inside regions with elements behind, but should work with other elements in the same region.
Comment 13 Alexandru Chiculita 2013-06-04 11:24:43 PDT
Created attachment 203713 [details]
Patch V6

Computed the layer repartition only once after the RenderFlowThread::layout is done. Preparation work to avoid making all the regions have layers.
Comment 14 Alexandru Chiculita 2013-06-04 17:19:14 PDT
Created attachment 203742 [details]
Patch V7

Added code to only promote make layers for Regions that contain layers from the flow thread. Also, removed the stacking context requirement on the RenderRegions. RenderRegions should now be part of the normal flow.
Comment 15 Alexandru Chiculita 2013-06-05 12:27:07 PDT
Created attachment 203872 [details]
Patch V8

Removed unnecessary call to update the layers.
Comment 16 Sudarshan 2013-07-03 02:13:24 PDT
Created attachment 205983 [details]
WIP Patch
Comment 17 Sudarshan 2013-07-03 02:18:59 PDT
Comment on attachment 205983 [details]
WIP Patch

Alternate approach for enabling compositing for regions. Right now requires RenderLayer::collectLayers not to skip RenderFlowThreads (disabled by 117270). Would like to know if there is any approach available for fixing 117270?
Comment 18 Mihnea Ovidenie 2013-07-03 02:27:22 PDT
(In reply to comment #17)
> (From update of attachment 205983 [details])
> Alternate approach for enabling compositing for regions. Right now requires RenderLayer::collectLayers not to skip RenderFlowThreads (disabled by 117270). Would like to know if there is any approach available for fixing 117270?

Hi,

Alex is working on this as you can see from the posted patches and blocking bugs. If you would like to contribute to this effort, i suggest you should sync with him. It is difficult to comment on your patch as it does not describe:
* the general approach
* why your approach is better than the one proposed by Alex
* it contains no tests
Comment 19 Build Bot 2013-07-03 13:45:52 PDT
Comment on attachment 205983 [details]
WIP Patch

Attachment 205983 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1028024

New failing tests:
fast/multicol/mixed-opacity-fixed-test.html
fast/multicol/mixed-positioning-stacking-order.html
Comment 20 Build Bot 2013-07-03 13:45:55 PDT
Created attachment 206022 [details]
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-09  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3
Comment 21 Sudarshan 2013-07-04 07:06:16 PDT
(In reply to comment #18)
> (In reply to comment #17)
> > (From update of attachment 205983 [details] [details])
> > Alternate approach for enabling compositing for regions. Right now requires RenderLayer::collectLayers not to skip RenderFlowThreads (disabled by 117270). Would like to know if there is any approach available for fixing 117270?
> 
> Hi,
> 
> Alex is working on this as you can see from the posted patches and blocking bugs. If you would like to contribute to this effort, i suggest you should sync with him. 
It is difficult to comment on your patch as it does not describe:
> * the general approach
> * why your approach is better than the one proposed by Alex
> * it contains no tests

Before submitting the patch I had conversed with Alex over mail. He was interested in seeing our approach. 

The general approach is to adjust the geometry of composited layers inside a flow thread to reflect their actual positions inside regions. This takes care of the composited layers getting drawn at the right positions.

When we discussed about the current proposed patch by Alex, he said it would only handle the first level Renderlayers would be collected. Our approach does not have this limitation.

I will add a test for multi level composited RenderLayers...
Comment 22 Sudarshan 2013-07-04 07:20:56 PDT
Created attachment 206085 [details]
Patch
Comment 23 Sudarshan 2013-07-04 07:23:47 PDT
Comment on attachment 206085 [details]
Patch

WIP patch, added test cases for heierarchical renderlayers being composited in a flow thread
Comment 24 Build Bot 2013-07-04 09:09:05 PDT
Comment on attachment 206085 [details]
Patch

Attachment 206085 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/945899

New failing tests:
fast/multicol/mixed-opacity-fixed-test.html
fast/multicol/mixed-positioning-stacking-order.html
Comment 25 Build Bot 2013-07-04 09:09:09 PDT
Created attachment 206095 [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.3
Comment 26 Build Bot 2013-07-04 16:26:47 PDT
Comment on attachment 205983 [details]
WIP Patch

Attachment 205983 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/1032110

New failing tests:
fast/forms/select/popup-closes-on-blur.html
Comment 27 Build Bot 2013-07-04 16:26:52 PDT
Created attachment 206115 [details]
Archive of layout-test-results from APPLE-EWS-5 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: APPLE-EWS-5  Port: win-future  Platform: CYGWIN_NT-6.1-WOW64-1.7.20-0.266-5-3-i686-32bit
Comment 28 Sudarshan 2013-07-07 23:33:47 PDT
Hi Alex, I would be great if you can give your opinion on the alternative approach. We had worked on this approach in a short time. So if you see some potential, than I will continue working on it. Thanks!
Comment 29 Alexandru Chiculita 2013-07-08 04:06:51 PDT
(In reply to comment #28)
> Hi Alex, I would be great if you can give your opinion on the alternative approach. We had worked on this approach in a short time. So if you see some potential, than I will continue working on it. Thanks!

Hi, thanks for your patch! I'm doing something very similar for the layers inside the regions based multi column implementation. However, for CSS Regions I would rather take my approach instead. Your patch does not take into account the transforms of the regions correctly and it will overlay the content of the regions on top of every other layer in the DOM.

My approach moves the graphics layers from the RenderFlowThread to the RenderRegions and are correctly displayed in the stacking context created by the Region. Also, the transforms and effects applied on the regions are correctly applied to the content.
Comment 30 Build Bot 2013-07-14 14:55:57 PDT
Comment on attachment 206085 [details]
Patch

Attachment 206085 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/1064678
Comment 31 Mihai Maerean 2013-07-23 08:58:25 PDT
Created attachment 207330 [details]
CSSRegionsAcceleratedCompositingTests.zip