Bug 84833 - creating single page ps from huge pdf slow / function "PSConverter::convert()" ignores page ranges
Summary: creating single page ps from huge pdf slow / function "PSConverter::convert()...
Status: RESOLVED FIXED
Alias: None
Product: poppler
Classification: Unclassified
Component: qt4 frontend (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: poppler-bugs
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-09 09:07 UTC by t_d_l_c
Modified: 2015-01-04 17:49 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:

Attachments
Proposed Patch (1.41 KB, patch)
2014-10-09 12:00 UTC, t_d_l_c 		Details | Splinter Review
The cc-File before and after the patch (1.41 KB, application/octet-stream)
2014-10-09 12:02 UTC, t_d_l_c 		Details
New patch with new constructor for PsOutputDev (9.32 KB, patch)
2014-10-23 07:34 UTC, t_d_l_c 		Details | Splinter Review
patch for poppler-0.24.5 (15.41 KB, patch)
2014-10-23 12:30 UTC, t_d_l_c 		Details | Splinter Review
My patch to implement non consecutive pspages (11.88 KB, patch)
2014-10-30 22:34 UTC, Albert Astals Cid 		Details | Splinter Review
View All

Description t_d_l_c 2014-10-09 09:07:41 UTC 
I try to print a single page from a huge (400MB) PDF-file with 8000 pages from okular. This takes ages. 
I noticed that the temporary ps-file created by okular using poppler in /tmp is also huge (about 400MB). Evince creates only 200kB. 

As we are using our own builds of okular and poppler I looked at the sources. 
I think the problem is in:

poppler-0.24.5/qt4/src/poppler-ps-converter.cc

in the function "bool PSConverter::convert()".

This function simply ignores all page ranges. Looking the source code of poppler 0.26, the problem seems to be still there. 
I will try to fix this bug myself and post a solution here.
Comment 1 t_d_l_c 2014-10-09 12:00:40 UTC 
Created attachment 107610 [details] [review]
Proposed Patch

Here is my proposed patch. Can anyone say if my patch might have some side effects? Is it possible that one page influences the rendering of another page?

I iterate over the page list to determine the min/max page. Would it be more appropriate to simply use the first and last element of the page list? Can I rely on that?
Comment 2 t_d_l_c 2014-10-09 12:02:09 UTC 
Created attachment 107611 [details]
The cc-File before and after the patch

The cc-File before and after the patch, along with the same patch as in the other attachment
Comment 3 Albert Astals Cid 2014-10-09 20:32:43 UTC 
Best solution would be actually adding a new constructor to PsOutputDev that accepts a list, do you think you can work on that?
Comment 4 t_d_l_c 2014-10-23 07:34:47 UTC 
Created attachment 108287 [details] [review]
New patch with new constructor for PsOutputDev

An external company has fixed it for us. This should be used as the top patch in the ubuntu paket in version 0.18.4-1ubuntu3.1. 
I have not tested this patch yet and will provide a patch for 0.24.5 shortly.
Comment 5 t_d_l_c 2014-10-23 12:30:05 UTC 
Created attachment 108296 [details] [review]
patch for poppler-0.24.5

I got the patch for poppler-0.24.5 now and attach it to this bug. It was made for our version of the ubuntu-packet poppler_0.24.5-0ubuntu3. Please feel free to contact me if it does not work with the upstream version. We are highly interested that this bug is fixed in the upstream version of poppler (and okular).
Comment 6 t_d_l_c 2014-10-23 12:31:22 UTC 
I tested the poppler-0.24.5 patch and it works for us.
Comment 7 Albert Astals Cid 2014-10-23 22:36:43 UTC 
Question: Who should get the copyright attribution for this patch?
Comment 8 Albert Astals Cid 2014-10-30 08:00:12 UTC 
t_d_l_c ping
Comment 9 t_d_l_c 2014-10-30 09:35:39 UTC 
I can't decide on the copyright myself, sorry. My Boss will provide feedback concerning the copyright. Most probable is "Landeshaupstadt München/City of Munich", even though the patch was written by credativ GmbH.
But please wait until my boss has decided on that.
Comment 10 Albert Astals Cid 2014-10-30 09:45:08 UTC 
No pressure, but i'm releasing poppler 0.28 today so it'd be awesome if we could get an answer today so i can include the patch ;)
Comment 11 Albert Astals Cid 2014-10-30 09:46:49 UTC 
even if the (C) is "Landeshaupstadt München/City of Munich" i'll also need an email address to put besides it
Comment 12 t_d_l_c 2014-10-30 12:09:19 UTC 
According to my boss you can take "Landeshaupstadt München/City of Munich" and my official address ludwig.cornelius {at} muenchen.de
Comment 13 Albert Astals Cid 2014-10-30 22:34:06 UTC 
Actually i just had a look at the patch and realized i can't use that, it's a huge copy and paste of code. I understand why a contractor would do that but it increases our maintaince costs too much so I can't accept it.

I'm doing my own patch that does the same but i'll need to test it so it may not end up on the next release.
Comment 14 Albert Astals Cid 2014-10-30 22:34:58 UTC 
Created attachment 108708 [details] [review]
My patch to implement non consecutive pspages
Comment 15 t_d_l_c 2014-11-13 12:43:04 UTC 
Thanks for fixing it upstream. That's very useful for us.
Comment 16 Albert Astals Cid 2015-01-04 17:49:43 UTC 
My patch has now been pushed.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.