SPv2: Add support for tracking raster paint invalidations in testing. (issue 2380683006 by chrishtr@chromium.org)

0 views
Skip to first unread message

chri...@chromium.org

unread,
Sep 29, 2016, 8:18:01 PM9/29/16
to wangx...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org
Reviewers: Xianzhu
CL: https://codereview.chromium.org/2380683006/

Description:
SPv2: Add support for tracking raster paint invalidations in testing.

Adds one test to demonstrate the output format and smoke test SPv2 paint
invalidation.

BUG=647831
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Affected files (+493, -250 lines):
M third_party/WebKit/LayoutTests/TestExpectations
D third_party/WebKit/LayoutTests/paint/invalidation/margin-expected.txt
A + third_party/WebKit/LayoutTests/virtual/spv2/paint/invalidation/margin-expected.txt
M third_party/WebKit/Source/core/frame/FrameView.h
M third_party/WebKit/Source/core/frame/FrameView.cpp
M third_party/WebKit/Source/core/frame/LocalFrame.cpp
M third_party/WebKit/Source/platform/BUILD.gn
M third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp
M third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.h
M third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
M third_party/WebKit/Source/platform/graphics/paint/PaintChunk.h
M third_party/WebKit/Source/platform/graphics/paint/PaintController.h
M third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp
M third_party/WebKit/Source/platform/graphics/paint/PaintControllerTest.cpp
A third_party/WebKit/Source/platform/graphics/paint/PaintInvalidationTracking.h
A third_party/WebKit/Source/platform/graphics/paint/PaintInvalidationTracking.cpp
M third_party/WebKit/Source/platform/testing/PaintPrinters.cpp


wangx...@chromium.org

unread,
Sep 29, 2016, 8:47:01 PM9/29/16
to chri...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org

https://codereview.chromium.org/2380683006/diff/80001/third_party/WebKit/Source/platform/graphics/paint/PaintChunk.h
File third_party/WebKit/Source/platform/graphics/paint/PaintChunk.h
(right):

https://codereview.chromium.org/2380683006/diff/80001/third_party/WebKit/Source/platform/graphics/paint/PaintChunk.h#newcode87
third_party/WebKit/Source/platform/graphics/paint/PaintChunk.h:87:
Vector<PaintInvalidationInfo> rasterInvalidationRects;
I suggest to keep this to still contain FloatRect, and create another
field for raster invalidation tracking, so that we don't need to
allocate the additional fields in the items of this vector when we are
not tracking invalidations.

I think we should use "RasterInvalidation" instead of
"PaintInvalidation" here. We can distinguish them in SPv2.

https://codereview.chromium.org/2380683006/diff/80001/third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp
File
third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp
(right):

https://codereview.chromium.org/2380683006/diff/80001/third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp#newcode612
third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp:612:
}
Can you extract this as a function?

We can get the paintInvalidationReason from the
client.getPaintInvalidationReason() (for clients in the new display
list, and clients in old chunks matching new chunks).

https://codereview.chromium.org/2380683006/diff/80001/third_party/WebKit/Source/platform/graphics/paint/PaintInvalidationTracking.cpp
File
third_party/WebKit/Source/platform/graphics/paint/PaintInvalidationTracking.cpp
(right):

https://codereview.chromium.org/2380683006/diff/80001/third_party/WebKit/Source/platform/graphics/paint/PaintInvalidationTracking.cpp#newcode77
third_party/WebKit/Source/platform/graphics/paint/PaintInvalidationTracking.cpp:77:

Can we move rect under-invalidation checking from GraphicsLayer to here
(can be in another patch)?

https://codereview.chromium.org/2380683006/

chri...@chromium.org

unread,
Sep 30, 2016, 2:19:11 PM9/30/16
to wangx...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org
I renamed paint to raster throughout.

Also, as a followup patch, I will try to adjust the SPv1 raster invalidation
output format
to be agnostic to the layer tree structure. We might be able to get SPv2 outputs
to match
SPv1 outputs exactly for many tests.



https://codereview.chromium.org/2380683006/diff/80001/third_party/WebKit/Source/platform/graphics/paint/PaintChunk.h
File third_party/WebKit/Source/platform/graphics/paint/PaintChunk.h
(right):

https://codereview.chromium.org/2380683006/diff/80001/third_party/WebKit/Source/platform/graphics/paint/PaintChunk.h#newcode87
third_party/WebKit/Source/platform/graphics/paint/PaintChunk.h:87:
Vector<PaintInvalidationInfo> rasterInvalidationRects;
On 2016/09/30 at 00:47:00, Xianzhu wrote:
> I suggest to keep this to still contain FloatRect, and create another
field for raster invalidation tracking, so that we don't need to
allocate the additional fields in the items of this vector when we are
not tracking invalidations.
>
> I think we should use "RasterInvalidation" instead of
"PaintInvalidation" here. We can distinguish them in SPv2.

Done.
On 2016/09/30 at 00:47:01, Xianzhu wrote:
> Can you extract this as a function?
>
> We can get the paintInvalidationReason from the
client.getPaintInvalidationReason() (for clients in the new display
list, and clients in old chunks matching new chunks).

Done.
On 2016/09/30 at 00:47:01, Xianzhu wrote:
> Can we move rect under-invalidation checking from GraphicsLayer to
here (can be in another patch)?

Ok will do that in a followup.

https://codereview.chromium.org/2380683006/

wangx...@chromium.org

unread,
Sep 30, 2016, 3:04:31 PM9/30/16
to chri...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org
lgtm




https://codereview.chromium.org/2380683006/diff/160001/third_party/WebKit/Source/platform/graphics/paint/PaintController.h
File third_party/WebKit/Source/platform/graphics/paint/PaintController.h
(right):

https://codereview.chromium.org/2380683006/diff/160001/third_party/WebKit/Source/platform/graphics/paint/PaintController.h#newcode281
third_party/WebKit/Source/platform/graphics/paint/PaintController.h:281:
bool m_isTrackingPaintInvalidations;
Nit: This seems not used.

https://codereview.chromium.org/2380683006/

chri...@chromium.org

unread,
Sep 30, 2016, 5:04:16 PM9/30/16
to wangx...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org

https://codereview.chromium.org/2380683006/diff/160001/third_party/WebKit/Source/platform/graphics/paint/PaintController.h
File third_party/WebKit/Source/platform/graphics/paint/PaintController.h
(right):

https://codereview.chromium.org/2380683006/diff/160001/third_party/WebKit/Source/platform/graphics/paint/PaintController.h#newcode281
third_party/WebKit/Source/platform/graphics/paint/PaintController.h:281:
bool m_isTrackingPaintInvalidations;
On 2016/09/30 at 19:04:30, Xianzhu wrote:
> Nit: This seems not used.

commit-bot@chromium.org via codereview.chromium.org

unread,
Sep 30, 2016, 5:05:27 PM9/30/16
to chri...@chromium.org, wangx...@chromium.org, commi...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org

commit-bot@chromium.org via codereview.chromium.org

unread,
Sep 30, 2016, 5:22:33 PM9/30/16
to chri...@chromium.org, wangx...@chromium.org, commi...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org
Try jobs failed on following builders:
android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_compile_dbg/builds/138090)

https://codereview.chromium.org/2380683006/

commit-bot@chromium.org via codereview.chromium.org

unread,
Sep 30, 2016, 6:12:24 PM9/30/16
to chri...@chromium.org, wangx...@chromium.org, commi...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org
CQ is trying da patch. Follow status at

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 1, 2016, 12:27:29 AM10/1/16
to chri...@chromium.org, wangx...@chromium.org, commi...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org
CQ is trying da patch. Follow status at

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 1, 2016, 12:36:11 AM10/1/16
to chri...@chromium.org, wangx...@chromium.org, commi...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org
Try jobs failed on following builders:

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 1, 2016, 12:43:02 AM10/1/16
to chri...@chromium.org, wangx...@chromium.org, commi...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org
CQ is trying da patch. Follow status at

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 1, 2016, 1:26:15 AM10/1/16
to chri...@chromium.org, wangx...@chromium.org, commi...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org
Try jobs failed on following builders:

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 1, 2016, 4:09:13 PM10/1/16
to chri...@chromium.org, wangx...@chromium.org, commi...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org
CQ is trying da patch. Follow status at

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 1, 2016, 4:49:43 PM10/1/16
to chri...@chromium.org, wangx...@chromium.org, commi...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org
Try jobs failed on following builders:
linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_FAILED,

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 1, 2016, 6:40:43 PM10/1/16
to chri...@chromium.org, wangx...@chromium.org, commi...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org
CQ is trying da patch. Follow status at

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 1, 2016, 7:13:06 PM10/1/16
to chri...@chromium.org, wangx...@chromium.org, commi...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org
Try jobs failed on following builders:
linux_layout_tests_slimming_paint_v2 on master.tryserver.chromium.linux
(JOB_FAILED,

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 2, 2016, 10:38:35 AM10/2/16
to chri...@chromium.org, wangx...@chromium.org, commi...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org
CQ is trying da patch. Follow status at

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 2, 2016, 11:36:44 AM10/2/16
to chri...@chromium.org, wangx...@chromium.org, commi...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org
Try jobs failed on following builders:

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 3, 2016, 12:28:28 PM10/3/16
to chri...@chromium.org, wangx...@chromium.org, commi...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org
CQ is trying da patch. Follow status at

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 3, 2016, 2:05:24 PM10/3/16
to chri...@chromium.org, wangx...@chromium.org, commi...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org
Committed patchset #18 (id:340001)

https://codereview.chromium.org/2380683006/

commit-bot@chromium.org via codereview.chromium.org

unread,
Oct 3, 2016, 2:07:03 PM10/3/16
to chri...@chromium.org, wangx...@chromium.org, commi...@chromium.org, ajuma...@chromium.org, blink-lay...@chromium.org, blink-...@chromium.org, blink-reviews-p...@chromium.org, caba...@adobe.com, chromium...@chromium.org, danakj...@chromium.org, dongseo...@intel.com, drott+bl...@chromium.org, dsch...@chromium.org, fma...@chromium.org, jbr...@chromium.org, ju...@chromium.org, pdr+graphi...@chromium.org, rob....@samsung.com, sche...@chromium.org
Patchset 18 (id:??) landed as
https://crrev.com/a3396125b597b2c83bd7617405c44ecf8c958985
Cr-Commit-Position: refs/heads/master@{#422459}

https://codereview.chromium.org/2380683006/
Reply all
Reply to author
Forward
0 new messages