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

Try pdf box to render pdf as images #452

Merged
merged 5 commits into from
Jun 13, 2024
Merged

Try pdf box to render pdf as images #452

merged 5 commits into from
Jun 13, 2024

Conversation

celuchmarek
Copy link
Member

No description provided.

@celuchmarek celuchmarek marked this pull request as ready for review June 3, 2024 12:30
@celuchmarek celuchmarek requested a review from a team as a code owner June 3, 2024 12:30
@celuchmarek
Copy link
Member Author

celuchmarek commented Jun 3, 2024

Je to pri dlhých dokumentoch trochu pomalšie na vyrenderovanie, ale tak väčšina podpisovaných dokumentov nie je dlhá.

Skúšal som 102 stranové PDF. Latest verzia 5,5 sekundy, túto verzia 9,5 sekundy.

@celuchmarek
Copy link
Member Author

Snímka obrazovky 2024-06-03 o 14 33 51

@slavino
Copy link

slavino commented Jun 5, 2024

mozno off-topic kedze som s PDFBox od Apache este nerobil - ale preco je pouzita ver. 2.0.x ked uz releasli aj 3.0.2 (a pripravuje sa 4.x)? V sekcii https://pdfbox.apache.org/3.0/migration.html popisuju vylepsenia, tak mozno to okrem vplyvu na RAM bude mat vplyv i na celkovu rychlost.

@celuchmarek
Copy link
Member Author

@slavino Pôvodne som tam tiež dával 3.0.2, ale zistil som, že DSS knižnica má dependency na PDFBox 2.0.x a teda neviem tam dať inú.

V DSS Jire už existuje priradená issue pre upgrade na 3.x, čiže niekedy to hádam spravia. Ale zatiaľ to má nízku prioritu, takže si na to určite počkáme ešte niekoľko mesiacov.

@slavino
Copy link

slavino commented Jun 5, 2024

I've added a comment to JIRA of DSS - let's see if @bsanchezb will find some time any soon :)

@celuchmarek
Copy link
Member Author

Upravil som to zobrazovanie do JavaFX Image v ScrollPane namiesto webview. Je to pocitovo rýchlejšie a najmä sa to o dosť lepšie scrolluje než kedysi PDF (aspoň u mňa). Pridal som DPI setting pre usera, aby si ho vedel znížiť, ak mu to pôjde príliš pomaly pri väčších dokumentoch.

Snímka obrazovky 2024-06-12 o 20 11 36 Snímka obrazovky 2024-06-12 o 20 05 56 Snímka obrazovky 2024-06-12 o 20 07 50

@celuchmarek celuchmarek requested a review from jsuchal June 12, 2024 18:15
Copy link
Member

@jsuchal jsuchal left a comment

Choose a reason for hiding this comment

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

Ten lazy loading by som fakt zvazil lebo to sice bude komplikovane, ale pri velkych pdf to moze byt dealbreaker. Skusali sme nejake 100MB?

@@ -73,7 +73,7 @@ public void startVisualization(SigningJob job) {
}

try {
var visualization = DocumentVisualizationBuilder.fromJob(job);
var visualization = DocumentVisualizationBuilder.fromJob(job, settings.getPdfDpi());
Copy link
Member

Choose a reason for hiding this comment

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

Tu by som posielal mozno cely settings lebo potom pri dalsich veciach nemusis pridavat parameter.

Comment on lines 30 to 40
var divs = new ArrayList<byte[]>();
for (int page = 0; page < pdfDocument.getNumberOfPages(); ++page) {
var os = new ByteArrayOutputStream();
var bim = pdfRenderer.renderImageWithDPI(page, pdfDpi, ImageType.RGB);
ImageIO.write(bim, "png", os);
divs.add(os.toByteArray());
}

pdfDocument.close();

return divs;
Copy link
Member

Choose a reason for hiding this comment

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

Toto ale vyrenderuje uplne vsetky strany, nie je to lazy ako to bolo predtym ze?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nie je to lazy loading. Neviem, ako by sme ho spravili.

Na JavaFX Image sa dá nastaviť nejaký background loading, ale musí ísť z disku. Vieme keď tak vyrenderované obrázky ukladať do nejakého temp dir a odtiaľ nastaviť loading.

Copy link
Member

@jsuchal jsuchal Jun 12, 2024

Choose a reason for hiding this comment

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

v tom html to fungovalo tak, ze som tam nahadzal placehoder divy nejakej velkosti (nacitane z prvej strany asik) a potom ked si scrollol tak to az potom nacitalo/renderovalo strany +- par stran z viewportu. ten scrollpane asi ma nejake eventy ked vies, ze to je visible alebo nie.

Copy link
Member

Choose a reason for hiding this comment

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

ale teda skusme tam 100MB pdf a ked to uvladze tak asi na to kaslime

Copy link
Member Author

@celuchmarek celuchmarek Jun 13, 2024

Choose a reason for hiding this comment

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

Skúsil som 100MB PDF 77 strán https://cartographicperspectives.org/index.php/journal/article/view/cp43-complete-issue/pdf

Trvalo to 13 sekúnd a pri ďalšom raze 9 sekúnd.

Comment on lines +322 to +331
Parent root;
try {
root = GUIUtils.loadFXML(controller, "signing-dialog.fxml");
} catch (AutogramException e) {
showError(e);
return;
} catch (Exception e) {
showError(new UnrecognizedException(e));
return;
}
Copy link
Member

Choose a reason for hiding this comment

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

Toto sa preco meni?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmm, neviem. Nepamätám si, že by som tu niečo zámerne menil. Vrátim.

Copy link
Member Author

Choose a reason for hiding this comment

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

už viem. Ak tam vyskočila nejaká exception, zhltla sa a nič sa neudialo. Ono počas toho loadFxml sa volá potom aj initialize, v ktorom sa deje všetko - generovanie obrázkov z PDF aj ich zobrazenie v GUI.

Problém je, že v Autogram.java::77 sa zdá, že sa handluje táto exception, no tam sa volá

ui.onUIThreadDo(() -> ui.showVisualization(visualization, this));

A čo vyskočí vo vnútri tej lambdy sa nechytá v tom bloku kódu. Preto som pridal, nech sa chytá rovno v tej metóde.

@celuchmarek
Copy link
Member Author

Skúšal som 110 stranové pdf 40MB. Pri 100 dpi to na mojom m2 air trvalo 6 sekúnd od výberu súboru po zobrazenie. To dpi robí veľký časový rozdiel pri generovaní obrázkov.

Ale pri 150 dpi to nielen dlhšie generovalo, ale potom aj veľmi sekalo pri scrollovaní. To scrollovanie ale ovplyvňuje iba počet strán, nie samotná veľkosť súboru. Veľkosť súboru hrá rolu pri rýchlosti generovania.

@jsuchal
Copy link
Member

jsuchal commented Jun 12, 2024

Skúšal som 110 stranové pdf 40MB. Pri 100 dpi to na mojom m2 air trvalo 6 sekúnd od výberu súboru po zobrazenie. To dpi robí veľký časový rozdiel pri generovaní obrázkov.

Ale pri 150 dpi to nielen dlhšie generovalo, ale potom aj veľmi sekalo pri scrollovaní. To scrollovanie ale ovplyvňuje iba počet strán, nie samotná veľkosť súboru. Veľkosť súboru hrá rolu pri rýchlosti generovania.

IMHO je to preto lebo tie canvasy co to generuje su bitmapy, ak tam das len nejaky rectangle prazdny, tak to nemusi kazdy pixel generovat (tak to fungovalo v tom pdf.js - ja som tam dal len prazdny div, nie canvas)

@celuchmarek
Copy link
Member Author

@jsuchal ja som tam zámerne dal aj border okolo celého scrollpane a nielen na page, pretože takto, ako si to upravil, nevidno, kde končí náhľad PDF - chýba mi tam tá spodná čiara.

Naozaj sa ti toto pozdáva viac? Ak je to kvôli tomu scrollbaru, vieme dať vpravo nejaký ten inset 30px, aby bol border vľavo od scrollbaru.

Snímka obrazovky 2024-06-13 o 7 37 41

@celuchmarek celuchmarek requested a review from jsuchal June 13, 2024 06:13
@celuchmarek celuchmarek merged commit 48d032d into main Jun 13, 2024
6 checks passed
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.

3 participants