[Webkit-unassigned] [Bug 61256] Print HTML with only 1 line, then 2 pages are printed

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=61256





--- Comment #22 from Kentaro Hara <haraken at google.com>  2011-05-27 04:21:00 PST ---
Thank you for the review. The things are a bit complicated and I would like to ask you how we should fix this problem. 

> I don't understand where computePageRects() is called from in your example. The test calls layoutTestController.numberOfPages(), but PrintContext::numberOfPages() never invokes computePageRects().

The call stack of the test is as follows:

LayoutTestController::numberOfPages() @ WebKit/Tools/DumpRenderTree/chromium/LayoutTestController.cpp
WebFrameImpl::printBegin() @ WebKit/Source/WebKit/chromium/src/WebFrameImpl.cpp
PrintContext::computePageRects() @ WebKit/Source/WebCore/page/PrintContext.cpp

I think that this call path makes sense. This call path does essentially the same calculation as PrintContext::numberOfPages(). (The reason why WebFrameImpl::printBegin() does not call PrintContext::numberOfPages() is just that WebFrameImpl holds PrintContext's instance but the PrintContext::numberOfPages() is not an instance method.) 

Anyway, the problem is the rounding error caused by transformation between int and float. Specifically, the following error does happen:

int WebFrameImpl::printBegin(pageSize) {
    ...;
    FloatRect rect(0, 0, static_cast<float>(pageSize.width), static_cast<float>(pageSize.height));
    m_printContext->begin(rect.width(), rect.height());
    m_printContext->computePageRects(rect, 0, 0, 1.0, pageHeight);
    ...;
    return m_printContext->pageCount();
}

Initially, assume that the |pageSize| is [539, 755] and thus |rect| is [539.0, 755.0]. Then, m_printContext->begin(...) calls PrintContext::begin():

void PrintContext::begin(float width, float height) // [539.0, 755.0]
{   
    ...;                                                                                                                    
    float minLayoutWidth = width * printingMinimumShrinkFactor; // 539.0 * 1.25 = 673.75
    float minLayoutHeight = height * printingMinimumShrinkFactor; // 755.0 * 1.25 = 943.75
    ...;
    m_frame->setPrinting(..., FloatSize(minLayoutWidth, minLayoutHeight), ...);
}

Then, m_frame->setPrinting(...) calls FrameView::forceLayoutForPagination(...) and it rounds [673.75, 943.75] to [673, 943], which will be later accessed by view->documentRect(). 

Finally, WebFrameImpl::printBegin() calls m_printContext->computePageRects(...) and the following calculation happens:

void PrintContext::computePageRects(printRect) { // printRect is [539, 755]
    ...;
    const IntRect& documentRect = view->documentRect();  // [673, 943]
    if (isHorizontal) {
        float ratio = printRect.height() / printRect.width(); // 539 / 755
        pageWidth = documentRect.width(); // 539
        pageHeight = floorf(pageWidth * ratio); // floorf(673 * (539 / 755)) = 942
    }
    ...;
    computePageRectsWithPageSizeInternal(FloatSize(pageWidth, pageHeight, ...);
}

void PrintContext::computePageRectsWithPageSizeInternal(pageWidth, pageHeight, ...) {
    ...;
    docLogicalHeight = documentRect.height(); // 943
    pageLogicalHeight = pageHeight; // 942
    pageCount = ceilf((float)docLogicalHeight / pageLogicalHeight); // ceilf(943 / 942) = 2!!!
    ...;
}

As you can see, the problem is that we lose accuracy by transforming float to int. Thus, I guess that the essential solution for this problem is to change the code so that all the widths and heights are managed by float (never int!!). However, since this change requires very large modification to the whole printing codes, I would like to seek more modest solution. 

Then, my idea is to allow the rounding error smaller than 1.0, (although there is no ground for the value "1.0", as you mentioned). Would you please give me your opinion?

nit:
If my idea is acceptable, the following change may be better than my current patch (that allows rounding error in PrintContext::computePageRects()):

void PrintContext::computePageRectsWithPageSizeInternal(...) {
    ...;
    if (docLogicalHeight - pageLogicalHeight <= 1)
        pageCount = 1;
    else
        pageCount = ceilf((float)docLogicalHeight / pageLogicalHeight);
    ...;
}

Sorry for this long reply.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.