Re: [LyX/master] Refine lang nesting fix
On Fri, Jun 19, 2015 at 07:03:14PM +0200, Juergen Spitzmueller wrote:
> commit 46aed6d2b90cafed93a6ee996ec72c09a6325f92
> Author: Juergen Spitzmueller
> Date: Fri Jun 19 19:02:35 2015 +0200
>
> Refine lang nesting fix
>
After this commit I believe that many tests fail (I have only checked
that one test fails because of the commit but I am guessing that many
other failures are due to it as well).
After this commit, the following file fails to compile
doc/de/Additional.lyx
The LaTeX errors I get are about \end{english}.
Jürgen if you don't have time to look into the regression, do you
recommend that this commit be reverted?
Scott
Re: [PATCH] Get rid of ParagraphMetrics::insetDimension
On Thu, Oct 15, 2015 at 03:30:21PM +0200, Jean-Marc Lasgouttes wrote:
> Scott, OK?
Yes.
> // FIXME (Abdel 23/09/2007): this is a bit messy
> because of the
> // elimination of Inset::dim_ cache. This coordOffset()
> method needs
> // to be rewritten in light of the new design.
> - Dimension const & dim = parMetrics(dit[i - 1].text(),
> - dit[i - 1].pit()).insetDimension(&sl.inset());
> + Dimension const & dim =
> coordCache().getInsets().dim(&sl.inset());
> lastw = dim.wid;
> } else {
> Dimension const dim = sl.inset().dimension(*this);
I don't suppose your fix addressed the FIXME ?
Scott
Re: When to court Qt 5.6?
On Thu, Oct 15, 2015 at 08:37:41AM +0200, Jean-Marc Lasgouttes wrote: > Le 15/10/15 01:06, Scott Kostyshak a écrit : > >LyX 2.2.0 and the following 2.2.x releases will still continue to work > >well with Qt 4.8.6 but will also support Qt 5.6, which brings some > >advantages most notably for users with HiDPI displays. Note that if you > >compile LyX with a Qt 5 release before 5.6 you are likely to run into > >several regressions with respect to Qt 4.8.6. See #9215 for a list of > >bugs related to compiling LyX with different versions of Qt. > > The minimum Qt version is actually 4.5, although I do not know who is able > to check that. I could try to test it in a virtual machine, though. Thanks for catching that. I changed it to mention 4.5. I'm not sure it's worth the time to test, but if you are curious then let us know what you find. Scott
Re: IBus compatibility
Guillaume Munch wrote: > Beware of any confusion, the deadlock bug is with qt4 and is solved in > qt5 (any version I could test). Another bug makes qt < 5.6 impractical > for (all) Linux users. So the situation is more complex than what you > wrote. Oh, sorry, then I misunderstood the situation completely. I agree that something has to be done in this case. Georg
Re: [patches] the last ones before 2.2
Le 15/10/2015 18:20, Georg Baum a écrit : Jean-Marc Lasgouttes wrote: Why do you need a new member variable at all? Why can't you just use the memory address? This would not consume memory and still be a unique id. I guess that the answer is the same as for paragraph ids: they can get copied at unexpected times. If my understanding of the patch is correct, then the id is incremented on each copy, therefore I don't see a different behaviour than it would be using the address. Yes. You are of course right in case I misunderstood something, and the id is kept on copying a math inset. Jean-Marc's comment seems to be: sometimes we would like the id to remain the same (which indeed would not be taken into account by the current patch). Jean-Marc, did I understand you remark correctly and if so, do you have an example in mind? Guillaume
Re: Form is function
Le 14/10/2015 09:38, Liviu Andronic a écrit : On Wed, Oct 14, 2015 at 7:55 AM, Guillaume Munch wrote: the hard truth is that our manpower is limited and our devels barely have the bandwidth to address crashes, regressions and the like (in between monitoring tickets, triaging, helping on the MLs, and implementing things sorely needed to keep up with software developments or just implementing things they need themselves). All those red color-coded tickets on Trac tend to focus spirits and keep devels around the bugs that absolutely need to get fixed until the next release. I already did all of what you describe in addition to implementing long-standing feature requests and last-minute fixes of compilation; I am not going to take an authoritative argument about what I need. This was never meant as a rebuke or criticism to any one devel in particular (or to the team, for the matter), but merely what seems like a factual description of how our devels operate and their constraints. No, of course. Guillaume
Re: Pixmap cache is broken for master
Le 15/10/2015 09:20, Jean-Marc Lasgouttes a écrit : Le 14/10/15 14:22, Stephan Witt a écrit : Indeed. What about just dumping it? :p I'm all for it. I did some profiling. Pixmaps are indeed a little bit faster visually. Here are the hard numbers Without Pixmaps: 66% LyX 32% GuiWorkArea::Private::updateScreen 12,6% BufferView::updateMetrics 8,5% TabWorkArea::paintEvent 12% WindowServer With Pixmaps: 62% LyX 26% GuiWorkArea::Private::updateScreen 15.4% BufferView::updateMetrics 11.7% TabWorkArea::paintEvent 12% WindowServer Note that these numbers should be normalized so that the updateMetrics numbers match (they are unrelated to pixmaps). Then everything will be easier to compare. I am in a hurry now, I will do it n a later message. By the way, it is also much slower with orthographic correction and lot of red-underlined words (e.g. a few lorem ipsum paragraphs). Is this known/expected? On the positive side, the underlining looks much nicer, which I assume is thanks to the text being rendered on a per-word basis. I could not figure whether the performance issue is a regression.
Re: IBus compatibility
Le 14/10/2015 20:00, Georg Baum a écrit : Jean-Marc Lasgouttes wrote: Le 11/10/2015 12:43, Guillaume Munch a écrit : Qt 5.6.0 will probably be released before LyX 2.2.0. Therefore the most simple solution would be to disable IBus if an older qt is detected (at runtime) and mention in the release notes that korean users need to use Qt 5.6. There is some time before a Qt version reaches repositories. Current Ubuntu is 5.3, next is going to be 5.4, and I could not manage to find a PPA with 5.5 when I last looked. This is why we need a better workaround. Depends IMHO how much work it is. We are talking about a problem which appears only on Linux where qt 4.8 works fine. Therefore, it would only affect users who are both using the hangul input method _and_ wanting some new qt5 features at the same time. This is probably a very very small fraction, so I would only spend a small amount of time for a possible workaround. Beware of any confusion, the deadlock bug is with qt4 and is solved in qt5 (any version I could test). Another bug makes qt < 5.6 impractical for (all) Linux users. So the situation is more complex than what you wrote. Even if we were to just write a warning at the top of the release notes it would count as having made a decision (but it's nice if we can make it easier for users). How do we detect that current input method is Hangul? Thanks to Jean-Marc for asking on the general list (but again the issue is with qt4 contrary to what you wrote!). Guillaume
Re: [patches] the last ones before 2.2
Le 14/10/2015 21:35, Georg Baum a écrit :
*#2: Add a unique id to math insets. I also print this information in
the DEVEL_VERSION in the status bar similarly to what is done currently
for paragraphs. It is interesting that while the paragraph id is usually
below 1, this new math inset id immediately goes into the millions
on opening an actual LyX document and it increases very quickly (which I
suspect is due to updateMacros()).
Please measure memory consumption. I am a bit afraid that this increases
memory usage a lot, since math insets contain very tiny bits of information,
and you may need _lots_ of them even for simple formula.
I did not realise that there were supposed to be so many InsetMaths live
at the same time. Although the increased memory use is quite small for
modern standard, I agree that it is a bad idea to increase it needlessly
both in principle and in the long run.
In fact I only need uids on InsetMathNest, which should represent a much
smaller population.
Why do you need a new member variable at all? Why can't you just use the
memory address? This would not consume memory and still be a unique id.
Yes, it would make sense to use pointers IMO. There was two reasons for
the UniqueId class: I needed to experiment with different copy semantics
before I finally decided for the current behaviour; and also I find that
using the address looked kludgy compared to the current code (e.g. the
paragraph id). Another aspect is that during debugging, having an id
that starts from 0 is easier to read and keep track of. I suggest seeing
how it goes with having ids only for InsetMathNest. Does that make sense?
*#6: Remove a deep copy of MathData in lyx::write that interfered with
TexRow. If anybody can explain to me the purpose of this antediluvian
call to extractStrings, I am all ears.
Fortunately I can answer this question: extractStrings() exists to make the
formulas better suitable for feeding them to external programs like computer
algebra systems. It is also used for debug output (dump()) to make it more
readable for humans.
Thank you for the explanation.
In the meanwhile, I wrote
equivalent code in case it was actually useful.
The new code looks rather complicated to me. I think the old one should be
kept.
ExtractStrings forces write to repeat a deep copy on a recursive call. I
would advise against keeping write as it is. There already was an
unexplained time explosion when generating previews in a document with a
lot of math macros, before Enrico rationalised the macro definition
output during preview. What was not explained is that it took more than
a time linear in the size of the generated preview tex file, in the
tests that I did at the time. Not to say that it's necessarily related
(though it may be), but this can be a concrete issue.
I am skeptical about "complicated" given that it is the same as before,
except that it was doing a deep copy before (which I call "complicated"
in this context). As for readability, see the attached diff where it is
rephrased using a separate function.
What I do not understand is why this code is executed at all for LaTeX
output. I believe that it can even produce wrong PDF contents.
Do you have an example of how it might invalidate the latex output?
Therefore, if
LaTeX output is fixed to not call the free function write() at all, then
extractStrings() can be kept and will not interfere.
Sorry, that's beyond my current knowledge of LyX internals. I am
confident in my patches because I make small local changes. I suggest
keeping my fix for now, made more readable as attached, unless somebody
is able to provide a patch along your lines in a timely manner.
I noticed a major
decrease of the math inset ids after this patch; hopefully this id is a
new metrics that will allow us to observe further improvements to the
implementation of maths.
I am not sure whether this is a good measure for mathed code quality. It is
a central idea of mathed that a math inset is a self contained data class.
If you assume that then it is not necessarily a problem if many copies are
made.
You might be right; I found interesting though to observe such a
decrease in the uid counter with the patch discussed above.
Guillaume
diff --git a/src/mathed/MathExtern.cpp b/src/mathed/MathExtern.cpp
index 1904843..afd224a 100644
--- a/src/mathed/MathExtern.cpp
+++ b/src/mathed/MathExtern.cpp
@@ -193,6 +193,23 @@ void extractStrings(MathData & ar)
}
+//In-place version of extractStrings. Returns false if there is no string to
+//extract. Otherwise, the extracted string is appended to s and the iterator
+//"from" is incremented appropriately.
+bool extractString(MathData::const_iterator & from,
+ MathData::const_iterator const & to,
+ docstring& s)
+{
+ bool ret = false;
+ for (; from != to && (*from)->asCharInset(); ++from) {
+ ret = true;
+ s += (*from)->getChar();
+ }
+ return ret;
+}
+
+
+
void ext
Re: [patches] the last ones before 2.2
Jean-Marc Lasgouttes wrote: >> Why do you need a new member variable at all? Why can't you just use the >> memory address? This would not consume memory and still be a unique id. > > I guess that the answer is the same as for paragraph ids: they can get > copied at unexpected times. If my understanding of the patch is correct, then the id is incremented on each copy, therefore I don't see a different behaviour than it would be using the address. You are of course right in case I misunderstood something, and the id is kept on copying a math inset. Georg
Re: [PATCH] Get rid of ParagraphMetrics::insetDimension
On 10/15/2015 09:30 AM, Jean-Marc Lasgouttes wrote: The following patch is a simple cleanup that was written before the acceptation of the role of release master by Scott. It removes the cache ParagraphMetrics:insetDimension, and relies on the already existing BufferView::coordCache instead. It is quite straightforward, but does not bring any real advantage beside deleting 48 lines of code and gaining a modest amount of memory. To be frank, I would not have started to write now, but now that I have it, I'd prefer to apply it rather to let it rot. The only changed behavior is that the CoordCache asserts when requiring a non existing cache entry, where insetDimension would return a dummy value. This can lead to some crashes, but I would say that these are welcome in some cases. Is there something sensible to do in release mode in this case? rather than crash? Richard
[PATCH] Get rid of ParagraphMetrics::insetDimension
The following patch is a simple cleanup that was written before the
acceptation of the role of release master by Scott. It removes the cache
ParagraphMetrics:insetDimension, and relies on the already existing
BufferView::coordCache instead.
It is quite straightforward, but does not bring any real advantage
beside deleting 48 lines of code and gaining a modest amount of memory.
To be frank, I would not have started to write now, but now that I have
it, I'd prefer to apply it rather to let it rot.
The only changed behavior is that the CoordCache asserts when requiring
a non existing cache entry, where insetDimension would return a dummy
value. This can lead to some crashes, but I would say that these are
welcome in some cases.
Scott, OK?
JMarc
>From bdb0bdf0211736891e7280009e6dfe23f9da8419 Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes
Date: Mon, 12 Oct 2015 16:11:58 +0200
Subject: [PATCH] Get rid of ParagraphMetrics::insetDimension
We already have a CoordCache of insets dimensions. It is not necessary
to store the same information in two places.
Give a name to CoordCache tables types to improve code readability.
Remove ParagraphMetrics::singleWidth, which is not used anymore.
---
development/PAINTING_ANALYSIS | 18 +-
src/BufferView.cpp|3 +--
src/CoordCache.cpp|2 +-
src/CoordCache.h | 14 --
src/ParagraphMetrics.cpp | 37 -
src/ParagraphMetrics.h| 11 ---
src/RowPainter.cpp|2 +-
src/TextMetrics.cpp | 31 +++
src/mathed/MathData.cpp |8
9 files changed, 35 insertions(+), 91 deletions(-)
diff --git a/development/PAINTING_ANALYSIS b/development/PAINTING_ANALYSIS
index f86ad2d..d1da54b 100644
--- a/development/PAINTING_ANALYSIS
+++ b/development/PAINTING_ANALYSIS
@@ -33,14 +33,14 @@ is acted on.
** Buffer::change issues
When calling Buffer::changed outside of bv::processUpdateFlags,
-how do we now that the update strategy is set correctly? It is
+how do we know that the update strategy is set correctly? It is
possible to reset the strategy at the end of bv::draw. What would be
a good value? NoScreenUpdate?
On a related note, what is the semantics of a call to
Buffer::changed(false)? What does the caller mean?
-** How to avoid redraw with FitCursor when the cursor is already OK?
+** How to avoid redraw with FitCursor when the cursor is already OK?
In this case, we invoke Buffer::change(false) with drawing disabled
and NoScreenUpdate strategy.
@@ -60,15 +60,9 @@ cursor.
* Proposals
-** get rid of pm::insetDimension.
-
-The information contained there is already in bv::coordCache.
-
-Effect: only code simplification.
-
** set inset position during metrics phase
-This implies to set inset positions relative to outer isnet during
+This implies to set inset positions relative to outer inset during
metrics phase and then in a second loop to descend into insets and
update positions correctly.
@@ -95,14 +89,13 @@ expensive for large files. This would though help scrolling.
* Description of current drawing mechanism
-** Two phase drawing
+** Two stage drawing
There are two parts to drawing the work area:
+ the metrics phase computes the size of insets and breaks the
paragraphs into rows. It stores the dimension of insets (both
- normal and math) in bv::coordCache, and the size of normal
- insets in pm::insetDimension.
+ normal and math) in bv::coordCache.
+ the drawing phase draws the contents and caches the inset
positions. Since the caching of positions is useful in itself,
@@ -110,7 +103,6 @@ There are two parts to drawing the work area:
thing we want is to cache inset positions
(Painter::setDrawingEnabled).
-
The machinery is controlled via bv::processUpdateFlags. This method is
called at the end of bv::mouseEventDispatch and in
GuiApplication::dispatch, via the updateCurrentView method. There are
diff --git a/src/BufferView.cpp b/src/BufferView.cpp
index b07eb2b..63cb873 100644
--- a/src/BufferView.cpp
+++ b/src/BufferView.cpp
@@ -2832,8 +2832,7 @@ Point BufferView::coordOffset(DocIterator const & dit) const
// FIXME (Abdel 23/09/2007): this is a bit messy because of the
// elimination of Inset::dim_ cache. This coordOffset() method needs
// to be rewritten in light of the new design.
- Dimension const & dim = parMetrics(dit[i - 1].text(),
-dit[i - 1].pit()).insetDimension(&sl.inset());
+ Dimension const & dim = coordCache().getInsets().dim(&sl.inset());
lastw = dim.wid;
} else {
Dimension const dim = sl.inset().dimension(*this);
diff --git a/src/CoordCache.cpp b/src/CoordCache.cpp
index 827697e..53f1d0a 100644
--- a/src/CoordCache.cpp
+++ b/src/CoordCache.cpp
@@ -42,7 +42,7 @@ void CoordCache::clear()
void CoordCache::dump() const
{
LYXERR0("InsetCache contains:");
- Co
Re: Pixmap cache is broken for master
Le 15/10/2015 14:47, Richard Heck a écrit : On 10/15/2015 04:20 AM, Jean-Marc Lasgouttes wrote: Le 14/10/15 14:22, Stephan Witt a écrit : Indeed. What about just dumping it? :p I'm all for it. I did some profiling. Pixmaps are indeed a little bit faster visually. Here are the hard numbers Without Pixmaps: 66% LyX 32% GuiWorkArea::Private::updateScreen 12,6% BufferView::updateMetrics 8,5% TabWorkArea::paintEvent 12% WindowServer With Pixmaps: 62% LyX 26% GuiWorkArea::Private::updateScreen 15.4% BufferView::updateMetrics 11.7% TabWorkArea::paintEvent 11% WindowServer Note that these numbers should be normalized so that the updateMetrics numbers match (they are unrelated to pixmaps). Then everything will be easier to compare. 50.8 LyX 21.3 GuiWorkArea::Private::updateScreen 12.6 BufferView::updateMetrics 9.6 TabWorkArea::paintEvent WindowServer 9 So quite a bit faster. Indeed, and the time spent in the window server is lower too. So the short version of Pixmap rendering is: * Pros: + faster * Cons: - bad image quality (subpixel aliasing?) - memory consumption: full pixmap of rows are kept in memory, whereas before we only kept single glyphs. Note however that QPixmapCache limits this to 10MB of memory by default, which is not much. - currently broken (but this can probably be fixed). So except for the problem iof image quality, Pixmap caching seems to kind of work. I will see how fixable it is. JMarc
Re: Pixmap cache is broken for master
On 10/15/2015 04:20 AM, Jean-Marc Lasgouttes wrote: Le 14/10/15 14:22, Stephan Witt a écrit : Indeed. What about just dumping it? :p I'm all for it. I did some profiling. Pixmaps are indeed a little bit faster visually. Here are the hard numbers Without Pixmaps: 66% LyX 32% GuiWorkArea::Private::updateScreen 12,6% BufferView::updateMetrics 8,5% TabWorkArea::paintEvent 12% WindowServer With Pixmaps: 62% LyX 26% GuiWorkArea::Private::updateScreen 15.4% BufferView::updateMetrics 11.7% TabWorkArea::paintEvent 12% WindowServer Note that these numbers should be normalized so that the updateMetrics numbers match (they are unrelated to pixmaps). Then everything will be easier to compare. 21.3% GuiWorkArea::Private::updateScreen 12.6% BufferView::updateMetrics 9.6% TabWorkArea::paintEvent So quite a bit faster. Richard
Re: Crash when quitting on OS X
They are definitely hard to reproduce, I spent a while trying to create a recipe, but with no success. I'd be happy to try the modified version. On Thu, Oct 15, 2015 at 12:41 AM, Stephan Witt wrote: > Am 15.10.2015 um 01:31 schrieb disinteres...@gmail.com: > > > With LyX 2.1.4 and OS X 10.10.5, I often get crashes when quitting using > command+q. Oddly, the release notes for 2.1.4 say that it fixes another > crashes on quitting bug (#8637). I was getting these crashes with previous > versions of LyX when I was on Mavericks, but upgrading to Yosemite solved > the problem until I updated to 2.1.4, which reintroduced the behavior. > > > > Is anyone else seeing this? > > These crashes are difficult to reproduce on a developer system. I had them > once and the change fixed it. Later I got them again and now they are gone > again without any change. Sorry, it's not clear what's triggering them. It > happens on destruction of the LinkBack-machinery on exit of LyX. > > I want to give it another try and made a change to address the cause of > the problem. I can make a LyX 2.1.4 with this change and will be happy if > you are willing to test it. > > Regards, > Stephan
Re: Form is function
Le 15/10/15 00:13, Scott Kostyshak a écrit : The version of trac we use is almost 5 years old now. There have been a lot of changes since then. I don't think we should update before a major release, and I don't even know if we have someone who would have the time and knowledge to do the update. I just bring this up now in case it would change any of the proposals that were mentioned as far as bug triaging so that if we do ever update we don't have to go through another process of workflow proposals. Does anyone know if the newest version of trac changes anything significantly as far as the proposals that Guillaume mentioned? Here is the change log: http://trac.edgewall.org/wiki/TracChangeLog I do not think that there will be significant improvements, but a newer trac will come with better security and probably a better choice of plugins. However, the best way to get that would be to upgrade our centos version. I just ran fearlessly "yum update" and we are now at CentOS 6.7. Everything still seems to work :) To update further, we would have to upgrade to CentOS 7, which a more difficult problem. JMarc
Re: Pixmap cache is broken for master
Le 14/10/15 14:22, Stephan Witt a écrit : Indeed. What about just dumping it? :p I'm all for it. I did some profiling. Pixmaps are indeed a little bit faster visually. Here are the hard numbers Without Pixmaps: 66% LyX 32% GuiWorkArea::Private::updateScreen 12,6% BufferView::updateMetrics 8,5% TabWorkArea::paintEvent 12% WindowServer With Pixmaps: 62% LyX 26% GuiWorkArea::Private::updateScreen 15.4% BufferView::updateMetrics 11.7% TabWorkArea::paintEvent 12% WindowServer Note that these numbers should be normalized so that the updateMetrics numbers match (they are unrelated to pixmaps). Then everything will be easier to compare. I am in a hurry now, I will do it n a later message. JMarc
Re: Moving towards a 2.2 release
Le 12/10/15 11:02, Jean-Marc Lasgouttes a écrit : 2. What are the bugs that should be fixed before we release 2.2.0? Please note which of these bugs are regressions. Please note also if you are volunteering to work on the bug yourself. I have a bug (just created for the record) on my plate about horizontal scrolling: http://www.lyx.org/trac/ticket/9796 There is also the open problem of metrics caching, which now caches almost complete rows (it used to be only single words). I fear that this can lead to performance/memory problems after long editing sessions. This can be fixed using a LRU map cache, which I have yet to implement. JMarc
