Bug 142783 - RenderLayerCompositor: only create backing for visible and non-empty layers
Summary: RenderLayerCompositor: only create backing for visible and non-empty layers
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-17 08:53 PDT by Julien Isorce
Modified: 2017-04-24 19:06 PDT (History)
9 users (show)

See Also:


Attachments
RenderLayerCompositor: only create backing for visible and non-empty layers (1.32 KB, patch)
2015-03-17 08:53 PDT, Julien Isorce
no flags Details | Formatted Diff | Diff
RenderLayerCompositor: only create backing for visible and non-empty layers (1.59 KB, patch)
2015-03-17 10:37 PDT, Julien Isorce
no flags Details | Formatted Diff | Diff
Print info on composited layers (1004 bytes, patch)
2015-03-18 11:12 PDT, Julien Isorce
no flags Details | Formatted Diff | Diff
Example of positionned divs (2.27 KB, text/html)
2015-03-18 11:22 PDT, Julien Isorce
no flags Details
Example of positionned divs (2.28 KB, text/html)
2015-03-19 11:41 PDT, Julien Isorce
no flags Details
WIP: only accept indirect reason if the layer can be a stacking context (composited) (1.15 KB, patch)
2015-03-19 11:42 PDT, Julien Isorce
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-mavericks-wk2 (246.29 KB, application/zip)
2015-03-19 12:12 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews101 for mac-mavericks (243.80 KB, application/zip)
2015-03-19 12:24 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews206 for win-future (17.15 MB, application/zip)
2015-03-20 16:31 PDT, Build Bot
no flags Details
WIP: for overlap composited reasins check if the layer is visually non empty (1.37 KB, patch)
2015-03-23 11:34 PDT, Julien Isorce
buildbot: commit-queue-
Details | Formatted Diff | Diff
WIP: for overlap composited reasins check if the layer is visually non empty and stacking context (1.42 KB, patch)
2015-03-23 11:41 PDT, Julien Isorce
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-mavericks-wk2 (572.91 KB, application/zip)
2015-03-23 12:28 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews102 for mac-mavericks (541.85 KB, application/zip)
2015-03-23 12:30 PDT, Build Bot
no flags Details
RenderLayerCompositor: avoid creating unecessary composited layers when not stacking context. (3.87 KB, patch)
2015-03-24 11:22 PDT, Julien Isorce
beidson: review-
Details | Formatted Diff | Diff
Test case2 (4.48 KB, text/html)
2015-03-24 11:25 PDT, Julien Isorce
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Julien Isorce 2015-03-17 08:53:16 PDT
Created attachment 248849 [details]
RenderLayerCompositor: only create backing for visible and non-empty layers

When there is at least one layer that requires compositing in the page, like accelerated canvas or so, all overlapped layers creates their own backing even if initially unvisible (and even with 0 width or height)

Not sure that the following patch has the best approach tough but worth to have a try on layout tests on webkit's buildbots.

Other approach could be to make the layer that are in the same zindex, use the same backing.
Comment 1 Simon Fraser (smfr) 2015-03-17 10:03:06 PDT
This will break all kinds of things: transitions on a visibility:hidden layer that toggles to visible, visible children of a hidden composited layer etc.
Comment 2 Julien Isorce 2015-03-17 10:37:19 PDT
Created attachment 248852 [details]
RenderLayerCompositor: only create backing for visible and non-empty layers

In last patch I forgot to actually activate my change :) so previous patch was doing nothing.
Comment 3 Julien Isorce 2015-03-18 11:12:22 PDT
Created attachment 248942 [details]
Print info on composited layers
Comment 4 Julien Isorce 2015-03-18 11:22:54 PDT
Created attachment 248943 [details]
Example of positionned divs

In the attached exmaple, all the divs become composited (creates a backing), because of the video tag.

If the video tag is added after all the divs or in a zindex different than 0 then all the divs do not become composited.

So is it really an expected behaviour  for neasted divs to all become composited this way ?

You can use the attached patched to print info on composited layers.

About stacking context the following doc is very helpful:

"When no z-index property is specified, elements are rendered on the default rendering layer 0 (zero)." , but if I put 0 manually the divs do not become composited (if I put auto they become composited)

"A stacking context is formed, anywhere in the document, by any element which is positioned (absolutely or relatively) with a z-index value other than "auto" "
It seems to behave the opposite as said juste before.

https://developer.mozilla.org/en-US/docs/Web/Guide/CSS/Understanding_z_index/The_stacking_context
https://developer.mozilla.org/en-US/docs/Web/Guide/CSS/Understanding_z_index/Stacking_without_z-index
Comment 5 Simon Fraser (smfr) 2015-03-18 11:25:05 PDT
Comment on attachment 248942 [details]
Print info on composited layers

There's a Compositing log channel which you may find useful.
Comment 6 Simon Fraser (smfr) 2015-03-18 11:28:52 PDT
(In reply to comment #4)
> Created attachment 248943 [details]
> Example of positionned divs
> 
> In the attached exmaple, all the divs become composited (creates a backing),
> because of the video tag.

Not on Mac. I get one big layer for the rootmost view and none inside of it.

> If the video tag is added after all the divs or in a zindex different than 0
> then all the divs do not become composited.

Odd that there's an ordering difference.

> So is it really an expected behaviour  for neasted divs to all become
> composited this way ?

No. Are you testing webkit TOT?

> About stacking context the following doc is very helpful:
> 
> "When no z-index property is specified, elements are rendered on the default
> rendering layer 0 (zero)." , but if I put 0 manually the divs do not become
> composited (if I put auto they become composited)
> 
> "A stacking context is formed, anywhere in the document, by any element
> which is positioned (absolutely or relatively) with a z-index value other
> than "auto" "
> It seems to behave the opposite as said juste before.

z-index:0 create stacking context, but has z-index 0 in its parent stacking context. z-index: auto does not create stacking context.
Comment 7 Julien Isorce 2015-03-18 12:02:49 PDT
I am testing on webkitgtk and webkitefl with latest webkit code.

That's good to know that you get good behavior on osx. I'll investigate more tomorrow. I'll try to understand why all the divs become composited even if I set z-index to auto (the default).
Comment 8 Anton Obzhirov 2015-03-19 04:26:28 PDT
(In reply to comment #6)
> (In reply to comment #4)
> > Created attachment 248943 [details]
> > Example of positionned divs
> > 
> > In the attached exmaple, all the divs become composited (creates a backing),
> > because of the video tag.
> 
> Not on Mac. I get one big layer for the rootmost view and none inside of it.
> 
> > If the video tag is added after all the divs or in a zindex different than 0
> > then all the divs do not become composited.
> 
> Odd that there's an ordering difference.
> 
> > So is it really an expected behaviour  for neasted divs to all become
> > composited this way ?
> 
> No. Are you testing webkit TOT?
> 
> > About stacking context the following doc is very helpful:
> > 
> > "When no z-index property is specified, elements are rendered on the default
> > rendering layer 0 (zero)." , but if I put 0 manually the divs do not become
> > composited (if I put auto they become composited)
> > 
> > "A stacking context is formed, anywhere in the document, by any element
> > which is positioned (absolutely or relatively) with a z-index value other
> > than "auto" "
> > It seems to behave the opposite as said juste before.
> 
> z-index:0 create stacking context, but has z-index 0 in its parent stacking
> context. z-index: auto does not create stacking context.

Actually, in the example Julien attached all divs have -index: 0;.
So if you used the same unmodified example you got wrong behaviour on Mac.
Comment 9 Julien Isorce 2015-03-19 06:03:33 PDT
(In reply to comment #8)

Indeed the example should have come with "z-index: auto;" (or no line at all)


Result with auto:

0x7faa397f06c0 (0.000000,0.000000-1288.000000,728.000000) 0.00KB (root) [opaque] RenderView
  0x7faa397e66c0 (0.000000,0.000000-680.000000,300.000000) 0.00KB (video) RenderVideo VIDEO
  0x7faa397e67e0 (0.000000,0.000000-680.000000,300.000000) 0.00KB (overlap) RenderFlexibleBox DIV
  0x7faa397e6900 (5.000000,265.000000-675.000000,295.000000) 0.00KB (overlap) RenderFlexibleBox DIV class='paused no-video show'
  0x7faa39799d80 (8.000000,8.000000-1288.000000,728.000000) 0.00KB (overlap) RenderBlock (positioned) DIV class='view'
  0x7faa39799ea0 (8.000000,8.000000-1288.000000,728.000000) 0.00KB (overlap) RenderBlock (positioned) DIV class='view'
  0x7faa39799000 (8.000000,8.000000-1288.000000,728.000000) 0.00KB (overlap) RenderBlock (positioned) DIV class='view'
  0x7faa39799120 (8.000000,8.000000-1288.000000,728.000000) 0.00KB (overlap) RenderBlock (positioned) DIV class='view'
  0x7faa39799240 (8.000000,8.000000-1288.000000,728.000000) 0.00KB (overlap) RenderBlock (positioned) DIV class='view'
  0x7faa39799360 (8.000000,8.000000-1288.000000,728.000000) 0.00KB (overlap) RenderBlock (positioned) DIV class='view'
  0x7faa397996c0 (108.000000,450.000000-108.000000,450.000000) 0.00KB (overlap) RenderBlock (positioned) DIV id='menu1' class='view'
  0x7faa397997e0 (252.000000,450.000000-1288.000000,510.000000) 0.00KB (overlap) RenderBlock (positioned) DIV class='view'
  0x7faa39799900 (252.000000,450.000000-706.000000,510.000000) 0.00KB (overlap) [opaque] RenderBlock (positioned) DIV class='view'
  0x7faa39799a20 (335.000000,463.000000-696.000000,493.000000) 0.00KB (overlap) RenderBlock (positioned) DIV class='view view-text'
Total layers   primary   secondary   obligatory backing (KB)   secondary backing(KB)   total backing (KB)  update time (ms)
      14           2        12                 0.00                   0.00                   0.00               0.69


Result with z-index: 0 

0x7fce2f7f16c0 (0.000000,0.000000-1288.000000,728.000000) 0.00KB (root) [opaque] RenderView
  0x7fce2f7e76c0 (0.000000,0.000000-680.000000,300.000000) 0.00KB (video) RenderVideo VIDEO
  0x7fce2f7e77e0 (0.000000,0.000000-680.000000,300.000000) 0.00KB (overlap) RenderFlexibleBox DIV
  0x7fce2f7e7900 (5.000000,265.000000-675.000000,295.000000) 0.00KB (overlap) RenderFlexibleBox DIV class='paused no-video show'
  0x7fce2f78fd80 (8.000000,8.000000-1288.000000,728.000000) 0.00KB (overlap) RenderBlock (positioned) DIV class='view'
Total layers   primary   secondary   obligatory backing (KB)   secondary backing(KB)   total backing (KB)  update time (ms)
       5           2         3                 0.00                   0.00                   0.00               0.81
Comment 10 Simon Fraser (smfr) 2015-03-19 10:24:54 PDT
(In reply to comment #8)
> Actually, in the example Julien attached all divs have -index: 0;.
> So if you used the same unmodified example you got wrong behaviour on Mac.

Can you attach a testcase that actually shows the bug?
Comment 11 Julien Isorce 2015-03-19 11:41:08 PDT
Created attachment 249039 [details]
Example of positionned divs

It'll try to make a proper layout test but for now this example make all the div be composited. You can see it using WEBKIT_DEBUG="compositing".  Whereas z-index=auto should prevent them creating a stacking context (backing).
Comment 12 Julien Isorce 2015-03-19 11:42:41 PDT
Created attachment 249040 [details]
WIP: only accept indirect reason if the layer can be a stacking context (composited)

WIP patch just to run layout tests.
Comment 13 Simon Fraser (smfr) 2015-03-19 11:43:45 PDT
OK, what I see on Mac with that testcase is a bunch of compositing layers, but most of them have no backing store so are cheap.

This is expected behavior; there's no enclosing stacking context, so each position:relative layer gets composited.

Blink wrote some "layer flattening" code to solve this, but it's complex and hard to get right.
Comment 14 WebKit Commit Bot 2015-03-19 11:44:50 PDT
Attachment 249040 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RenderLayer.h:946:  More than one command on the same line in if  [whitespace/parens] [4]
Total errors found: 1 in 1 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Build Bot 2015-03-19 12:12:27 PDT
Comment on attachment 249040 [details]
WIP: only accept indirect reason if the layer can be a stacking context (composited)

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

Number of test failures exceeded the failure limit.
Comment 16 Build Bot 2015-03-19 12:12:32 PDT
Created attachment 249044 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 17 Build Bot 2015-03-19 12:24:12 PDT
Comment on attachment 249040 [details]
WIP: only accept indirect reason if the layer can be a stacking context (composited)

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

Number of test failures exceeded the failure limit.
Comment 18 Build Bot 2015-03-19 12:24:16 PDT
Created attachment 249047 [details]
Archive of layout-test-results from ews101 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 19 Build Bot 2015-03-20 16:31:33 PDT
Comment on attachment 249040 [details]
WIP: only accept indirect reason if the layer can be a stacking context (composited)

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

New failing tests:
js/slow-stress/fold-strict-eq.html
Comment 20 Build Bot 2015-03-20 16:31:38 PDT
Created attachment 249146 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-WOW64-1.7.20-0.266-5-3-i686-32bit
Comment 21 Julien Isorce 2015-03-23 09:50:46 PDT
I checked an indeed a backingstore is created only when CoordinatedGraphicsScene/layerShouldHaveBackingStore 
"drawsContent() && contentsAreVisible() && !size().isEmpty()" return true.

At each call to CoordinatedGraphicsScene::setLayerState it checks if this condition return false to see if it can remove the backingstore.
Comment 22 Julien Isorce 2015-03-23 10:34:55 PDT
So in RenderLayerCompositor::computeCompositingRequirements, when the layer is "RenderBlock HTML":

if (layer.isStackingContainer())
  if (Vector<RenderLayer*>* posZOrderList = layer.posZOrderList())
    for (auto* renderLayer : *posZOrderList)
      computeCompositingRequirements

The posZOrderList contains the 11 divs.

Then within a call to computeCompositingRequirements for each of these 11 divs, compositingReason = overlapMap.overlapsLayers returns IndirectCompositingReason::Overlap (which make it composited) because m_overlapStack.last() is the rect of the video layer.

Indeed each divs is push and then pop from the overlapmap because of:  
if (childState.compositingAncestor == &layer && !layer.isRootLayer())
  overlapMap.popCompositingContainer();

So for each div it always compares to video rect, even if these divs are nested.

As opposite, when z-index is force to 0 in the attached example, posZOrderList is 2 (video tag and first div) and the m_overlapStack.last() is the first div (which one is made composited).
So except the first div, compositingReason = overlapMap.overlapsLayers is compared to the first div. So it returns IndirectCompositingReason::None.

In the end having z-index to auto make it the nested divs behaves as if there were not nested. Is it expected ? Well as least it follows the rules that auto means it does not create a stacking context (except for the root layer)
Comment 23 Julien Isorce 2015-03-23 11:34:59 PDT
Created attachment 249242 [details]
WIP: for overlap composited reasins check if the layer is visually non empty

I am still not happy with this patch since it should make a composite layer for the first div (in both zindex cases: 0/auto)

But I am curious of the layout tests result.
Comment 24 Julien Isorce 2015-03-23 11:41:25 PDT
Created attachment 249244 [details]
WIP: for overlap composited reasins check if the layer is visually non empty and stacking context

This one is better for z-index 0 case.
Comment 25 Build Bot 2015-03-23 12:28:06 PDT
Comment on attachment 249242 [details]
WIP: for overlap composited reasins check if the layer is visually non empty

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

New failing tests:
compositing/geometry/bounds-ignores-hidden-dynamic-negzindex.html
compositing/geometry/fixed-position-flipped-writing-mode.html
compositing/visibility/layer-visible-content.html
css3/blending/blend-mode-blended-element-overlapping-composited-sibling-should-have-compositing-layer.html
Comment 26 Build Bot 2015-03-23 12:28:10 PDT
Created attachment 249250 [details]
Archive of layout-test-results from ews107 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 27 Build Bot 2015-03-23 12:30:00 PDT
Comment on attachment 249242 [details]
WIP: for overlap composited reasins check if the layer is visually non empty

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

New failing tests:
compositing/geometry/bounds-ignores-hidden-dynamic-negzindex.html
compositing/visibility/layer-visible-content.html
css3/blending/blend-mode-blended-element-overlapping-composited-sibling-should-have-compositing-layer.html
Comment 28 Build Bot 2015-03-23 12:30:06 PDT
Created attachment 249251 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 29 Julien Isorce 2015-03-24 11:22:00 PDT
Created attachment 249335 [details]
RenderLayerCompositor: avoid creating unecessary composited layers when not stacking context.
Comment 30 Julien Isorce 2015-03-24 11:25:25 PDT
Created attachment 249336 [details]
Test case2
Comment 31 Brady Eidson 2017-04-24 19:06:29 PDT
Comment on attachment 249335 [details]
RenderLayerCompositor: avoid creating unecessary composited layers when not stacking context.

This patch has been pending review since 2015 with no recent activity.
It seems unlikely that it would even still apply to trunk in its current form.

Clearing from the review queue.

Feel free to update and resubmit if the patch is still relevant.