Hello again, Am 24.01.2015 um 00:20 schrieb Adam Reichold: > Hello Razi, > > Am 23.01.2015 um 23:37 schrieb Seyyed Razi Alavizadeh: >> Hello Adam, >> >> Sorry for my late reply. > > No, thanks for looking into it! > >> I tested search with trunk 1834: >> If I enable "Whole Words" and I search for "a space" then results >> contains something like for example "a KB-space", I'm not sure if it is >> expected or not (?) but it's because "wordBegins&&wordEnds" is true when >> partial matching for "space". > > Yes, the thing is that after your last change, we began to accept > non-letter (instead of whitespace) as word boundaries which I found > pretty useful. Of course this does not match what DjVuLibre thinks are > words and so we have two (DjVu) words "a" and "KB-space" on which the > second one fits when using whole words matching because of the hyphen. > > Personally, I think that is actually useful and we can live with the > slightly larger number of false positives. Otherwise we'd have to revert > back to match our words with DjVuLibre's words. > >> Another issue is highlighting search results within tree-view when >> searching "several words" with "Whole Words" disabled. >> For example the search phrase is "a space" one of result is "a subspace" >> that is not highlighted. > > Yes, that seems because "emphasizeText" does not split the text to > search for into individual words before highlighting. I see a few > options here: > > 1) Make the "emphasizeText" function exactly as complicated as DjVu's > "findText" function with the possibility of breaking synchronicity with > other plugins. > > 2) Make the "findText" function's matching less fuzzy only finding > proper substrings (with consecutive whitespace being counted as one > symbol) and stop matching something like "a space" with "a KB-space". > > 3) Not only query the surroundingText but also the text to be > highlighted using "Page::text" with the actual result rectangle which > can then be highlighted a proper substring. > > 4) Ignore the differences in highlighting the more outlying cases. > > Personally, I dislike option 2) and feel like 3) is the cleanest way of > going about this (also w.r.t. to other plugins) but I am unsure about > the performance implications. 1) may be a practical compromise. What do > you think?
Attached, you also find what I would consider a clean approach to separating the plug-in internal matching and the text highlighting for you to have a look at and consider performance-wise. Best regards, Adam. >> Best Regards, >> Razi. >> > > Best regards, Adam. > >> >> 2015-01-11 18:32 GMT+03:30 Adam Reichold <[email protected] >> <mailto:[email protected]>>: >> >> Hello Razi, >> >> I am currently working on getting the whole-words search function of >> Poppler exposed in the Qt frontends. I did already add the necessary >> UI elements to qpdfview and started work on the DjVu backend. >> >> As you know, the DjVu search function, i.e. the "findText" method, has >> always been pretty "hand waving". To make it fulfil the requirements >> made by the UI at least to some degree, I extended it to respect the >> whole words flag and I also tried to restore the matching of search >> texts that span several words (which was gone after the last change to >> make it match words within words at non-letters). >> >> It is now pretty complicated again (doing a word-by-word possibly with >> words being substrings of DjVu words match) and I would be grateful if >> you could review it as well when you find some spare time to do so. >> Thanks! >> >> Best regards, Adam. >> >> -- >> Mailing list: https://launchpad.net/~qpdfview >> Post to : [email protected] >> <mailto:[email protected]> >> Unsubscribe : https://launchpad.net/~qpdfview >> More help : https://help.launchpad.net/ListHelp >> >> >> >> >> -- >> 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 >> > > >
=== modified file 'sources/documentview.cpp'
--- sources/documentview.cpp 2015-01-11 13:17:34 +0000
+++ sources/documentview.cpp 2015-01-24 11:07:39 +0000
@@ -820,11 +820,11 @@
return m_searchTask->wholeWords();
}
-QString DocumentView::surroundingText(int page, const QRectF& rect) const
+QPair< QString, QString > DocumentView::surroundingText(int page, const QRectF& rect) const
{
if(page < 1 || page > m_pages.size() || rect.isEmpty())
{
- return QString();
+ return qMakePair(QString(), QString());
}
// Fetch at most half of a line as centered on the given rectangle as possible.
@@ -834,7 +834,10 @@
const QRectF surroundingRect(x, rect.top(), width, rect.height());
- return m_pages.at(page - 1)->text(surroundingRect).simplified();
+ const QString& matchedText = m_pages.at(page - 1)->text(rect).simplified();
+ const QString& surroundingText = m_pages.at(page - 1)->text(surroundingRect).simplified();
+
+ return qMakePair(matchedText, surroundingText);
}
void DocumentView::show()
=== modified file 'sources/documentview.h'
--- sources/documentview.h 2015-01-11 13:17:34 +0000
+++ sources/documentview.h 2015-01-24 11:05:05 +0000
@@ -134,7 +134,7 @@
bool searchMatchCase() const;
bool searchWholeWords() const;
- QString surroundingText(int page, const QRectF& rect) const;
+ QPair< QString, QString > surroundingText(int page, const QRectF& rect) const;
signals:
void documentChanged();
=== modified file 'sources/miscellaneous.cpp'
--- sources/miscellaneous.cpp 2015-01-17 14:08:33 +0000
+++ sources/miscellaneous.cpp 2015-01-24 11:18:38 +0000
@@ -56,7 +56,7 @@
return true;
}
-void emphasizeText(const QString& text, bool matchCase, bool wholeWords, const QString& surroundingText, QTextLayout& textLayout)
+void emphasizeText(const QString& matchedText, const QString& surroundingText, QTextLayout& textLayout)
{
QFont font = textLayout.font();
font.setWeight(QFont::Light);
@@ -65,30 +65,14 @@
QList< QTextLayout::FormatRange > additionalFormats;
- const Qt::CaseSensitivity sensitivity = matchCase ? Qt::CaseSensitive : Qt::CaseInsensitive;
- int index = 0;
-
- while((index = surroundingText.indexOf(text, index, sensitivity)) != -1)
+ for(int index = 0; (index = surroundingText.indexOf(matchedText, index)) != -1; index += matchedText.length())
{
- const int nextIndex = index + text.length();
-
- const bool wordBegins = index == 0 || !surroundingText.at(index - 1).isLetterOrNumber();
- const bool wordEnds = nextIndex == surroundingText.length() || !surroundingText.at(nextIndex).isLetterOrNumber();
-
- if(!wholeWords || (wordBegins && wordEnds))
- {
- QTextLayout::FormatRange formatRange;
- formatRange.start = index;
- formatRange.length = text.length();
- formatRange.format.setFontWeight(QFont::Bold);
-
- additionalFormats.append(formatRange);
- }
-
- if((index = nextIndex) >= surroundingText.length())
- {
- break;
- }
+ QTextLayout::FormatRange formatRange;
+ formatRange.start = index;
+ formatRange.length = matchedText.length();
+ formatRange.format.setFontWeight(QFont::Bold);
+
+ additionalFormats.append(formatRange);
}
textLayout.setAdditionalFormats(additionalFormats);
@@ -647,15 +631,12 @@
return;
}
- const QString text = index.data(SearchModel::TextRole).toString();
+ const QString matchedText = index.data(SearchModel::MatchedTextRole).toString();
const QString surroundingText = index.data(SearchModel::SurroundingTextRole).toString();
- if(!text.isEmpty() && !surroundingText.isEmpty())
+ if(!matchedText.isEmpty() && !surroundingText.isEmpty())
{
- const bool matchCase = index.data(SearchModel::MatchCaseRole).toBool();
- const bool wholeWords = index.data(SearchModel::WholeWordsRole).toBool();
-
- paintSurroundingText(painter, option, text, surroundingText, matchCase, wholeWords);
+ paintSurroundingText(painter, option, matchedText, surroundingText);
return;
}
}
@@ -675,7 +656,7 @@
}
void SearchItemDelegate::paintSurroundingText(QPainter* painter, const QStyleOptionViewItem& option,
- const QString& text, const QString& surroundingText, bool matchCase, bool wholeWords) const
+ const QString& matchedText, const QString& surroundingText) const
{
const int textMargin = QApplication::style()->pixelMetric(QStyle::PM_FocusFrameHMargin) + 1;
const QRect textRect = option.rect.adjusted(textMargin, 0, -textMargin, 0);
@@ -691,7 +672,7 @@
textLayout.setText(elidedText);
textLayout.setFont(option.font);
- emphasizeText(text, matchCase, wholeWords, surroundingText, textLayout);
+ emphasizeText(matchedText, surroundingText, textLayout);
textLayout.beginLayout();
=== modified file 'sources/miscellaneous.h'
--- sources/miscellaneous.h 2015-01-17 12:47:36 +0000
+++ sources/miscellaneous.h 2015-01-24 11:17:40 +0000
@@ -345,7 +345,7 @@
void paintProgress(QPainter* painter, const QStyleOptionViewItem& option,
int progress) const;
void paintSurroundingText(QPainter* painter, const QStyleOptionViewItem& option,
- const QString& text, const QString& surroundingText, bool matchCase, bool wholeWords) const;
+ const QString& matchedText, const QString& surroundingText) const;
};
=== modified file 'sources/searchmodel.cpp'
--- sources/searchmodel.cpp 2015-01-11 13:17:34 +0000
+++ sources/searchmodel.cpp 2015-01-24 11:19:51 +0000
@@ -174,6 +174,8 @@
return view->searchMatchCase();
case WholeWordsRole:
return view->searchWholeWords();
+ case MatchedTextRole:
+ return fetchMatchedText(view, result);
case SurroundingTextRole:
return fetchSurroundingText(view, result);
case Qt::ToolTipRole:
@@ -377,7 +379,9 @@
return;
}
- m_textCache.insert(job.key, job.object, job.object->length());
+ const int cost = job.object->first.length() + job.object->second.length();
+
+ m_textCache.insert(job.key, job.object, cost);
emit dataChanged(createIndex(0, 0, view), createIndex(results->count() - 1, 0, view));
}
@@ -424,19 +428,28 @@
return createIndex(row, 0);
}
+QString SearchModel::fetchMatchedText(DocumentView* view, const SearchModel::Result& result) const
+{
+ const TextCacheObject* object = fetchText(view, result);
+
+ return object != 0 ? object->first : QString();
+}
+
QString SearchModel::fetchSurroundingText(DocumentView* view, const Result& result) const
{
- if(view == 0)
- {
- return QString();
- }
-
+ const TextCacheObject* object = fetchText(view, result);
+
+ return object != 0 ? object->second : QString();
+}
+
+const SearchModel::TextCacheObject* SearchModel::fetchText(DocumentView* view, const SearchModel::Result& result) const
+{
const TextCacheKey key = textCacheKey(view, result);
const TextCacheObject* object = m_textCache.object(key);
if(object != 0)
{
- return *object;
+ return object;
}
if(m_textWatchers.size() < 20 && !m_textWatchers.contains(key))
@@ -449,7 +462,7 @@
watcher->setFuture(QtConcurrent::run(textJob, key, result));
}
- return QLatin1String("...");
+ return 0;
}
inline SearchModel::TextCacheKey SearchModel::textCacheKey(DocumentView* view, const Result& result)
@@ -465,9 +478,9 @@
SearchModel::TextJob SearchModel::textJob(const TextCacheKey& key, const Result& result)
{
- const QString surroundingText = key.first->surroundingText(result.first, result.second);
+ const QPair< QString, QString > text = key.first->surroundingText(result.first, result.second);
- return TextJob(key, new QString(surroundingText));
+ return TextJob(key, new TextCacheObject(text));
}
} // qpdfview
=== modified file 'sources/searchmodel.h'
--- sources/searchmodel.h 2015-01-11 13:17:34 +0000
+++ sources/searchmodel.h 2015-01-24 11:14:34 +0000
@@ -58,6 +58,7 @@
TextRole,
MatchCaseRole,
WholeWordsRole,
+ MatchedTextRole,
SurroundingTextRole
};
@@ -107,7 +108,7 @@
typedef QPair< DocumentView*, QByteArray > TextCacheKey;
- typedef QString TextCacheObject;
+ typedef QPair< QString, QString > TextCacheObject;
struct TextJob
{
@@ -124,8 +125,11 @@
mutable QCache< TextCacheKey, TextCacheObject > m_textCache;
mutable QHash< TextCacheKey, TextWatcher* > m_textWatchers;
+ QString fetchMatchedText(DocumentView* view, const Result& result) const;
QString fetchSurroundingText(DocumentView* view, const Result& result) const;
+ const TextCacheObject* fetchText(DocumentView* view, const Result& result) const;
+
static TextCacheKey textCacheKey(DocumentView* view, const Result& result);
static TextJob textJob(const TextCacheKey& key, const Result& result);
signature.asc
Description: OpenPGP digital signature
-- Mailing list: https://launchpad.net/~qpdfview Post to : [email protected] Unsubscribe : https://launchpad.net/~qpdfview More help : https://help.launchpad.net/ListHelp
