[waterfallplot] crashes if requested waterfall range overflows

[frankrolf]

waterfallplot doesn’t like the requested waterfall range to exceed the physical page size.
In other words: the waterfall doesn’t cascade.

waterfallplot -wfr 60-70 font.pfa

Proofing font font.pfa. Start time: Tue Feb 18 14:02:22 2020.
Traceback (most recent call last):
  File "/Users/fgriessh/.pyenv/versions/3.7.4/bin/waterfallplot", line 8, in <module>
    sys.exit(waterfallplot())
  File "/Users/fgriessh/.pyenv/versions/3.7.4/lib/python3.7/site-packages/afdko/proofpdf.py", line 945, in waterfallplot
    main()
  File "/Users/fgriessh/.pyenv/versions/3.7.4/lib/python3.7/site-packages/afdko/proofpdf.py", line 972, in main
    proofMakePDF(params.rt_fileList, params, txPath)
  File "/Users/fgriessh/.pyenv/versions/3.7.4/lib/python3.7/site-packages/afdko/proofpdf.py", line 893, in proofMakePDF
    pdfFilePath = makePDF(pdfFont, params, doProgressBar)
  File "/Users/fgriessh/.pyenv/versions/3.7.4/lib/python3.7/site-packages/afdko/pdflib/fontpdf.py", line 1670, in makePDF
    makeWaterfallPDF(params, pdfFont, doProgressBar)
  File "/Users/fgriessh/.pyenv/versions/3.7.4/lib/python3.7/site-packages/afdko/pdflib/fontpdf.py", line 1992, in makeWaterfallPDF
    numPages = int(ceil(float(numWaterFalls) / numWaterfallsOnPage))
ZeroDivisionError: float division by zero

Test font attached:
font.pfa.zip

[josh-hadley]

Playing around with this a bit, it looks like this happens when it can't fit all of the sizes in the specified range onto the first page (e.g. -wfr 60-67 works, but attempting to add one more size, 68, would put the first line for size 68 onto the second page of the PDF).

What do you think the right behavior here should be? It seems like it should be pretty easy to pre-calculate and catch this condition, so we could warn about such cases, and then either just quit (with advice to reduce the range), or maybe just start chopping off sizes from the upper end until it fits.

I note that specifying a single very large size also causes this problem (e.g. -wfr 1000), not sure how that case should get resolved (although it's arguable that there's a practical upper size limit for waterfalls on A4/US Letter size, probably somewhere < 100).

[readroberts]

I'm for making the use case work. The immediate problem is that numWaterfallsOnPage is an integer, so it evaluates to zero when a single waterfall needs more than a single page, hence the divide by zero. I think problem is fixed by replacing line 1992 with:

if numWaterfallsOnPage > 0:
	numPages = int(ceil(float(numWaterFalls) / numWaterfallsOnPage))
else:
	numPages = int(ceil(float(numWaterFalls)*pageHeight / waterfallHeight))

Of course, this does allow the user to do things like -wfr 1000, which is unreadable, but it doesn't crash, and the user can immediately see why the large point size isn't useful.

[josh-hadley]

A quick test implementation of Read's suggestion works, but I'm not sure about the output, it's not very clean and probably not useful (see font.pdf attachment). But, it doesn't crash, so 🤷‍♂

It seems like the original intent of waterfallplot was to be able to fit all specified sizes onto one page at a time (wrapping all sizes at the same place, so that each page of the result always shows the same size set).

[frankrolf]

@josh-hadley I think the file you attached is mostly fine (please note the point size label getting lost on page 2).
Waterfalls are most useful in smaller sizes, so I expect this scenario to be an edge case anyway.

Two other possible solutions:

• do not collate pages but write the pages for the smaller waterfall first, then the pages for the larger waterfall
• switch paper size to tabloid if the waterfall range exceeds the size of the letter paper format (this might actually be the most useful, since I don’t see a lot of waterfall plots being printed).

Thanks for making those experiments!

[josh-hadley]

It seems like this tool could use some refactoring and reworking in many places, which would be nice to do, but is a bit more of a project. I'm inclined to just go with this simple fix which avoids the crash for the edge case example, and resolve to make deeper changes at a later time.

[frankrolf]

Sounds good! Thank you.