-
Notifications
You must be signed in to change notification settings - Fork 500
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dry run doesn't always return correct height #789
Comments
Here's another point to consider. When Asciidoctor PDF was first released, it couldn't draw a background behind a block that split across pages. So we applied "keep together" logic to most panel-like blocks (listing, sidebar, example, etc). That's no longer necessary. We could return to splitting blocks across pages and only using "keep together" when requested. |
Hey, I promised to look at this a while ago, but then life happened and I got pulled off to a bunch of other tasks (I'm sure you can relate :)). So I've read through what you've written above, and it all makes sense to me - certainly it matches up to what I was thinking previously. Also very much +1 to your last comment. There is a slight wrinkle in the ointment which could cause an issue, possibly, though it could be deliberately not handled in the first iteration of this. Let's say we fix #578, and make "keep_together" configurable for all blocks. Then someone does a "keep together" open block, and within that, another keep_together block (doesn't matter what). The behaviour I'd like to see in that case is that first the whole lot tries to fit onto a single page, and if it can't, you then try to see if the sub-element can fit onto a single page (and so on and so forth - hopefully that makes sense?). That maybe is something we can punt on for a while, but whenever someone tries to fix this, it'd be nice to at least have the notion of doing this in mind. It might just fall out of how it's coded. |
I'm in the process of working on this. Here's the solution (assuming keep together logic):
|
This issue has been replaced by #2003 which summarizes the new logic being implemented to compute the extent of a block's boundaries. |
The dry run logic doesn't always return the correct height if the content exceeds the height of a page. The problem has to do with page boundaries.
When we want to pre-calculate the height of a document fragment, we move to a scratch document, clone the page settings, and write into the document. We then measure the distance from where the cursor started to where the cursor ended, taking into account page traversals.
However, when we do this, we always start the cursor at the top of the page in the scratch document, regardless of where the cursor is in the main document. When we go back to write the content in the main document, we often end up with a different result because of where the content falls in relation to the page boundaries.
The fix seems simple. Just start at the same position in the scratch document that we do in the main document. The problem is, we sometimes want to know if the content will fit on a single page. In that case, we need to start at the top of the document so we get a clean measurement. This is necessary for the "keep together" logic.
If we're doing "keep together" logic, then we first need to start at the top of the document and write until we hit the end of the page. We need to intercept this event and stop writing. We know we can't fit in a single page. We then need to back up, move the cursor to the location it is in the main document and start again, this time proceeding until we finish writing. We return the height and a flag that says we can't fit on a single page.
If we aren't doing "keep together" logic, then there is no reason to do the first step. We can just move the cursor to the same location it is in the main document, write all the content, then measure.
If we rewrite the logic as described, we'll always get accurate measurements. We'll also have the ability to use dry run for either "keep together" logic or to simply pre-calculate the height of content. This will enable features like #578.
In short, here's the logic for keep together:
Here's the logic when not using keep together:
NOTE: We might be able to avoid a dry run in obvious cases; a kind of engineering estimate.
The text was updated successfully, but these errors were encountered: