Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2555283002:
Add mask layer and offsetFromLayoutObject info to GraphicsLayer JSON (Closed)
|
Created:
4 years ago by Stephen Chennney Modified:
4 years ago Reviewers:
ajuma CC:
chromium-reviews, krit, drott+blinkwatch_chromium.org, blink-reviews-platform-graphics_chromium.org, dshwang, pdr+graphicswatchlist_chromium.org, jbroman, Justin Novosad, blink-layers+watch_chromium.org, Rik, f(malita), blink-reviews-paint_chromium.org, blink-reviews, danakj+watch_chromium.org, ajuma+watch_chromium.org, rwlbuis Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd mask layer and offsetFromLayoutObject info to GraphicsLayer JSON
R=ajuma
BUG=672077
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Committed: https://crrev.com/b0246be63c67f341421ee64196857479e7e2b9ef
Cr-Commit-Position: refs/heads/master@{#437549}
Patch Set 1 #
Total comments: 5
Patch Set 2 : Fix asserts and rebaseline #Patch Set 3 : Re-add restoreLayer. #Patch Set 4 : Rebased #
Messages
Total messages: 27 (14 generated)
Description was changed from ========== Add mask layer and offsetFromLayoutObject info to GraphicsLayer JSON R=ajuma BUG=672077 ========== to ========== Add mask layer and offsetFromLayoutObject info to GraphicsLayer JSON R=ajuma BUG=672077 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ==========
Test changes are expected from this patch and I'm getting new baselines. Having said that, most of the change is due to outputting offsetFromLayoutObject which I think is only marginally positive. I'd leave it out if you prefer it that way. https://codereview.chromium.org/2555283002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/paint/BoxPainter.cpp (left): https://codereview.chromium.org/2555283002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/paint/BoxPainter.cpp:795: context.endLayer(); This is code cleanup. https://codereview.chromium.org/2555283002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/paint/FilterPainter.cpp (right): https://codereview.chromium.org/2555283002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/paint/FilterPainter.cpp:60: } Style fix forced upon us. https://codereview.chromium.org/2555283002/diff/1/third_party/WebKit/Source/p... File third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp (right): https://codereview.chromium.org/2555283002/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp:654: } I'm not so sure about outputting this. Lots of layers have non-zero and it changes lots of test text output. Your thoughts? https://codereview.chromium.org/2555283002/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp:792: } These missing layers were a big issue in originally trying to figure out how mask layers worked.
https://codereview.chromium.org/2555283002/diff/1/third_party/WebKit/Source/p... File third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp (right): https://codereview.chromium.org/2555283002/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp:654: } On 2016/12/07 20:29:11, Stephen Chennney wrote: > I'm not so sure about outputting this. Lots of layers have non-zero and it > changes lots of test text output. Your thoughts? What about adding a LayerTreeFlag for it and only outputting when the flag is set? Then tests that care about this can opt into it.
On 2016/12/07 22:53:18, ajuma wrote: > https://codereview.chromium.org/2555283002/diff/1/third_party/WebKit/Source/p... > File third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp (right): > > https://codereview.chromium.org/2555283002/diff/1/third_party/WebKit/Source/p... > third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp:654: } > On 2016/12/07 20:29:11, Stephen Chennney wrote: > > I'm not so sure about outputting this. Lots of layers have non-zero and it > > changes lots of test text output. Your thoughts? > > What about adding a LayerTreeFlag for it and only outputting when the flag is > set? Then tests that care about this can opt into it. I think that's a good idea. I'll get that done.
The CQ bit was checked by schenney@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
I put the offsetFromLayoutObject behind the debug output flag, which makes sense to me because it's only useful if you dump layer pointers as in debug mode. Only 2 tests have mask layers, so I leave that as part of the normal layer output and rebase the tests.
On 2016/12/08 18:48:29, Stephen Chennney wrote: > I put the offsetFromLayoutObject behind the debug output flag, which makes sense > to me because it's only useful if you dump layer pointers as in debug mode. > > Only 2 tests have mask layers, so I leave that as part of the normal layer > output and rebase the tests. Thanks, lgtm
The CQ bit was unchecked by schenney@chromium.org
The CQ bit was checked by schenney@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by schenney@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from ajuma@chromium.org Link to the patchset: https://codereview.chromium.org/2555283002/#ps40001 (title: "Re-add restoreLayer.")
Failed to revert a change in the previous patch set. Now it should be right.
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for
third_party/WebKit/Source/core/paint/FilterPainter.cpp:
While running git apply --index -p1;
error: patch failed: third_party/WebKit/Source/core/paint/FilterPainter.cpp:53
error: third_party/WebKit/Source/core/paint/FilterPainter.cpp: patch does not
apply
Patch: third_party/WebKit/Source/core/paint/FilterPainter.cpp
Index: third_party/WebKit/Source/core/paint/FilterPainter.cpp
diff --git a/third_party/WebKit/Source/core/paint/FilterPainter.cpp
b/third_party/WebKit/Source/core/paint/FilterPainter.cpp
index
6f7d9e4c640af8e7fbe0576f3a4b1f03693cc350..1629ba1a749a2cf5d4250b86aaab7962ac70c4e6
100644
--- a/third_party/WebKit/Source/core/paint/FilterPainter.cpp
+++ b/third_party/WebKit/Source/core/paint/FilterPainter.cpp
@@ -53,10 +53,11 @@ FilterPainter::FilterPainter(PaintLayer& layer,
DCHECK(m_layoutObject);
- if (clipRect.rect() != paintingInfo.paintDirtyRect || clipRect.hasRadius())
+ if (clipRect.rect() != paintingInfo.paintDirtyRect || clipRect.hasRadius()) {
m_clipRecorder = wrapUnique(new LayerClipRecorder(
context, *layer.layoutObject(), DisplayItem::kClipLayerFilter,
clipRect,
&paintingInfo, LayoutPoint(), paintFlags));
+ }
if (!context.getPaintController().displayItemConstructionIsDisabled()) {
CompositorFilterOperations compositorFilterOperations =
The CQ bit was checked by schenney@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from ajuma@chromium.org Link to the patchset: https://codereview.chromium.org/2555283002/#ps60001 (title: "Rebased")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481294723183830,
"parent_rev": "8a13694642d7bbf97a225ab256c9de0ffb4f5073", "commit_rev":
"1bc71cc7f828579a1308974c2df77ac5d424f53c"}
Message was sent while issue was closed.
Description was changed from ========== Add mask layer and offsetFromLayoutObject info to GraphicsLayer JSON R=ajuma BUG=672077 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ========== to ========== Add mask layer and offsetFromLayoutObject info to GraphicsLayer JSON R=ajuma BUG=672077 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2555283002 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Add mask layer and offsetFromLayoutObject info to GraphicsLayer JSON R=ajuma BUG=672077 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2555283002 ========== to ========== Add mask layer and offsetFromLayoutObject info to GraphicsLayer JSON R=ajuma BUG=672077 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Committed: https://crrev.com/b0246be63c67f341421ee64196857479e7e2b9ef Cr-Commit-Position: refs/heads/master@{#437549} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/b0246be63c67f341421ee64196857479e7e2b9ef Cr-Commit-Position: refs/heads/master@{#437549} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
