2014-09-24 23:03 GMT+03:30 Adam Reichold <[email protected]>:
> -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Hello Razi, > > Hello Adam, > thanks for putting so much work into this! I just had a short look at > the branch and noticed some things: > > * It does not build. It fails first in "PdfPage::search" missing > "m_size" for me. Maybe just "m_page->m_size"? > > Oops... it's because I'm using Poppler 0.10 and I just forgot to update that part of code to the latest code. * Did you measure the increase in memory consumption because of > storing the text of the whole line? I suspect it is significant. Did you test with a DjVu file? I think maybe what you saw about memory consumption is because of removing "Page::search" overload and not other part of this patch I used Windows Task Manager for test and for example for ~15000 occurrences it used ~5MB that I think is as expected. Also > extracting this directly inside the model adds UI specifics to it > which smells like a layering violation. Wouldn't it be better to > request that text on-demand so that people not using the search dock > don't pay the additional overhead? > > Yes you are right. The best solution that I'm aware is that we request text on vertical scrollbar position change. I wrote a solution (workaround?) for it (revision.1704 [1]) now for me DjVu search seems as fast as it should be. By "on-demand" do you mean an option to disable storing result in model, i.e. disable this patch completely? * Also not using the "Page::search" overload introduced in Poppler > 0.22 (funnily enough by us specifically for qpdfview) which returns a > whole list of occurrences drastically reduces search performance. It > was up to a factor of 10 if I remember correctly. I do not think users > will like that and it also joins with the problem stated before. > > Again because I'm using older Poppler I didn't see the regression you mentioned, but if we request text in other part for example my above suggestion or in SearchTask::run() we can solve this issue. (revision.1703 [2]) * You are also right that I strongly dislike the kind of reuse that > "SearchDelegate" embodies. Did you try using a widget delegate to > achieve the same effect? What exactly are the modifications to > QItemDelegate? I suppose it is the additional formats highlighting the > keyword? Is there no way to inject this behavior into an existing > delegate or widget? > Yes I just add additional formats to QTextLayout. I re-wrote it. (revision.1702 [3]) > > * I do not like "DocumentView::prepareHighlight" becoming public as it > is an internal helper method. If anything there could be another > public slot to jump to a specific search result using some form of > index where it is ensured that the method is called with proper input. > > Revision.1705 [4] > * If we are going to invest significant resources (code size and > complexity, memory consumption and processing power) into this, then I > think we should aim to implement the broader feature, i.e. a search > interface showing results from all open tabs. Hence placing the > "SearchModel" outside of the "DocumentView" and connecting to it via > signals and slots to keep its data current. > > I am sorry if this sounds overly critical. It is just that I feel that > my job description as maintainer is to work the program into the best > state achievable with the given resources, often meaning to work it > and its contributors a bit harder to cope with the increasing > complexity of the project. So please don't feel discouraged and let's discuss the various issues > as I am sure we can resolve them. If you want to, you can also reply > to this mail via the mailing list to make this a public discussion so > that other people can join in and maybe reweigh some of my objections. > > No problem :) the code of QPDFView is clean and in a very good state because of you, please keep up your great work. Best regards, Adam. > > [1] http://bazaar.launchpad.net/~srazi/qpdfview /extend-search-dock2/revision/1704 [2] http://bazaar.launchpad.net/~srazi/qpdfview /extend-search-dock2/revision/1703 [3] http://bazaar.launchpad.net/~srazi/qpdfview /extend-search-dock2/revision/1702 [4] http://bazaar.launchpad.net/~srazi/qpdfview /extend-search-dock2/revision/1705 Best Regards, Razi > Am 24.09.2014 um 14:12 schrieb Seyyed Razi Alavizadeh: > > Hello Adam, I pushed it here [1], indeed I duplicated and renamed > > "BookmarkModel" to "SearchModel". And for "SearchDelegate" I know > > that you don't like copy-paste from Qt codes but I wrote this > > class more than a year ago and it is tested on two applications, I > > used QTextLayout because it internally runs bidi-algorithm on text > > and because of this it works correctly with RTL languages (like my > > native language: Persian) using QPainter::drawText() is just good > > when we don't need to support RTL languages. > > > > [1] https://code.launchpad.net/~srazi/qpdfview/extend-search-dock2 > > > > Best Regards, Razi. > > > > 2014-09-23 22:18 GMT+03:30 Adam Reichold <[email protected] > > <mailto:[email protected]>>: > > > > Hello Razi, > > > > you mentioned a branch implementing a search dock once. I justed > > wanted to suggest to you to publish it on Launchpad so that other > > people can test it, even though we can't merge it into trunk yet. > > To get people notified, you could use the mailing list and link > > your branch to the related bug report [1]. > > > > Best regards, Adam. > > > > [1] https://bugs.launchpad.net/qpdfview7/+bug/1028295 > > > > > > > > > > -- Alavizadeh, Sayed Razi My Blog: http://pozh.org > > <http://pozh.org/> Saaghar (نرمافزار شعر): > > http://saaghar.pozh.org/ Saaghar Fan Page: > > http://www.facebook.com/saaghar.p Saaghar Mailing List: > > http://groups.google.com/group/saaghar > > > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v2 > > iQEcBAEBAgAGBQJUIxyRAAoJEPSSjE3STU34/vQIAL2XreiaOGc1hsBORmyisXln > j6Exq3n8KRpBlUW4vKtFmA90yatty/6btIz0PlTLPJbCBGJcl0qGz5KH3Id/sGKf > orL/gQFduz8hjp3RKGVbs91q0vLpCim+oxcS/RPQNGxJlZcqJstPRUSVHHJI3fdo > qM2NoE3hXqt8cqwiIVxhrQeIr7iOzW6IW6TqUrcQk9Y8t9/NsxaGYbgvoUMAfzIk > 4qI5y0EGudl2TjSK9MnjKX5Sl/nZMPSYiIfMjhZnt6BFq+xZQK9FxdKAjCa4UkCH > 0B92CDN3fUwPOAuLxEvIuD0x0Gy6QCRT7j1VAB0ntg7ffwPe0oEMj69tSZYAKUU= > =i7QQ > -----END PGP SIGNATURE----- > -- Alavizadeh, Sayed Razi My Blog: http://pozh.org Saaghar (نرمافزار شعر): http://saaghar.pozh.org/ Saaghar Fan Page: http://www.facebook.com/saaghar.p Saaghar Mailing List: http://groups.google.com/group/saaghar
-- Mailing list: https://launchpad.net/~qpdfview Post to : [email protected] Unsubscribe : https://launchpad.net/~qpdfview More help : https://help.launchpad.net/ListHelp
