Prevent image taller than page from creating empty page at start of block

[mojavelinux]

When an image taller than the height of a page is placed as the first child of a block (an element with a background and/or border), the converter leaves behind an empty page at the start of the block. This happens because the converter advances the page in order to give the image the most amount of space it can to fit. However, the image already had the most amount of space it could have under the circumstances. As a result, the converter skips a perfectly good page, which is not what the reader expects to see.

This issue has a dependency on #2003 (resolved).

[mojavelinux]

This issue can be addressed by making the following two changes:

1. When arranging a block, set the at-top attribute on the node to the page number if the block falls at the top of the page (do this inside of the arrange_block proc).
2. When arranging an image, and the cursor is not at the top of the page, check if the image is the first child and if the value of the parent's at-top attribute matches the current page number. If the parent is at the page top, don't advance the image to a new page.

[mojavelinux]

It's tempting, instead, to want to check if the extent has an empty first page and shift the start of the extent. However, that approach has problems when moving between the scratch and main document.

[mojavelinux]

When dry_run is complete, we're checking whether we are at the top of a new page. This may need to be changed to check for an empty page since rendering the image may push the cursor down. It's something to check if we are ending up with an extra page.

[mojavelinux]

I decided to go with the simpler solution, which is to mark an image that does not fit in available height as held and do not advance page during a dry run in the case the current page is empty. If this mark is still set during rendering, do not advance the page and clear the mark. This keeps the change scoped to the image handling.

[mojavelinux]

I'll leave the diff here for the original proposal in case we want to return to it:

diff --git lib/asciidoctor/pdf/converter.rb lib/asciidoctor/pdf/converter.rb
index dcec4f98..31e8e40e 100644
--- lib/asciidoctor/pdf/converter.rb
+++ lib/asciidoctor/pdf/converter.rb
@@ -1819,7 +1819,7 @@ def convert_image node, opts = {}
               end
               # NOTE: shrink image so it fits within available space; group image & caption
               if (rendered_h = svg_size.output_height) > (available_h = cursor - caption_h)
-                unless pinned || at_page_top?
+                unless pinned || at_page_top? || (node.first_child? && (node.parent.attr? 'at-top', page_number))
                   advance_page
                   available_h = cursor - caption_h
                 end
@@ -1860,7 +1860,7 @@ def convert_image node, opts = {}
               rendered_w, rendered_h = image_info.calc_image_dimensions width: (width || [available_w, actual_w].min)
               # NOTE: shrink image so it fits within available space; group image & caption
               if rendered_h > (available_h = cursor - caption_h)
-                unless pinned || at_page_top?
+                unless pinned || at_page_top? || (node.first_child? && (node.parent.attr? 'at-top', page_number))
                   advance_page
                   available_h = cursor - caption_h
                 end
diff --git lib/asciidoctor/pdf/ext/asciidoctor/abstract_block.rb lib/asciidoctor/pdf/ext/asciidoctor/abstract_block.rb
index daf13c01..fbc175c0 100644
--- lib/asciidoctor/pdf/ext/asciidoctor/abstract_block.rb
+++ lib/asciidoctor/pdf/ext/asciidoctor/abstract_block.rb
@@ -13,6 +13,10 @@ def last_child
     blocks[-1]
   end
 
+  def first_child?
+    self == parent.blocks[0]
+  end
+
   def last_child?
     self == parent.blocks[-1]
   end
diff --git lib/asciidoctor/pdf/ext/prawn/extensions.rb lib/asciidoctor/pdf/ext/prawn/extensions.rb
index c37c4df4..a5cd8ba3 100644
--- lib/asciidoctor/pdf/ext/prawn/extensions.rb
+++ lib/asciidoctor/pdf/ext/prawn/extensions.rb
@@ -990,7 +990,11 @@ def with_dry_run &block
       # start_new_page. Using start_new_page can mangle the calculation of content block's extent.
       #
       def arrange_block node, &block
-        keep_together = (node.option? 'unbreakable') && !at_page_top?
+        if (at_top = at_page_top?)
+          node.set_attr 'at-top', page_number
+        elsif node.option? 'unbreakable'
+          keep_together = true
+        end
         doc = node.document
         block_for_scratch = proc do
           push_scratch doc
@@ -1000,6 +1004,8 @@ def arrange_block node, &block
         end
         extent = dry_run keep_together: keep_together, onto: [self, keep_together], &block_for_scratch
         scratch? ? block_for_scratch.call : (yield extent)
+      ensure
+        node.remove_attr 'at-top' if at_top
       end

Note that this proposal only works for a single level of block nesting. It tracks whether the current parent started at the top of the page and assumes the image does do if it's the first block.

[mojavelinux]

It turns out setting the marker on the image block itself isn't going to work. The reason for this is that dry run creates new pages (and tares the content on the first page), so it will look like the current page is empty even when it is not. So back to the other solution ;)

[mojavelinux]

Here's the diff for the solution that sets the marker, which we have determined does not work:

diff --git lib/asciidoctor/pdf/converter.rb lib/asciidoctor/pdf/converter.rb
index dcec4f98..f7ea79cc 100644
--- lib/asciidoctor/pdf/converter.rb
+++ lib/asciidoctor/pdf/converter.rb
@@ -1819,7 +1819,7 @@ def convert_image node, opts = {}
               end
               # NOTE: shrink image so it fits within available space; group image & caption
               if (rendered_h = svg_size.output_height) > (available_h = cursor - caption_h)
-                unless pinned || at_page_top?
+                unless pinned || at_page_top? || (scratch? ? page.empty? && (node.set_attr 'pdf-hold', '') : (node.remove_attr 'pdf-hold'))
                   advance_page
                   available_h = cursor - caption_h
                 end
@@ -1860,7 +1860,7 @@ def convert_image node, opts = {}
               rendered_w, rendered_h = image_info.calc_image_dimensions width: (width || [available_w, actual_w].min)
               # NOTE: shrink image so it fits within available space; group image & caption
               if rendered_h > (available_h = cursor - caption_h)
-                unless pinned || at_page_top?
+                unless pinned || at_page_top? || (scratch? ? page.empty? && (node.set_attr 'pdf-hold', '') : (node.remove_attr 'pdf-hold'))
                   advance_page
                   available_h = cursor - caption_h
                 end