rkflx requested changes to this revision. rkflx added a comment. This revision now requires changes to proceed.
>> 5. `ASSERT: "page()" in file okular/core/generator_p.cpp, line 129` (Happened two times already, but not everytime.) > > You got this one changing the size of the sidebar? That's correct. Now I even have somewhat reliable steps to reproduce (you'd still need a bit of luck, though). Open the document and while it is rendering resize the sidebar so X gets stuck, then wait for the `ASSERT`: F5676036: assert-after-resize.webm <https://phabricator.kde.org/F5676036> Also, the `ASSERT` problem is not unique to vector heavy documents like `dublin-center-street.pdf`, I get it too for an image heavy 50 page document while holding [Space] to scroll until it crashes. After reopening the document, Okular would even crash immediately then, meaning this probably has nothing to do with the stuck sidebar (which is only the trigger in one case). In fact, scrolling in almost every multipage document triggers the problem, which is quite serious. This was not there before (I checked specifically by only undoing the Okular patch). --- >> 6. Text page generation not cancellable > > cancel text extraction on document close I'm still not sure whether it's not working correctly or I'm not understanding what "cancelling" means. In the following video, I Close during rendering (immediate reaction) and then I Close during text extraction (long delay): F5676037: close-not-cancelling.webm <https://phabricator.kde.org/F5676037> To me this looks like text extraction in just one huge uninterruptible chunk, and only after it has finished Okular will react with whatever I wanted to do (in this case Close). The same happens for Zoom In, where text extraction is not "stopped" and thus I get to see the pixelated rendering for quite a while (I did not test any other actions yet): F5676039: zoom-not-cancelling.webm <https://phabricator.kde.org/F5676039> --- > I'll finalize testing […] tomorrow. Done. Other than the issues noted above I did not find any additional oddities or crashes when testing more extensively (different documents, more features). (In HiDPI mode we still have those double redrawing / no redrawing problems, but those are unrelated to this patch. Mentioning it only so that nobody testing in HiDPI mode gets confused.) I'm setting the status to "Changes requested" for now, because the crashes happen for almost every scientific document I tested (we have lots of users in this area) and apply to a Release build too. Apart from that, the overall state of the patch is quite good. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D9328 To: aacid, ervin, rkflx Cc: rkflx, ervin, michaelweghorn, ngraham, #okular, aacid
