<table><tr><td style="">aacid marked 9 inline comments as done.<br />aacid added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D9328" rel="noreferrer">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D9328#193130" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;" rel="noreferrer">D9328#193130</a>, <a href="https://phabricator.kde.org/p/rkflx/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;" rel="noreferrer">@rkflx</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>First things first: The patch marks an impressive improvement in (perceived) drawing performance for slow-rendering PDFs, I'm glad your hard work payed off.</p>
<p>See below for some minor comments regarding the code, but obviously I lack Okular/Poppler knowledge to review this in depth. I noticed <a href="https://phabricator.kde.org/D8379" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: line-through;" rel="noreferrer">D8379</a> added some autotests, but here we get nothing in that regard.</p></div>
</blockquote>
<p>There's no autotests in <a href="https://phabricator.kde.org/D8379" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: line-through;" rel="noreferrer">D8379</a>, no?</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>Do you think there might be a way to autotest the intended behaviour of the rendering a bit, e.g. correct order of main canvas / thumbnail rendering, cancelling on zoom/resize/pan etc.? Just asking, not a requirement for acceptance of course.</p></blockquote>
<p>It is inside the realms of doable, but doesn't seem trivial sadly, not sure if i have enough time to devote to adding them</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>As for testing:</p>
<ul class="remarkup-list">
<li class="remarkup-list-item">I'm using Qt 5.10.0, Okular master and Poppler master (Poppler patch did not apply cleanly, though) with <tt style="background: #ebebeb; font-size: 13px;">dublin-center-street.pdf</tt> for now.</li>
</ul></blockquote>
<p>I've updated the poppler patch now.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ul class="remarkup-list">
<li class="remarkup-list-item">So far I could not yet perform very extensive test runs, but I thought I'd share the first problems I found so you can start looking into it. Will do more testing in a bit. Let me know if you think the problem is somewhere on my side.</li>
<li class="remarkup-list-item">Segfault when resizing window. Backtrace:</li>
</ul></blockquote>
<p>Right, this should be fixed with the new patch i'll upload</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ol class="remarkup-list" start="2">
<li class="remarkup-list-item">OOM killed when changing sidebar size.</li>
</ol></blockquote>
<p>Can not reproduce, maybe fixed by the fix to 1) since it was sidebar related. What i can reproduce is the UI getting a bit stuck if you resize the sidebar while the page is rendering, but that already happens without this patch so at least it's not a regression, i had a quick look and can't really figure out what is going on, i'll try to have another look, but don't think we should block on this</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ol class="remarkup-list" start="3">
<li class="remarkup-list-item">ASSERT when deselecting "parking (level 7)" in <span><span class="phui-tag-view phui-tag-type-shade phui-tag-grey phui-tag-shade "><span class="phui-tag-core ">Layers</span></span></span> sidebar:</li>
</ol></blockquote>
<p>Right, fixed.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><ol class="remarkup-list" start="4">
<li class="remarkup-list-item">No text rendering in some situations (observed this multiple times when wildly zooming and jumping around via the thumbnail view, sadly don't have a concise video yet).</li>
</ol></blockquote>
<p>That's really weird since text is "just" part of the regular rendering, you should either get no rendering at all or it working, but having no text doesn't make much sense. It may be that the tile got cancelled and wasn't cleared. I'll investigate a bit more and see if i can reproduce it.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D9328#inline-45653" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">rkflx</span> wrote in <span style="color: #4b4d51; font-weight: bold;">document.cpp:3327</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Add <tt style="background: #ebebeb; font-size: 13px;">{ }</tt>.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Nope, not okular style (if it has any :D) see 5 lines below</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R223 Okular</div></div></div><br /><div><strong>BRANCH</strong><div><div>cancellable (branched from master)</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D9328" rel="noreferrer">https://phabricator.kde.org/D9328</a></div></div><br /><div><strong>To: </strong>aacid, ervin<br /><strong>Cc: </strong>rkflx, ervin, michaelweghorn, ngraham, Okular, aacid<br /></div>