New issue
Advanced search Search tips

Issue 873387 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 23
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Task



Sign in to add a comment

Clean up BlockPainter.

Project Member Reported by vmp...@chromium.org, Aug 10

Issue description

BlockPainter (https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/paint/block_painter.h?l=24) is responsible for painting LayoutBlocks (https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/layout/layout_block.cc?g=0&l=876). 

It's a pretty complicated algorithm which is outlined in https://www.w3.org/TR/CSS21/zindex.html

We should clean up the BlockPainter class and refactor it, comment it, in a way that has a clear mapping from the spec to the code.
 
Status: Fixed (was: Assigned)
Fixed in https://chromium-review.googlesource.com/c/chromium/src/+/1175226
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 23

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

commit b474444cbdfc77dbb1e840540e0c352b439e08e0
Author: Mason Freed <masonfreed@chromium.org>
Date: Thu Aug 23 02:02:17 2018

[CI] Refactoring BlockPainter and BlockFlowPainter

Previously, BlockPainter's PaintObject method:
 1) was rather uncommented, particularly as it related to its correspondence
    to the W3C "Appendix E" spec for stacking context painting (at
    https://www.w3.org/TR/CSS21/zindex.html).
 2) was a bit convoluted, in the parts that dealt with painting LayoutBlockFlow
    objects. BlockPainter handled painting most parts of painting both
    LayoutBlocks and LayoutBlockFlows, except for in two places. In those
    places, a BlockFlowPainter was constructed and called, and then
    at one point in BlockFlowPainter::PaintContents, a BlockPainter
    was constructed and PaintContents called on that. Functional but confusing.

With this CL, comments are added to link BlockPainter to Appendix E, and
BlockFlowPainter is eliminated. That code is folded back into BlockPainter,
inline where it was called.

Bug:  873387 
Change-Id: Idc041eacbcfadd1df1a610f81a2a98de024b7d49
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/1175226
Commit-Queue: Mason Freed <masonfreed@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585366}
[modify] https://crrev.com/b474444cbdfc77dbb1e840540e0c352b439e08e0/third_party/blink/renderer/core/paint/BUILD.gn
[delete] https://crrev.com/d8e750e79e63fa0539c0d018db01ab3b1365587b/third_party/blink/renderer/core/paint/block_flow_painter.cc
[delete] https://crrev.com/d8e750e79e63fa0539c0d018db01ab3b1365587b/third_party/blink/renderer/core/paint/block_flow_painter.h
[modify] https://crrev.com/b474444cbdfc77dbb1e840540e0c352b439e08e0/third_party/blink/renderer/core/paint/block_painter.cc
[modify] https://crrev.com/b474444cbdfc77dbb1e840540e0c352b439e08e0/third_party/blink/renderer/core/paint/block_painter.h
[modify] https://crrev.com/b474444cbdfc77dbb1e840540e0c352b439e08e0/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.h
[modify] https://crrev.com/b474444cbdfc77dbb1e840540e0c352b439e08e0/third_party/blink/renderer/core/paint/object_painter.cc
[modify] https://crrev.com/b474444cbdfc77dbb1e840540e0c352b439e08e0/third_party/blink/renderer/core/paint/object_painter.h
[modify] https://crrev.com/b474444cbdfc77dbb1e840540e0c352b439e08e0/third_party/blink/renderer/core/paint/paint_phase.h

Sign in to add a comment