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

Issue 559733005: Move painting code from RenderBoxModelObject into BoxPainter. (Closed)

Created:
6 years, 3 months ago by chrishtr
Modified:
6 years, 3 months ago
Reviewers:
pdr.
CC:
blink-reviews, blink-reviews-rendering, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, rune+blink, slimming-paint-reviews_chromium.org, zoltan1
Project:
blink
Visibility:
Public.

Description

Move painting code from RenderBoxModelObject into BoxPainter. BUG=412088 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181880

Patch Set 1 #

Total comments: 5

Patch Set 2 : Fix bug, address comments. #

Patch Set 3 : Fix bug #

Patch Set 4 : Remove bad assert #

Patch Set 5 : Fix #

Patch Set 6 : gs #

Total comments: 2

Patch Set 7 : Make one method const. #

Patch Set 8 : Fixed. #

Patch Set 9 : Merged. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+780 lines, -734 lines) Patch
M Source/core/core.gypi View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -0 lines 0 comments Download
A Source/core/paint/BackgroundImageGeometry.h View 1 chunk +79 lines, -0 lines 0 comments Download
A Source/core/paint/BackgroundImageGeometry.cpp View 1 chunk +41 lines, -0 lines 0 comments Download
M Source/core/paint/BoxPainter.h View 1 2 3 4 5 6 1 chunk +12 lines, -0 lines 0 comments Download
M Source/core/paint/BoxPainter.cpp View 1 2 3 4 5 6 7 4 chunks +629 lines, -3 lines 0 comments Download
M Source/core/rendering/InlineFlowBox.cpp View 1 2 3 4 5 3 chunks +4 lines, -3 lines 0 comments Download
M Source/core/rendering/RenderBox.h View 1 1 chunk +2 lines, -1 line 0 comments Download
M Source/core/rendering/RenderBox.cpp View 1 2 3 4 5 6 7 8 3 chunks +3 lines, -2 lines 0 comments Download
M Source/core/rendering/RenderBoxModelObject.h View 4 chunks +1 line, -81 lines 0 comments Download
M Source/core/rendering/RenderBoxModelObject.cpp View 7 chunks +0 lines, -639 lines 0 comments Download
M Source/core/rendering/RenderImage.cpp View 1 2 3 4 5 6 7 8 2 chunks +2 lines, -1 line 0 comments Download
M Source/core/rendering/RenderObject.h View 1 2 3 4 5 6 7 8 2 chunks +1 line, -2 lines 0 comments Download
M Source/core/rendering/RenderReplaced.cpp View 2 chunks +2 lines, -1 line 0 comments Download
M Source/core/rendering/RenderWidget.cpp View 2 chunks +2 lines, -1 line 0 comments Download

Messages

Total messages: 28 (10 generated)
chrishtr
https://codereview.chromium.org/559733005/diff/1/Source/core/rendering/RenderBox.h File Source/core/rendering/RenderBox.h (right): https://codereview.chromium.org/559733005/diff/1/Source/core/rendering/RenderBox.h#newcode635 Source/core/rendering/RenderBox.h:635: bool getBackgroundPaintedExtent(LayoutRect&); This is fallout from BoxPainter::m_renderBox not yet ...
6 years, 3 months ago (2014-09-10 21:56:33 UTC) #2
pdr.
This looks great. LGTM with a nit and a comment. https://codereview.chromium.org/559733005/diff/1/Source/core/rendering/RenderBox.cpp File Source/core/rendering/RenderBox.cpp (right): https://codereview.chromium.org/559733005/diff/1/Source/core/rendering/RenderBox.cpp#newcode1108 ...
6 years, 3 months ago (2014-09-11 05:59:13 UTC) #3
chrishtr
https://codereview.chromium.org/559733005/diff/1/Source/core/rendering/RenderBox.cpp File Source/core/rendering/RenderBox.cpp (right): https://codereview.chromium.org/559733005/diff/1/Source/core/rendering/RenderBox.cpp#newcode1108 Source/core/rendering/RenderBox.cpp:1108: BoxPainter(*this).calculateBackgroundImageGeometry(0, style()->backgroundLayers(), backgroundRect, geometry); On 2014/09/11 at 05:59:13, pdr ...
6 years, 3 months ago (2014-09-11 17:45:53 UTC) #4
pdr.
On 2014/09/11 17:45:53, chrishtr wrote: > > Could we add a fixme here so we ...
6 years, 3 months ago (2014-09-11 17:47:04 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patchset/559733005/40001
6 years, 3 months ago (2014-09-11 18:08:43 UTC) #7
commit-bot: I haz the power
Try jobs failed on following builders: linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/24468)
6 years, 3 months ago (2014-09-11 18:55:00 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patchset/559733005/60001
6 years, 3 months ago (2014-09-11 20:39:30 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/24484)
6 years, 3 months ago (2014-09-11 21:19:08 UTC) #13
chrishtr
I ran into a bug, which was that InlineFlowBox's renderer inherits directly from RenderBoxModelObject. I ...
6 years, 3 months ago (2014-09-11 23:25:44 UTC) #14
pdr.
https://codereview.chromium.org/559733005/diff/100001/Source/core/paint/BoxPainter.h File Source/core/paint/BoxPainter.h (right): https://codereview.chromium.org/559733005/diff/100001/Source/core/paint/BoxPainter.h#newcode28 Source/core/paint/BoxPainter.h:28: static void paintFillLayerExtended(RenderBoxModelObject&, const PaintInfo&, const Color&, const FillLayer&, ...
6 years, 3 months ago (2014-09-11 23:28:06 UTC) #15
pdr.
Explicit LGTM
6 years, 3 months ago (2014-09-11 23:38:11 UTC) #16
chrishtr
https://codereview.chromium.org/559733005/diff/100001/Source/core/paint/BoxPainter.h File Source/core/paint/BoxPainter.h (right): https://codereview.chromium.org/559733005/diff/100001/Source/core/paint/BoxPainter.h#newcode28 Source/core/paint/BoxPainter.h:28: static void paintFillLayerExtended(RenderBoxModelObject&, const PaintInfo&, const Color&, const FillLayer&, ...
6 years, 3 months ago (2014-09-11 23:40:19 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patchset/559733005/120001
6 years, 3 months ago (2014-09-11 23:41:50 UTC) #19
commit-bot: I haz the power
Try jobs failed on following builders: linux_blink_dbg on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/24893)
6 years, 3 months ago (2014-09-12 00:15:33 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patchset/559733005/140001
6 years, 3 months ago (2014-09-12 04:21:07 UTC) #23
commit-bot: I haz the power
Failed to apply patch for Source/core/rendering/RenderImage.cpp: While running patch -p1 --forward --force --no-backup-if-mismatch; patching file ...
6 years, 3 months ago (2014-09-12 04:21:18 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patchset/559733005/160001
6 years, 3 months ago (2014-09-12 04:31:17 UTC) #27
commit-bot: I haz the power
6 years, 3 months ago (2014-09-12 05:40:23 UTC) #28
Message was sent while issue was closed.
Committed patchset #9 (id:160001) as 181880

Powered by Google App Engine
This is Rietveld 408576698