Feature: PDF support

[nicholas477]

Not really sure if this would be wanted, but I added PDF support to my fork and I was curious if y'all would want this upstream

[liamw1]

This looks cool! I'm totally willing to support additional formats as long as they meet two conditions:

1. It's a format that is supported by all the major browsers
2. There's some way to extract an image that represents its contents

Seems like PDFs satisfy both, so they're a good candidate.

I reviewed the implementation: the structure seems fine, just needs a bit more massaging to get it consistent with the rest of the codebase. For the comments about adding ApiError variants: wiring those up can be a bit tedious, so if you want I can handle those changes.

I have one comment on the design overall. What do you think of using max_image_width and max_image_height for the max dimensions of the PDF instead of having separate config values just for PDFs? I say this because no other post type has separate dimension limits, so I feel like it may make sense to merge them.

[nicholas477]

Thank you for taking the time to look over my code! I'm going to implement all of the code changes in a bit. About the image size though:

I have one comment on the design overall. What do you think of using max_image_width and max_image_height for the max dimensions of the PDF instead of having separate config values just for PDFs? I say this because no other post type has separate dimension limits, so I feel like it may make sense to merge them.

I don't think that would make sense since PDFs don't really have a pixel width/height. I'm not too sure how the library calculates the size, but for some of the PDFs i was giving it, it would give me somewhere in the range of 16k image width/height for the first page.

Also the max_pdf_width/height is only used for:

• limiting the size of the preview image shown in the posts list
• limiting the size of the PDF preview inside the post page

I think there's a better way to do both. I'd like to uncouple the size of the pdf viewer from the size of the preview image. Maybe the preview image should be around 300px? I didn't look into how oxibooru calculates size of the preview images for pngs, jpgs, etc. For the size of the pdf shown on the page, I'm not really sure how that should be done right. Here's an example of how this looks right now:

I'm not much of a frontend dev unfortunately, but scaling it to page seems like the way

[liamw1]

I'm not too sure how the library calculates the size, but for some of the PDFs i was giving it, it would give me somewhere in the range of 16k image width/height for the first page.

So if you were to use default render settings:

// Defaults to x/y scale of 1.0 and library chooses width/height
RenderSettings {
    bg_color: WHITE,
    ..Default::default()
}

the library would try to render a huge preview? These settings seem to work OK for the PDFs I've uploaded, but there might be some weird edge cases I haven't run into.

[nicholas477]

I'm not too sure how the library calculates the size, but for some of the PDFs i was giving it, it would give me somewhere in the range of 16k image width/height for the first page.

So if you were to use default render settings:

// Defaults to x/y scale of 1.0 and library chooses width/height
RenderSettings {
    bg_color: WHITE,
    ..Default::default()
}

the library would try to render a huge preview? These settings seem to work OK for the PDFs I've uploaded, but there might be some weird edge cases I haven't run into.

Yeah, I've been making PDFs for sewing patterns (exported from clo3d) and the calculated dimensions seem to be absolutely massive. for this pdf: https://booru.cactus.vg/data/posts/000000/00/1040_GdEGVuUVFyo6s6yh9Kasbzse3uI09CaKsA5P3EViUd8.pdf it was giving me 7746 x 9804 for the size

also once again, i think the rendered image is only used for the preview thumbnail in the post list. i dont show it in the post viewer itself at least

[liamw1]

Alright, in that case it seems like the separate dimension limits for PDFs defensible. I'm only harping on this because taking away settings from the config is more painful than adding them, so I want to be confident they're necessary.

[nicholas477]

I feel that! If there is a better way to do any of this please let me know, I'm also not really happy with adding more config variables

[liamw1]

Alright, I went ahead and committed some minor stylistic changes that weren't worth commenting on here. I also removed the need for cloning the pixel buffer with a bit of unsafe code.

Everything looks pretty good to me. I'm aiming to get this into version 0.9. Until then, I'll keep thinking about alternative ways of handling the preview scaling and see if we can remove the need for those two additional config settings.

Something else that's worth thinking about is up-scaling. One PDF that I uploaded while testing came out to have a fairly small preview image (around 200px in width/height). Because the preview dimensions are used to determine the size of the post view, the PDF is rendered in a very small area when viewing the post. Perhaps the preview image should be scaled to some minimum size as well?

[nicholas477]

Looks great! Thanks for cleaning up the code. I agree we also need a minimum size and I can code that up, just where do I figure out what the minimum size needs to be?

[nicholas477]

Also is there a discord or something for this project?

[liamw1]

Also is there a discord or something for this project?

No, not at the moment

Also, I think most of these scaling issues can be solved by the client. The post content template was silently broken and so PDFs did not respect scaling buttons. I fixed that and also made it so that PDFs will small preview images are always scaled up on the client, which looks much better IMO. The dimensions of a PDF are somewhat arbitrary anyway.

So instead of having a minimum size for the preview, the client will automatically enlarge PDFs that happened to have small previews for whatever reason. And after thinking about it for a bit, I think it actually makes sense to make the max preview dimension hard-coded constants. The dimension limiting is mostly an implementation detail now, since the client is handling the scaling.

Resolved.

[nicholas477]

I think the fit width/height buttons should work differently for documents. I don't see why it should keep the aspect ratio the same. I think i'm going to try changing how this works