Cc: msten...@opera.com chrishtr@chromium.org Summary: Paint properties for multicol contents (was: PaintPropertyTreeBuilder generates wrong paintOffset for multicolumn contents)
The issue is more complicated than just the paintOffset. The mapping from multicol flow thread space to multicol container space is currently not represented in paint properties.
We need to answer the following questions:
- What space should paint offsets of the paint properties be in?
- Should we create multicol node in paint property tree?
Blocking:-646176 Cc: wangxianzhu@chromium.org Owner: ---- Status: Available (was: Assigned)
For now we workaround this issue in PaintInvalidator for slimmingPaintInvalidation by forcing slow path rect mapping for multicol contents, so we can unblock bug 646176 from this bug.
Mark this bug Available because I'm unlikely to work on it soon.
Layout tests affected by this bug in slimmingPaintInvalidation mode (for now they are passing with the workaround):
paint/invalidation/column-rule-change.html
paint/invalidation/column-rules-fixed-height.html
paint/invalidation/multicol-as-paint-container.html
paint/invalidation/multicol-nested.html
paint/invalidation/multicol-repaint.html
paint/invalidation/multicol-with-abspos-in-relpos.html
paint/invalidation/multicol-with-block.html
paint/invalidation/multicol-with-inline.html
paint/invalidation/multicol-with-relpos.html
paint/invalidation/multicol-with-text.html
This is mainly for SPv2.
The SlimmingPaintInvalidation path is forcing slow path for multicol, and all of the above tests pass except virtual/spinvalidation/fast/multicol/span/abspos-containing-block-outside-spanner.html. Will file another bug for it.
After much reflection, I have concluded that:
1. We should not represent the mapping from flow thread to
container space in property trees. It should instead be treated as a purely
paint effect, within the 2D transform space of pagination.
2. "Paint offset" should mean paint offset only for the first fragment, even
if there are multiple.
*** Why: adding support in property trees more or less will end up implying
that then entire system, including cc, will need to understand it (or else
we'd end up with a bunch of weird corner case and hacks in PAC to avoid that).
Also, as shown below it appears that the current status quo can be preserved
in SPv2 just fine with minimal changes to the code.
*** Implications:
a. SPv2 GeometryMapper-type code explicitly does not support fragmenting
content across property tree spaces, in particular transforms and effects.
The fragmentation code in layout should be implemented to not allow
fragmenting of transforms (including scroll) or effects.
(Overflow) clips inside of multicolumn are a special case, which are already
when possible, and should continue to be in the future, implemented explicitly
during paint, since fragment clips are not representable in the property trees.
b. Paint invalidation should continue to use the "slow" path for multicolumn
content when computing visual rects. If we were clever enough to know that
there was only one fragment, it seems we could avoid the slow path, but that
is an optimization for the future.
c. Paint invalidation should (?) be able to not special-case multicolumn when
detecting object movement via paint offset changes.
d. The pre-paint tree walk will have to take into account fragmentation in
order to determine the correct paint offset per (2).
e. Code which references "paint offset" should be careful to do so only in
cases where the computation does not cross a pagination container boundary.
Example: if there is content under a subtree of property tree nodes which
are in turn under a pagination container + transform/effect, we can do
geometry relative to them correctly without reference to pagination. On
the other hand, we cannot use "paint offset" during paint. IOW. We still
need to dofragmentation during paint like PaintLayerPainter is already doing.
Based on these conclusions, I have a couple of action items before closing this
bug. I think (d) needs coding to implement correctly. I also need to remove/
adjust the comments referencing this bug.
What would we do for the following example?
http://jsbin.com/bexoziruvu/quiet/
This has a transformed div that's in the second column of a multicolumn. What positions the transform node for the green box to be in the second column?
Re comment 12:
The paint offset of #transform would be its painting position before applying
the transform, i.e. in the second column. This is what positions it there.
This kind of example was part of why I concluded that paint offset should point
to the "first fragment visual location".
So we'll basically just keep the paint offset up-to-date considering columns during pre-paint? I like the simplicity of that approach.
In (a) you say we'll disable fragmentation of transforms. What would that mean for an example like http://output.jsbin.com/pidujamawi/quiet/? In this example, we have a transformed div spans two columns--would we change to just overflow the green div out of the first column and not paint into the second column?
Where's the slow path multicolumn code for visual rects?
Update: comment 18 is correct, my plan was too simple/naive.
New plan:
In the pre-paint tree walk:
1. For LayoutObjects which have PaintLayers and are under a fragmentation
container, compute its fragments, and store them as a linked list of
ObjectPaintProperties. (LayoutObjects which have no fragmentation will end
up with just one entry in this linked list.)
2. LayoutObjects which are fragmented and have a transform or effect
will get a new transform or effect node for each of its fragments, with
its origin set as appropriate, and parented under the corresponding
fragment for its PaintingLayer() ancestor.
3. LayoutObjects which have PaintLayers and are under a fragmentation
container will receive a new "fragmentation clip" on each of their
fragments' ObjectPaintProperties, parented in the same way as effects and
transforms.
4. Scroll will reset the fragmentation container to nullptr.
Paint invalidation code for computing visual rects of fragmented
LayoutObjects will work according to this algorithm:
Algorithm VisualRect(LayoutObject object, LayoutObject ancestor):
if (HasFragmentation(object) && FragmentationRoot(object) == FragmentationRoot(ancestor)) {
FloatRect local_visual_rect = LocalVisualRect()+ Offset(object, object.PaintingLayer()
FloatRect visual_rect;
For each fragment in fragments:
ObjectPaintProperties properties = PropertiesFor(object.PaintProperties(), fragment))
ObjectPaintProperties ancestor_properties = PropertiesFor(object.PaintProperties(), fragment))
visual_rect.Unite(
GeometryMapper::mapToVisualRect(properties, ancestor_properties, local_visual_rect))
} else if (HasFragmentation(object) || HasFragmentation(ancestor)) {
// Fall back to slow path?
} else {
Behave as normal.
}
Paint code will use the fragments from the pre-paint tree walk to paint, rather than
re-computing them.
Comment 1 by wangxianzhu@chromium.org
, Sep 26 2016Summary: Paint properties for multicol contents (was: PaintPropertyTreeBuilder generates wrong paintOffset for multicolumn contents)