asciidoctor / asciidoctor-pdf

:page_with_curl: Asciidoctor PDF: A native PDF converter for AsciiDoc based on Asciidoctor and Prawn, written entirely in Ruby.
https://docs.asciidoctor.org/pdf-converter/latest/
MIT License
1.14k stars 500 forks source link

asciidoctor-pdf version 1.5.0.beta.1 broke PDF styling #1182

Closed chuck-confluent closed 5 years ago

chuck-confluent commented 5 years ago

Before with asciidoctor-pdf 1.5.0.alpha16:

Screen Shot 2019-07-31 at 3 01 56 PM

After with asciidoctor-pdf 1.5.0.beta1:

Screen Shot 2019-07-31 at 3 00 52 PM

mojavelinux commented 5 years ago

You likely need to specify the size and position of the background image. Something like:

:title-page-background-image: image:bg.png[fit=contain,position=top]

The sizing, scaling, and positioning defaults were very inconsistent before. Now they're all run through the same method so the same logic is always used.

I am curious why it isn't fitting by default, though. Could you point me at the repo?

chuck-confluent commented 5 years ago

Thanks for replying @mojavelinux ! I'm afraid I can't point you to the repo since it's private. I will, however, point my colleagues to your response and we can investigate. Thanks!

mojavelinux commented 5 years ago

It would be really helpful if you could either send me a zip or set up a temporary repository with the minimum viable reproducible case so that I can investigate too. If that doesn't work, just send me the background image and the attribute declaration so I can at least try it with that image.

chuck-confluent commented 5 years ago

Here is a zip of the fonts, images and yml declarations:

ascii-pdf-themes.zip

Thanks again for looking into it!

mojavelinux commented 5 years ago

Thanks Chuck!

mojavelinux commented 5 years ago

Oh, I see what it is. You're specifying a background image for every content page as a way of putting a line below the running header. The background image is now positioned in the center of the page by default. To change this, you can use the position attribute.

Here's the current line in your theme you need to update.

page:
  ...
  background_image: images/top_divider_ig.png

Here's how it should be:

page:
  ...
  background_image: image:images/top_divider_ig.png[position=top]

That will restore the previous layout.

When I try your theme, the title page looks fine. It's the content pages that were the problem.

mojavelinux commented 5 years ago

Btw, you don't have to use a white image to tare the background of the title page. Instead, you can use:

title_page:
  background_image: none

That will remove the background image just for the title page.

(I'm not sure why you have the page background color set to black. You may wan to change that to white.)

chuck-confluent commented 5 years ago

Thanks Dan!

Tagging @gnschenker and @russau -- I will file this as an issue in our training-tools repo and we can work out when to fix this

chuck-confluent commented 5 years ago

@mojavelinux Hey Dan, I tried to make the changes you suggested, and the regular pages are back to normal but the title pages aren't adhering to the title page declaration:

Screen Shot 2019-08-02 at 6 34 54 PM

Looking at this documentation, I see that the proper format for the title page is title-page rather than title_page, but the result is the same. I also tried changing the logo image from

logo:
  image: images/color_logo.png

to

logo:
  image: image:images/color_logo.png[]

but that didn't work either. Let me know if I'm doing something obviously wrong. The fact that the zip I sent you worked well before is also confusing me.

mojavelinux commented 5 years ago

It sounds to me like you gems got messed up (since this is now thoroughly tested). I recommend installing clean or trying another computer. I'm just not getting the results you are.

Btw, underscore and hyphen in the theme are interchangeable.

chuck-confluent commented 5 years ago

@mojavelinux I'm running on the ruby:alpine3.10 docker image, so I'm confident it's not a configuration issue, unless there's an issue with apk

mojavelinux commented 5 years ago

Is there any way you could give me temporary access to this private repository? Since this seems to be an issue with Asciidoctor PDF, I would like to track down the problem. You can grab my e-mail from the git history.

mojavelinux commented 5 years ago

How are you installing Asciidoctor and Asciidoctor PDF in the container?

mojavelinux commented 5 years ago

Could you also provide a sample AsciiDoc file that reproduces the issue? There could be a dependency here on how the document is structured, such as its doctype or document attributes.

mojavelinux commented 5 years ago

Btw, here's how the page, base, and title-page keys should look:

page:
  layout: portrait
  # 36 is equivalent to 0.5in
  margin: [0.5in,0.5in,0.5in,0.7in]
  size: LETTER
  background_image: image:images/top_divider_ig.png[position=top]
base:
  align: left
  font:
    family: Roboto
    color: #000000
    size: 12
  line:
    height: 1.15
    height_length: 12
  text_indent: 100
  logo:
    top: 0%
    align: left
title_page:
  align: center
  logo:
    image: images/color_logo.png
    top: 10%
    align: center
  title:
    top: 30%
    left: 15%
    font_size: 48
    font_color: 000000
  background_image: none
  subtitle_font_size: 36
  authors_margin_top: 12

Notice that the background_image for the title page is none. Unless you set that, it's going to use the background image set on the page.

mojavelinux commented 5 years ago

I tried the configuration I just shared in the docker container and it works as expected.

Here are the steps I took:

  1. Launch container
    podman run -it --rm --entrypoint=sh --privileged -v `pwd`:/workspace -w /workspace ruby:alpine3.10
  2. Add the following Gemfile:
    source 'https://rubygems.org'
    gem 'asciidoctor-pdf'
  3. Install the gems using:
    $ bundle --path=.bundle/gems
  4. Run Asciidoctor PDF:
    $ bundle exec asciidoctor-pdf -a pdf-theme=./handbook-theme.yml -a pdf-fontsdir=./fonts example.adoc

Here's the title page:

basic-example-1

mojavelinux commented 5 years ago

The fact that the zip I sent you worked well before is also confusing me.

This is understandable because a lot has changed with how the background images are proceeded. There were loads of bugs in the logic, and those bugs have been ironed out. You were likely relying on a bug ;)

chuck-confluent commented 5 years ago

Thanks! This is a lot to dig into on a Saturday, so I'll drop my Dockerfile and check back in on Monday:

FROM ruby:alpine3.10

RUN apk add --no-cache ghostscript \
    && gem install asciidoctor \
    && gem install asciidoctor-pdf --version 1.5.0.beta.2 \
    && gem install rouge \
    && gem install asciidoctor-rouge \
    && gem install concurrent-ruby \
    && mkdir -p /app
WORKDIR /app
COPY ./entrypoint.sh ./
COPY ./themes ./themes
CMD ./entrypoint.sh 

The command I was using inside the container (after some volume mounts to get everything into the right place) is:

asciidoctor-pdf -a pdf-themesdir=/app/themes \
    -a pdf-theme=ig \
    -a pdf-fontsdir=/app/themes/fonts \
    -o build/artifacts/test1.pdf /app/data/build/staging/modules/02-fundamentals.adoc

I notice I used to use pdfstylesdir and pdfstyle, but had to change them to pdf-themesdir and pdf-theme.

I am not using the bundle command. Maybe this is the issue. Maybe I'm missing some necessary gems.

I will also change the theme yml to more closely resemble yours. Thanks again! I'll investigate more soon.

mojavelinux commented 5 years ago

FYI: You don't need asciidoctor-rouge anymore. That gem is obsolete. Asciidoctor provides Rouge integration out of the box.

concurrent-ruby is installed automatically when you install asciidoctor-pdf.

mojavelinux commented 5 years ago

I notice I used to use pdfstylesdir and pdfstyle, but had to change them to pdf-themesdir and pdf-theme.

They are aliases, though "theme" is the preferred terminology moving forward.

mojavelinux commented 5 years ago

I am not using the bundle command. Maybe this is the issue. Maybe I'm missing some necessary gems.

It shouldn't make a different, though I believe that using Bundler gives you more control and consistency. Think of it like Maven, Gradle, or npm. It keeps the dependencies with the project.

mojavelinux commented 5 years ago

I will also change the theme yml to more closely resemble yours.

I'm rather confident at this point that the problem is that you aren't setting the background image on the title page to none.

chuck-confluent commented 5 years ago

This is a fun puzzle. I'm still thoroughly confused. I trimmed down the dockerfile per your suggestion to try to reduce the chance of conflicting gems:

FROM ruby:alpine3.10

RUN apk add --no-cache ghostscript \
    && gem install asciidoctor-pdf --version 1.5.0.beta.2 \
    && mkdir -p /app
WORKDIR /app
COPY ./entrypoint.sh ./
COPY ./themes ./themes
CMD ./entrypoint.sh 

I also copypasta'd your theme, only adding the proper font information and called it example-theme.yml:

page:
  layout: portrait
  # 36 is equivalent to 0.5in
  margin: [0.5in,0.5in,0.5in,0.7in]
  size: LETTER
  background_image: image:images/top_divider_ig.png[position=top]
base:
  align: left
  font:
    family: Roboto
    color: #000000
    size: 12
  line:
    height: 1.15
    height_length: 12
  text_indent: 100
  logo:
    top: 0%
    align: left
title_page:
  align: center
  logo:
    image: images/color_logo.png
    top: 10%
    align: center
  title:
    top: 30%
    left: 15%
    font_size: 48
    font_color: 000000
  background_image: none
  subtitle_font_size: 36
  authors_margin_top: 12

font:
  catalog:
    Roboto:
      normal: Roboto-Regular.ttf
      bold: Roboto-Bold.ttf
      italic: Roboto-Italic.ttf
      bold_italic: Roboto-BoldItalic.ttf

    Carlito:
      normal: carlito-normal.ttf
      bold: carlito-bold.ttf
      italic: carlito-italic.ttf
      bold_italic: carlito-bold_italic.ttf

    Inconsolata:
      normal: inconsolata-regular.ttf
      bold: inconsolata-bold.ttf
  fallbacks: [Carlito]

I built the docker image and ran:

asciidoctor-pdf -a pdf-themesdir=/app/themes \
    -a pdf-theme=example \
    -a pdf-fontsdir=/app/themes/fonts \
    -o /app/data/build/artifacts/test1.pdf /app/data/build/staging/modules/02-fundamentals.adoc

I got the following result:

Screen Shot 2019-08-03 at 9 48 10 PM

The first page of the adoc looks like this:

:artifact-type: IG
:module-number: <<module-number>>
:imagesdir: ./images/fundamentals
:source-highlighter: rouge
:icons: font

= Fundamentals of Apache Kafka
Module {module-number}

<<<

Really nothing special. Just a title and a subtitle.

mojavelinux commented 5 years ago

Aha! Now the truth is revealed. You're document doesn't actually have a title page.

Here's how it should look:

= Fundamentals of Apache Kafka: Module {module-number}
:artifact-type: IG
:imagesdir: ./images/fundamentals
:source-highlighter: rouge
:icons: font
:title-page:

CONTENT GOES HERE

(assuming you pass -a module-number=16 to the processor)

A few things going on here.

First, if you use the article doctype (which is the default), you don't get a title page by default. You need to either use the book doctype (:doctype: book) or set the title-page attribute. See footnote [8] in this section: https://github.com/asciidoctor/asciidoctor-pdf/blob/master/docs/theming-guide.adoc#theme-related-document-attributes And there's no need for a force page break if you enable the title page.

Second, there should be no blank lines in the document header. The preferred order is to use the doctitle and put the attribute entries directly underneath it. Alternately, you can put the attribute entries above the doctitle line. But there should be no blank lines in the document header. See https://asciidoctor.org/docs/user-manual/#doc-header

Lastly, if you want a subtitle, it should be embedded in the doctitle line, offset by a colon. See https://asciidoctor.org/docs/user-manual/#document-subtitle

chuck-confluent commented 5 years ago

Success! Thanks so much! It makes me wonder why it ever worked in the first place. But like you said, there were a lot of changes to the title page rendering logic, so we were probably benefitting from a bug. This should make our document base more resistant to breakages in the future. Woohoo!

mojavelinux commented 5 years ago

:tada:

But like you said, there were a lot of changes to the title page rendering logic, so we were probably benefitting from a bug.

Exactly. Before we had a test suite, Asciidoctor PDF was wild west (and why it was still in alpha). Now we have a very robust test suite and we were able to test out a lot of these inconsistencies.

This should make our document base more resistant to breakages in the future.

:100: