Bug 247857

Summary: Get rid of subtract-one-whole-pixel hack in Multi-column
Product: WebKit Reporter: Ahmad Saleem <ahmad.saleem792>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Normal CC: bfulgham, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
URL: https://jsfiddle.net/7ouv0qbr/show

Description Ahmad Saleem 2022-11-13 04:02:05 PST
Hi Team,

Just going through Blink Commits, I noticed another commit, where we have still one pixel hack present in our code.

Webkit GitHub - https://github.com/WebKit/WebKit/blob/5a9504aef82d929917a680a05a3fc17f7f6b4dde/Source/WebCore/rendering/RenderMultiColumnSet.cpp#LL816 & https://github.com/WebKit/WebKit/blob/5a9504aef82d929917a680a05a3fc17f7f6b4dde/Source/WebCore/rendering/RenderMultiColumnSet.cpp#L821 & 

Blink Commit - https://chromium.googlesource.com/chromium/blink/+/b0bf11d2c22b4b0a6775322b9dd955cdfe9f8c19

JSFiddle Failing - https://jsfiddle.net/1o9fy8gx/show (Firefox Nightly 108 shows line below & Chrome Canary 110 was broken and was not rendering this for some time)

Just wanted to raise this bug so we can get rid of it as well.

Thanks!
Comment 1 Radar WebKit Bug Importer 2022-11-20 04:03:15 PST
<rdar://problem/102559272>
Comment 2 Ahmad Saleem 2023-11-03 03:08:51 PDT
LayoutUnit repaintLogicalBottom = (isHorizontalWritingMode() ? fragmentedFlowRepaintRect.maxY() : fragmentedFlowRepaintRect.maxX());
    
    // Figure out the start and end columns for the layer and only check within that range so that
    // we don't walk the entire column row.
    unsigned startColumn;
    unsigned endColumn;
    columnIntervalForBlockRangeInFragmentedFlow(repaintLogicalTop, repaintLogicalBottom, startColumn, endColumn);

and

void RenderMultiColumnSet::columnIntervalForBlockRangeInFragmentedFlow(LayoutUnit logicalTopInFragmentedFlow, LayoutUnit logicalBottomInFragmentedFlow, unsigned& firstColumn, unsigned& lastColumn) const
{
    ASSERT(logicalTopInFragmentedFlow <= logicalBottomInFragmentedFlow);
    firstColumn = columnIndexAtOffset(logicalTopInFragmentedFlow);
    lastColumn = columnIndexAtOffset(logicalBottomInFragmentedFlow);
    // logicalBottomInFragmentedFlow is an exclusive endpoint, so some additional adjustments may be necessary.
    if (lastColumn > firstColumn && LayoutUnit(logicalTopInFragmentedFlow(lastColumn)) == LayoutUnit(logicalBottomInFragmentedFlow))
        --lastColumn;
}

__

^ this might work. Didn't tried.
__

I attached unprefixed test case in URL field.