<html> <head> <base href="https://bugs.freedesktop.org/"> </head> <body> <div> <a class="bz_bug_link bz_status_NEW " title="NEW - add a displayAnnot to Page class" href="https://bugs.freedesktop.org/show_bug.cgi?id=83642#c7">Comment # 7</a> on <a class="bz_bug_link bz_status_NEW " title="NEW - add a displayAnnot to Page class" href="https://bugs.freedesktop.org/show_bug.cgi?id=83642">bug 83642</a> from <a class="email" href="mailto:aacid@kde.org" title="Albert Astals Cid <aacid@kde.org>"> Albert Astals Cid</a> <pre>> --- <a href="show_bug.cgi?id=83642#c6">Comment #6</a> from Tobias Deiminger <<a href="mailto:haxtibal@posteo.de">haxtibal@posteo.de</a>> --- > Whitespace changes are removed now. See rev. 2 of > <a href="https://gist.github.com/haxtibal/eab9320d43e47dfefe47249ee1e6d01a">https://gist.github.com/haxtibal/eab9320d43e47dfefe47249ee1e6d01a</a>. Sorry, > wanted to cleanup interleaved tabs and thought one could ignore changes > during review. But obviously the "?w=1" trick doesn't work for gists. Btw., > if you prefer we can shift the patch over to bugs.freedesktop.org anytime. Could you try to split this into a series of patches so they are smaller and easier to review since each patch has a clear focus/idea behind it? Seems like there's various stuff going on there (specially all the qt5 code moving seems like could be easily split). > You'll probably find more to dislike. I do, just had no better ideas yet. > Some reasoning in advance: > The changes around Qt5SplashOutputDev/ArthurOutputDev [0] are a bit heavy > and unfinished (dislike!). But I wanted to share as much "factory code" as > possible between Poppler::Page and Poppler::Annotation, thus factored out > common stuff. The changes regarding Annot::draw [1] are a bit heavy and > unfinished (again dislike). But I had to find a common place amongst all > annotation types where one can hook in to get the real size of the > dynamically generated appearance *1). Plus the template pattern approach > hopefully helps poppler newbies like me to understand things better, as it > makes common steps in Annot::draw schematic and explicit. makeAppearance(obj, drawRect, getXRef()); is a bit ackward API, I think it would be better if you return obj instead of passing it as output parameter. > > Regarding API... QImage Poppler::Annotation::renderToImage(double hDPI, > double vDPI) was the simplest API I could imagine. Functionality explicitly > cares about one single annotation and no other objects, thus the method is > bound to Annotation, instead of Page. We don't need to bother clients with > position/size arguments, poppler knows it all, and better. We probably > don't need callbacks. Maybe we want to allow fine tuning some rendering > settings? E.g. an enableTransparentBackground option could make sense to > chose either page color or transparency as background. What would be the use case for wanting the page color as background? > Also a further > method > Poppler::Annotation::renderToArthur(...) could be useful and would be > consistent to Poppler::Page::renderToArthur. Guess so yeah, but i wouldnt' really worry about that for now.</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are the assignee for the bug.</li> </ul> </body> </html>