Enhancements for search

[bdjnk]

Although none of the following are actual bugs, I think they are critical features for a useful find feature. (If you don't have patience to read everything, I'm just advising that qpdfview try to mimic the behavior of the Firefox find toolbar)

Ctrl-F displays the find toolbar and gives it focus, which is expected behavior. However, unlike most other programs I have used, pressing Escape while the find toolbar has focus does nothing. Often that will close the find toolbar, which to me would be the expected behavior. Meanwhile, if Ctrl-F is pressed while the find toolbar is open, it could just refocus and select the content.

An additional niggle is the highlight color, which, being a light grey, is very hard to pick out from a white background. Of course, any color choice would be wrong on some combination of background and text colors. Perhaps there is some way to grab the background and text colors and generate a sensible highlight color choice, but that's getting ahead of myself.

Other features worth adding would be find previous, match case, highlight all, ect. For long PDFs, a powerful search feature seems pretty important.

p.s. I'm really amazed at how quickly this program is improving. Thank you!

[Adam Reichold (adamreichold)]

I agree with your recommendations. After releasing version 0.2 and creating a mailing list today, I will hopefully implement these changes for the next minor release. Do you think blue will is a sensible highlight colour until we implement a smarter heuristic?

[Adam Reichold (adamreichold)]

By the way, you could join the qpdfview team and its mailing list at https://launchpad.net/~qpdfview if you would like to influence the development of this little program more directly.

[Adam Reichold (adamreichold)]

Implemented the changed visibility behaviour, changed the highlight colour to a strange blue and added a "Match case" option. I will probably have to rework the handling of highlighting to add "Find previous" and "Highlight all." This will probably be tackled in connection with bug #958634 as this will necessitate the use of Poppler's TextBox-API anyway.

[Adam Reichold (adamreichold)]

I think the current trunk revision contains all the necessary changes. Please test it exhaustively. Thanks.

[bdjnk (bdjnks)]

I just did the following several minutes ago:

$ bzr branch lp:qpdfview
$ cd qpdfview/
$ qmake
$ make
$ sudo make install
$ qpdfview

I have several problems. Entering a search term does not highlight any matching text, nor does it more the view to an area of the pdf containing the text. Finding next or previous by any method almost always results in a segmentation fault. I even managed to cause qpdfview to pin the cpu by entering a search term very slowly, one character at a time.

[Adam Reichold (adamreichold)]

This is obviously not an improvement. Marking this as "In progress" again.

First of all, if you test development versions, there is no need to install the program. You skip "$ sudo make install" and replace the last step by "./qpdfview". This way you don't touch your existing installation of a older version.

Now we need to find out why it does not work for you. Can you attach one of the files that caused the crash? This would help me testing.

Also just entering text without the "Highlight option" enabled currently does nothing. The highlight should only appear after you activate "Find previous" or "Find next." (It seems this will have to change as well after this runs reliably.)

Pinning the CPU seems very possible currently as there currently is no way to stop a running search. So if a search is still running and you type in a new word, the program first waits for the old search to finish. This will probably have to change as well.

Hopefully I will be able to reproduce some of this behaviour.

[Adam Reichold (adamreichold)]

I did find and fix several errors in the threading. Maybe this explains the crashs. But I will have to think of something to make this more robust against slow typing...

[bdjnk (bdjnks)]

Initially I did try running qpdfview without installing it, but when I experienced those issues, I figured it my somehow be related to it not having been installed. As unlikely as this probably is, I wanted to make sure.

I ran some tests using gdb. When hitting next or previous I got:

Program received signal SIGSEGV, Segmentation fault.
0xb7c23a04 in QGraphicsItemPrivate::setVisibleHelper(bool, bool, bool) () from /usr/lib/libQtGui.so.4

While typing characters slowly one at a time gave me:

Program received signal SIGSEGV, Segmentation fault.
0xb7c0ed77 in QGraphicsItem::scene() const () from /usr/lib/libQtGui.so.4

Speaking of which, not only was qpdfview using all the CPU, but it was also hung.

I've attached a PDF that I tested, but every PDF I've tried has had these same issues.

Oh, and I've been trying to get my IDE to behave normally. When I manage it, I'll have more information. For now, I can tell you that the problem manifests when m_lastResult.value()->setVisible(); is called from documentview.cpp

[Adam Reichold (adamreichold)]

Thank you for being thorough. I run the program from the source code directory all the time, so I will hopefully notice any problems with that.

It is hard for me to interpret the GDB output with the exact revision number you were running. (Problematically enough I am still treating the trunk branch like my personal one, so I push changes to frequently. I will try to change this in the future.)

The problem of hanging is that if a search is running a the GUI thread tries to access the result list (e.g. to clear the results when closing the search bar), it will block until that search is over. (So the real problem is that searches can't currently be canceled.)

I also agree that this was completely unrelated to the specific files. I really made some gross mistakes when calling into the GUI thread from the search thread. I am sorry for pushing the feature in such a rough state.

Nevertheless Benjamin Eltzner and I did some more testing and have now (trunk revision 75) hopefully found the biggest problems. It should also be a bit harder to bring the program down, as changes are ignored if a search is already running. (Maybe suboptimal, but again canceling seems to be the problem.) But if one types really slowly, the program will start to search for a single letter which will of course take a very long time. Do you think the timer should be remove so that a search has to be started using the return key manually?

[Adam Reichold (adamreichold)]

I remove the timer (so searches have to be triggered using the return key) and made searches cancelable. (trunk revision 76) Just give me a comment if this works for you now. I will not mark this "Fix committed" early again. Who knows what will happen. :-)

[bdjnk (bdjnks)]

Well, typing really slowly no longer causes any trouble, now that the enter key is required. That you can mark as fixed.

However, issues still exist. For me, searching has no discernible effects. In the code (I got my IDE working normally again) hitting find previous results in a call to findPrevious() and hitting find next a call to findNext(), but no visible change occurs (except a small jump downward on a search initiated by hitting enter). Nothing is highlighted, despite having highlight all checked.

This is using revision 76.

[Adam Reichold (adamreichold)]

I am sincerely puzzled. Hopefully, I will have something more substantial to say tomorrow...

[Adam Reichold (adamreichold)]

Ok, I made some changes to the visibility cycle of the search results. I am not sure they actually fix anything. This code is really getting convoluted and will need to be refactored.

If this does not do any good, can you try to give me a precise guide how to produce these errors, i.e. the exact settings in qpdfview that you use. (page layout, scaling, rotation, etc.) From an earlier comment, I also assume you run an up-to-date Arch Linux? I am sorry for making this so tedious for you, but debugging without reproducing is quite difficult. :-(

[bdjnk (bdjnks)]

Excellent call asking what my setting were. I played around and discovered that when I set a percentage zoom, rather than using fit to page or page width like I usually do, the highlighting and jumping from result to result suddenly works! It works irregardless of rotation, the level of zoom, and the page view, although the search does need to be redone if any of them are changed.

Try testing with fit to page or page width. If searching still works properly for you, I will try and provide further info.

[bdjnk (bdjnks)]

Okay, the problem is as follows. When searching in fit to page or page width mode, documentview's resizeEvent is called, containing the following:

if(m_scaling == FitToPage || m_scaling == FitToPageWidth)
{
    prepareScene();
    prepareView();
}

And prepareScene calls cancelSearch which means the search never completes, etc.

[Adam Reichold (adamreichold)]

Yes, this seems to be the problem. As a workaround, I made the status bar permanently visible. This will have to be resolved but I would like to start some heavy refactoring in a separate branch to make this easier. Does trunk revision 81 work for you? If so, I will come back to this after establishing some separation of concern in this mess of a program.

[bdjnk (bdjnks)]

Trunk revision 81 works!