Source/core/paint/DeprecatedPaintLayerClipper.h - Issue 1327483003: Add documentation to 2 DeprecatedPaintLayer objects

Side by Side Diff: Source/core/paint/DeprecatedPaintLayerClipper.h

Issue 1327483003: Add documentation to 2 DeprecatedPaintLayer objects (Closed) Base URL: svn://svn.chromium.org/blink/trunk

Patch Set: Created 5 years, 3 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch | Annotate | Revision Log

OLD	NEW
1 /*	1 /*

2 * Copyright (C) 2003, 2009, 2012 Apple Inc. All rights reserved.	2 * Copyright (C) 2003, 2009, 2012 Apple Inc. All rights reserved.

3 * Copyright (C) 2013 Intel Corporation. All rights reserved.	3 * Copyright (C) 2013 Intel Corporation. All rights reserved.

4 *	4 *

5 * Portions are Copyright (C) 1998 Netscape Communications Corporation.	5 * Portions are Copyright (C) 1998 Netscape Communications Corporation.

6 *	6 *

7 * Other contributors:	7 * Other contributors:

8 * Robert O'Callahan <roc+@cs.cmu.edu>	8 * Robert O'Callahan <roc+@cs.cmu.edu>

9 * David Baron <dbaron@fas.harvard.edu>	9 * David Baron <dbaron@fas.harvard.edu>

10 * Christian Biesinger <cbiesinger@web.de>	10 * Christian Biesinger <cbiesinger@web.de>

(...skipping 109 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
120	120

121 private:	121 private:

122 friend class DeprecatedPaintLayerClipper;	122 friend class DeprecatedPaintLayerClipper;

123	123

124 ClipRectsCacheSlot m_cacheSlot;	124 ClipRectsCacheSlot m_cacheSlot;

125 LayoutSize subPixelAccumulation;	125 LayoutSize subPixelAccumulation;

126 ShouldRespectOverflowClip respectOverflowClip;	126 ShouldRespectOverflowClip respectOverflowClip;

127 ShouldRespectOverflowClip respectOverflowClipForViewport;	127 ShouldRespectOverflowClip respectOverflowClipForViewport;

128 };	128 };

129	129

	130 // DeprecatedPaintLayerClipper is responsible for caching clip rects.
	wkorman 2015/09/01 17:58:20 Worth adding a note for any class with 'Deprecated Worth adding a note for any class with 'Deprecated' in the name re: why it is considered deprecated and, if we have a sense, what we expect to eventually take its place? Would apply to all three classes in this change. If in some cases we've just renamed the classes so they're consistent with the DeprecatedPaintLayer naming then IMO still worth noting that, since otherwise a new eng seeing a class named like this will absolutely think they should avoid using the class and instead look for something else. mstensho (USE GERRIT) 2015/09/01 18:59:06 Calculating and caching, perhaps? Calculating and caching, perhaps? Julien - ping for review 2015/09/03 14:25:54 I have added, that should clarify the status of th Show quoted text On 2015/09/01 at 17:58:20, wkorman wrote: > Worth adding a note for any class with 'Deprecated' in the name re: why it is considered deprecated and, if we have a sense, what we expect to eventually take its place? Would apply to all three classes in this change. I have added, that should clarify the status of the classes: // This class is NOT DEPRECATED, DeprecatedPaintLayer is and we match its // naming. Also modified DeprecatedPaintLayer to explain the rationale for deprecation: // The class is DEPRECATED, which means that we would like to remove it. The // reason for removal is that it has been a dumping ground for features for too // long and is the wrong level of abstraction, bearing no correspondence to any // CSS concept. Its associated objects and some of its feature need to be // migrated to LayoutObject (or the appropriate sub-class). Show quoted text > If in some cases we've just renamed the classes so they're consistent with the DeprecatedPaintLayer naming then IMO still worth noting that, since otherwise a new eng seeing a class named like this will absolutely think they should avoid using the class and instead look for something else. Makes sense! On 2015/09/01 at 18:59:06, mstensho wrote: Show quoted text > Calculating and caching, perhaps? Updated.
	131 //

	132 // The main reason for this cache is that we compute the clip rects during

	133 // a layout tree walk but need them during a paint tree walk (see example

	134 // below for some explanations).

	135 //

	136 // A lot of complexity in this class come from the difference in inheritance

	137 // between 'overflow' and 'clip':

	138 // * 'overflow' applies based on the containing blocks chain.

	139 // (http://www.w3.org/TR/CSS2/visufx.html#propdef-overflow)

	140 // * 'clip' applies to all descendants.

	141 // (http://www.w3.org/TR/CSS2/visufx.html#propdef-clip)

	142 //

	143 // Let's take an example:

	144 // <!DOCTYPE html>

	145 // <div id="container" style="position: absolute; height: 100px; width: 100px">

	146 // <div id="inflow" style="height: 200px; width: 200px;

	147 // background-color: purple"></div>

	148 // <div id="fixed" style="height: 200px; width: 200px; position: fixed;

	149 // background-color: orange"></div>

	150 // </div>

	151 //

	152 // The paint tree looks like:

	153 // html

	154 // / \

	155 // / \

	156 // / \

	157 // container fixed

	158 // \|

	159 // \|

	160 // inflow

	161 //

	162 // If we add "overflow: hidden" to #container, the overflow clip will apply to

	163 // #inflow but not to #fixed. That's because #fixed's containing block is above

	164 // #container and thus 'overflow' doesn't apply to it. During our tree walk,

	165 // #fixed is a child of #container, which is the reason why we keep 3 clip rects

	166 // depending on the 'position' of the elements.

	167 //

	168 // Now instead if we add "clip: rect(0px, 100px, 100px, 0px)" to #container,

	169 // the clip will apply to both #inflow and #fixed. That's because 'clip'

	170 // applies to any descendant, regardless of containing blocks. Note that

	171 // #container and #fixed are siblings in the paint tree but #container does

	172 // clip #fixed. This is the reason why we compute the painting clip rects during
	wkorman 2015/09/01 17:58:20 The way this is written up makes it sound as if, i The way this is written up makes it sound as if, if it weren't for this functionality requirement, we wouldn't cache these. But I'm expecting we'd want to cache them anyway so that paint phase has them available and doesn't need to recalculate them unnecessarily when we potentially have to repaint an area multiple times. True or false? Julien - ping for review 2015/09/03 14:25:54 I don't really have a definite answer to your ques Show quoted text On 2015/09/01 at 17:58:20, wkorman wrote: > The way this is written up makes it sound as if, if it weren't for this functionality requirement, we wouldn't cache these. But I'm expecting we'd want to cache them anyway so that paint phase has them available and doesn't need to recalculate them unnecessarily when we potentially have to repaint an area multiple times. True or false? I don't really have a definite answer to your question without data :-) The aim of this paragraph is to highlight the 2 different walks (layout vs paint tree), which motivates the need for caching. If we could do the computation during a paint tree walk, we may not need caching (by recomputing them on the fly) but it turns out that it's really hard to do that. As a counter argument to the multiple recomputation argument, we switched the normal flow list to be generated on the fly, potentially several times per paint and we didn't see any performance degradation (https://codereview.chromium.org/1188363002). This was motivated by a code simplification and less memory use.
	173 // a layout tree walk and cache them for painting.

130 class DeprecatedPaintLayerClipper {	174 class DeprecatedPaintLayerClipper {

131 DISALLOW_ALLOCATION();	175 DISALLOW_ALLOCATION();

132 WTF_MAKE_NONCOPYABLE(DeprecatedPaintLayerClipper);	176 WTF_MAKE_NONCOPYABLE(DeprecatedPaintLayerClipper);

133 public:	177 public:

134 explicit DeprecatedPaintLayerClipper(LayoutBoxModelObject&);	178 explicit DeprecatedPaintLayerClipper(LayoutBoxModelObject&);

135	179

136 void clearClipRectsIncludingDescendants();	180 void clearClipRectsIncludingDescendants();

137 void clearClipRectsIncludingDescendants(ClipRectsCacheSlot);	181 void clearClipRectsIncludingDescendants(ClipRectsCacheSlot);

138	182

139 LayoutRect childrenClipRect() const; // Returns the foreground clip rect of the layer in the document's coordinate space.	183 LayoutRect childrenClipRect() const; // Returns the foreground clip rect of the layer in the document's coordinate space.

(...skipping 26 matching lines...) Expand all Loading...
166 bool shouldRespectOverflowClip(const ClipRectsContext&) const;	210 bool shouldRespectOverflowClip(const ClipRectsContext&) const;

167	211

168 // FIXME: Could this be a LayoutBox?	212 // FIXME: Could this be a LayoutBox?

169 LayoutBoxModelObject& m_layoutObject;	213 LayoutBoxModelObject& m_layoutObject;

170 mutable OwnPtr<ClipRect> m_clips[NumberOfClipRectsCacheSlots];	214 mutable OwnPtr<ClipRect> m_clips[NumberOfClipRectsCacheSlots];

171 };	215 };

172	216

173 } // namespace blink	217 } // namespace blink

174	218

175 #endif // LayerClipper_h	219 #endif // LayerClipper_h

OLD	NEW

« no previous file with comments | « no previous file | Source/core/paint/DeprecatedPaintLayerStackingNode.h » ('j') | Source/core/paint/DeprecatedPaintLayerStackingNode.h » ('J')