Created attachment 138157 [details] [review] The patch file Hello, I needed some extra functionality what is available in the pdftoppm utility, but there were some missing options in the image and the page_renderer classes and I also added some small bugfixes. On image I made the following modifications: - new poppler::image modes - corrected bytes_per_row for poppler::image::rgb24, new image modes added - NetPBMWriter & save also handles new modes On page_renderer: - page_renderer_private got image_format and line_mode fields - this two new property is exposed in a same way as paper_color and hints - default values are the same as before This is my first patch in this project, so please be gentle with me if I did something wrong. :) Thank you, have a nice day!
Please remove the changes to CMakeLists.txt Can you explain why poppler::image::rgb24 was wrong? Also did you change save behaviour for case format_rgb24? You can't include include splash/SplashTypes.h in a public header
I removed the changes from CMakeLists.txt The calc_bytes_per_row (poppler::image::rgb24) bug was the following: we are using 4-padded lines. The current version simply multiplies the width by 3, so that produces distorted images when the output image width isn't a multiply of 4. The version I propose manages to render the pdf correctly with any width. I attached a test pdf and two renders, one with the original code, one with my modifications. On image::save I accidentally exchanged RGB24 with BGR24 in my previous patch, I fixed that. My BGR24 saving method also had a bug, that's also fixed. According to our conversation with Cid, I removed the include of splash/SplashTypes.h from the public header and added converter functions to handle image_mode and line_modes both on public and on Splash side.
Created attachment 138247 [details] [review] Fixed patch according to comment 1
Created attachment 138248 [details] test file to image::calc_bytes_per_row fix
Created attachment 138249 [details] image::calc_bytes_per_row original render
Created attachment 138250 [details] image::calc_bytes_per_row fixed render
Sorry i took too much to come back. Is there any reason you really need - format_invalid, + format_invalid = -1, I think not, but want to double check with you. ? That's bad since changes the value of format_invalid between versions. Also the two new enums should be at the end so that the existing ones don't change value. Other than that looks good, no need to upload a new patch, just tell me if format_invalid = -1 is needed or not.
I just forgot to remove the default value on this patch for `format_invalid`, sorry. It was only important when I exposed splash by mistake, but not anymore. Thank you for taking care of the rest! I'll be more careful on my next patch :)
Pushed, i also removed the - image::format_enum format : 3; + image::format_enum format : 4; change because 6 enums fit fine in 3 bits
That was also present because of the format_enum::invalid_format = -1 and I forgot to remove it. Thank you!
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.