Re: About the rowpainter2 branch (testers, have a look!)

[Abdelrazak Younes]

On 23/07/2015 16:45, Jean-Marc Lasgouttes wrote:

Le 23/07/2015 16:29, Richard Heck a écrit :

I wonder how bad this will be. In practice, people do not actually use
that many different words when writing. Probably no more than a few
hundred, and a couple thousand, at most.


It is not a problem of words, it is a problem of lines of text. For 
example, for displaying this previous sentence while typing it, I have 
to evaluate the length of


I
It
It_
It i
It is
It is_
It is n
[... lots of entries ...]]
It is not a problem of words, it is a problem of lines of te
It is not a problem of words, it is a problem of lines of tex
It is not a problem of words, it is a problem of lines of text
It is not a problem of words, it is a problem of lines of text.

These strings will remain forever in the map, and I will never need 
them anymore.


Easy way out (IIUC): Don't use the cache in the currently edited row or 
even the 2 rows after the current row in the current paragraph to cope 
with automatic justification.


Abdel

Re: About the rowpainter2 branch (testers, have a look!)

[Abdelrazak Younes]

On 23/07/2015 16:45, Jean-Marc Lasgouttes wrote:

Le 23/07/2015 16:29, Richard Heck a écrit :

I wonder how bad this will be. In practice, people do not actually use
that many different words when writing. Probably no more than a few
hundred, and a couple thousand, at most.


It is not a problem of words, it is a problem of lines of text. For 
example, for displaying this previous sentence while typing it, I have 
to evaluate the length of


I
It
It_
It i
It is
It is_
It is n
[... lots of entries ...]]
It is not a problem of words, it is a problem of lines of te
It is not a problem of words, it is a problem of lines of tex
It is not a problem of words, it is a problem of lines of text
It is not a problem of words, it is a problem of lines of text.

These strings will remain forever in the map, and I will never need 
them anymore.


Easy way out (IIUC): Don't use the cache in the currently edited row or 
even the 2 rows after the current row in the current paragraph to cope 
with automatic justification.


Abdel

Re: About the rowpainter2 branch (testers, have a look!)

[Helge Hafting]

Den 23. juli 2015 10:16, skrev Jean-Marc Lasgouttes:


Except for #9691.

The big remaining problem though is that I rely on a 
mapdocstring,int for caching word widths and this will probably fill 
up with long editing sessions. In the original rowpainter2 code, there 
were only words in the cache, and the number of words in a language is 
much less than the number of string that can apear in a row...
There are many strings - but instead the number of rows in a document is 
limited. There is one entry per screen line, right? (Assuming all are 
indeed different.) Even a 1000-page book will have a very limited number 
of screen lines - compared to the size of computer memory.


And this grows as the user resizes the window or edit text - but it'd 
take a long time to get into memory performance problems. I rarely have 
editing sessions lasting more than a week. I sometimes leaves an editor 
open overnight, but there is no point over a weekend.


Helge Hafting

Re: About the rowpainter2 branch (testers, have a look!)

[Jean-Marc Lasgouttes]

Le 23/07/2015 16:19, Helge Hafting a écrit :

There are many strings - but instead the number of rows in a document is
limited. There is one entry per screen line, right? (Assuming all are
indeed different.) Even a 1000-page book will have a very limited number
of screen lines - compared to the size of computer memory.


Well, while you edit there is probably a hundred of small variations 
around the same line. I assumes that this adds up...


JMarc

Re: About the rowpainter2 branch (testers, have a look!)

[Jean-Marc Lasgouttes]

Le 23/07/2015 16:29, Richard Heck a écrit :

I wonder how bad this will be. In practice, people do not actually use
that many different words when writing. Probably no more than a few
hundred, and a couple thousand, at most.


It is not a problem of words, it is a problem of lines of text. For 
example, for displaying this previous sentence while typing it, I have 
to evaluate the length of


I
It
It_
It i
It is
It is_
It is n
[... lots of entries ...]]
It is not a problem of words, it is a problem of lines of te
It is not a problem of words, it is a problem of lines of tex
It is not a problem of words, it is a problem of lines of text
It is not a problem of words, it is a problem of lines of text.

These strings will remain forever in the map, and I will never need them 
anymore.



Use of a hash, as you suggested
earlier, would probably be sufficient. I gather you just need to
implement qHash(docstring), as said in the FIXME, but that could be as
easy as:

 uint qHash(const  docstring d) { return qHash(toqstr(d)); }


Strangely enough, I tried to do this, but the compiler never accepted to 
link. I finally gave up and used a hash on QString instead. This is not 
has good since the docstring-QString has to be done every time.


However, my approximate profiling with Instruments showed that, while 
with current code display cost it 60% painting and 40% metrics, with 
QCacheQString the cost i more like 50%/50%, so that metrics 
computation is less efficient.


I suspect that computing hash on long strings is less efficient than 
sorting them in a map.


There is code floating around for having a map with an additional list 
that orders entries. It is not so complicated to use, but my time is 
running out right now.


JMarc

Re: About the rowpainter2 branch (testers, have a look!)

[Richard Heck]

On 07/23/2015 04:16 AM, Jean-Marc Lasgouttes wrote:

Le 23/07/2015 08:09, Jürgen Spitzmüller a écrit :

Am Donnerstag 23 Juli 2015, 00:43:35 schrieb Jean-Marc Lasgouttes:

Le 22/07/2015 17:42, Jürgen Spitzmüller a écrit :
Yes, this is with recent master. I can easily reproduce it with sec 
2.5.1

of the User Guide, for instance (see attached screenshot).


OK, try again now. Let's see how it fares.


The problem is fixed. Looks quite good in general now, from what I 
can see.


Except for #9691.

The big remaining problem though is that I rely on a 
mapdocstring,int for caching word widths and this will probably fill 
up with long editing sessions. In the original rowpainter2 code, there 
were only words in the cache, and the number of words in a language is 
much less than the number of string that can apear in a row...


I wonder how bad this will be. In practice, people do not actually use 
that many different words when writing. Probably no more than a few 
hundred, and a couple thousand, at most. Use of a hash, as you suggested 
earlier, would probably be sufficient. I gather you just need to 
implement qHash(docstring), as said in the FIXME, but that could be as 
easy as:


uint qHash(const  docstring d) { return qHash(toqstr(d)); }

Alternatively, we could borrow the qHash code:

https://searchcode.com/codesearch/view/47469703/

From a quick look, it actually converts the QString to unicode before 
operating on it:


uint qHash(const QString key)
{
return hash(key.unicode(), key.size());
}

static uint hash(const uchar *p, int n)
{
uint h = 0;

while (n--) {
h = (h  4) + *p++;
h ^= (h  0xf000)  23;
h = 0x0fff;
}
return h;
}

So we might just need some version of the latter.

Richard

Re: About the rowpainter2 branch (testers, have a look!)

[Jürgen Spitzmüller]

Am Donnerstag 23 Juli 2015, 00:43:35 schrieb Jean-Marc Lasgouttes:
 Le 22/07/2015 17:42, Jürgen Spitzmüller a écrit :
  Yes, this is with recent master. I can easily reproduce it with sec 2.5.1
  of the User Guide, for instance (see attached screenshot).
 
 OK, try again now. Let's see how it fares.

The problem is fixed. Looks quite good in general now, from what I can see.

Jürgen

 JMarc

Re: About the rowpainter2 branch (testers, have a look!)

[Jean-Marc Lasgouttes]

Le 23/07/2015 08:09, Jürgen Spitzmüller a écrit :

Am Donnerstag 23 Juli 2015, 00:43:35 schrieb Jean-Marc Lasgouttes:

Le 22/07/2015 17:42, Jürgen Spitzmüller a écrit :

Yes, this is with recent master. I can easily reproduce it with sec 2.5.1
of the User Guide, for instance (see attached screenshot).


OK, try again now. Let's see how it fares.


The problem is fixed. Looks quite good in general now, from what I can see.


Except for #9691.

The big remaining problem though is that I rely on a mapdocstring,int 
for caching word widths and this will probably fill up with long editing 
sessions. In the original rowpainter2 code, there were only words in the 
cache, and the number of words in a language is much less than the 
number of string that can apear in a row...


JMarc

Re: About the rowpainter2 branch (testers, have a look!)

[Jürgen Spitzmüller]

Am Donnerstag 23 Juli 2015, 00:43:35 schrieb Jean-Marc Lasgouttes:
> Le 22/07/2015 17:42, Jürgen Spitzmüller a écrit :
> > Yes, this is with recent master. I can easily reproduce it with sec 2.5.1
> > of the User Guide, for instance (see attached screenshot).
> 
> OK, try again now. Let's see how it fares.

The problem is fixed. Looks quite good in general now, from what I can see.

Jürgen

> JMarc

Re: About the rowpainter2 branch (testers, have a look!)

[Jean-Marc Lasgouttes]

Le 23/07/2015 08:09, Jürgen Spitzmüller a écrit :

Am Donnerstag 23 Juli 2015, 00:43:35 schrieb Jean-Marc Lasgouttes:

Le 22/07/2015 17:42, Jürgen Spitzmüller a écrit :

Yes, this is with recent master. I can easily reproduce it with sec 2.5.1
of the User Guide, for instance (see attached screenshot).


OK, try again now. Let's see how it fares.


The problem is fixed. Looks quite good in general now, from what I can see.


Except for #9691.

The big remaining problem though is that I rely on a map 
for caching word widths and this will probably fill up with long editing 
sessions. In the original rowpainter2 code, there were only words in the 
cache, and the number of words in a language is much less than the 
number of string that can apear in a row...


JMarc

Re: About the rowpainter2 branch (testers, have a look!)

[Helge Hafting]

Den 23. juli 2015 10:16, skrev Jean-Marc Lasgouttes:


Except for #9691.

The big remaining problem though is that I rely on a 
map for caching word widths and this will probably fill 
up with long editing sessions. In the original rowpainter2 code, there 
were only words in the cache, and the number of words in a language is 
much less than the number of string that can apear in a row...
There are many strings - but instead the number of rows in a document is 
limited. There is one entry per screen line, right? (Assuming all are 
indeed different.) Even a 1000-page book will have a very limited number 
of screen lines - compared to the size of computer memory.


And this grows as the user resizes the window or edit text - but it'd 
take a long time to get into memory performance problems. I rarely have 
editing sessions lasting more than a week. I sometimes leaves an editor 
open overnight, but there is no point over a weekend.


Helge Hafting

Re: About the rowpainter2 branch (testers, have a look!)

[Richard Heck]

On 07/23/2015 04:16 AM, Jean-Marc Lasgouttes wrote:

Le 23/07/2015 08:09, Jürgen Spitzmüller a écrit :

Am Donnerstag 23 Juli 2015, 00:43:35 schrieb Jean-Marc Lasgouttes:

Le 22/07/2015 17:42, Jürgen Spitzmüller a écrit :
Yes, this is with recent master. I can easily reproduce it with sec 
2.5.1

of the User Guide, for instance (see attached screenshot).


OK, try again now. Let's see how it fares.


The problem is fixed. Looks quite good in general now, from what I 
can see.


Except for #9691.

The big remaining problem though is that I rely on a 
map for caching word widths and this will probably fill 
up with long editing sessions. In the original rowpainter2 code, there 
were only words in the cache, and the number of words in a language is 
much less than the number of string that can apear in a row...


I wonder how bad this will be. In practice, people do not actually use 
that many different words when writing. Probably no more than a few 
hundred, and a couple thousand, at most. Use of a hash, as you suggested 
earlier, would probably be sufficient. I gather you just need to 
implement qHash(docstring), as said in the FIXME, but that could be as 
easy as:


uint qHash(const & docstring d) { return qHash(toqstr(d)); }

Alternatively, we could borrow the qHash code:

https://searchcode.com/codesearch/view/47469703/

From a quick look, it actually converts the QString to unicode before 
operating on it:


uint qHash(const QString )
{
return hash(key.unicode(), key.size());
}

static uint hash(const uchar *p, int n)
{
uint h = 0;

while (n--) {
h = (h << 4) + *p++;
h ^= (h & 0xf000) >> 23;
h &= 0x0fff;
}
return h;
}

So we might just need some version of the latter.

Richard

Re: About the rowpainter2 branch (testers, have a look!)

[Jean-Marc Lasgouttes]

Le 23/07/2015 16:19, Helge Hafting a écrit :

There are many strings - but instead the number of rows in a document is
limited. There is one entry per screen line, right? (Assuming all are
indeed different.) Even a 1000-page book will have a very limited number
of screen lines - compared to the size of computer memory.


Well, while you edit there is probably a hundred of small variations 
around the same line. I assumes that this adds up...


JMarc

Re: About the rowpainter2 branch (testers, have a look!)

[Jean-Marc Lasgouttes]

Le 23/07/2015 16:29, Richard Heck a écrit :

I wonder how bad this will be. In practice, people do not actually use
that many different words when writing. Probably no more than a few
hundred, and a couple thousand, at most.


It is not a problem of words, it is a problem of lines of text. For 
example, for displaying this previous sentence while typing it, I have 
to evaluate the length of


I
It
It_
It i
It is
It is_
It is n
[... lots of entries ...]]
It is not a problem of words, it is a problem of lines of te
It is not a problem of words, it is a problem of lines of tex
It is not a problem of words, it is a problem of lines of text
It is not a problem of words, it is a problem of lines of text.

These strings will remain forever in the map, and I will never need them 
anymore.



Use of a hash, as you suggested
earlier, would probably be sufficient. I gather you just need to
implement qHash(docstring), as said in the FIXME, but that could be as
easy as:

 uint qHash(const & docstring d) { return qHash(toqstr(d)); }


Strangely enough, I tried to do this, but the compiler never accepted to 
link. I finally gave up and used a hash on QString instead. This is not 
has good since the docstring->QString has to be done every time.


However, my approximate profiling with Instruments showed that, while 
with current code display cost it 60% painting and 40% metrics, with 
QCache the cost i more like 50%/50%, so that metrics 
computation is less efficient.


I suspect that computing hash on long strings is less efficient than 
sorting them in a map.


There is code floating around for having a map with an additional list 
that orders entries. It is not so complicated to use, but my time is 
running out right now.


JMarc

Re: About the rowpainter2 branch (testers, have a look!)

[Jean-Marc Lasgouttes]

Le 22/07/2015 13:34, Jürgen Spitzmüller a écrit :

Am Mittwoch 22 Juli 2015, 11:55:32 schrieb Jean-Marc Lasgouttes:

I fixed this problem. Now I am interested in any row-breaking behavior
that does not sound good.


It breaks between xref inset and closing bracket, e.g. in (for details, cf.
[XRef Inset]).


Are you sure this is after 7d19265ba02aa0f7 ? I cannot reproduce.

JMarc

Re: About the rowpainter2 branch (testers, have a look!)

[Jean-Marc Lasgouttes]

Le 22/07/2015 05:56, Scott Kostyshak a écrit :

On Tue, Jul 21, 2015 at 6:04 PM, Jean-Marc Lasgouttes
lasgout...@lyx.org wrote:


I landed in master now.


I could be imagining things, but scrolling feels smoother to me.


Yes it should. The number of QPainter::drawText calls is reduced a lot 
(but the strings are longer).



Are you interested in *any* difference in behavior I notice when
testing? For example, if a linebreak is at a different (but still
sensible) place than it was before?


No, because I am not sure that my row breaking after str-metrics was 
sensible. I am interested though in the worst offenders. A good example 
is given by breaking that can happen just before or just after a quote 
inset. This is definitely bad.


JMarc

Re: About the rowpainter2 branch (testers, have a look!)

[Jürgen Spitzmüller]

Am Mittwoch 22 Juli 2015, 11:55:32 schrieb Jean-Marc Lasgouttes:
 I fixed this problem. Now I am interested in any row-breaking behavior 
 that does not sound good.

It breaks between xref inset and closing bracket, e.g. in (for details, cf. 
[XRef Inset]).

Jürgen

Re: About the rowpainter2 branch (testers, have a look!)

[Jean-Marc Lasgouttes]

Le 22/07/2015 09:40, Jean-Marc Lasgouttes a écrit :

Are you interested in *any* difference in behavior I notice when
testing? For example, if a linebreak is at a different (but still
sensible) place than it was before?


No, because I am not sure that my row breaking after str-metrics was
sensible. I am interested though in the worst offenders. A good example
is given by breaking that can happen just before or just after a quote
inset. This is definitely bad.


I fixed this problem. Now I am interested in any row-breaking behavior 
that does not sound good.


JMarc

Re: About the rowpainter2 branch (testers, have a look!)

[Jean-Marc Lasgouttes]

Le 22/07/2015 17:42, Jürgen Spitzmüller a écrit :

Yes, this is with recent master. I can easily reproduce it with sec 2.5.1 of
the User Guide, for instance (see attached screenshot).


OK, try again now. Let's see how it fares.

JMarc

Re: About the rowpainter2 branch (testers, have a look!)

[Jean-Marc Lasgouttes]

Le 22/07/2015 05:56, Scott Kostyshak a écrit :

On Tue, Jul 21, 2015 at 6:04 PM, Jean-Marc Lasgouttes
 wrote:


I landed in master now.


I could be imagining things, but scrolling feels smoother to me.


Yes it should. The number of QPainter::drawText calls is reduced a lot 
(but the strings are longer).



Are you interested in *any* difference in behavior I notice when
testing? For example, if a linebreak is at a different (but still
sensible) place than it was before?


No, because I am not sure that my row breaking after str-metrics was 
sensible. I am interested though in the worst offenders. A good example 
is given by breaking that can happen just before or just after a quote 
inset. This is definitely bad.


JMarc

Re: About the rowpainter2 branch (testers, have a look!)

[Jean-Marc Lasgouttes]

Le 22/07/2015 09:40, Jean-Marc Lasgouttes a écrit :

Are you interested in *any* difference in behavior I notice when
testing? For example, if a linebreak is at a different (but still
sensible) place than it was before?


No, because I am not sure that my row breaking after str-metrics was
sensible. I am interested though in the worst offenders. A good example
is given by breaking that can happen just before or just after a quote
inset. This is definitely bad.


I fixed this problem. Now I am interested in any row-breaking behavior 
that does not sound good.


JMarc

Re: About the rowpainter2 branch (testers, have a look!)

[Jürgen Spitzmüller]

Am Mittwoch 22 Juli 2015, 11:55:32 schrieb Jean-Marc Lasgouttes:
> I fixed this problem. Now I am interested in any row-breaking behavior 
> that does not sound good.

It breaks between xref inset and closing bracket, e.g. in "(for details, cf. 
[XRef Inset])."

Jürgen

Re: About the rowpainter2 branch (testers, have a look!)

[Jean-Marc Lasgouttes]

Le 22/07/2015 13:34, Jürgen Spitzmüller a écrit :

Am Mittwoch 22 Juli 2015, 11:55:32 schrieb Jean-Marc Lasgouttes:

I fixed this problem. Now I am interested in any row-breaking behavior
that does not sound good.


It breaks between xref inset and closing bracket, e.g. in "(for details, cf.
[XRef Inset])."


Are you sure this is after 7d19265ba02aa0f7 ? I cannot reproduce.

JMarc

Re: About the rowpainter2 branch (testers, have a look!)

[Jean-Marc Lasgouttes]

Le 22/07/2015 17:42, Jürgen Spitzmüller a écrit :

Yes, this is with recent master. I can easily reproduce it with sec 2.5.1 of
the User Guide, for instance (see attached screenshot).


OK, try again now. Let's see how it fares.

JMarc

Re: About the rowpainter2 branch (testers, have a look!)

[Richard Heck]

On 07/21/2015 09:36 AM, Scott Kostyshak wrote:

On Tue, Jul 21, 2015 at 8:10 AM, Jean-Marc Lasgouttes
lasgout...@lyx.org wrote:


Basically, I'd appreciate if some people could test it so that we can see
what's right and what's wrong. Since leave on thursday for vacation, the
choice is between landing it now or landing it in late August when I come
back. But I have more time now to fix bugs than I will have in August.

What shall I do?

I propose that you land it in master.


I agree, for all Scott's reasons. 2.2 is not exactly imminent (though we 
ought seriously to try to get a schedule in place).


Richard

Re: About the rowpainter2 branch (testers, have a look!)

[Stephan Witt]

Am 21.07.2015 um 18:04 schrieb Richard Heck rgh...@lyx.org:

 On 07/21/2015 09:36 AM, Scott Kostyshak wrote:
 On Tue, Jul 21, 2015 at 8:10 AM, Jean-Marc Lasgouttes
 lasgout...@lyx.org wrote:
 
 Basically, I'd appreciate if some people could test it so that we can see
 what's right and what's wrong. Since leave on thursday for vacation, the
 choice is between landing it now or landing it in late August when I come
 back. But I have more time now to fix bugs than I will have in August.
 
 What shall I do?
 I propose that you land it in master.
 
 I agree, for all Scott's reasons. 2.2 is not exactly imminent (though we 
 ought seriously to try to get a schedule in place).

+1

I tried it on Mac OS X with Qt 5 and cannot see any issue at a first glance.

Stephan

Re: About the rowpainter2 branch (testers, have a look!)

[Scott Kostyshak]

On Tue, Jul 21, 2015 at 6:04 PM, Jean-Marc Lasgouttes
lasgout...@lyx.org wrote:

 I landed in master now.

I could be imagining things, but scrolling feels smoother to me.

Are you interested in *any* difference in behavior I notice when
testing? For example, if a linebreak is at a different (but still
sensible) place than it was before?

Scott

Re: About the rowpainter2 branch (testers, have a look!)

[Georg Baum]

Richard Heck wrote:

 On 07/21/2015 09:36 AM, Scott Kostyshak wrote:
 On Tue, Jul 21, 2015 at 8:10 AM, Jean-Marc Lasgouttes
 lasgout...@lyx.org wrote:

 Basically, I'd appreciate if some people could test it so that we can
 see what's right and what's wrong. Since leave on thursday for vacation,
 the choice is between landing it now or landing it in late August when I
 come back. But I have more time now to fix bugs than I will have in
 August.

 What shall I do?
 I propose that you land it in master.

+1
 
 I agree, for all Scott's reasons. 2.2 is not exactly imminent (though we
 ought seriously to try to get a schedule in place).

+1 as well.


Georg

Re: About the rowpainter2 branch (testers, have a look!)

[Jean-Marc Lasgouttes]

Le 21/07/15 15:36, Scott Kostyshak a écrit :

I propose that you land it in master. If something goes very wrong, we
can always revert it without you here. This is why I really appreciate
your having done the work on a branch (as opposed to master directly),
so it is easy to merge/unmerge.


I landed in master now. I tried a bit to use QCache to make call to 
speed-up computation of string width, but it des not sem to work better 
than a normal std::map on my older iMac. It might be that hash tables 
are not good when keys are long-ish strings. I attach the patch in case 
somebody good at Qt and/or STL wants to have a look


JMarc

From f76e728dbc026e54ff6bdb9d5cf7568012a148e7 Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes lasgout...@lyx.org
Date: Tue, 21 Jul 2015 23:19:40 +0200
Subject: [PATCH] Use QCache to cache string width

This buys us a hash table and some LRU eviction of old strings.
---
 src/frontends/qt4/GuiFontLoader.cpp  | 12 
 src/frontends/qt4/GuiFontMetrics.cpp | 15 ++-
 src/frontends/qt4/GuiFontMetrics.h   |  5 ++---
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/frontends/qt4/GuiFontLoader.cpp 
b/src/frontends/qt4/GuiFontLoader.cpp
index 69f1ed2..22df6e8 100644
--- a/src/frontends/qt4/GuiFontLoader.cpp
+++ b/src/frontends/qt4/GuiFontLoader.cpp
@@ -269,9 +269,9 @@ static QString makeFontName(QString const  family, QString 
const  foundry)
 }
 
 
-GuiFontInfo::GuiFontInfo(FontInfo const  f)
-   : metrics(QFont())
+static QFont makeQFont(FontInfo const  f)
 {
+   QFont font;
QString const pat = symbolFamily(f.family());
if (!pat.isEmpty()) {
bool ok;
@@ -346,11 +346,15 @@ GuiFontInfo::GuiFontInfo(FontInfo const  f)
   * lyxrc.zoom / 100.0);
 
LYXERR(Debug::FONT, The font has size:   font.pointSizeF());
-
-   metrics = GuiFontMetrics(font);
+   return font;
 }
 
 
+GuiFontInfo::GuiFontInfo(FontInfo const  f)
+   : font(makeQFont(f)), metrics(font)
+{}
+
+
 bool FontLoader::available(FontInfo const  f)
 {
// FIXME THREAD
diff --git a/src/frontends/qt4/GuiFontMetrics.cpp 
b/src/frontends/qt4/GuiFontMetrics.cpp
index e41ef3c..54d60fd 100644
--- a/src/frontends/qt4/GuiFontMetrics.cpp
+++ b/src/frontends/qt4/GuiFontMetrics.cpp
@@ -12,7 +12,6 @@
 #include config.h
 
 #include GuiFontMetrics.h
-
 #include qt_helpers.h
 
 #include Dimension.h
@@ -120,14 +119,12 @@ int GuiFontMetrics::rbearing(char_type c) const
 
 int GuiFontMetrics::width(docstring const  s) const
 {
-   int w = 0;
-   mapdocstring, int::const_iterator it = strwidth_cache_.find(s);
-   if (it != strwidth_cache_.end()) {
-   w = it-second;
-   } else {
-   w = metrics_.width(toqstr(s));
-   strwidth_cache_[s] = w;
-   }
+   QString const qs = toqstr(s);
+   int * pw = strwidth_cache_[qs];
+   if (pw)
+   return *pw;
+   int w = metrics_.width(qs);
+   strwidth_cache_.insert(qs, new int(w));
return w;
 }
 
diff --git a/src/frontends/qt4/GuiFontMetrics.h 
b/src/frontends/qt4/GuiFontMetrics.h
index a382253..8797e53 100644
--- a/src/frontends/qt4/GuiFontMetrics.h
+++ b/src/frontends/qt4/GuiFontMetrics.h
@@ -16,8 +16,7 @@
 
 #include support/docstring.h
 
-#include map
-
+#include QCache
 #include QFont
 #include QFontMetrics
 #include QHash
@@ -71,7 +70,7 @@ private:
 
/// Cache of string widths
/// FIXME Try to use a QHash (this requires to define qHash(docstring))
-   mutable std::mapdocstring, int strwidth_cache_;
+   mutable QCacheQString, int strwidth_cache_;
 
struct AscendDescend {
int ascent;
-- 
2.1.4

About the rowpainter2 branch (testers, have a look!)

[Jean-Marc Lasgouttes]

Hello,

I have now done most of what I wanted on the rowpainter2 branch. When I 
decided a few month ago that it would be a good JMarc Week of Code 
project I thought that 2.2 would be near enough then and that this work 
would be 2.3 material. Now I guess it makes more sense to land it for 
2.2. Actually, there is less code involved that I thought there would be.


What is done:

* the RowPainter code relies directly on Row tokenization,except for 
labels, endlabels and stuff like that. This could be added, but I am not 
sure it is worth the hassle.


* full ranges of text are now painted directly, instead of doing it 
word-by-word. This gives a nice speed up.


* Chinese text should be shown correctly on screen.


What remains to do:

* there are some situations where the text is broken at inset boundary 
and breaking at a space would have been better. I have to tweak the 
shortenIfNeeded method.


* since we compute metrics for full sentences instead of words, the 
width cache, which is a plain map, does not really make sense. The cache 
hit rate cannot be good, and after long editing sessions I suspect that 
the memory use will be horrible. The solution is either to get rid of it 
or to replace it with a LRU cache container. Does anyone know of a good 
simple one with appropriate license ? Or shall I write it myself?



What will (probably) not be done:

* I gave up on my plan to get rid of Bidi completely. The last user is 
Cursor::getSurroundingPos, and one needs to replace unpleasant code by 
some other unpleasant code. I will be neither worse nor better, but it 
will not rely on Bidi. Since it is peripheral to the main effort, I did 
not do it.




Basically, I'd appreciate if some people could test it so that we can 
see what's right and what's wrong. Since leave on thursday for vacation, 
the choice is between landing it now or landing it in late August when I 
come back. But I have more time now to fix bugs than I will have in August.


What shall I do?

JMarc

Re: About the rowpainter2 branch (testers, have a look!)

[Scott Kostyshak]

On Tue, Jul 21, 2015 at 8:10 AM, Jean-Marc Lasgouttes
lasgout...@lyx.org wrote:

 Basically, I'd appreciate if some people could test it so that we can see
 what's right and what's wrong. Since leave on thursday for vacation, the
 choice is between landing it now or landing it in late August when I come
 back. But I have more time now to fix bugs than I will have in August.

 What shall I do?

I propose that you land it in master. If something goes very wrong, we
can always revert it without you here. This is why I really appreciate
your having done the work on a branch (as opposed to master directly),
so it is easy to merge/unmerge.

Another (more minor) reason to put it in master is that the LyX daily
PPA will have the work and people on Ubuntu who want to test it can
easily do so.

If someone disagrees with putting it in master, I can test the features branch.

Scott

About the rowpainter2 branch (testers, have a look!)

[Jean-Marc Lasgouttes]

Hello,

I have now done most of what I wanted on the rowpainter2 branch. When I 
decided a few month ago that it would be a good JMarc Week of Code 
project I thought that 2.2 would be near enough then and that this work 
would be 2.3 material. Now I guess it makes more sense to land it for 
2.2. Actually, there is less code involved that I thought there would be.


What is done:

* the RowPainter code relies directly on Row tokenization,except for 
labels, endlabels and stuff like that. This could be added, but I am not 
sure it is worth the hassle.


* full ranges of text are now painted directly, instead of doing it 
word-by-word. This gives a nice speed up.


* Chinese text should be shown correctly on screen.


What remains to do:

* there are some situations where the text is broken at inset boundary 
and breaking at a space would have been better. I have to tweak the 
shortenIfNeeded method.


* since we compute metrics for full sentences instead of words, the 
width cache, which is a plain map, does not really make sense. The cache 
hit rate cannot be good, and after long editing sessions I suspect that 
the memory use will be horrible. The solution is either to get rid of it 
or to replace it with a LRU cache container. Does anyone know of a good 
simple one with appropriate license ? Or shall I write it myself?



What will (probably) not be done:

* I gave up on my plan to get rid of Bidi completely. The last user is 
Cursor::getSurroundingPos, and one needs to replace unpleasant code by 
some other unpleasant code. I will be neither worse nor better, but it 
will not rely on Bidi. Since it is peripheral to the main effort, I did 
not do it.




Basically, I'd appreciate if some people could test it so that we can 
see what's right and what's wrong. Since leave on thursday for vacation, 
the choice is between landing it now or landing it in late August when I 
come back. But I have more time now to fix bugs than I will have in August.


What shall I do?

JMarc

Re: About the rowpainter2 branch (testers, have a look!)

[Scott Kostyshak]

On Tue, Jul 21, 2015 at 8:10 AM, Jean-Marc Lasgouttes
 wrote:

> Basically, I'd appreciate if some people could test it so that we can see
> what's right and what's wrong. Since leave on thursday for vacation, the
> choice is between landing it now or landing it in late August when I come
> back. But I have more time now to fix bugs than I will have in August.
>
> What shall I do?

I propose that you land it in master. If something goes very wrong, we
can always revert it without you here. This is why I really appreciate
your having done the work on a branch (as opposed to master directly),
so it is easy to merge/unmerge.

Another (more minor) reason to put it in master is that the LyX daily
PPA will have the work and people on Ubuntu who want to test it can
easily do so.

If someone disagrees with putting it in master, I can test the features branch.

Scott

Re: About the rowpainter2 branch (testers, have a look!)

[Richard Heck]

On 07/21/2015 09:36 AM, Scott Kostyshak wrote:

On Tue, Jul 21, 2015 at 8:10 AM, Jean-Marc Lasgouttes
 wrote:


Basically, I'd appreciate if some people could test it so that we can see
what's right and what's wrong. Since leave on thursday for vacation, the
choice is between landing it now or landing it in late August when I come
back. But I have more time now to fix bugs than I will have in August.

What shall I do?

I propose that you land it in master.


I agree, for all Scott's reasons. 2.2 is not exactly imminent (though we 
ought seriously to try to get a schedule in place).


Richard

Re: About the rowpainter2 branch (testers, have a look!)

[Stephan Witt]

Am 21.07.2015 um 18:04 schrieb Richard Heck :

> On 07/21/2015 09:36 AM, Scott Kostyshak wrote:
>> On Tue, Jul 21, 2015 at 8:10 AM, Jean-Marc Lasgouttes
>>  wrote:
>> 
>>> Basically, I'd appreciate if some people could test it so that we can see
>>> what's right and what's wrong. Since leave on thursday for vacation, the
>>> choice is between landing it now or landing it in late August when I come
>>> back. But I have more time now to fix bugs than I will have in August.
>>> 
>>> What shall I do?
>> I propose that you land it in master.
> 
> I agree, for all Scott's reasons. 2.2 is not exactly imminent (though we 
> ought seriously to try to get a schedule in place).

+1

I tried it on Mac OS X with Qt 5 and cannot see any issue at a first glance.

Stephan

Re: About the rowpainter2 branch (testers, have a look!)

[Georg Baum]

Richard Heck wrote:

> On 07/21/2015 09:36 AM, Scott Kostyshak wrote:
>> On Tue, Jul 21, 2015 at 8:10 AM, Jean-Marc Lasgouttes
>>  wrote:
>>
>>> Basically, I'd appreciate if some people could test it so that we can
>>> see what's right and what's wrong. Since leave on thursday for vacation,
>>> the choice is between landing it now or landing it in late August when I
>>> come back. But I have more time now to fix bugs than I will have in
>>> August.
>>>
>>> What shall I do?
>> I propose that you land it in master.

+1
 
> I agree, for all Scott's reasons. 2.2 is not exactly imminent (though we
> ought seriously to try to get a schedule in place).

+1 as well.


Georg

Re: About the rowpainter2 branch (testers, have a look!)

[Jean-Marc Lasgouttes]

Le 21/07/15 15:36, Scott Kostyshak a écrit :

I propose that you land it in master. If something goes very wrong, we
can always revert it without you here. This is why I really appreciate
your having done the work on a branch (as opposed to master directly),
so it is easy to merge/unmerge.


I landed in master now. I tried a bit to use QCache to make call to 
speed-up computation of string width, but it des not sem to work better 
than a normal std::map on my older iMac. It might be that hash tables 
are not good when keys are long-ish strings. I attach the patch in case 
somebody good at Qt and/or STL wants to have a look


JMarc

From f76e728dbc026e54ff6bdb9d5cf7568012a148e7 Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes 
Date: Tue, 21 Jul 2015 23:19:40 +0200
Subject: [PATCH] Use QCache to cache string width

This buys us a hash table and some LRU eviction of old strings.
---
 src/frontends/qt4/GuiFontLoader.cpp  | 12 
 src/frontends/qt4/GuiFontMetrics.cpp | 15 ++-
 src/frontends/qt4/GuiFontMetrics.h   |  5 ++---
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/frontends/qt4/GuiFontLoader.cpp 
b/src/frontends/qt4/GuiFontLoader.cpp
index 69f1ed2..22df6e8 100644
--- a/src/frontends/qt4/GuiFontLoader.cpp
+++ b/src/frontends/qt4/GuiFontLoader.cpp
@@ -269,9 +269,9 @@ static QString makeFontName(QString const & family, QString 
const & foundry)
 }
 
 
-GuiFontInfo::GuiFontInfo(FontInfo const & f)
-   : metrics(QFont())
+static QFont makeQFont(FontInfo const & f)
 {
+   QFont font;
QString const pat = symbolFamily(f.family());
if (!pat.isEmpty()) {
bool ok;
@@ -346,11 +346,15 @@ GuiFontInfo::GuiFontInfo(FontInfo const & f)
   * lyxrc.zoom / 100.0);
 
LYXERR(Debug::FONT, "The font has size: " << font.pointSizeF());
-
-   metrics = GuiFontMetrics(font);
+   return font;
 }
 
 
+GuiFontInfo::GuiFontInfo(FontInfo const & f)
+   : font(makeQFont(f)), metrics(font)
+{}
+
+
 bool FontLoader::available(FontInfo const & f)
 {
// FIXME THREAD
diff --git a/src/frontends/qt4/GuiFontMetrics.cpp 
b/src/frontends/qt4/GuiFontMetrics.cpp
index e41ef3c..54d60fd 100644
--- a/src/frontends/qt4/GuiFontMetrics.cpp
+++ b/src/frontends/qt4/GuiFontMetrics.cpp
@@ -12,7 +12,6 @@
 #include 
 
 #include "GuiFontMetrics.h"
-
 #include "qt_helpers.h"
 
 #include "Dimension.h"
@@ -120,14 +119,12 @@ int GuiFontMetrics::rbearing(char_type c) const
 
 int GuiFontMetrics::width(docstring const & s) const
 {
-   int w = 0;
-   map::const_iterator it = strwidth_cache_.find(s);
-   if (it != strwidth_cache_.end()) {
-   w = it->second;
-   } else {
-   w = metrics_.width(toqstr(s));
-   strwidth_cache_[s] = w;
-   }
+   QString const qs = toqstr(s);
+   int * pw = strwidth_cache_[qs];
+   if (pw)
+   return *pw;
+   int w = metrics_.width(qs);
+   strwidth_cache_.insert(qs, new int(w));
return w;
 }
 
diff --git a/src/frontends/qt4/GuiFontMetrics.h 
b/src/frontends/qt4/GuiFontMetrics.h
index a382253..8797e53 100644
--- a/src/frontends/qt4/GuiFontMetrics.h
+++ b/src/frontends/qt4/GuiFontMetrics.h
@@ -16,8 +16,7 @@
 
 #include "support/docstring.h"
 
-#include 
-
+#include 
 #include 
 #include 
 #include 
@@ -71,7 +70,7 @@ private:
 
/// Cache of string widths
/// FIXME Try to use a QHash (this requires to define qHash(docstring))
-   mutable std::map strwidth_cache_;
+   mutable QCache strwidth_cache_;
 
struct AscendDescend {
int ascent;
-- 
2.1.4

Re: About the rowpainter2 branch (testers, have a look!)

[Scott Kostyshak]

On Tue, Jul 21, 2015 at 6:04 PM, Jean-Marc Lasgouttes
 wrote:

> I landed in master now.

I could be imagining things, but scrolling feels smoother to me.

Are you interested in *any* difference in behavior I notice when
testing? For example, if a linebreak is at a different (but still
sensible) place than it was before?

Scott