Multi-page PDF rendering broken

[michaelgmiller]

1. Which version of PhantomJS are you using? Tip: run phantomjs --version.
   Master
2. What steps will reproduce the problem?
   1. Render a webpage to a PDF which spans over 1 page
3. Which operating system are you using?
   CentOS
4. Did you use binary PhantomJS or did you compile it from source?
   Compile from source
5. Please provide any additional information below.
   n/a

[Connorhd]

What sha of qtbase did you build against? I'm wondering if this is fixed in qt 5.6

[michaelgmiller]

@Connorhd I used the version of qtbase that's in master: b5cc008

[Travyse]

Also having this problem.

[michaelgmiller]

friendly ping. can we please revert the commit?

[ricokahler]

I'm also having this issue and I think I know why it's happening. Before, the QWebFrame was printed by a QPrinter using the slot QWebFrame::print(...), now the PDF is being rendered with QWebFrame::render(...) with a QPdfWriter. I'm thinking the render() method doesn't have any logic for new pages or uhh printing stuff.

I actually desperately need the renderBase64('pdf') so I'll looking into a fix for this.

[ricokahler]

Yup, just like i thought. This is the logic for QWebPage::render(...)

    /*!
Render the frame into \a painter.
*/
void QWebFrame::render(QPainter* painter)
{
    if (!d->frame->view())
        return;

    GraphicsContext context(painter);
    if (context.paintingDisabled() && !context.updatingControlTints())
        return;

    d->renderRelativeCoords(&context, AllLayers, QRegion(d->frame->view()->frameRect()));
}

and this is the logic for QWebPage::print(...):

#ifndef QT_NO_PRINTER
/*!
    Prints the frame to the given \a printer.
    \sa render()
*/
void QWebFrame::print(QPrinter *printer) const
{
#if HAVE(QTPRINTSUPPORT)
    QPainter painter;
    if (!painter.begin(printer))
        return;

    const qreal zoomFactorX = (qreal)printer->logicalDpiX() / qt_defaultDpi();
    const qreal zoomFactorY = (qreal)printer->logicalDpiY() / qt_defaultDpi();

    PrintContext printContext(d->frame);
    float pageHeight = 0;

    QRect qprinterRect = printer->pageRect();

    IntRect pageRect(0, 0,
                    int(qprinterRect.width() / zoomFactorX),
                    int(qprinterRect.height() / zoomFactorY));

    printContext.begin(pageRect.width(), pageRect.height());

    printContext.computePageRects(pageRect, /* headerHeight */ 0, /* footerHeight */ 0, /* userScaleFactor */ 1.0, pageHeight);

    int docCopies;
    int pageCopies;
    if (printer->collateCopies()) {
        docCopies = 1;
        pageCopies = printer->numCopies();
    } else {
        docCopies = printer->numCopies();
        pageCopies = 1;
    }

    int fromPage = printer->fromPage();
    int toPage = printer->toPage();
    bool ascending = true;

    if (fromPage == 0 && toPage == 0) {
        fromPage = 1;
        toPage = printContext.pageCount();
    }
    // paranoia check
    fromPage = qMax(1, fromPage);
    toPage = qMin(static_cast<int>(printContext.pageCount()), toPage);
    if (toPage < fromPage) {
        // if the user entered a page range outside the actual number
        // of printable pages, just return
        return;
    }

    if (printer->pageOrder() == QPrinter::LastPageFirst) {
        int tmp = fromPage;
        fromPage = toPage;
        toPage = tmp;
        ascending = false;
    }

    painter.scale(zoomFactorX, zoomFactorY);
    GraphicsContext ctx(&painter);

    for (int i = 0; i < docCopies; ++i) {
        int page = fromPage;
        while (true) {
            for (int j = 0; j < pageCopies; ++j) {
                if (printer->printerState() == QPrinter::Aborted
                    || printer->printerState() == QPrinter::Error) {
                    printContext.end();
                    return;
                }
                printContext.spoolPage(ctx, page - 1, pageRect.width());
                if (j < pageCopies - 1)
                    printer->newPage();
            }

            if (page == toPage)
                break;

            if (ascending)
                ++page;
            else
                --page;

            printer->newPage();
        }

        if ( i < docCopies - 1)
            printer->newPage();
    }

    printContext.end();
#endif // HAVE(PRINTSUPPORT)
}
#endif // QT_NO_PRINTER

I might be overlooking something but it looks like render(...) never calls newPage() lol

sigh

[JvanderHeide]

Could this also be the reason that currently my (and a lot of other's) PDF's are rendered "too large"? Compared to actually using print functionality in my browser and generation the text and images in generated PDF's are like 1.4-1.5 times larger.

[michaelgmiller]

@JvanderHeide that's a separate issue. I have a hacky workaround in #13997.

[JvanderHeide]

@michaelgmiller Thank you for your reply, that's not what I ment however.
I see some stuff regarding zooming in the QWebPage::print(...): that's not in the QWebPage::render(...);. That's why I thought it might be the cause of this:

Left Safari Print -> Save as PDF, Right PhantomJS PDF generation.
(Chrome looks like Safari in terms of image and text sizes)

So actual "rendering size" instead of "file size". Not looking to highjack this issue just wondering if they might be related. Also seeing lots of other people with similar issues, with multi-page rendering and people having stuff bleed on to the next page when it shouldn't or items just being partially out of view.

To me it looks like it could relate to issues like:
#12936, #14138, #12685, #13417, #13742, #13524, ...

[ricokahler]

@JvanderHeide I'm also having the issue with the page being rendered too big. What operating system did you use to render that PDF? On ubuntu, phantom renders the text bigger than chrome would. However in windows, the text renders the same size as chrome would.

I can tell you that my current patch does not fix this issue. I actually didn't scale the painter at all because in the current qtwebkit that phantomJS uses scales the painter by:

Edit: my patch now fixes the issue

const qreal zoomFactorX = (qreal)printer->logicalDpiX() / printer->resolution() which works out to a zoomFactor of 1 all the time.

I found that printer->logicalDpiX() and printer->resolution() will always hold the same value. See QPdfEngine::metric() and QPdfPrintEngine::metric() with the case QPaintDevice::PdmDpiX in the qtBase.

In the current github repo of qtbase, the code divides by qt_defaultDpi(). Compare with the current qtWebkit that phantom uses

I think this scaling is the issue. I'm gonna try to patch it and I'll post my results back here because my patch to fix the multi-paged rendering should scale the painter appropriately. @michaelgmiller 's patch for embedded fonts might fix another issue but on the simple page i'm testing on, the text is selectable.

[ricokahler]

Welp @JvanderHeide, my PDFs now have the same size as chrome but it only worked when i hard coded the DPI to 96.

I think qt_defaultDpi() returns 72 on linux but 96 on windows. If this property is just replaced with a hard-coded 96, it works and it works consistently.

[michaelgmiller]

@ricokahler Are you still working on this? Would be awesome to get a fix landed for phantomjs 2.5!

[ricokahler]

@michaelgmiller sure, give me a week

[michaelgmiller]

@ricokahler any luck?

[jayvelasco1990]

Hello, is there any progress in this? The memory leak is solved, but now we cannot have multiple PDF pages. Is there a timeline for next release and what will be included? @ariya

[stale]

Due to our very limited maintenance capacity (see #14541 for more details), we need to prioritize our development focus on other tasks. Therefore, this issue will be automatically closed. In the future, if we see the need to attend to this issue again, then it will be reopened. Thank you for your contribution!