search

spajak / cef-pdf

cef-pdf HTML to PDF utility
MIT License
77 stars 33 forks source link

Add 4 new command line options: #29

Closed jcoleman1969 closed 5 years ago

jcoleman1969 commented 5 years ago

-scale= to set pdfSettings.scale_factor -delay= to delay after page load before rendering pdf (in my case allowed D3 charts to finish animating before creating the PDF) -viewwidth= to set viewport width (in my case setting the width to 1600 fixed a table that was trying to size to 100% and getting cut off at 128px wide, which is the default) -viewheight= added for parity, could be useful if element was specified as height=100%

anko commented 5 years ago

Nice!

I think -delay is fragile, because the necessary timeout differs depending on host machine specs, system load, input size, etc. In long-term, I'd want a callback for the page to fire when ready. For one-shots, -delay is useful as-is.

jcoleman1969 commented 5 years ago

I just noticed that -delay doesn't seem to work consistently in the release build. I'm not sure why it's different than the debug build, but maybe just doing a sleep in Client::OnLoadEnd() isn't allowing the message loop to run? I was thinking it may be better to set a flag in OnLoadEnd() that the load is finished and then call m_jobManager->Process() from elsewhere after the timeout has expired. Do you have any ideas on where we could call it from? Thanks!

jcoleman1969 commented 5 years ago

I changed to using CefPostDelayedTask() instead of sleeping, and now --delay works reliably on both builds.

jcoleman1969 commented 5 years ago

The CefRefPtr is declared as CefRefPtr, not to our derived CefRefPtr, so I either have to downcast it, or I need to change the declaration to CefRefPtr in Client.h. Which would you prefer?

On Sun, Apr 14, 2019 at 4:41 PM Sebastian Pająk notifications@github.com wrote:

@spajak commented on this pull request.

In src/Client.cpp https://github.com/spajak/cef-pdf/pull/29#discussion_r275173359:

@@ -308,4 +322,16 @@ void Client::OnRenderProcessTerminated( DLOG(INFO) << "Client::OnRenderProcessTerminated"; }

+void Client::SetViewWidth(int viewWidth) +{

  • RenderHandler renderHandler = (RenderHandler)(m_renderHandler.get());

Use CefRefPtr instead

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/spajak/cef-pdf/pull/29#pullrequestreview-226412366, or mute the thread https://github.com/notifications/unsubscribe-auth/AtaTBsPngZy22h01YdQgN8WFmdWz_Ecbks5vg5KEgaJpZM4a6tQ8 .

spajak commented 5 years ago

Yes. Change declaration. It should not break anything

spajak commented 5 years ago

Thanks. You could also update README if you can