Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(6)

Issue 239513010: Only boxes should have child transform layers. (Closed)

Created:
6 years, 8 months ago by Ian Vollick
Modified:
6 years, 8 months ago
CC:
blink-reviews, bemjb+rendering_chromium.org, dsinclair, zoltan1, eae+blinkwatch, leviw+renderwatch, abarth-chromium, jchaffraix+rendering, pdr., rune+blink
Visibility:
Public.

Description

Only boxes should have child transform layers. The child transform layer exists to apply a perspective transform on its composited descendants. If this were to be necessary, the renderer housing the transform should be a box. Without this requirement, we cannot determine the correct size of the composited layer which will own the perpective transform. Rationale, According to http://www.w3.org/TR/css-transforms-1/ Perspective can apply to "transformable elements." That is, to """ an element whose layout is governed by the CSS box model which is either a block-level or atomic inline-level element, or whose display property computes to table-row, table-row-group, table-header-group, table-footer-group, table-cell, or table-caption [CSS21] an element in the SVG namespace and not governed by the CSS box model which has the attributes transform, ‘patternTransform‘ or gradientTransform [SVG11]. """ block-level elements and atomic inline-level elements are boxes (see http://www.w3.org/TR/CSS2/visuren.html#x13 for more info on atomic inline-level elements), as are the table-* things. SVG elements could cause problems if they could be separately composited, but they cannot be currently and the SVG root is a box. BUG=363873 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=171832

Patch Set 1 #

Patch Set 2 : de-crazified the test. #

Total comments: 6

Patch Set 3 : Address reviewer feedback. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -1 line) Patch
A LayoutTests/compositing/child-transform-layer-requires-box.html View 1 2 1 chunk +23 lines, -0 lines 0 comments Download
A LayoutTests/compositing/child-transform-layer-requires-box-expected.txt View 1 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/rendering/compositing/CompositedLayerMapping.cpp View 1 2 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 9 (0 generated)
Ian Vollick
Glenn, can you please sanity-check / pre-review? Julien, can you tell me if my rationale ...
6 years, 8 months ago (2014-04-16 15:50:18 UTC) #1
hartmanng
sanity-check lgtm
6 years, 8 months ago (2014-04-16 16:06:40 UTC) #2
esprehn
How can a non-box get a composited layer?
6 years, 8 months ago (2014-04-16 19:27:39 UTC) #3
Ian Vollick
On 2014/04/16 19:27:39, esprehn wrote: > How can a non-box get a composited layer? It's ...
6 years, 8 months ago (2014-04-16 19:49:32 UTC) #4
Julien - ping for review
lgtm > How can a non-box get a composited layer? Also nothing prevents a non-box ...
6 years, 8 months ago (2014-04-17 00:02:38 UTC) #5
Ian Vollick
https://codereview.chromium.org/239513010/diff/20001/LayoutTests/compositing/child-transform-layer-requires-box.html File LayoutTests/compositing/child-transform-layer-requires-box.html (right): https://codereview.chromium.org/239513010/diff/20001/LayoutTests/compositing/child-transform-layer-requires-box.html#newcode1 LayoutTests/compositing/child-transform-layer-requires-box.html:1: <style> On 2014/04/17 00:02:38, Julien Chaffraix - PST wrote: ...
6 years, 8 months ago (2014-04-17 01:31:03 UTC) #6
Ian Vollick
The CQ bit was checked by vollick@chromium.org
6 years, 8 months ago (2014-04-17 01:52:52 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/vollick@chromium.org/239513010/40001
6 years, 8 months ago (2014-04-17 01:53:05 UTC) #8
commit-bot: I haz the power
6 years, 8 months ago (2014-04-17 03:00:41 UTC) #9
Message was sent while issue was closed.
Change committed as 171832

Powered by Google App Engine
This is Rietveld 408576698