New issue
Advanced search Search tips

Issue 743221 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task


Participants' hotlists:
layoutng


Sign in to add a comment

Refactor BoxPainter::PaintFillLayer

Project Member Reported by e...@chromium.org, Jul 14 2017

Issue description

BoxPainter::PaintFillLayer is used to paint LayoutBoxes, LayoutInlines, and LayoutViews and has a lot of branching and special case logic.
 

Comment 1 by e...@chromium.org, Jul 14 2017

Status: Started (was: Available)
Labels: BugSource-Chromium PaintTeamTriaged-20170717
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc76ea2e43c179b98bc8e299dfe141a4087b472d

commit dc76ea2e43c179b98bc8e299dfe141a4087b472d
Author: Emil A Eklund <eae@chromium.org>
Date: Tue Jul 18 01:56:41 2017

Move BoxPainter::PaintFillLayer to BoxModelObjectPainter

The static BoxPainter::PaintFillLayer method is used to paint LayoutBox,
LayoutInline, and LayoutView content. Thus it is marked static and takes
a LayoutBoxModelObject (rather than a LayoutBox) as LayoutInline isn't a
sub-class of LayoutBox. This isn't ideal and requires unsafe casting and
makes code sharing across legacy layout and LayoutNG problematic.

This change moves the BoxPainter::PaintFillLayer method into a new class
called BoxModelObjectPainter that can be used to paint blocks & inlines.

Bug:  743221 
Change-Id: Ib7c9ef6095a4034d2c56a6a1c6d081245b494198
Reviewed-on: https://chromium-review.googlesource.com/572336
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487338}
[modify] https://crrev.com/dc76ea2e43c179b98bc8e299dfe141a4087b472d/third_party/WebKit/Source/core/paint/BUILD.gn
[add] https://crrev.com/dc76ea2e43c179b98bc8e299dfe141a4087b472d/third_party/WebKit/Source/core/paint/BoxModelObjectPainter.cpp
[add] https://crrev.com/dc76ea2e43c179b98bc8e299dfe141a4087b472d/third_party/WebKit/Source/core/paint/BoxModelObjectPainter.h
[modify] https://crrev.com/dc76ea2e43c179b98bc8e299dfe141a4087b472d/third_party/WebKit/Source/core/paint/BoxPainter.cpp
[modify] https://crrev.com/dc76ea2e43c179b98bc8e299dfe141a4087b472d/third_party/WebKit/Source/core/paint/BoxPainter.h
[modify] https://crrev.com/dc76ea2e43c179b98bc8e299dfe141a4087b472d/third_party/WebKit/Source/core/paint/BoxPainterBase.cpp
[modify] https://crrev.com/dc76ea2e43c179b98bc8e299dfe141a4087b472d/third_party/WebKit/Source/core/paint/InlineFlowBoxPainter.cpp
[modify] https://crrev.com/dc76ea2e43c179b98bc8e299dfe141a4087b472d/third_party/WebKit/Source/core/paint/ViewPainter.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/743d373765288f763465be423f313dbe0d027b37

commit 743d373765288f763465be423f313dbe0d027b37
Author: Emil A Eklund <eae@chromium.org>
Date: Tue Jul 18 17:45:44 2017

Also Move BoxPainter::PaintFillLayers to BoxModelObjectPainter

Follow up to the CL that moved PaintFillLayer to BoxModelObjectPainter,
this change moves the PaintFillLayers method to the box model class.

Bug:  743221 
Change-Id: Iefcb91e4e2ec21e338fa0d6f6ea00f5a87ebaa7a
Reviewed-on: https://chromium-review.googlesource.com/572146
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487518}
[modify] https://crrev.com/743d373765288f763465be423f313dbe0d027b37/third_party/WebKit/Source/core/paint/BoxModelObjectPainter.cpp
[modify] https://crrev.com/743d373765288f763465be423f313dbe0d027b37/third_party/WebKit/Source/core/paint/BoxModelObjectPainter.h
[modify] https://crrev.com/743d373765288f763465be423f313dbe0d027b37/third_party/WebKit/Source/core/paint/BoxPainter.cpp
[modify] https://crrev.com/743d373765288f763465be423f313dbe0d027b37/third_party/WebKit/Source/core/paint/BoxPainter.h
[modify] https://crrev.com/743d373765288f763465be423f313dbe0d027b37/third_party/WebKit/Source/core/paint/FieldsetPainter.cpp
[modify] https://crrev.com/743d373765288f763465be423f313dbe0d027b37/third_party/WebKit/Source/core/paint/InlineFlowBoxPainter.cpp
[modify] https://crrev.com/743d373765288f763465be423f313dbe0d027b37/third_party/WebKit/Source/core/paint/TableCellPainter.cpp

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c686dec1a1253f02cbde035b086717bbd4588ea8

commit c686dec1a1253f02cbde035b086717bbd4588ea8
Author: Emil A Eklund <eae@chromium.org>
Date: Wed Jul 19 00:28:17 2017

Remove unused PaintInsetBoxShadowInBounds method

The BoxPainterBase::PaintInsetBoxShadowInBounds is only called from the
BoxPainterBase::PaintInsetBoxShadow method. Remove the former signature
and merge the two implementations.

Bug:  743221 
Change-Id: I45c12a143a8362c0400b263ed210d5dfe34c22a8
Reviewed-on: https://chromium-review.googlesource.com/576661
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487689}
[modify] https://crrev.com/c686dec1a1253f02cbde035b086717bbd4588ea8/third_party/WebKit/Source/core/paint/BoxPainterBase.cpp
[modify] https://crrev.com/c686dec1a1253f02cbde035b086717bbd4588ea8/third_party/WebKit/Source/core/paint/BoxPainterBase.h

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a61014fad30efbfc90135865dc7d79299ce7e44d

commit a61014fad30efbfc90135865dc7d79299ce7e44d
Author: Emil A Eklund <eae@chromium.org>
Date: Wed Jul 19 01:19:55 2017

Remove unused BoxPainterBase::BoundsForDrawingRecorder definition

Bug:  743221 
Change-Id: Ib4ec6f8a9ed8de3b63b55d32419d8f4b5f4f9b39
Tbr: ikilpatrick@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/576787
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487703}
[modify] https://crrev.com/a61014fad30efbfc90135865dc7d79299ce7e44d/third_party/WebKit/Source/core/paint/BoxPainterBase.h

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2ae8ffc7be3b2d3f87c261b210a2538602282942

commit 2ae8ffc7be3b2d3f87c261b210a2538602282942
Author: Emil A Eklund <eae@chromium.org>
Date: Tue Aug 01 07:49:13 2017

Move shared BoxModelObjectPainter logic to BoxPainterBase

Move box painting logic that can be shared across LayoutNG/legacy layout
to BoxPainterBase. Layout implementation specific logic, such as scroll;
text; and inline handling, is handled by the appropriate sub-class.

Bug:  743221 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I2d10403f955a0d18ab63b175fe250b7780225835
Reviewed-on: https://chromium-review.googlesource.com/576386
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490908}
[modify] https://crrev.com/2ae8ffc7be3b2d3f87c261b210a2538602282942/third_party/WebKit/Source/core/layout/LayoutBoxModelObject.h
[modify] https://crrev.com/2ae8ffc7be3b2d3f87c261b210a2538602282942/third_party/WebKit/Source/core/paint/BoxModelObjectPainter.cpp
[modify] https://crrev.com/2ae8ffc7be3b2d3f87c261b210a2538602282942/third_party/WebKit/Source/core/paint/BoxModelObjectPainter.h
[modify] https://crrev.com/2ae8ffc7be3b2d3f87c261b210a2538602282942/third_party/WebKit/Source/core/paint/BoxPainter.cpp
[modify] https://crrev.com/2ae8ffc7be3b2d3f87c261b210a2538602282942/third_party/WebKit/Source/core/paint/BoxPainter.h
[modify] https://crrev.com/2ae8ffc7be3b2d3f87c261b210a2538602282942/third_party/WebKit/Source/core/paint/BoxPainterBase.cpp
[modify] https://crrev.com/2ae8ffc7be3b2d3f87c261b210a2538602282942/third_party/WebKit/Source/core/paint/BoxPainterBase.h
[modify] https://crrev.com/2ae8ffc7be3b2d3f87c261b210a2538602282942/third_party/WebKit/Source/core/paint/FieldsetPainter.cpp
[modify] https://crrev.com/2ae8ffc7be3b2d3f87c261b210a2538602282942/third_party/WebKit/Source/core/paint/InlineFlowBoxPainter.cpp
[modify] https://crrev.com/2ae8ffc7be3b2d3f87c261b210a2538602282942/third_party/WebKit/Source/core/paint/ListMarkerPainter.cpp
[modify] https://crrev.com/2ae8ffc7be3b2d3f87c261b210a2538602282942/third_party/WebKit/Source/core/paint/TableCellPainter.cpp
[modify] https://crrev.com/2ae8ffc7be3b2d3f87c261b210a2538602282942/third_party/WebKit/Source/core/paint/TableRowPainter.cpp
[modify] https://crrev.com/2ae8ffc7be3b2d3f87c261b210a2538602282942/third_party/WebKit/Source/core/paint/TableSectionPainter.cpp
[modify] https://crrev.com/2ae8ffc7be3b2d3f87c261b210a2538602282942/third_party/WebKit/Source/core/paint/TextPainterBase.cpp
[modify] https://crrev.com/2ae8ffc7be3b2d3f87c261b210a2538602282942/third_party/WebKit/Source/core/paint/ViewPainter.cpp
[modify] https://crrev.com/2ae8ffc7be3b2d3f87c261b210a2538602282942/third_party/WebKit/Source/platform/geometry/LayoutRectOutsets.h

Comment 8 by e...@chromium.org, Aug 2 2017

Status: Fixed (was: Started)

Sign in to add a comment