Bug 84900 - [CSS Regions] Enable accelerated layers on elements inside a RenderFlowThread
Summary: [CSS Regions] Enable accelerated layers on elements inside a RenderFlowThread
Status: ASSIGNED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mihnea Ovidenie
URL:
Keywords:
Depends on: 117409 123142 126755 116242 117074 117270 117365 120457 120760 123143 123193 124042 124887 124978 125144 128218 128527 129371 131485
Blocks: 57312
  Show dependency treegraph
 
Reported: 2012-04-25 15:58 PDT by Alexandru Chiculita
Modified: 2014-04-10 01:43 PDT (History)
17 users (show)

See Also:


Attachments
First attempt (18.94 KB, patch)
2013-05-20 13:48 PDT, Alexandru Chiculita
no flags Details | Formatted Diff | Diff
Patch V2 (19.00 KB, patch)
2013-05-21 11:13 PDT, Alexandru Chiculita
no flags Details | Formatted Diff | Diff
Patch V3 (19.76 KB, patch)
2013-05-21 13:15 PDT, Alexandru Chiculita
no flags Details | Formatted Diff | Diff
Patch V4 (26.51 KB, patch)
2013-05-30 16:26 PDT, Alexandru Chiculita
no flags Details | Formatted Diff | Diff
Patch V5 (35.67 KB, patch)
2013-05-31 14:47 PDT, Alexandru Chiculita
no flags Details | Formatted Diff | Diff
Patch V6 (36.77 KB, patch)
2013-06-04 11:24 PDT, Alexandru Chiculita
no flags Details | Formatted Diff | Diff
Patch V7 (44.84 KB, patch)
2013-06-04 17:19 PDT, Alexandru Chiculita
no flags Details | Formatted Diff | Diff
Patch V8 (44.05 KB, patch)
2013-06-05 12:27 PDT, Alexandru Chiculita
no flags Details | Formatted Diff | Diff
WIP Patch (6.94 KB, patch)
2013-07-03 02:13 PDT, Sudarshan
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 (566.14 KB, application/zip)
2013-07-03 13:45 PDT, Build Bot
no flags Details
Patch (10.73 KB, patch)
2013-07-04 07:20 PDT, Sudarshan
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-11 for mac-mountainlion-wk2 (498.29 KB, application/zip)
2013-07-04 09:09 PDT, Build Bot
no flags Details
Archive of layout-test-results from APPLE-EWS-5 for win-future (842.99 KB, application/zip)
2013-07-04 16:26 PDT, Build Bot
no flags Details
CSSRegionsAcceleratedCompositingTests.zip (11.41 KB, application/zip)
2013-07-23 08:58 PDT, Mihai Maerean
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
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