Skip to content
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

[waterfallplot] fix crash #1094

Merged
merged 2 commits into from
Feb 19, 2020
Merged

Conversation

josh-hadley
Copy link
Collaborator

update pdflib/fontpdf.py to address crash
closes #1092

update pdflib/fontpdf.py to address crash
closes #1092
if numWaterfallsOnPage > 0:
numPages = int(ceil(float(numWaterFalls) / numWaterfallsOnPage))
else:
numPages = int(ceil(float(numWaterFalls)*pageHeight / waterfallHeight))
Copy link
Contributor

@cjchapman cjchapman Feb 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the code at line 1996 should be:

numPages = int(ceil(float(numWaterFalls) * waterfallHeight / pageHeight))

to be more like the calculation in line 1994.

Alternatively, perhaps you could replace lines 1993-1996 with:

if numWaterfallsOnPage == 0:
	numWaterfallsOnPage = 1
numPages = int(ceil(float(numWaterFalls) / numWaterfallsOnPage))

similar to what is done in lines 1998-1999.

Copy link
Collaborator Author

@josh-hadley josh-hadley Feb 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried both of these suggestions locally using the example font and command from #1092 , and the only difference that I'm able to discern is the total page count displayed in the header (and timestamp).

In my original commit (using code suggested by @readroberts ), the total page count is shown as 16 (which is incorrect; the actual number of pages is 28).

Using the first method suggested above (change line 1996), the total page count is shown as 26, which is closer, but still wrong. For the alternative method above, the page count is 20

I want to be clear here that the actual number of pages generated for all three of these is 28, and the waterfall content on the pages is identical, apart from the "## of ##" page indicator and timestamp in the headers.

(per discussion in PR, using method that yields number closer to actual count)
@josh-hadley josh-hadley merged commit 692e944 into develop Feb 19, 2020
@josh-hadley josh-hadley deleted the jh-fix-waterfallplot-overflow branch February 19, 2020 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[waterfallplot] crashes if requested waterfall range overflows
2 participants