diegomura / react-pdf

πŸ“„ Create PDF files using React
https://react-pdf.org
MIT License
14.9k stars 1.18k forks source link

Content in pages with dynamic components can get squashed #2476

Open mskec opened 10 months ago

mskec commented 10 months ago

Describe the bug Content in pages with dynamic components can get squashed and rendered incorrectly. Let's say we have a document with fixed header (dynamic content), and other content that wraps from page to page.

<Document>
  <Page size={{ height: 500, width: 200 }}>
    <View fixed render={() => <Text style={{ height: 50 }}>Header</Text>} />
    <View style={{ height: 900, backgroundColor: 'lightgray' }} />
    <Text>Hello World</Text>
  </Page>
</Document>

The current behaviour is creating a layout with 2 pages (numbers are heights):

Page 1 => header: 50, content: 450
Page 2 => header: 50, content: 401, text: 20

The expected behavior is to have 3 pages (numbers are heights):

Page 1 => header: 50, content: 450
Page 2 => header: 50, content: 450
Page 3 => header: 50, text: 20

From doing some debugging directly in the library, it seems like the content is fit into a shorter page. Somewhere along the way, the page is assigned a wrong height which squashes the content.

To Reproduce REPL with the reproduction case.

Proposed solution There is one solution that works and fixes the wrong page height, but it requires reseting page dimensions and extra relayout call in resolveDynamicPage. The extra relayout has significant performance impact when a template splits in many sub pages.

const resolveDynamicPage = (props, page, fontStore, yoga) => {
  if (shouldResolveDynamicNodes(page)) {
    const resolvedPage = resolveDynamicNodes(props, page);
    let savedHeight;
    if (resolvedPage.type === 'PAGE' && props?.pageNumber === 1) {
      savedHeight = resolvedPage.box?.height;
      delete resolvedPage.box.height;
    }
    let relayoutedPage = relayoutPage(resolvedPage, fontStore, yoga);

    if (resolvedPage.type === 'PAGE' && props?.pageNumber === 1) {
      const boxHeight = relayoutedPage.box?.height;
      const styleHeight = relayoutedPage.style?.height || savedHeight;

      if (styleHeight - boxHeight > SAFETY_THRESHOLD) {
        relayoutedPage = resetDimensions(relayoutedPage);
        relayoutedPage.box.height = styleHeight;
        relayoutedPage = relayoutPage(relayoutedPage, fontStore, yoga);
      }
    }
    return relayoutedPage;
  }

  return page;
};

Potentially similar issues https://github.com/diegomura/react-pdf/issues/2380, https://github.com/diegomura/react-pdf/issues/2436,

jafeth-calderon commented 9 months ago

Currently having the same problem and related to #2380 and #2436 as well. With or without fixed elements the rendering of dynamic components seems like a problem, text gets squashed to try and fit more content in one page apparently.

jgo80 commented 9 months ago

@mskec I just patch-packaged node_modules/@react-pdf/layout/lib/index.cjs with your approach and this works brilliant! I am gonna live with the performance decrease for now - due to the urgent need of production usage. But I hope there will be an official solution soon 🀞

There was a small typo in your proposed solution: SAFTY_THRESHOLD vs SAFETY_THRESHOLD. Also the yoga prop was missing.

// Temporary fix through Patch Package
var resolveDynamicPage = function resolveDynamicPage(props, page, fontStore, yoga) {
  if (shouldResolveDynamicNodes(page)) {
    var resolvedPage = resolveDynamicNodes(props, page);
    var savedHeight;
    if (resolvedPage.type === 'PAGE' && props?.pageNumber === 1) {
      savedHeight = resolvedPage.box?.height;
      delete resolvedPage.box.height;
    }
    var relayoutedPage = relayoutPage(resolvedPage, fontStore, yoga);
    if (resolvedPage.type === 'PAGE' && props?.pageNumber === 1) {
      const boxHeight = relayoutedPage.box?.height;
      const styleHeight = relayoutedPage.style?.height || savedHeight;

      if (styleHeight - boxHeight > SAFETY_THRESHOLD) {
        relayoutedPage = resetDimensions(relayoutedPage);
        relayoutedPage.box.height = styleHeight;
        relayoutedPage = relayoutPage(relayoutedPage, fontStore);
      }
    }

    return relayoutedPage
  }
  return page;
};
mskec commented 9 months ago

Hi @jgo80, thanks for typos, I updated the code. Glad it's working for you as well. We are running this successfully since the last summer.

jgo80 commented 9 months ago

Hi @jgo80, thanks for typos, I updated the code. Glad it's working for you as well. We are running this successfully since the last summer.

@mskec sorry to bother - you did not specify where you've got resetDimensions from. Is it a custom function? It does not live in node_modules/@react-pdf/layout/lib/index.cjs. Thx for your help!

mskec commented 9 months ago

Here is our resetDimensions:

export const resetDimensions = node => {
  const newNode = Object.assign({}, node, { box: {} });

  if (!node.children) return newNode;

  const children = node.children.map(resetDimensions);

  return Object.assign({}, newNode, { children });
};
anonfreak commented 8 months ago

@mskec I implemented your solution for a really long pdf, unfortunately on the last two pages the phenomenon will occur again. It seems that on the last and last but one page the content gets squashed again, because trailing dynamic height won't be calculated correctly to the last and last but one page. Does someone also experienced this? (The border shows the View which gets wrapped throught all pages)

image
mskec commented 8 months ago

@anonfreak, do you have a minimal reproduction that I can try with our internal version of the library?

anonfreak commented 8 months ago

@mskec I found the problem myself now... I tried to wrap={false} text blocks (headline + text) in my flexlayout on the right side. This caused the height renderings to fail, when it needed to break on a new page to keep one view together. We now just removed the wrap={false} views and see there: works! I think it's a compromise, which we accept for now. But if you like to debug this further i can provide you with an reproduction.

jgo80 commented 2 months ago

I revised my workaround posted earlier today. For any reason my old version only recalculated page 1 but not the subsequent. Here is the updated code (hello future me, this is what you're looking for):

// Temporary fix through Patch Package
// Elminiates https://github.com/diegomura/react-pdf/issues/2476
// Content in pages with dynamic components can get squashed
var resetDimensions = function resetDimensions(node) {
  var newNode = Object.assign({}, node, { box: {} });
  if (!node.children) return newNode;
  var children = node.children.map(resetDimensions);
  return Object.assign({}, newNode, { children });
};

var resolveDynamicPage = function resolveDynamicPage(props, page, fontStore, yoga) {
  if (shouldResolveDynamicNodes(page)) {
    var resolvedPage = resolveDynamicNodes(props, page);

    var savedHeight;
    if (resolvedPage.type === 'PAGE') {
      savedHeight = resolvedPage.box?.height;
      delete resolvedPage.box.height;
    }
    var relayoutedPage = relayoutPage(resolvedPage, fontStore, yoga);

    if (resolvedPage.type === 'PAGE') {
      var boxHeight = relayoutedPage.box?.height;
      var styleHeight = relayoutedPage.style?.height || savedHeight;

      if (styleHeight - boxHeight > SAFETY_THRESHOLD) {
        relayoutedPage = resetDimensions(relayoutedPage);
        relayoutedPage.box.height = styleHeight;
        relayoutedPage = relayoutPage(relayoutedPage, fontStore, yoga);
      }
    }

    return relayoutedPage;
  }
  return page;
};

I still patch, I would really love to see this implemented, thanks for considering 😌

mskec commented 2 months ago

We are still running internal fork with this and other patches.

The problem with this suggested solution is eg. when you try to render a large list which overflows many pages. Because of constant relayoutPage calls, things slow down exponentially. There's a way to implement a template to counter it, but it's a nightmare to maintain.

I wonder if anyone sees a better solution to prevent content from getting squashed.