Changeset 143395 in webkit


Ignore:
Timestamp:
Feb 19, 2013 3:23:39 PM (11 years ago)
Author:
hyatt@apple.com
Message:

[New Multicolumn] REGRESSION: RenderMultiColumnSets broken by the RenderRegion -> RenderBlock subclassing.
https://bugs.webkit.org/show_bug.cgi?id=110239.

Reviewed by Simon Fraser.

Source/WebCore:

Test: fast/multicol/newmulticol/column-rules-fixed-height.html

  • rendering/RenderBlock.cpp:

(WebCore::RenderBlock::columnRectAt):
Make sure the columnGap() in the old multicolumn code is always expressed as a LayoutUnit. This was the
one place where it was still an int.

  • rendering/RenderFlowThread.cpp:

(WebCore::RenderFlowThread::paintFlowThreadPortionInRegion):
Rework the painting of flow thread portions to account for the fact that regions paint at an integral
translation. This means you have to construct clipping around that integral destination. Subpixel layout
regions did not clip correctly as a result of this issue.

  • rendering/RenderMultiColumnSet.cpp:

(WebCore::RenderMultiColumnSet::columnRectAt):
Fix the same bug with columnGap() that the old column code has, i.e., one spot where it was an int.

(WebCore::RenderMultiColumnSet::paintObject):
RenderMultiColumnSet should be using paintObject and not paint and it needs to check for visibility
and phases now that it is a RenderBlock subclass.

(WebCore::RenderMultiColumnSet::paintColumnRules):
Fix the bug that Opera guys fixed in the old multi-column code. They didn't patch the new code, so this
takes care of that.

  • rendering/RenderMultiColumnSet.h:

(RenderMultiColumnSet):
Change to use paintObject instead of paint.

LayoutTests:

  • fast/multicol/newmulticol: Added.
  • fast/multicol/newmulticol/column-rules-fixed-height-expected.html: Added.
  • fast/multicol/newmulticol/column-rules-fixed-height.html: Added.
Location:
trunk
Files:
3 added
6 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r143393 r143395  
     12013-02-19  David Hyatt  <hyatt@apple.com>
     2
     3        [New Multicolumn] REGRESSION: RenderMultiColumnSets broken by the RenderRegion -> RenderBlock subclassing.
     4        https://bugs.webkit.org/show_bug.cgi?id=110239.
     5
     6        Reviewed by Simon Fraser.
     7
     8        * fast/multicol/newmulticol: Added.
     9        * fast/multicol/newmulticol/column-rules-fixed-height-expected.html: Added.
     10        * fast/multicol/newmulticol/column-rules-fixed-height.html: Added.
     11
    1122013-02-19  Christian Biesinger  <cbiesinger@chromium.org>
    213
  • trunk/Source/WebCore/ChangeLog

    r143389 r143395  
     12013-02-19  David Hyatt  <hyatt@apple.com>
     2
     3        [New Multicolumn] REGRESSION: RenderMultiColumnSets broken by the RenderRegion -> RenderBlock subclassing.
     4        https://bugs.webkit.org/show_bug.cgi?id=110239.
     5
     6        Reviewed by Simon Fraser.
     7
     8        Test: fast/multicol/newmulticol/column-rules-fixed-height.html
     9
     10        * rendering/RenderBlock.cpp:
     11        (WebCore::RenderBlock::columnRectAt):
     12        Make sure the columnGap() in the old multicolumn code is always expressed as a LayoutUnit. This was the
     13        one place where it was still an int.
     14
     15        * rendering/RenderFlowThread.cpp:
     16        (WebCore::RenderFlowThread::paintFlowThreadPortionInRegion):
     17        Rework the painting of flow thread portions to account for the fact that regions paint at an integral
     18        translation. This means you have to construct clipping around that integral destination. Subpixel layout
     19        regions did not clip correctly as a result of this issue.
     20
     21        * rendering/RenderMultiColumnSet.cpp:
     22        (WebCore::RenderMultiColumnSet::columnRectAt):
     23        Fix the same bug with columnGap() that the old column code has, i.e., one spot where it was an int.
     24
     25        (WebCore::RenderMultiColumnSet::paintObject):
     26        RenderMultiColumnSet should be using paintObject and not paint and it needs to check for visibility
     27        and phases now that it is a RenderBlock subclass.
     28
     29        (WebCore::RenderMultiColumnSet::paintColumnRules):
     30        Fix the bug that Opera guys fixed in the old multi-column code. They didn't patch the new code, so this
     31        takes care of that.
     32
     33        * rendering/RenderMultiColumnSet.h:
     34        (RenderMultiColumnSet):
     35        Change to use paintObject instead of paint.
     36
    1372013-02-19  Branimir Lambov  <blambov@google.com>
    238
  • trunk/Source/WebCore/rendering/RenderBlock.cpp

    r143357 r143395  
    54245424    LayoutUnit colLogicalTop = borderBefore() + paddingBefore();
    54255425    LayoutUnit colLogicalLeft = logicalLeftOffsetForContent();
    5426     int colGap = columnGap();
     5426    LayoutUnit colGap = columnGap();
    54275427    if (colInfo->progressionAxis() == ColumnInfo::InlineAxis) {
    54285428        if (style()->isLeftToRightDirection() ^ colInfo->progressionIsReversed())
  • trunk/Source/WebCore/rendering/RenderFlowThread.cpp

    r143322 r143395  
    256256        return;
    257257
    258     // Adjust the clipping rect for the region.
    259     // paintOffset contains the offset where the painting should occur
    260     // adjusted with the region padding and border.
    261     LayoutRect regionClippingRect = computeRegionClippingRect(paintOffset, flowThreadPortionRect, flowThreadPortionOverflowRect);
     258    // RenderFlowThread should start painting its content in a position that is offset
     259    // from the region rect's current position. The amount of offset is equal to the location of
     260    // the flow thread portion in the flow thread's local coordinates.
     261    // Note that we have to pixel snap the location at which we're going to paint, since this is necessary
     262    // to minimize the amount of incorrect snapping that would otherwise occur.
     263    // If we tried to paint by applying a non-integral translation, then all the
     264    // layout code that attempted to pixel snap would be incorrect.
     265    IntPoint adjustedPaintOffset;
     266    LayoutPoint portionLocation;
     267    if (style()->isFlippedBlocksWritingMode()) {
     268        LayoutRect flippedFlowThreadPortionRect(flowThreadPortionRect);
     269        flipForWritingMode(flippedFlowThreadPortionRect);
     270        portionLocation = flippedFlowThreadPortionRect.location();
     271    } else
     272        portionLocation = flowThreadPortionRect.location();
     273    adjustedPaintOffset = roundedIntPoint(paintOffset - portionLocation);
     274
     275    // The clipping rect for the region is set up by assuming the flowThreadPortionRect is going to paint offset from adjustedPaintOffset.
     276    // Remember that we pixel snapped and moved the paintOffset and stored the snapped result in adjustedPaintOffset. Now we add back in
     277    // the flowThreadPortionRect's location to get the spot where we expect the portion to actually paint. This can be non-integral and
     278    // that's ok. We then pixel snap the resulting clipping rect to account for snapping that will occur when the flow thread paints.
     279    IntRect regionClippingRect = pixelSnappedIntRect(computeRegionClippingRect(adjustedPaintOffset + portionLocation, flowThreadPortionRect, flowThreadPortionOverflowRect));
    262280
    263281    PaintInfo info(paintInfo);
    264     info.rect.intersect(pixelSnappedIntRect(regionClippingRect));
     282    info.rect.intersect(regionClippingRect);
    265283
    266284    if (!info.rect.isEmpty()) {
     
    269287        context->clip(regionClippingRect);
    270288
    271         // RenderFlowThread should start painting its content in a position that is offset
    272         // from the region rect's current position. The amount of offset is equal to the location of
    273         // the flow thread portion in the flow thread's local coordinates.
    274         IntPoint renderFlowThreadOffset;
    275         if (style()->isFlippedBlocksWritingMode()) {
    276             LayoutRect flippedFlowThreadPortionRect(flowThreadPortionRect);
    277             flipForWritingMode(flippedFlowThreadPortionRect);
    278             renderFlowThreadOffset = roundedIntPoint(paintOffset - flippedFlowThreadPortionRect.location());
    279         } else
    280             renderFlowThreadOffset = roundedIntPoint(paintOffset - flowThreadPortionRect.location());
    281 
    282         context->translate(renderFlowThreadOffset.x(), renderFlowThreadOffset.y());
    283         info.rect.moveBy(-renderFlowThreadOffset);
     289        context->translate(adjustedPaintOffset.x(), adjustedPaintOffset.y());
     290        info.rect.moveBy(-adjustedPaintOffset);
    284291       
    285292        layer()->paint(context, info.rect, 0, 0, region, RenderLayer::PaintLayerTemporaryClipRects);
  • trunk/Source/WebCore/rendering/RenderMultiColumnSet.cpp

    r142984 r143395  
    120120    LayoutUnit colLogicalTop = borderBefore() + paddingBefore();
    121121    LayoutUnit colLogicalLeft = borderAndPaddingLogicalLeft();
    122     int colGap = columnGap();
     122    LayoutUnit colGap = columnGap();
    123123    if (style()->isLeftToRightDirection())
    124124        colLogicalLeft += index * (colLogicalWidth + colGap);
     
    206206}
    207207
    208 void RenderMultiColumnSet::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset)
    209 {
    210     // FIXME: RenderRegions are replaced elements right now and so they only paint in the foreground phase.
     208void RenderMultiColumnSet::paintObject(PaintInfo& paintInfo, const LayoutPoint& paintOffset)
     209{
     210    if (style()->visibility() != VISIBLE)
     211        return;
     212
     213    RenderBlock::paintObject(paintInfo, paintOffset);
     214
     215    // FIXME: Right now we're only painting in the foreground phase.
    211216    // Columns should technically respect phases and allow for background/float/foreground overlap etc., just like
    212     // RenderBlocks do. We can't correct this, however, until RenderRegions are changed to actually be
    213     // RenderBlocks. Note this is a pretty minor issue, since the old column implementation clipped columns
     217    // RenderBlocks do. Note this is a pretty minor issue, since the old column implementation clipped columns
    214218    // anyway, thus making it impossible for them to overlap one another. It's also really unlikely that the columns
    215219    // would overlap another block.
     220    if (!m_flowThread || !isValid() || (paintInfo.phase != PaintPhaseForeground && paintInfo.phase != PaintPhaseSelection))
     221        return;
     222
    216223    setRegionObjectsRegionStyle();
    217224    paintColumnRules(paintInfo, paintOffset);
     
    231238    LayoutUnit ruleThickness = blockStyle->columnRuleWidth();
    232239    LayoutUnit colGap = columnGap();
    233     bool renderRule = ruleStyle > BHIDDEN && !ruleTransparent && ruleThickness <= colGap;
     240    bool renderRule = ruleStyle > BHIDDEN && !ruleTransparent;
    234241    if (!renderRule)
    235242        return;
  • trunk/Source/WebCore/rendering/RenderMultiColumnSet.h

    r142984 r143395  
    9595    virtual void computeLogicalHeight(LayoutUnit logicalHeight, LayoutUnit logicalTop, LogicalExtentComputedValues&) const OVERRIDE;
    9696
    97     virtual void paint(PaintInfo&, const LayoutPoint& paintOffset) OVERRIDE;
     97    virtual void paintObject(PaintInfo&, const LayoutPoint& paintOffset) OVERRIDE;
    9898    virtual bool nodeAtPoint(const HitTestRequest&, HitTestResult&, const HitTestLocation&, const LayoutPoint& accumulatedOffset, HitTestAction) OVERRIDE;
    9999
Note: See TracChangeset for help on using the changeset viewer.