Re: test of math previews

[Enrico Forestieri]

On Fri, Jun 26, 2015 at 02:00:33AM +0100, Guillaume M-M wrote:
 Le 25/06/2015 23:45, Richard Heck a écrit :
 On 06/25/2015 02:58 PM, Enrico Forestieri wrote:
 On Thu, Jun 25, 2015 at 02:16:19PM -0400, Richard Heck wrote:
 This more or less completes this work?
 I think so. The most important glitches were identified and solved.
 
 (For which thanks very much, to you
 and Guillaume.)
 The credit goes mainly to Guillaume for his accurate testing.
 
 Somebody had to write the code
 
 rh
 
 
 
 This is also what I believe. Thank you!
 
 By the way will f3b03699 be backported as well? It seems that segfaults are
 almost guaranteed for people with previews on during Ctrl+C...

It was backported at 2a9f0733. It is curious that this one was not reported
earlier, but perhaps there are not so many macro users also using instant
preview out there :). However, most probably, it was hidden by #9418.

-- 
Enrico

Re: test of math previews

[Enrico Forestieri]

On Fri, Jun 26, 2015 at 02:00:33AM +0100, Guillaume M-M wrote:
> Le 25/06/2015 23:45, Richard Heck a écrit :
> >On 06/25/2015 02:58 PM, Enrico Forestieri wrote:
> >>On Thu, Jun 25, 2015 at 02:16:19PM -0400, Richard Heck wrote:
> >>>This more or less completes this work?
> >>I think so. The most important glitches were identified and solved.
> >>
> >>>(For which thanks very much, to you
> >>>and Guillaume.)
> >>The credit goes mainly to Guillaume for his accurate testing.
> >
> >Somebody had to write the code
> >
> >rh
> >
> >
> 
> This is also what I believe. Thank you!
> 
> By the way will f3b03699 be backported as well? It seems that segfaults are
> almost guaranteed for people with previews on during Ctrl+C...

It was backported at 2a9f0733. It is curious that this one was not reported
earlier, but perhaps there are not so many macro users also using instant
preview out there :). However, most probably, it was hidden by #9418.

-- 
Enrico

Re: test of math previews

[Enrico Forestieri]

On Thu, Jun 25, 2015 at 10:16:21AM +0200, Enrico Forestieri wrote:
 
 The third patch is the simpler one proposed by Guillaume. I have already
 applied the better patch to master but there is a glitch with it.
 Namely, if a preview snippet produces a blank image, its metrics are
 not invalidated and thus the image would not be visible on the LyX
 screen (actually, it is replaced by a small red square). I still have
 to take a look at it, so I think that for the time being the simpler
 patch will do.

Actually, it seems doing the right thing, as things such as $\quad$ or
$\relax$ are left as is, while $\raisebox{6mm}{}$ or $\raisebox{-6mm}{}$
produce a small vertical strip with the required characteristics (as
revealed by selecting the images in LyX).
So, I think the better patch can also be applied.

-- 
Enrico

Re: test of math previews

[Enrico Forestieri]

On Thu, Jun 25, 2015 at 01:44:15PM +0200, Enrico Forestieri wrote:
 On Thu, Jun 25, 2015 at 10:16:21AM +0200, Enrico Forestieri wrote:
  
  The third patch is the simpler one proposed by Guillaume. I have already
  applied the better patch to master but there is a glitch with it.
  Namely, if a preview snippet produces a blank image, its metrics are
  not invalidated and thus the image would not be visible on the LyX
  screen (actually, it is replaced by a small red square). I still have
  to take a look at it, so I think that for the time being the simpler
  patch will do.
 
 Actually, it seems doing the right thing, as things such as $\quad$ or
 $\relax$ are left as is, while $\raisebox{6mm}{}$ or $\raisebox{-6mm}{}$
 produce a small vertical strip with the required characteristics (as
 revealed by selecting the images in LyX).

I had a closer look and this behavior is independent of the patch.
Only when using dvipng blank images can be detected reliably, so what
described above occurs only when using the legacy route for generating
images.

 So, I think the better patch can also be applied.

More rightly, as in stable the legacy route is always used as a fallback.

-- 
Enrico

Re: test of math previews

[Richard Heck]

On 06/25/2015 02:13 PM, Enrico Forestieri wrote:

On Thu, Jun 25, 2015 at 01:44:15PM +0200, Enrico Forestieri wrote:

On Thu, Jun 25, 2015 at 10:16:21AM +0200, Enrico Forestieri wrote:

The third patch is the simpler one proposed by Guillaume. I have already
applied the better patch to master but there is a glitch with it.
Namely, if a preview snippet produces a blank image, its metrics are
not invalidated and thus the image would not be visible on the LyX
screen (actually, it is replaced by a small red square). I still have
to take a look at it, so I think that for the time being the simpler
patch will do.

Actually, it seems doing the right thing, as things such as $\quad$ or
$\relax$ are left as is, while $\raisebox{6mm}{}$ or $\raisebox{-6mm}{}$
produce a small vertical strip with the required characteristics (as
revealed by selecting the images in LyX).

I had a closer look and this behavior is independent of the patch.
Only when using dvipng blank images can be detected reliably, so what
described above occurs only when using the legacy route for generating
images.


So, I think the better patch can also be applied.

More rightly, as in stable the legacy route is always used as a fallback.


OK with me then.

This more or less completes this work? (For which thanks very much, to 
you and Guillaume.)


Richard

Re: test of math previews

[Enrico Forestieri]

On Thu, Jun 25, 2015 at 02:16:19PM -0400, Richard Heck wrote:
 On 06/25/2015 02:13 PM, Enrico Forestieri wrote:
 On Thu, Jun 25, 2015 at 01:44:15PM +0200, Enrico Forestieri wrote:
 On Thu, Jun 25, 2015 at 10:16:21AM +0200, Enrico Forestieri wrote:
 The third patch is the simpler one proposed by Guillaume. I have already
 applied the better patch to master but there is a glitch with it.
 Namely, if a preview snippet produces a blank image, its metrics are
 not invalidated and thus the image would not be visible on the LyX
 screen (actually, it is replaced by a small red square). I still have
 to take a look at it, so I think that for the time being the simpler
 patch will do.
 Actually, it seems doing the right thing, as things such as $\quad$ or
 $\relax$ are left as is, while $\raisebox{6mm}{}$ or $\raisebox{-6mm}{}$
 produce a small vertical strip with the required characteristics (as
 revealed by selecting the images in LyX).
 I had a closer look and this behavior is independent of the patch.
 Only when using dvipng blank images can be detected reliably, so what
 described above occurs only when using the legacy route for generating
 images.
 
 So, I think the better patch can also be applied.
 More rightly, as in stable the legacy route is always used as a fallback.
 
 OK with me then.
 
 This more or less completes this work?

I think so. The most important glitches were identified and solved.

 (For which thanks very much, to you
 and Guillaume.)

The credit goes mainly to Guillaume for his accurate testing.

-- 
Enrico

Re: test of math previews

[Richard Heck]

On 06/25/2015 02:58 PM, Enrico Forestieri wrote:

On Thu, Jun 25, 2015 at 02:16:19PM -0400, Richard Heck wrote:

This more or less completes this work?

I think so. The most important glitches were identified and solved.


(For which thanks very much, to you
and Guillaume.)

The credit goes mainly to Guillaume for his accurate testing.


Somebody had to write the code

rh

Re: test of math previews

[Guillaume M-M]

Le 25/06/2015 23:45, Richard Heck a écrit :

On 06/25/2015 02:58 PM, Enrico Forestieri wrote:

On Thu, Jun 25, 2015 at 02:16:19PM -0400, Richard Heck wrote:

This more or less completes this work?

I think so. The most important glitches were identified and solved.


(For which thanks very much, to you
and Guillaume.)

The credit goes mainly to Guillaume for his accurate testing.


Somebody had to write the code

rh




This is also what I believe. Thank you!

By the way will f3b03699 be backported as well? It seems that segfaults 
are almost guaranteed for people with previews on during Ctrl+C...

Re: test of math previews

[Enrico Forestieri]

On Wed, Jun 24, 2015 at 09:36:50PM -0400, Richard Heck wrote:
 On 06/24/2015 08:49 PM, Guillaume M-M wrote:
 Le 25/06/2015 00:44, Enrico Forestieri a écrit :
 On Mon, Jun 22, 2015 at 11:32:17PM +0100, Guillaume M-M wrote:
 
 3) Previews have the incorrect size with layouts that offer font sizes
 other
 than 1[0-2]pt (lyx-preview-fontsize.lyx). The legacy method offers a
 function that retrieves the actual font size and calculates the actual
 dpi.
 Attached is a patch (extract_resolution.diff) that imports this
 function in
 the new script.
 
 Thank you, this is much better than trying to catch all possible cases.
 I have applied the patch to master at 0751f96d.
 
 
 Thank you for applying it.
 
 Here's a trivial patch for stable, if you are not ready to backport
 0751f96d. Both were tested in stable.
 
 This is fine with me, as is the original patch, as Enrico sees fit.

Richard, I am attaching 3 patches for stable. The first one fix crashes
occurring when zooming after latex completely failed to generate an
output and when copying a macro with instant preview on. This last
crash also occurs in 2.1.3.

The second patch assures that a lonely math inset is previewed.
The third patch is the simpler one proposed by Guillaume. I have already
applied the better patch to master but there is a glitch with it.
Namely, if a preview snippet produces a blank image, its metrics are
not invalidated and thus the image would not be visible on the LyX
screen (actually, it is replaced by a small red square). I still have
to take a look at it, so I think that for the time being the simpler
patch will do.

-- 
Enrico
diff --git a/src/graphics/PreviewLoader.cpp b/src/graphics/PreviewLoader.cpp
index eb25805..e20f1ad 100644
--- a/src/graphics/PreviewLoader.cpp
+++ b/src/graphics/PreviewLoader.cpp
@@ -709,6 +709,7 @@ void PreviewLoader::Impl::finishedGenerating(pid_t pid, int 
retval)
if (git == in_progress_.end()) {
lyxerr  PreviewLoader::finishedGenerating(): unable to find 
data for PID   pid  endl;
+   finished_generating_ = true;
return;
}
 
@@ -717,8 +718,11 @@ void PreviewLoader::Impl::finishedGenerating(pid_t pid, 
int retval)
LYXERR(Debug::GRAPHICS, PreviewLoader::finishedInProgress(
 retval  ): processing   status
  for   command);
-   if (retval  0)
+   if (retval  0) {
+   in_progress_.erase(git);
+   finished_generating_ = true;
return;
+   }
 
// Read the metrics file, if it exists
vectordouble ascent_fractions(git-second.snippets.size());
diff --git a/src/mathed/MacroTable.h b/src/mathed/MacroTable.h
index 3bd04ea..030e6f7 100644
--- a/src/mathed/MacroTable.h
+++ b/src/mathed/MacroTable.h
@@ -69,6 +69,8 @@ public:
char const * MathMLtype() const;
///
void setSymbol(latexkeys const * sym) { sym_ = sym; }
+   ///
+   DocIterator const  pos() { return pos_; }
 
/// lock while being drawn to avoid recursions
int lock() const { return ++lockCount_; }
diff --git a/src/mathed/MathMacro.cpp b/src/mathed/MathMacro.cpp
index 06cfd5c..a25c5db 100644
--- a/src/mathed/MathMacro.cpp
+++ b/src/mathed/MathMacro.cpp
@@ -176,6 +176,14 @@ void MathMacro::assign(MathMacro const  that)
if (p)
p-setOwner(this);
}
+   if (macro_  lyxrc.preview == LyXRC::PREVIEW_ON) {
+   // We need to update macro_ by ourselves because in this case
+   // MathData::metrics() is not called when selecting a math inset
+   DocIterator const  pos = macroBackup_.pos();
+   macro_ = pos.buffer() ? pos.buffer()-getMacro(name(), pos) : 0;
+   if (!macro_)
+   macro_ = macroBackup_;
+   }
 }
 
 
diff --git a/src/mathed/MathMacro.cpp b/src/mathed/MathMacro.cpp
index 06cfd5c..9744366 100644
--- a/src/mathed/MathMacro.cpp
+++ b/src/mathed/MathMacro.cpp
@@ -604,6 +612,20 @@ bool MathMacro::validName() const
 
 void MathMacro::validate(LaTeXFeatures  features) const
 {
+   // Immediately after a document is loaded, in some cases the MacroData
+   // of the global macros defined in the lib/symbols file may still not
+   // be known to the macro machinery because it will be set only after
+   // the first call to updateMacros(). This is not a problem unless
+   // instant preview is on for math, in which case we will be missing
+   // the corresponding requirements.
+   // In this case, we get the required info from the global macro table.
+   if (requires_.empty()  !macro_) {
+   // Update requires for known global macros.
+   MacroData const * data = MacroTable::globalMacros().get(name());
+   if (data  !data-requires().empty())
+   features.require(data-requires());
+   }
+
 

Re: test of math previews

[Enrico Forestieri]

On Wed, Jun 24, 2015 at 09:36:50PM -0400, Richard Heck wrote:
> On 06/24/2015 08:49 PM, Guillaume M-M wrote:
> >Le 25/06/2015 00:44, Enrico Forestieri a écrit :
> >>On Mon, Jun 22, 2015 at 11:32:17PM +0100, Guillaume M-M wrote:
> >>>
> >>>3) Previews have the incorrect size with layouts that offer font sizes
> >>>other
> >>>than 1[0-2]pt (lyx-preview-fontsize.lyx). The legacy method offers a
> >>>function that retrieves the actual font size and calculates the actual
> >>>dpi.
> >>>Attached is a patch (extract_resolution.diff) that imports this
> >>>function in
> >>>the new script.
> >>
> >>Thank you, this is much better than trying to catch all possible cases.
> >>I have applied the patch to master at 0751f96d.
> >>
> >
> >Thank you for applying it.
> >
> >Here's a trivial patch for stable, if you are not ready to backport
> >0751f96d. Both were tested in stable.
> 
> This is fine with me, as is the original patch, as Enrico sees fit.

Richard, I am attaching 3 patches for stable. The first one fix crashes
occurring when zooming after latex completely failed to generate an
output and when copying a macro with instant preview on. This last
crash also occurs in 2.1.3.

The second patch assures that a lonely math inset is previewed.
The third patch is the simpler one proposed by Guillaume. I have already
applied the better patch to master but there is a glitch with it.
Namely, if a preview snippet produces a blank image, its metrics are
not invalidated and thus the image would not be visible on the LyX
screen (actually, it is replaced by a small red square). I still have
to take a look at it, so I think that for the time being the simpler
patch will do.

-- 
Enrico
diff --git a/src/graphics/PreviewLoader.cpp b/src/graphics/PreviewLoader.cpp
index eb25805..e20f1ad 100644
--- a/src/graphics/PreviewLoader.cpp
+++ b/src/graphics/PreviewLoader.cpp
@@ -709,6 +709,7 @@ void PreviewLoader::Impl::finishedGenerating(pid_t pid, int 
retval)
if (git == in_progress_.end()) {
lyxerr << "PreviewLoader::finishedGenerating(): unable to find "
"data for PID " << pid << endl;
+   finished_generating_ = true;
return;
}
 
@@ -717,8 +718,11 @@ void PreviewLoader::Impl::finishedGenerating(pid_t pid, 
int retval)
LYXERR(Debug::GRAPHICS, "PreviewLoader::finishedInProgress("
<< retval << "): processing " << status
<< " for " << command);
-   if (retval > 0)
+   if (retval > 0) {
+   in_progress_.erase(git);
+   finished_generating_ = true;
return;
+   }
 
// Read the metrics file, if it exists
vector ascent_fractions(git->second.snippets.size());
diff --git a/src/mathed/MacroTable.h b/src/mathed/MacroTable.h
index 3bd04ea..030e6f7 100644
--- a/src/mathed/MacroTable.h
+++ b/src/mathed/MacroTable.h
@@ -69,6 +69,8 @@ public:
char const * MathMLtype() const;
///
void setSymbol(latexkeys const * sym) { sym_ = sym; }
+   ///
+   DocIterator const & pos() { return pos_; }
 
/// lock while being drawn to avoid recursions
int lock() const { return ++lockCount_; }
diff --git a/src/mathed/MathMacro.cpp b/src/mathed/MathMacro.cpp
index 06cfd5c..a25c5db 100644
--- a/src/mathed/MathMacro.cpp
+++ b/src/mathed/MathMacro.cpp
@@ -176,6 +176,14 @@ void MathMacro::assign(MathMacro const & that)
if (p)
p->setOwner(this);
}
+   if (macro_ && lyxrc.preview == LyXRC::PREVIEW_ON) {
+   // We need to update macro_ by ourselves because in this case
+   // MathData::metrics() is not called when selecting a math inset
+   DocIterator const & pos = macroBackup_.pos();
+   macro_ = pos.buffer() ? pos.buffer()->getMacro(name(), pos) : 0;
+   if (!macro_)
+   macro_ = _;
+   }
 }
 
 
diff --git a/src/mathed/MathMacro.cpp b/src/mathed/MathMacro.cpp
index 06cfd5c..9744366 100644
--- a/src/mathed/MathMacro.cpp
+++ b/src/mathed/MathMacro.cpp
@@ -604,6 +612,20 @@ bool MathMacro::validName() const
 
 void MathMacro::validate(LaTeXFeatures & features) const
 {
+   // Immediately after a document is loaded, in some cases the MacroData
+   // of the global macros defined in the lib/symbols file may still not
+   // be known to the macro machinery because it will be set only after
+   // the first call to updateMacros(). This is not a problem unless
+   // instant preview is on for math, in which case we will be missing
+   // the corresponding requirements.
+   // In this case, we get the required info from the global macro table.
+   if (requires_.empty() && !macro_) {
+   // Update requires for known global macros.
+   MacroData const * data = MacroTable::globalMacros().get(name());
+   if (data && 

Re: test of math previews

[Enrico Forestieri]

On Thu, Jun 25, 2015 at 10:16:21AM +0200, Enrico Forestieri wrote:
> 
> The third patch is the simpler one proposed by Guillaume. I have already
> applied the better patch to master but there is a glitch with it.
> Namely, if a preview snippet produces a blank image, its metrics are
> not invalidated and thus the image would not be visible on the LyX
> screen (actually, it is replaced by a small red square). I still have
> to take a look at it, so I think that for the time being the simpler
> patch will do.

Actually, it seems doing the right thing, as things such as $\quad$ or
$\relax$ are left as is, while $\raisebox{6mm}{}$ or $\raisebox{-6mm}{}$
produce a small vertical strip with the required characteristics (as
revealed by selecting the images in LyX).
So, I think the better patch can also be applied.

-- 
Enrico

Re: test of math previews

[Enrico Forestieri]

On Thu, Jun 25, 2015 at 01:44:15PM +0200, Enrico Forestieri wrote:
> On Thu, Jun 25, 2015 at 10:16:21AM +0200, Enrico Forestieri wrote:
> > 
> > The third patch is the simpler one proposed by Guillaume. I have already
> > applied the better patch to master but there is a glitch with it.
> > Namely, if a preview snippet produces a blank image, its metrics are
> > not invalidated and thus the image would not be visible on the LyX
> > screen (actually, it is replaced by a small red square). I still have
> > to take a look at it, so I think that for the time being the simpler
> > patch will do.
> 
> Actually, it seems doing the right thing, as things such as $\quad$ or
> $\relax$ are left as is, while $\raisebox{6mm}{}$ or $\raisebox{-6mm}{}$
> produce a small vertical strip with the required characteristics (as
> revealed by selecting the images in LyX).

I had a closer look and this behavior is independent of the patch.
Only when using dvipng blank images can be detected reliably, so what
described above occurs only when using the legacy route for generating
images.

> So, I think the better patch can also be applied.

More rightly, as in stable the legacy route is always used as a fallback.

-- 
Enrico

Re: test of math previews

[Richard Heck]

On 06/25/2015 02:13 PM, Enrico Forestieri wrote:

On Thu, Jun 25, 2015 at 01:44:15PM +0200, Enrico Forestieri wrote:

On Thu, Jun 25, 2015 at 10:16:21AM +0200, Enrico Forestieri wrote:

The third patch is the simpler one proposed by Guillaume. I have already
applied the better patch to master but there is a glitch with it.
Namely, if a preview snippet produces a blank image, its metrics are
not invalidated and thus the image would not be visible on the LyX
screen (actually, it is replaced by a small red square). I still have
to take a look at it, so I think that for the time being the simpler
patch will do.

Actually, it seems doing the right thing, as things such as $\quad$ or
$\relax$ are left as is, while $\raisebox{6mm}{}$ or $\raisebox{-6mm}{}$
produce a small vertical strip with the required characteristics (as
revealed by selecting the images in LyX).

I had a closer look and this behavior is independent of the patch.
Only when using dvipng blank images can be detected reliably, so what
described above occurs only when using the legacy route for generating
images.


So, I think the better patch can also be applied.

More rightly, as in stable the legacy route is always used as a fallback.


OK with me then.

This more or less completes this work? (For which thanks very much, to 
you and Guillaume.)


Richard

Re: test of math previews

[Enrico Forestieri]

On Thu, Jun 25, 2015 at 02:16:19PM -0400, Richard Heck wrote:
> On 06/25/2015 02:13 PM, Enrico Forestieri wrote:
> >On Thu, Jun 25, 2015 at 01:44:15PM +0200, Enrico Forestieri wrote:
> >>On Thu, Jun 25, 2015 at 10:16:21AM +0200, Enrico Forestieri wrote:
> >>>The third patch is the simpler one proposed by Guillaume. I have already
> >>>applied the better patch to master but there is a glitch with it.
> >>>Namely, if a preview snippet produces a blank image, its metrics are
> >>>not invalidated and thus the image would not be visible on the LyX
> >>>screen (actually, it is replaced by a small red square). I still have
> >>>to take a look at it, so I think that for the time being the simpler
> >>>patch will do.
> >>Actually, it seems doing the right thing, as things such as $\quad$ or
> >>$\relax$ are left as is, while $\raisebox{6mm}{}$ or $\raisebox{-6mm}{}$
> >>produce a small vertical strip with the required characteristics (as
> >>revealed by selecting the images in LyX).
> >I had a closer look and this behavior is independent of the patch.
> >Only when using dvipng blank images can be detected reliably, so what
> >described above occurs only when using the legacy route for generating
> >images.
> >
> >>So, I think the better patch can also be applied.
> >More rightly, as in stable the legacy route is always used as a fallback.
> 
> OK with me then.
> 
> This more or less completes this work?

I think so. The most important glitches were identified and solved.

> (For which thanks very much, to you
> and Guillaume.)

The credit goes mainly to Guillaume for his accurate testing.

-- 
Enrico

Re: test of math previews

[Richard Heck]

On 06/25/2015 02:58 PM, Enrico Forestieri wrote:

On Thu, Jun 25, 2015 at 02:16:19PM -0400, Richard Heck wrote:

This more or less completes this work?

I think so. The most important glitches were identified and solved.


(For which thanks very much, to you
and Guillaume.)

The credit goes mainly to Guillaume for his accurate testing.


Somebody had to write the code

rh

Re: test of math previews

[Guillaume M-M]

Le 25/06/2015 23:45, Richard Heck a écrit :

On 06/25/2015 02:58 PM, Enrico Forestieri wrote:

On Thu, Jun 25, 2015 at 02:16:19PM -0400, Richard Heck wrote:

This more or less completes this work?

I think so. The most important glitches were identified and solved.


(For which thanks very much, to you
and Guillaume.)

The credit goes mainly to Guillaume for his accurate testing.


Somebody had to write the code

rh




This is also what I believe. Thank you!

By the way will f3b03699 be backported as well? It seems that segfaults 
are almost guaranteed for people with previews on during Ctrl+C...

Re: test of math previews

[Enrico Forestieri]

On Tue, Jun 23, 2015 at 11:14:29AM -0400, Richard Heck wrote:
 On 06/22/2015 06:59 PM, Enrico Forestieri wrote:
 On Sat, Jun 20, 2015 at 02:11:53AM +0200, Enrico Forestieri wrote:
 On Fri, Jun 19, 2015 at 04:50:43PM +0100, Guillaume M-M wrote:
 
 Also, lyx-preview-macros-2-lassert.lyx no longer lasserts but on opening 
 the
 preview is not generated because \Coloneqq is not defined. This looks like
 the bug that you are referring to in your message where packages for 
 symbols
 are not properly loaded. But, notice that the preview is correctly 
 generated
 on a second time, when entering and leaving the math inset. So maybe it's
 easier to fix than it seemed. But indeed it's not a regression.
 Yes, this has always been the case, seemingly. However, now one can obtain
 a preview after entering and exiting a math inset, while previously the
 preview was simply never generated. So, this is a step forward. As regards
 the reason it occurs, I have still to investigate the matter.
 I had a closer look and this is a quite strange issue. It seems that this
 occurs only for a lonely math inset. Enable instant preview for math and
 then try loading the attached examples. Only the first one will fail.
 
 I think that this is due to the fact that the MacroData of the global
 macros defined in the lib/symbols file is set only after the first
 call to updateMacros() and when there is only a math inset, that method
 has no chance of being called and thus we miss the requirements.
 This seems to be confirmed by the attached patch for master which allows
 to preview all of the attached documents on loading.
 
 The patch is quite simple but it seems to cure a very particular case,
 so I am not sure it is worth applying. What do others think?
 
 I always imagine there are other ways to cause this sort of problem, e.g.,
 on reload of a document, or maybe someone has a small child document with
 only one macro in it. (They use that complicated macro in a lot of different
 papers, say, but not all the time.)

Ok, I will commit it, then.

-- 
Enrico

Re: test of math previews

[Enrico Forestieri]

On Tue, Jun 23, 2015 at 10:20:21PM +0200, Georg Baum wrote:
 Enrico Forestieri wrote:
 
  I don't know, maybe this is a case of a race condition. Most probably,
  with preview on, updateMacros() is not called (or is called later) and
  the MacroData:sym_ pointer is not updated at the time MacroData::xlmname()
  is called so that it contains bogus values. This seems to be confirmed by
  the attached patch that explicitly sets to null the MacroData::sym_
  pointer whenever a macro is copied. With this patch the crash is gone for
  me.
  
  I think that this is to be considered as a different instance of #9418,
  as also witnessed by the exact same backtrace.
 
 This is interesting. In theory, sym_ should only be set once, and only for 
 global macros from initSymbols(). For user defined macros it should always 
 be 0. According to my valgrind investigations of #9490 (which is exactly the 
 same problem as reported in this thread IMHO), the crash happens because the 
 MathMacro memory is already freed when MathMacro::mathmlize() is called. 
 Therefore, accessing d-macro_ invokes undefined behaviour. If this is true 
 then setting sym_ to 0 would only cure the symptom, but since I was unable 
 up to now to pinpoint the real cause of the crash I rather do not trust my 
 own findings very much anymore.

I think I understand what happens here and it seems to be a different
problem than #9490. All macros are stored using a map and when
Buffer::updateMacros() is called, it first clears the map and then
reconstructs it. The d-macro_ pointer points to a MacroData in this map
but is only updated when MathData::metrics() is called. Now, when instant
preview for math is on, MathData::metrics() is never called when
selecting a math inset, so that d-macro_ is not updated and now
d-macro_-sym_ is a bogus pointer and hence the crash when hitting Ctrl+C.

Luckily, when d-macros_ is updated by MathMacro::updateMacro(), a copy
of the pointed to data is also made. This is the data the now obsolete
d-macro_ was pointing to before Buffer::updateMacros() reorganized
things. This data contains the necessary info for trying to update
d-macro_ in the copy constructor and, if it still fails, we can simply
let d-macro_ point to that data directly.

The attached patch fixes the crash for me and I am going to commit it.

-- 
Enrico
diff --git a/src/mathed/MacroTable.h b/src/mathed/MacroTable.h
index 3bd04ea..030e6f7 100644
--- a/src/mathed/MacroTable.h
+++ b/src/mathed/MacroTable.h
@@ -69,6 +69,8 @@ public:
char const * MathMLtype() const;
///
void setSymbol(latexkeys const * sym) { sym_ = sym; }
+   ///
+   DocIterator const  pos() { return pos_; }
 
/// lock while being drawn to avoid recursions
int lock() const { return ++lockCount_; }
diff --git a/src/mathed/MathMacro.cpp b/src/mathed/MathMacro.cpp
index 24ebe77..8f2d15f 100644
--- a/src/mathed/MathMacro.cpp
+++ b/src/mathed/MathMacro.cpp
@@ -203,6 +203,14 @@ MathMacro::MathMacro(MathMacro const  that)
: InsetMathNest(that), d(new Private(*that.d))
 {
d-updateChildren(this);
+   if (d-macro_  lyxrc.preview == LyXRC::PREVIEW_ON) {
+   // We need to update d-macro_ by ourselves because in this case
+   // MathData::metrics() is not called when selecting a math inset
+   DocIterator const  pos = d-macroBackup_.pos();
+   d-macro_ = pos.buffer()-getMacro(name(), pos);
+   if (!d-macro_)
+   d-macro_ = d-macroBackup_;
+   }
 }
 
 
@@ -213,6 +221,14 @@ MathMacro  MathMacro::operator=(MathMacro const  that)
InsetMathNest::operator=(that);
*d = *that.d;
d-updateChildren(this);
+   if (d-macro_  lyxrc.preview == LyXRC::PREVIEW_ON) {
+   // We need to update d-macro_ by ourselves because in this case
+   // MathData::metrics() is not called when selecting a math inset
+   DocIterator const  pos = d-macroBackup_.pos();
+   d-macro_ = pos.buffer()-getMacro(name(), pos);
+   if (!d-macro_)
+   d-macro_ = d-macroBackup_;
+   }
return *this;
 }
 

Re: test of math previews

[Enrico Forestieri]

On Mon, Jun 22, 2015 at 11:32:17PM +0100, Guillaume M-M wrote:
 
 3) Previews have the incorrect size with layouts that offer font sizes other
 than 1[0-2]pt (lyx-preview-fontsize.lyx). The legacy method offers a
 function that retrieves the actual font size and calculates the actual dpi.
 Attached is a patch (extract_resolution.diff) that imports this function in
 the new script.

Thank you, this is much better than trying to catch all possible cases.
I have applied the patch to master at 0751f96d.

-- 
Enrico

Re: test of math previews

[Richard Heck]

On 06/24/2015 08:49 PM, Guillaume M-M wrote:

Le 25/06/2015 00:44, Enrico Forestieri a écrit :

On Mon, Jun 22, 2015 at 11:32:17PM +0100, Guillaume M-M wrote:


3) Previews have the incorrect size with layouts that offer font 
sizes other

than 1[0-2]pt (lyx-preview-fontsize.lyx). The legacy method offers a
function that retrieves the actual font size and calculates the 
actual dpi.
Attached is a patch (extract_resolution.diff) that imports this 
function in

the new script.


Thank you, this is much better than trying to catch all possible cases.
I have applied the patch to master at 0751f96d.



Thank you for applying it.

Here's a trivial patch for stable, if you are not ready to backport 
0751f96d. Both were tested in stable.


This is fine with me, as is the original patch, as Enrico sees fit.

Richard

Re: test of math previews

[Guillaume M-M]

Le 25/06/2015 00:44, Enrico Forestieri a écrit :

On Mon, Jun 22, 2015 at 11:32:17PM +0100, Guillaume M-M wrote:


3) Previews have the incorrect size with layouts that offer font sizes other
than 1[0-2]pt (lyx-preview-fontsize.lyx). The legacy method offers a
function that retrieves the actual font size and calculates the actual dpi.
Attached is a patch (extract_resolution.diff) that imports this function in
the new script.


Thank you, this is much better than trying to catch all possible cases.
I have applied the patch to master at 0751f96d.



Thank you for applying it.

Here's a trivial patch for stable, if you are not ready to backport 
0751f96d. Both were tested in stable.


Guillaume
diff --git a/lib/scripts/lyxpreview2bitmap.py b/lib/scripts/lyxpreview2bitmap.py
index 9775b0e..8cd3c09 100755
--- a/lib/scripts/lyxpreview2bitmap.py
+++ b/lib/scripts/lyxpreview2bitmap.py
@@ -159,7 +159,7 @@ def extract_metrics_info(dvipng_stdout):
 
 
 def fix_latex_file(latex_file, pdf_output):
-documentclass_re = re.compile((documentclass\[)(1[012]pt,?)(.+))
+documentclass_re = re.compile((documentclass\[)([0-9]+pt,?)(.+))
 def_re = re.compile(r(\\newcommandx|\\global\\long\\def)(\\[a-zA-Z]+))
 
 tmp = mkstemp()

Re: test of math previews

[Enrico Forestieri]

On Tue, Jun 23, 2015 at 10:20:21PM +0200, Georg Baum wrote:
> Enrico Forestieri wrote:
> 
> > I don't know, maybe this is a case of a race condition. Most probably,
> > with preview on, updateMacros() is not called (or is called later) and
> > the MacroData:sym_ pointer is not updated at the time MacroData::xlmname()
> > is called so that it contains bogus values. This seems to be confirmed by
> > the attached patch that explicitly sets to null the MacroData::sym_
> > pointer whenever a macro is copied. With this patch the crash is gone for
> > me.
> > 
> > I think that this is to be considered as a different instance of #9418,
> > as also witnessed by the exact same backtrace.
> 
> This is interesting. In theory, sym_ should only be set once, and only for 
> global macros from initSymbols(). For user defined macros it should always 
> be 0. According to my valgrind investigations of #9490 (which is exactly the 
> same problem as reported in this thread IMHO), the crash happens because the 
> MathMacro memory is already freed when MathMacro::mathmlize() is called. 
> Therefore, accessing d->macro_ invokes undefined behaviour. If this is true 
> then setting sym_ to 0 would only cure the symptom, but since I was unable 
> up to now to pinpoint the real cause of the crash I rather do not trust my 
> own findings very much anymore.

I think I understand what happens here and it seems to be a different
problem than #9490. All macros are stored using a map and when
Buffer::updateMacros() is called, it first clears the map and then
reconstructs it. The d->macro_ pointer points to a MacroData in this map
but is only updated when MathData::metrics() is called. Now, when instant
preview for math is on, MathData::metrics() is never called when
selecting a math inset, so that d->macro_ is not updated and now
d->macro_->sym_ is a bogus pointer and hence the crash when hitting Ctrl+C.

Luckily, when d->macros_ is updated by MathMacro::updateMacro(), a copy
of the pointed to data is also made. This is the data the now obsolete
d->macro_ was pointing to before Buffer::updateMacros() reorganized
things. This data contains the necessary info for trying to update
d->macro_ in the copy constructor and, if it still fails, we can simply
let d->macro_ point to that data directly.

The attached patch fixes the crash for me and I am going to commit it.

-- 
Enrico
diff --git a/src/mathed/MacroTable.h b/src/mathed/MacroTable.h
index 3bd04ea..030e6f7 100644
--- a/src/mathed/MacroTable.h
+++ b/src/mathed/MacroTable.h
@@ -69,6 +69,8 @@ public:
char const * MathMLtype() const;
///
void setSymbol(latexkeys const * sym) { sym_ = sym; }
+   ///
+   DocIterator const & pos() { return pos_; }
 
/// lock while being drawn to avoid recursions
int lock() const { return ++lockCount_; }
diff --git a/src/mathed/MathMacro.cpp b/src/mathed/MathMacro.cpp
index 24ebe77..8f2d15f 100644
--- a/src/mathed/MathMacro.cpp
+++ b/src/mathed/MathMacro.cpp
@@ -203,6 +203,14 @@ MathMacro::MathMacro(MathMacro const & that)
: InsetMathNest(that), d(new Private(*that.d))
 {
d->updateChildren(this);
+   if (d->macro_ && lyxrc.preview == LyXRC::PREVIEW_ON) {
+   // We need to update d->macro_ by ourselves because in this case
+   // MathData::metrics() is not called when selecting a math inset
+   DocIterator const & pos = d->macroBackup_.pos();
+   d->macro_ = pos.buffer()->getMacro(name(), pos);
+   if (!d->macro_)
+   d->macro_ = >macroBackup_;
+   }
 }
 
 
@@ -213,6 +221,14 @@ MathMacro & MathMacro::operator=(MathMacro const & that)
InsetMathNest::operator=(that);
*d = *that.d;
d->updateChildren(this);
+   if (d->macro_ && lyxrc.preview == LyXRC::PREVIEW_ON) {
+   // We need to update d->macro_ by ourselves because in this case
+   // MathData::metrics() is not called when selecting a math inset
+   DocIterator const & pos = d->macroBackup_.pos();
+   d->macro_ = pos.buffer()->getMacro(name(), pos);
+   if (!d->macro_)
+   d->macro_ = >macroBackup_;
+   }
return *this;
 }
 

Re: test of math previews

[Enrico Forestieri]

On Tue, Jun 23, 2015 at 11:14:29AM -0400, Richard Heck wrote:
> On 06/22/2015 06:59 PM, Enrico Forestieri wrote:
> >On Sat, Jun 20, 2015 at 02:11:53AM +0200, Enrico Forestieri wrote:
> >>On Fri, Jun 19, 2015 at 04:50:43PM +0100, Guillaume M-M wrote:
> >>
> >>>Also, lyx-preview-macros-2-lassert.lyx no longer lasserts but on opening 
> >>>the
> >>>preview is not generated because \Coloneqq is not defined. This looks like
> >>>the bug that you are referring to in your message where packages for 
> >>>symbols
> >>>are not properly loaded. But, notice that the preview is correctly 
> >>>generated
> >>>on a second time, when entering and leaving the math inset. So maybe it's
> >>>easier to fix than it seemed. But indeed it's not a regression.
> >>Yes, this has always been the case, seemingly. However, now one can obtain
> >>a preview after entering and exiting a math inset, while previously the
> >>preview was simply never generated. So, this is a step forward. As regards
> >>the reason it occurs, I have still to investigate the matter.
> >I had a closer look and this is a quite strange issue. It seems that this
> >occurs only for a lonely math inset. Enable instant preview for math and
> >then try loading the attached examples. Only the first one will fail.
> >
> >I think that this is due to the fact that the MacroData of the global
> >macros defined in the lib/symbols file is set only after the first
> >call to updateMacros() and when there is only a math inset, that method
> >has no chance of being called and thus we miss the requirements.
> >This seems to be confirmed by the attached patch for master which allows
> >to preview all of the attached documents on loading.
> >
> >The patch is quite simple but it seems to cure a very particular case,
> >so I am not sure it is worth applying. What do others think?
> 
> I always imagine there are other ways to cause this sort of problem, e.g.,
> on reload of a document, or maybe someone has a small child document with
> only one macro in it. (They use that complicated macro in a lot of different
> papers, say, but not all the time.)

Ok, I will commit it, then.

-- 
Enrico

Re: test of math previews

[Enrico Forestieri]

On Mon, Jun 22, 2015 at 11:32:17PM +0100, Guillaume M-M wrote:
> 
> 3) Previews have the incorrect size with layouts that offer font sizes other
> than 1[0-2]pt (lyx-preview-fontsize.lyx). The legacy method offers a
> function that retrieves the actual font size and calculates the actual dpi.
> Attached is a patch (extract_resolution.diff) that imports this function in
> the new script.

Thank you, this is much better than trying to catch all possible cases.
I have applied the patch to master at 0751f96d.

-- 
Enrico

Re: test of math previews

[Guillaume M-M]

Le 25/06/2015 00:44, Enrico Forestieri a écrit :

On Mon, Jun 22, 2015 at 11:32:17PM +0100, Guillaume M-M wrote:


3) Previews have the incorrect size with layouts that offer font sizes other
than 1[0-2]pt (lyx-preview-fontsize.lyx). The legacy method offers a
function that retrieves the actual font size and calculates the actual dpi.
Attached is a patch (extract_resolution.diff) that imports this function in
the new script.


Thank you, this is much better than trying to catch all possible cases.
I have applied the patch to master at 0751f96d.



Thank you for applying it.

Here's a trivial patch for stable, if you are not ready to backport 
0751f96d. Both were tested in stable.


Guillaume
diff --git a/lib/scripts/lyxpreview2bitmap.py b/lib/scripts/lyxpreview2bitmap.py
index 9775b0e..8cd3c09 100755
--- a/lib/scripts/lyxpreview2bitmap.py
+++ b/lib/scripts/lyxpreview2bitmap.py
@@ -159,7 +159,7 @@ def extract_metrics_info(dvipng_stdout):
 
 
 def fix_latex_file(latex_file, pdf_output):
-documentclass_re = re.compile("(documentclass\[)(1[012]pt,?)(.+)")
+documentclass_re = re.compile("(documentclass\[)([0-9]+pt,?)(.+)")
 def_re = re.compile(r"(\\newcommandx|\\global\\long\\def)(\\[a-zA-Z]+)")
 
 tmp = mkstemp()

Re: test of math previews

[Richard Heck]

On 06/24/2015 08:49 PM, Guillaume M-M wrote:

Le 25/06/2015 00:44, Enrico Forestieri a écrit :

On Mon, Jun 22, 2015 at 11:32:17PM +0100, Guillaume M-M wrote:


3) Previews have the incorrect size with layouts that offer font 
sizes other

than 1[0-2]pt (lyx-preview-fontsize.lyx). The legacy method offers a
function that retrieves the actual font size and calculates the 
actual dpi.
Attached is a patch (extract_resolution.diff) that imports this 
function in

the new script.


Thank you, this is much better than trying to catch all possible cases.
I have applied the patch to master at 0751f96d.



Thank you for applying it.

Here's a trivial patch for stable, if you are not ready to backport 
0751f96d. Both were tested in stable.


This is fine with me, as is the original patch, as Enrico sees fit.

Richard

Re: test of math previews

[Richard Heck]

On 06/22/2015 06:59 PM, Enrico Forestieri wrote:

On Sat, Jun 20, 2015 at 02:11:53AM +0200, Enrico Forestieri wrote:

On Fri, Jun 19, 2015 at 04:50:43PM +0100, Guillaume M-M wrote:


Also, lyx-preview-macros-2-lassert.lyx no longer lasserts but on opening the
preview is not generated because \Coloneqq is not defined. This looks like
the bug that you are referring to in your message where packages for symbols
are not properly loaded. But, notice that the preview is correctly generated
on a second time, when entering and leaving the math inset. So maybe it's
easier to fix than it seemed. But indeed it's not a regression.

Yes, this has always been the case, seemingly. However, now one can obtain
a preview after entering and exiting a math inset, while previously the
preview was simply never generated. So, this is a step forward. As regards
the reason it occurs, I have still to investigate the matter.

I had a closer look and this is a quite strange issue. It seems that this
occurs only for a lonely math inset. Enable instant preview for math and
then try loading the attached examples. Only the first one will fail.

I think that this is due to the fact that the MacroData of the global
macros defined in the lib/symbols file is set only after the first
call to updateMacros() and when there is only a math inset, that method
has no chance of being called and thus we miss the requirements.
This seems to be confirmed by the attached patch for master which allows
to preview all of the attached documents on loading.

The patch is quite simple but it seems to cure a very particular case,
so I am not sure it is worth applying. What do others think?


I always imagine there are other ways to cause this sort of problem, 
e.g., on reload of a document, or maybe someone has a small child 
document with only one macro in it. (They use that complicated macro in 
a lot of different papers, say, but not all the time.)


Richard

Re: test of math previews

[Guillaume M-M]

Le 23/06/2015 10:46, Enrico Forestieri a écrit :

On Tue, Jun 23, 2015 at 11:18:56AM +0200, Enrico Forestieri wrote:

On Tue, Jun 23, 2015 at 02:44:00AM +0100, Guillaume M-M wrote:


Still there at cd046f0e (master). But here's the trace I get when it crashes
right at the first Ctrl+C:


I doubt this has anything to do with the recent changes as I can also
reproduce the crash with 2.1.3.

BTW, the backtrace you show is what lyx spits out automatically. I don't
know from which thread that backtrace is taken (I also am very skeptical
about this kde-ish automatic bt thingie), but if you directly run lyx
with gdb, you would get the following (more appropriate) backtrace:


Thank you for the info (the above one-line traces came from gdb).



Indeed, this has to do with the general problem of copying macros and
how the various structures are carried on when copying.


OK for the fact that this is not a regression, but why should it be 
different depending on whether preview is on or off?




The attached patch avoids the crash for me (at least I don't get any
crash even after hitting Ctrl+C one hundred times).



Yes, I confirm, thank you.

Re: test of math previews

[Enrico Forestieri]

On Tue, Jun 23, 2015 at 02:23:18PM +0100, Guillaume M-M wrote:
 Le 23/06/2015 10:46, Enrico Forestieri a écrit :
 
 Indeed, this has to do with the general problem of copying macros and
 how the various structures are carried on when copying.
 
 OK for the fact that this is not a regression, but why should it be
 different depending on whether preview is on or off?

I don't know, maybe this is a case of a race condition. Most probably,
with preview on, updateMacros() is not called (or is called later) and
the MacroData:sym_ pointer is not updated at the time MacroData::xlmname()
is called so that it contains bogus values. This seems to be confirmed by
the attached patch that explicitly sets to null the MacroData::sym_ pointer
whenever a macro is copied. With this patch the crash is gone for me.

I think that this is to be considered as a different instance of #9418,
as also witnessed by the exact same backtrace.

-- 
Enrico
diff --git a/src/mathed/MathMacro.cpp b/src/mathed/MathMacro.cpp
index 24ebe77..4597ceb 100644
--- a/src/mathed/MathMacro.cpp
+++ b/src/mathed/MathMacro.cpp
@@ -203,6 +203,8 @@ MathMacro::MathMacro(MathMacro const  that)
: InsetMathNest(that), d(new Private(*that.d))
 {
d-updateChildren(this);
+   if (d-macro_)
+   const_castMacroData *(d-macro_)-setSymbol(0);
 }
 
 
@@ -213,6 +215,8 @@ MathMacro  MathMacro::operator=(MathMacro const  that)
InsetMathNest::operator=(that);
*d = *that.d;
d-updateChildren(this);
+   if (d-macro_)
+   const_castMacroData *(d-macro_)-setSymbol(0);
return *this;
 }
 

Re: test of math previews

[Enrico Forestieri]

On Tue, Jun 23, 2015 at 02:23:18PM +0100, Guillaume M-M wrote:
 Le 23/06/2015 10:46, Enrico Forestieri a écrit :
 
 Indeed, this has to do with the general problem of copying macros and
 how the various structures are carried on when copying.
 
 OK for the fact that this is not a regression, but why should it be
 different depending on whether preview is on or off?

I don't know, maybe this is a case of a race condition. Most probably,
with preview on, updateMacros() is not called (or is called later) and
the MacroData:sym_ pointer is not updated at the time MacroData::xlmname()
is called so that it contains bogus values. This seems to be confirmed by
the attached patch that explicitly sets to null the MacroData::sym_ pointer
whenever a macro is copied. With this patch the crash is gone for me.

I think that this is to be considered as a different instance of #9418,
as also witnessed by the exact same backtrace.

-- 
Enrico
diff --git a/src/mathed/MathMacro.cpp b/src/mathed/MathMacro.cpp
index 24ebe77..4597ceb 100644
--- a/src/mathed/MathMacro.cpp
+++ b/src/mathed/MathMacro.cpp
@@ -203,6 +203,8 @@ MathMacro::MathMacro(MathMacro const  that)
: InsetMathNest(that), d(new Private(*that.d))
 {
d-updateChildren(this);
+   if (d-macro_)
+   const_castMacroData *(d-macro_)-setSymbol(0);
 }
 
 
@@ -213,6 +215,8 @@ MathMacro  MathMacro::operator=(MathMacro const  that)
InsetMathNest::operator=(that);
*d = *that.d;
d-updateChildren(this);
+   if (d-macro_)
+   const_castMacroData *(d-macro_)-setSymbol(0);
return *this;
 }
 

Re: test of math previews

[Georg Baum]

Enrico Forestieri wrote:

 I don't know, maybe this is a case of a race condition. Most probably,
 with preview on, updateMacros() is not called (or is called later) and
 the MacroData:sym_ pointer is not updated at the time MacroData::xlmname()
 is called so that it contains bogus values. This seems to be confirmed by
 the attached patch that explicitly sets to null the MacroData::sym_
 pointer whenever a macro is copied. With this patch the crash is gone for
 me.
 
 I think that this is to be considered as a different instance of #9418,
 as also witnessed by the exact same backtrace.

This is interesting. In theory, sym_ should only be set once, and only for 
global macros from initSymbols(). For user defined macros it should always 
be 0. According to my valgrind investigations of #9490 (which is exactly the 
same problem as reported in this thread IMHO), the crash happens because the 
MathMacro memory is already freed when MathMacro::mathmlize() is called. 
Therefore, accessing d-macro_ invokes undefined behaviour. If this is true 
then setting sym_ to 0 would only cure the symptom, but since I was unable 
up to now to pinpoint the real cause of the crash I rather do not trust my 
own findings very much anymore.


Georg

Re: test of math previews

[Enrico Forestieri]

On Tue, Jun 23, 2015 at 10:20:21PM +0200, Georg Baum wrote:
 Enrico Forestieri wrote:
 
  I don't know, maybe this is a case of a race condition. Most probably,
  with preview on, updateMacros() is not called (or is called later) and
  the MacroData:sym_ pointer is not updated at the time MacroData::xlmname()
  is called so that it contains bogus values. This seems to be confirmed by
  the attached patch that explicitly sets to null the MacroData::sym_
  pointer whenever a macro is copied. With this patch the crash is gone for
  me.
  
  I think that this is to be considered as a different instance of #9418,
  as also witnessed by the exact same backtrace.
 
 This is interesting. In theory, sym_ should only be set once, and only for 
 global macros from initSymbols(). For user defined macros it should always 
 be 0. According to my valgrind investigations of #9490 (which is exactly the 
 same problem as reported in this thread IMHO), the crash happens because the 
 MathMacro memory is already freed when MathMacro::mathmlize() is called. 
 Therefore, accessing d-macro_ invokes undefined behaviour. If this is true 
 then setting sym_ to 0 would only cure the symptom, but since I was unable 
 up to now to pinpoint the real cause of the crash I rather do not trust my 
 own findings very much anymore.

It may very well be like that. In this particular crash, I verified
that sym_ is not 0 but has bogus values, such as 0x01 or 0x25, so
that the crash is triggered by trying to access sym_-xmlname, even
if d-macro_-xmlname() can be accessed.

Of course, setting sym_ to 0 is not the solution. It was only expedient
to show the cause of the crash. Indeed, it may very well cause other
crashes because most probably some innocent memory gets trashed.

-- 
Enrico

Re: test of math previews

[Enrico Forestieri]

On Tue, Jun 23, 2015 at 11:18:56AM +0200, Enrico Forestieri wrote:
 On Tue, Jun 23, 2015 at 02:44:00AM +0100, Guillaume M-M wrote:
  
  Still there at cd046f0e (master). But here's the trace I get when it crashes
  right at the first Ctrl+C:
 
 I doubt this has anything to do with the recent changes as I can also
 reproduce the crash with 2.1.3.
 
 BTW, the backtrace you show is what lyx spits out automatically. I don't
 know from which thread that backtrace is taken (I also am very skeptical
 about this kde-ish automatic bt thingie), but if you directly run lyx
 with gdb, you would get the following (more appropriate) backtrace:

Indeed, this has to do with the general problem of copying macros and
how the various structures are carried on when copying.

The attached patch avoids the crash for me (at least I don't get any
crash even after hitting Ctrl+C one hundred times).

-- 
Enrico
diff --git a/src/mathed/MathMacro.cpp b/src/mathed/MathMacro.cpp
index 24ebe77..09cfa0c 100644
--- a/src/mathed/MathMacro.cpp
+++ b/src/mathed/MathMacro.cpp
@@ -949,8 +949,9 @@ void MathMacro::mathmlize(MathStream  os) const
 {
// macro_ is 0 if this is an unknown macro
LATTEST(d-macro_ || d-displayMode_ != DISPLAY_NORMAL);
-   if (d-macro_) {
-   docstring const xmlname = d-macro_-xmlname();
+   MacroData const * data = MacroTable::globalMacros().get(name());
+   if (data) {
+   docstring const xmlname = data-xmlname();
if (!xmlname.empty()) {
char const * type = d-macro_-MathMLtype();
os  ''  type xmlname   /

Re: test of math previews

[Enrico Forestieri]

On Tue, Jun 23, 2015 at 02:44:00AM +0100, Guillaume M-M wrote:
 Le 23/06/2015 02:36, Guillaume M-M a écrit :
 Le 23/06/2015 02:34, Guillaume M-M a écrit :
 Another segfault in the stable branch involving previews (and master as
 well). When Instant preview is on and one copies a formula, it randomly
 crashes. In the attached, select the math inset and hold Ctrl+C until it
 segfaults. Sometimes it happens on the first press. This is pretty
 annoying because it's not a corner case at all. There is a very simple
 math macro, but it does not crash with preview off.
 
 The positive side is that I am getting a glimpse that having instant
 previews that actually work is very pleasing :)
 
 
 Stack trace:
 
 Stable:
 
 0x76360a48 in std::basic_stringwchar_t,
 std::char_traitswchar_t, std::allocatorwchar_t
  ::basic_string(std::basic_stringwchar_t, std::char_traitswchar_t,
 std::allocatorwchar_t  const) () from
 /usr/lib/x86_64-linux-gnu/libstdc++.so.6
 
 Master:
 
 0x0044e2e2 in ?? ()
 
 Does not take cd046f0e into account. Let me test again.
 
 
 
 
 Still there at cd046f0e (master). But here's the trace I get when it crashes
 right at the first Ctrl+C:

I doubt this has anything to do with the recent changes as I can also
reproduce the crash with 2.1.3.

BTW, the backtrace you show is what lyx spits out automatically. I don't
know from which thread that backtrace is taken (I also am very skeptical
about this kde-ish automatic bt thingie), but if you directly run lyx
with gdb, you would get the following (more appropriate) backtrace:

Program received signal SIGSEGV, Segmentation fault.
0xb6cafc1c in std::basic_stringwchar_t, std::char_traitswchar_t, 
std::allocatorwchar_t ::basic_string(std::basic_stringwchar_t, 
std::char_traitswchar_t, std::allocatorwchar_t  const) ()
   from /usr/lib/i386-linux-gnu/libstdc++.so.6
(gdb) bt
#0  0xb6cafc1c in std::basic_stringwchar_t, std::char_traitswchar_t, 
std::allocatorwchar_t ::basic_string(std::basic_stringwchar_t, 
std::char_traitswchar_t, std::allocatorwchar_t  const) ()
   from /usr/lib/i386-linux-gnu/libstdc++.so.6
#1  0x08383432 in lyx::MacroData::xmlname (this=0xa445428)
at ../../src/mathed/MacroTable.cpp:132
#2  0x0837fd00 in lyx::MathMacro::mathmlize (this=0x9a600c0, os=...)
at ../../src/mathed/MathMacro.cpp:953
#3  0x0839ce3f in lyx::operator (ms=..., at=...)
at ../../src/mathed/MathStream.cpp:294
#4  0x08373c23 in lyx::mathmlize (dat=..., os=...)
at ../../src/mathed/MathExtern.cpp:1446
#5  0x0839ce6c in lyx::operator (ms=..., ar=...)
at ../../src/mathed/MathStream.cpp:301
#6  0x08343b06 in lyx::InsetMathHull::mathmlize (this=0x93d95f8, os=...)
at ../../src/mathed/InsetMathHull.cpp:2180
#7  0x083441dd in lyx::InsetMathHull::xhtml (this=0x93d95f8, xs=..., op=...)
at ../../src/mathed/InsetMathHull.cpp:2252
#8  0x0823cc1d in lyx::Paragraph::simpleLyXHTMLOnePar (this=0x92bb3a8, 
buf=..., xs=..., runparams=..., outerfont=..., initial=0)
at ../../src/Paragraph.cpp:3051
#9  0x0822b47f in lyx::(anonymous namespace)::makeParagraphs (buf=..., xs=..., 
runparams=..., text=..., pbegin=..., pend=...)
at ../../src/output_xhtml.cpp:898
#10 0x0822c886 in lyx::xhtmlParagraphs (text=..., buf=..., xs=..., 
runparams=...) at ../../src/output_xhtml.cpp:1197
#11 0x080ddec0 in lyx::Buffer::writeLyXHTMLSource (this=0x9a29608, os=..., 
runparams=..., output=lyx::Buffer::FullSource) at ../../src/Buffer.cpp:2085
#12 0x0816f8a1 in lyx::(anonymous namespace)::putClipboard (paragraphs=..., 
docclass=..., plaintext=...) at ../../src/CutAndPaste.cpp:563
#13 0x081723eb in lyx::cap::copySelection (cur=..., plaintext=...)
at ../../src/CutAndPaste.cpp:1028
#14 0x08171842 in lyx::cap::copySelection (cur=...)
at ../../src/CutAndPaste.cpp:929
#15 0x0827d779 in lyx::Text::dispatch (this=0x9aab848, cur=..., cmd=...)
at ../../src/Text3.cpp:1369
#16 0x084c2329 in lyx::InsetText::doDispatch (this=0x9aab838, cur=..., cmd=...)
at ../../src/insets/InsetText.cpp:312
#17 0x083eb007 in lyx::Inset::dispatch (this=0x9aab838, cur=..., cmd=...)
at ../../src/insets/Inset.cpp:319
#18 0x08160c1d in lyx::Cursor::dispatch (this=0xa448e60, cmd0=...)
at ../../src/Cursor.cpp:433
#19 0x08539fd9 in lyx::frontend::GuiView::dispatchToBufferView (
this=0x90e79e8, cmd=..., dr=...)
at ../../../../src/frontends/qt4/GuiView.cpp:3448
#20 0x0853d7c8 in lyx::frontend::GuiView::dispatch (this=0x90e79e8, cmd=..., 
dr=...) at ../../../../src/frontends/qt4/GuiView.cpp:4005
#21 0x084f5b10 in lyx::frontend::GuiApplication::dispatch (this=0x8c72a10, 
cmd=..., dr=...) at ../../../../src/frontends/qt4/GuiApplication.cpp:2041
#22 0x084f1fae in lyx::frontend::GuiApplication::dispatch (this=0x8c72a10, 
cmd=...) at ../../../../src/frontends/qt4/GuiApplication.cpp:1389
#23 0x081f37d1 in lyx::dispatch (action=...) at ../../src/LyX.cpp:1392
#24 0x084f76cb in lyx::frontend::GuiApplication::processFuncRequest (
this=0x8c72a10, 

Re: test of math previews

[Enrico Forestieri]

On Tue, Jun 23, 2015 at 02:44:00AM +0100, Guillaume M-M wrote:
> Le 23/06/2015 02:36, Guillaume M-M a écrit :
> >Le 23/06/2015 02:34, Guillaume M-M a écrit :
> >>Another segfault in the stable branch involving previews (and master as
> >>well). When Instant preview is on and one copies a formula, it randomly
> >>crashes. In the attached, select the math inset and hold Ctrl+C until it
> >>segfaults. Sometimes it happens on the first press. This is pretty
> >>annoying because it's not a corner case at all. There is a very simple
> >>math macro, but it does not crash with preview off.
> >>
> >>The positive side is that I am getting a glimpse that having instant
> >>previews that actually work is very pleasing :)
> >>
> >>
> >>Stack trace:
> >>
> >>Stable:
> >>
> >>0x76360a48 in std::basic_string >>std::char_traits, std::allocator
> >> >::basic_string(std::basic_string >>std::allocator > const&) () from
> >>/usr/lib/x86_64-linux-gnu/libstdc++.so.6
> >>
> >>Master:
> >>
> >>0x0044e2e2 in ?? ()
> >
> >Does not take cd046f0e into account. Let me test again.
> >
> >
> 
> 
> Still there at cd046f0e (master). But here's the trace I get when it crashes
> right at the first Ctrl+C:

I doubt this has anything to do with the recent changes as I can also
reproduce the crash with 2.1.3.

BTW, the backtrace you show is what lyx spits out automatically. I don't
know from which thread that backtrace is taken (I also am very skeptical
about this kde-ish automatic bt thingie), but if you directly run lyx
with gdb, you would get the following (more appropriate) backtrace:

Program received signal SIGSEGV, Segmentation fault.
0xb6cafc1c in std::basic_string::basic_string(std::basic_string const&) ()
   from /usr/lib/i386-linux-gnu/libstdc++.so.6
(gdb) bt
#0  0xb6cafc1c in std::basic_string::basic_string(std::basic_string const&) ()
   from /usr/lib/i386-linux-gnu/libstdc++.so.6
#1  0x08383432 in lyx::MacroData::xmlname (this=0xa445428)
at ../../src/mathed/MacroTable.cpp:132
#2  0x0837fd00 in lyx::MathMacro::mathmlize (this=0x9a600c0, os=...)
at ../../src/mathed/MathMacro.cpp:953
#3  0x0839ce3f in lyx::operator<< (ms=..., at=...)
at ../../src/mathed/MathStream.cpp:294
#4  0x08373c23 in lyx::mathmlize (dat=..., os=...)
at ../../src/mathed/MathExtern.cpp:1446
#5  0x0839ce6c in lyx::operator<< (ms=..., ar=...)
at ../../src/mathed/MathStream.cpp:301
#6  0x08343b06 in lyx::InsetMathHull::mathmlize (this=0x93d95f8, os=...)
at ../../src/mathed/InsetMathHull.cpp:2180
#7  0x083441dd in lyx::InsetMathHull::xhtml (this=0x93d95f8, xs=..., op=...)
at ../../src/mathed/InsetMathHull.cpp:2252
#8  0x0823cc1d in lyx::Paragraph::simpleLyXHTMLOnePar (this=0x92bb3a8, 
buf=..., xs=..., runparams=..., outerfont=..., initial=0)
at ../../src/Paragraph.cpp:3051
#9  0x0822b47f in lyx::(anonymous namespace)::makeParagraphs (buf=..., xs=..., 
runparams=..., text=..., pbegin=..., pend=...)
at ../../src/output_xhtml.cpp:898
#10 0x0822c886 in lyx::xhtmlParagraphs (text=..., buf=..., xs=..., 
runparams=...) at ../../src/output_xhtml.cpp:1197
#11 0x080ddec0 in lyx::Buffer::writeLyXHTMLSource (this=0x9a29608, os=..., 
runparams=..., output=lyx::Buffer::FullSource) at ../../src/Buffer.cpp:2085
#12 0x0816f8a1 in lyx::(anonymous namespace)::putClipboard (paragraphs=..., 
docclass=..., plaintext=...) at ../../src/CutAndPaste.cpp:563
#13 0x081723eb in lyx::cap::copySelection (cur=..., plaintext=...)
at ../../src/CutAndPaste.cpp:1028
#14 0x08171842 in lyx::cap::copySelection (cur=...)
at ../../src/CutAndPaste.cpp:929
#15 0x0827d779 in lyx::Text::dispatch (this=0x9aab848, cur=..., cmd=...)
at ../../src/Text3.cpp:1369
#16 0x084c2329 in lyx::InsetText::doDispatch (this=0x9aab838, cur=..., cmd=...)
at ../../src/insets/InsetText.cpp:312
#17 0x083eb007 in lyx::Inset::dispatch (this=0x9aab838, cur=..., cmd=...)
at ../../src/insets/Inset.cpp:319
#18 0x08160c1d in lyx::Cursor::dispatch (this=0xa448e60, cmd0=...)
at ../../src/Cursor.cpp:433
#19 0x08539fd9 in lyx::frontend::GuiView::dispatchToBufferView (
this=0x90e79e8, cmd=..., dr=...)
at ../../../../src/frontends/qt4/GuiView.cpp:3448
#20 0x0853d7c8 in lyx::frontend::GuiView::dispatch (this=0x90e79e8, cmd=..., 
dr=...) at ../../../../src/frontends/qt4/GuiView.cpp:4005
#21 0x084f5b10 in lyx::frontend::GuiApplication::dispatch (this=0x8c72a10, 
cmd=..., dr=...) at ../../../../src/frontends/qt4/GuiApplication.cpp:2041
#22 0x084f1fae in lyx::frontend::GuiApplication::dispatch (this=0x8c72a10, 
cmd=...) at ../../../../src/frontends/qt4/GuiApplication.cpp:1389
#23 0x081f37d1 in lyx::dispatch (action=...) at ../../src/LyX.cpp:1392
#24 0x084f76cb in lyx::frontend::GuiApplication::processFuncRequest (

Re: test of math previews

[Enrico Forestieri]

On Tue, Jun 23, 2015 at 11:18:56AM +0200, Enrico Forestieri wrote:
> On Tue, Jun 23, 2015 at 02:44:00AM +0100, Guillaume M-M wrote:
> > 
> > Still there at cd046f0e (master). But here's the trace I get when it crashes
> > right at the first Ctrl+C:
> 
> I doubt this has anything to do with the recent changes as I can also
> reproduce the crash with 2.1.3.
> 
> BTW, the backtrace you show is what lyx spits out automatically. I don't
> know from which thread that backtrace is taken (I also am very skeptical
> about this kde-ish automatic bt thingie), but if you directly run lyx
> with gdb, you would get the following (more appropriate) backtrace:

Indeed, this has to do with the general problem of copying macros and
how the various structures are carried on when copying.

The attached patch avoids the crash for me (at least I don't get any
crash even after hitting Ctrl+C one hundred times).

-- 
Enrico
diff --git a/src/mathed/MathMacro.cpp b/src/mathed/MathMacro.cpp
index 24ebe77..09cfa0c 100644
--- a/src/mathed/MathMacro.cpp
+++ b/src/mathed/MathMacro.cpp
@@ -949,8 +949,9 @@ void MathMacro::mathmlize(MathStream & os) const
 {
// macro_ is 0 if this is an unknown macro
LATTEST(d->macro_ || d->displayMode_ != DISPLAY_NORMAL);
-   if (d->macro_) {
-   docstring const xmlname = d->macro_->xmlname();
+   MacroData const * data = MacroTable::globalMacros().get(name());
+   if (data) {
+   docstring const xmlname = data->xmlname();
if (!xmlname.empty()) {
char const * type = d->macro_->MathMLtype();
os << '<' << type << "> " << xmlname << " /<"

Re: test of math previews

[Guillaume M-M]

Le 23/06/2015 10:46, Enrico Forestieri a écrit :

On Tue, Jun 23, 2015 at 11:18:56AM +0200, Enrico Forestieri wrote:

On Tue, Jun 23, 2015 at 02:44:00AM +0100, Guillaume M-M wrote:


Still there at cd046f0e (master). But here's the trace I get when it crashes
right at the first Ctrl+C:


I doubt this has anything to do with the recent changes as I can also
reproduce the crash with 2.1.3.

BTW, the backtrace you show is what lyx spits out automatically. I don't
know from which thread that backtrace is taken (I also am very skeptical
about this kde-ish automatic bt thingie), but if you directly run lyx
with gdb, you would get the following (more appropriate) backtrace:


Thank you for the info (the above one-line traces came from gdb).



Indeed, this has to do with the general problem of copying macros and
how the various structures are carried on when copying.


OK for the fact that this is not a regression, but why should it be 
different depending on whether preview is on or off?




The attached patch avoids the crash for me (at least I don't get any
crash even after hitting Ctrl+C one hundred times).



Yes, I confirm, thank you.

Re: test of math previews

[Richard Heck]

On 06/22/2015 06:59 PM, Enrico Forestieri wrote:

On Sat, Jun 20, 2015 at 02:11:53AM +0200, Enrico Forestieri wrote:

On Fri, Jun 19, 2015 at 04:50:43PM +0100, Guillaume M-M wrote:


Also, lyx-preview-macros-2-lassert.lyx no longer lasserts but on opening the
preview is not generated because \Coloneqq is not defined. This looks like
the bug that you are referring to in your message where packages for symbols
are not properly loaded. But, notice that the preview is correctly generated
on a second time, when entering and leaving the math inset. So maybe it's
easier to fix than it seemed. But indeed it's not a regression.

Yes, this has always been the case, seemingly. However, now one can obtain
a preview after entering and exiting a math inset, while previously the
preview was simply never generated. So, this is a step forward. As regards
the reason it occurs, I have still to investigate the matter.

I had a closer look and this is a quite strange issue. It seems that this
occurs only for a lonely math inset. Enable instant preview for math and
then try loading the attached examples. Only the first one will fail.

I think that this is due to the fact that the MacroData of the global
macros defined in the lib/symbols file is set only after the first
call to updateMacros() and when there is only a math inset, that method
has no chance of being called and thus we miss the requirements.
This seems to be confirmed by the attached patch for master which allows
to preview all of the attached documents on loading.

The patch is quite simple but it seems to cure a very particular case,
so I am not sure it is worth applying. What do others think?


I always imagine there are other ways to cause this sort of problem, 
e.g., on reload of a document, or maybe someone has a small child 
document with only one macro in it. (They use that complicated macro in 
a lot of different papers, say, but not all the time.)


Richard

Re: test of math previews

[Enrico Forestieri]

On Tue, Jun 23, 2015 at 02:23:18PM +0100, Guillaume M-M wrote:
> Le 23/06/2015 10:46, Enrico Forestieri a écrit :
> >
> >Indeed, this has to do with the general problem of copying macros and
> >how the various structures are carried on when copying.
> 
> OK for the fact that this is not a regression, but why should it be
> different depending on whether preview is on or off?

I don't know, maybe this is a case of a race condition. Most probably,
with preview on, updateMacros() is not called (or is called later) and
the MacroData:sym_ pointer is not updated at the time MacroData::xlmname()
is called so that it contains bogus values. This seems to be confirmed by
the attached patch that explicitly sets to null the MacroData::sym_ pointer
whenever a macro is copied. With this patch the crash is gone for me.

I think that this is to be considered as a different instance of #9418,
as also witnessed by the exact same backtrace.

-- 
Enrico
diff --git a/src/mathed/MathMacro.cpp b/src/mathed/MathMacro.cpp
index 24ebe77..4597ceb 100644
--- a/src/mathed/MathMacro.cpp
+++ b/src/mathed/MathMacro.cpp
@@ -203,6 +203,8 @@ MathMacro::MathMacro(MathMacro const & that)
: InsetMathNest(that), d(new Private(*that.d))
 {
d->updateChildren(this);
+   if (d->macro_)
+   const_cast(d->macro_)->setSymbol(0);
 }
 
 
@@ -213,6 +215,8 @@ MathMacro & MathMacro::operator=(MathMacro const & that)
InsetMathNest::operator=(that);
*d = *that.d;
d->updateChildren(this);
+   if (d->macro_)
+   const_cast(d->macro_)->setSymbol(0);
return *this;
 }
 

Re: test of math previews

[Enrico Forestieri]

On Tue, Jun 23, 2015 at 02:23:18PM +0100, Guillaume M-M wrote:
> Le 23/06/2015 10:46, Enrico Forestieri a écrit :
> >
> >Indeed, this has to do with the general problem of copying macros and
> >how the various structures are carried on when copying.
> 
> OK for the fact that this is not a regression, but why should it be
> different depending on whether preview is on or off?

I don't know, maybe this is a case of a race condition. Most probably,
with preview on, updateMacros() is not called (or is called later) and
the MacroData:sym_ pointer is not updated at the time MacroData::xlmname()
is called so that it contains bogus values. This seems to be confirmed by
the attached patch that explicitly sets to null the MacroData::sym_ pointer
whenever a macro is copied. With this patch the crash is gone for me.

I think that this is to be considered as a different instance of #9418,
as also witnessed by the exact same backtrace.

-- 
Enrico
diff --git a/src/mathed/MathMacro.cpp b/src/mathed/MathMacro.cpp
index 24ebe77..4597ceb 100644
--- a/src/mathed/MathMacro.cpp
+++ b/src/mathed/MathMacro.cpp
@@ -203,6 +203,8 @@ MathMacro::MathMacro(MathMacro const & that)
: InsetMathNest(that), d(new Private(*that.d))
 {
d->updateChildren(this);
+   if (d->macro_)
+   const_cast(d->macro_)->setSymbol(0);
 }
 
 
@@ -213,6 +215,8 @@ MathMacro & MathMacro::operator=(MathMacro const & that)
InsetMathNest::operator=(that);
*d = *that.d;
d->updateChildren(this);
+   if (d->macro_)
+   const_cast(d->macro_)->setSymbol(0);
return *this;
 }
 

Re: test of math previews

[Georg Baum]

Enrico Forestieri wrote:

> I don't know, maybe this is a case of a race condition. Most probably,
> with preview on, updateMacros() is not called (or is called later) and
> the MacroData:sym_ pointer is not updated at the time MacroData::xlmname()
> is called so that it contains bogus values. This seems to be confirmed by
> the attached patch that explicitly sets to null the MacroData::sym_
> pointer whenever a macro is copied. With this patch the crash is gone for
> me.
> 
> I think that this is to be considered as a different instance of #9418,
> as also witnessed by the exact same backtrace.

This is interesting. In theory, sym_ should only be set once, and only for 
global macros from initSymbols(). For user defined macros it should always 
be 0. According to my valgrind investigations of #9490 (which is exactly the 
same problem as reported in this thread IMHO), the crash happens because the 
MathMacro memory is already freed when MathMacro::mathmlize() is called. 
Therefore, accessing d->macro_ invokes undefined behaviour. If this is true 
then setting sym_ to 0 would only cure the symptom, but since I was unable 
up to now to pinpoint the real cause of the crash I rather do not trust my 
own findings very much anymore.


Georg

Re: test of math previews

[Enrico Forestieri]

On Tue, Jun 23, 2015 at 10:20:21PM +0200, Georg Baum wrote:
> Enrico Forestieri wrote:
> 
> > I don't know, maybe this is a case of a race condition. Most probably,
> > with preview on, updateMacros() is not called (or is called later) and
> > the MacroData:sym_ pointer is not updated at the time MacroData::xlmname()
> > is called so that it contains bogus values. This seems to be confirmed by
> > the attached patch that explicitly sets to null the MacroData::sym_
> > pointer whenever a macro is copied. With this patch the crash is gone for
> > me.
> > 
> > I think that this is to be considered as a different instance of #9418,
> > as also witnessed by the exact same backtrace.
> 
> This is interesting. In theory, sym_ should only be set once, and only for 
> global macros from initSymbols(). For user defined macros it should always 
> be 0. According to my valgrind investigations of #9490 (which is exactly the 
> same problem as reported in this thread IMHO), the crash happens because the 
> MathMacro memory is already freed when MathMacro::mathmlize() is called. 
> Therefore, accessing d->macro_ invokes undefined behaviour. If this is true 
> then setting sym_ to 0 would only cure the symptom, but since I was unable 
> up to now to pinpoint the real cause of the crash I rather do not trust my 
> own findings very much anymore.

It may very well be like that. In this particular crash, I verified
that sym_ is not 0 but has bogus values, such as 0x01 or 0x25, so
that the crash is triggered by trying to access sym_->xmlname, even
if d->macro_->xmlname() can be accessed.

Of course, setting sym_ to 0 is not the solution. It was only expedient
to show the cause of the crash. Indeed, it may very well cause other
crashes because most probably some innocent memory gets trashed.

-- 
Enrico

Re: test of math previews

[Enrico Forestieri]

On Mon, Jun 22, 2015 at 11:32:17PM +0100, Guillaume M-M wrote:
 
 1) SIGSEGV with previews (no math macros involved). I believe this is the
 one I mentioned in a previous message. See the attached
 lyx-preview-sigsegv.lyx for instructions.

Thanks for catching this. I fixed this issue at cd046f0e. The other two
issues are not critical and I'll have a look later.

-- 
Enrico

Re: test of math previews

[Guillaume M-M]

Three bugs in the current stable (93c0512b). The third one has an easy 
fix (and also a very trivial partial fix if you want to move forward 
with the release). The first two could be postponed after the release, 
or maybe they are trivial as well to Enrico. Found the first two while 
testing my solution to the third one.


1) SIGSEGV with previews (no math macros involved). I believe this is 
the one I mentioned in a previous message. See the attached 
lyx-preview-sigsegv.lyx for instructions.



2) Closing a file whose previews are being generated results in an error 
dialog Could not remove the temporary directory 
/tmp/lyx_tmpdir.LXSnBrW17125 and the following output on the terminal:


Warning: Could not remove temporary directory

Could not remove the temporary directory 
/tmp/lyx_tmpdir.LXSnBrW17125/lyx_tmpbuf0
Warning: Warning in extract_resolution! Unable to open 
lyxpreviewQ17125_legacy.log

Warning: type 'exceptions.IOError',IOError(2, 'No such file or directory')
Warning: check_latex_log: Unable to open lyxpreviewQ17125_legacy.log
Warning: type 'exceptions.IOError',IOError(2, 'No such file or directory')
Warning: Warning in legacy_extract_metrics_info! Unable to open 
lyxpreviewQ17125_legacy.log

Warning: type 'exceptions.IOError',IOError(2, 'No such file or directory')
Error: Failed to extract metrics info from lyxpreviewQ17125_legacy.log

So there's some additional cleanup to do now when closing a file.


3) Previews have the incorrect size with layouts that offer font sizes 
other than 1[0-2]pt (lyx-preview-fontsize.lyx). The legacy method offers 
a function that retrieves the actual font size and calculates the actual 
dpi. Attached is a patch (extract_resolution.diff) that imports this 
function in the new script.


(It's possible to just replace:
documentclass_re = re.compile((documentclass\[)(1[012]pt,?)(.+))
with:
documentclass_re = re.compile((documentclass\[)([0-9]+pt,?)(.+))
but this does not take into account custom classes, options, preambles, 
etc. If this path is retained, here's the result of $grep FontSize 
*.layout:


achemso.layout: FontSize10|11|12
acmsiggraph.layout:  FontSize   9|10|11|12
agutex.layout:  FontSize10|11|12
amsart.layout:  FontSize   8|9|10|11|12
amsbook.layout: FontSize   8|9|10|11|12
apa6.layout:FontSize  10|11|12
apa.layout: FontSize  6|8|10|12
broadway.layout:FontSize12
dtk.layout: FontSize  default # only 10pt in fact of A5
elsarticle.layout:  FontSize10|11|12
elsart.layout:#  FontSize default   # controlled by class
entcs.layout:   FontSize   11
extarticle.layout:  FontSize  8|9|10|11|12|14|17|20
extbook.layout: FontSize  8|9|10|11|12|14|17|20
extletter.layout:   FontSize  8|9|10|11|12|14|17|20
extreport.layout:   FontSize  8|9|10|11|12|14|17|20
foils.layout:   FontSize  17|20|25|30
hollywood.layout:   FontSize  12
IEEEtran-CompSoc.layout:  FontSize  12
IEEEtran.layout:  FontSize  9|10|11|12
iopart.layout:  FontSize10|12
jasatex.layout: FontSize10|11|12
latex8.layout:   FontSize 10
ltugboat.layout:FontSize  default # only 11pt
memoir.layout:  FontSize 9|10|11|12|14|17
powerdot.layout:  FontSize 
size=8|size=9|size=10|size=11|size=12|size=14|size=17|size=20

sciposter.layout:   FontSize  14|17|20|25|30|36
seminar.layout: FontSize8|9|10|11|12|14|17
siamltex.layout:FontSize   8|9|10|11|12
sigplanconf.layout:  FontSize   9|10|11
slides.layout:  FontSize   |
svmono.layout:  FontSize10
svmult.layout:  FontSize

In particular, Powerdot has a special syntax, but the default font size 
is too big anyways (magnification=2000) so it's better to compute the 
font size from the log file, and this is precisely what 
extract_resolution already does.)


lyx-preview-sigsegv.lyx
Description: application/lyx


lyx-preview-fontsize.lyx
Description: application/lyx
diff --git a/lib/scripts/lyxpreview2bitmap.py b/lib/scripts/lyxpreview2bitmap.py
index 9775b0e..f9ee3b3 100755
--- a/lib/scripts/lyxpreview2bitmap.py
+++ b/lib/scripts/lyxpreview2bitmap.py
@@ -77,7 +77,7 @@
 
 import getopt, glob, os, re, shutil, string, sys
 
-from legacy_lyxpreview2ppm import legacy_conversion_step1
+from legacy_lyxpreview2ppm import legacy_conversion_step1, extract_resolution
 
 from lyxpreview_tools import bibtex_commands, check_latex_log, copyfileobj, \
  error, filter_pages, find_exe, find_exe_or_terminate, \
@@ -159,7 +159,6 @@ def extract_metrics_info(dvipng_stdout):
 
 
 def fix_latex_file(latex_file, pdf_output):
-documentclass_re = re.compile((documentclass\[)(1[012]pt,?)(.+))
 def_re = re.compile(r(\\newcommandx|\\global\\long\\def)(\\[a-zA-Z]+))
 
 tmp = mkstemp()
@@ -167,14 

Re: test of math previews

[Enrico Forestieri]

On Sat, Jun 20, 2015 at 02:11:53AM +0200, Enrico Forestieri wrote:
 On Fri, Jun 19, 2015 at 04:50:43PM +0100, Guillaume M-M wrote:
 
  Also, lyx-preview-macros-2-lassert.lyx no longer lasserts but on opening the
  preview is not generated because \Coloneqq is not defined. This looks like
  the bug that you are referring to in your message where packages for symbols
  are not properly loaded. But, notice that the preview is correctly generated
  on a second time, when entering and leaving the math inset. So maybe it's
  easier to fix than it seemed. But indeed it's not a regression.
 
 Yes, this has always been the case, seemingly. However, now one can obtain
 a preview after entering and exiting a math inset, while previously the
 preview was simply never generated. So, this is a step forward. As regards
 the reason it occurs, I have still to investigate the matter.

I had a closer look and this is a quite strange issue. It seems that this
occurs only for a lonely math inset. Enable instant preview for math and
then try loading the attached examples. Only the first one will fail.

I think that this is due to the fact that the MacroData of the global
macros defined in the lib/symbols file is set only after the first
call to updateMacros() and when there is only a math inset, that method
has no chance of being called and thus we miss the requirements.
This seems to be confirmed by the attached patch for master which allows
to preview all of the attached documents on loading.

The patch is quite simple but it seems to cure a very particular case,
so I am not sure it is worth applying. What do others think?

-- 
Enrico
diff --git a/src/mathed/MathMacro.cpp b/src/mathed/MathMacro.cpp
index 24ebe77..657856e 100644
--- a/src/mathed/MathMacro.cpp
+++ b/src/mathed/MathMacro.cpp
@@ -707,8 +707,21 @@ size_t MathMacro::appetite() const
 
 void MathMacro::validate(LaTeXFeatures  features) const
 {
+   // Immediately after a document is loaded, in some cases the MacroData
+   // of the global macros defined in the lib/symbols file may still not
+   // be known to the macro machinery because it will be set only after
+   // the first call to updateMacros(). This is not a problem unless
+   // instant preview is on for math, in which case we will be missing
+   // the corresponding requirements.
+   // In this case, we get the required info from the global macro table.
if (!d-requires_.empty())
features.require(d-requires_);
+   else if (!d-macro_) {
+   // Update requires for known global macros.
+   MacroData const * data = MacroTable::globalMacros().get(name());
+   if (data)
+   features.require(data-requires());
+   }
 
if (name() == binom)
features.require(binom);
#LyX 2.1 created this file. For more info see http://www.lyx.org/
\lyxformat 474
\begin_document
\begin_header
\textclass article
\use_default_options false
\maintain_unincluded_children false
\language english
\language_package default
\inputencoding auto
\fontencoding global
\font_roman default
\font_sans default
\font_typewriter default
\font_math auto
\font_default_family default
\use_non_tex_fonts false
\font_sc false
\font_osf false
\font_sf_scale 100
\font_tt_scale 100
\graphics default
\default_output_format default
\output_sync 0
\bibtex_command default
\index_command default
\paperfontsize default
\use_hyperref false
\papersize default
\use_geometry false
\use_package amsmath 1
\use_package amssymb 1
\use_package cancel 1
\use_package esint 1
\use_package mathdots 0
\use_package mathtools 0
\use_package mhchem 1
\use_package stackrel 1
\use_package stmaryrd 1
\use_package undertilde 1
\cite_engine basic
\cite_engine_type default
\biblio_style plain
\use_bibtopic false
\use_indices false
\paperorientation portrait
\suppress_date false
\justification true
\use_refstyle 0
\index Index
\shortcut idx
\color #008000
\end_index
\secnumdepth 3
\tocdepth 3
\paragraph_separation indent
\paragraph_indentation default
\quotes_language english
\papercolumns 1
\papersides 1
\paperpagestyle default
\tracking_changes false
\output_changes false
\html_math_output 0
\html_css_as_file 0
\html_be_strict false
\end_header

\begin_body

\begin_layout Standard
\begin_inset Formula $\Coloneqq$
\end_inset

 
\begin_inset Note Note
status open

\begin_layout Plain Layout
This fails to preview on loading
\end_layout

\end_inset

 
\end_layout

\end_body
\end_document
#LyX 2.1 created this file. For more info see http://www.lyx.org/
\lyxformat 474
\begin_document
\begin_header
\textclass article
\use_default_options false
\maintain_unincluded_children false
\language english
\language_package default
\inputencoding auto
\fontencoding global
\font_roman default
\font_sans default
\font_typewriter default
\font_math auto
\font_default_family default
\use_non_tex_fonts false
\font_sc false
\font_osf false
\font_sf_scale 100
\font_tt_scale 

Re: test of math previews

[Guillaume M-M]

Le 23/06/2015 02:34, Guillaume M-M a écrit :

Another segfault in the stable branch involving previews (and master as
well). When Instant preview is on and one copies a formula, it randomly
crashes. In the attached, select the math inset and hold Ctrl+C until it
segfaults. Sometimes it happens on the first press. This is pretty
annoying because it's not a corner case at all. There is a very simple
math macro, but it does not crash with preview off.

The positive side is that I am getting a glimpse that having instant
previews that actually work is very pleasing :)


Stack trace:

Stable:

0x76360a48 in std::basic_stringwchar_t,
std::char_traitswchar_t, std::allocatorwchar_t
 ::basic_string(std::basic_stringwchar_t, std::char_traitswchar_t,
std::allocatorwchar_t  const) () from
/usr/lib/x86_64-linux-gnu/libstdc++.so.6

Master:

0x0044e2e2 in ?? ()


Does not take cd046f0e into account. Let me test again.

Re: test of math previews

[Guillaume M-M]

Another segfault in the stable branch involving previews (and master as 
well). When Instant preview is on and one copies a formula, it randomly 
crashes. In the attached, select the math inset and hold Ctrl+C until it 
segfaults. Sometimes it happens on the first press. This is pretty 
annoying because it's not a corner case at all. There is a very simple 
math macro, but it does not crash with preview off.


The positive side is that I am getting a glimpse that having instant 
previews that actually work is very pleasing :)



Stack trace:

Stable:

0x76360a48 in std::basic_stringwchar_t, 
std::char_traitswchar_t, std::allocatorwchar_t 
::basic_string(std::basic_stringwchar_t, std::char_traitswchar_t, 
std::allocatorwchar_t  const) () from 
/usr/lib/x86_64-linux-gnu/libstdc++.so.6


Master:

0x0044e2e2 in ?? ()


lyx-preview-sigsegv3.lyx
Description: application/lyx

Re: test of math previews

[Guillaume M-M]

Le 23/06/2015 02:36, Guillaume M-M a écrit :

Le 23/06/2015 02:34, Guillaume M-M a écrit :

Another segfault in the stable branch involving previews (and master as
well). When Instant preview is on and one copies a formula, it randomly
crashes. In the attached, select the math inset and hold Ctrl+C until it
segfaults. Sometimes it happens on the first press. This is pretty
annoying because it's not a corner case at all. There is a very simple
math macro, but it does not crash with preview off.

The positive side is that I am getting a glimpse that having instant
previews that actually work is very pleasing :)


Stack trace:

Stable:

0x76360a48 in std::basic_stringwchar_t,
std::char_traitswchar_t, std::allocatorwchar_t
 ::basic_string(std::basic_stringwchar_t, std::char_traitswchar_t,
std::allocatorwchar_t  const) () from
/usr/lib/x86_64-linux-gnu/libstdc++.so.6

Master:

0x0044e2e2 in ?? ()


Does not take cd046f0e into account. Let me test again.





Still there at cd046f0e (master). But here's the trace I get when it 
crashes right at the first Ctrl+C:



(  1) src/lyx: src/lyx() [0x9c14e8]
(  2) src/lyx: src/lyx() [0x9c257b]
(  3) src/lyx: src/lyx() [0x9c2fc3]
(  4) src/lyx: src/lyx() [0xa2c091]
(  5) src/lyx: src/lyx() [0x9bfe57]
(  6) src/lyx: src/lyx() [0x5ef49a]
(  7) /lib/x86_64-linux-gnu/libc.so.6: 
/lib/x86_64-linux-gnu/libc.so.6(+0x352f0) [0x7f15240122f0]
(  8) src/lyx: std::basic_stringwchar_t, std::char_traitswchar_t, 
std::allocatorwchar_t ::basic_string(std::basic_stringwchar_t, 
std::char_traitswchar_t, std::allocatorwchar_t  const)

(  9) src/lyx: src/lyx() [0x7fb7da]
( 10) src/lyx: src/lyx() [0x7f1f9e]
( 11) src/lyx: src/lyx() [0x819460]
( 12) src/lyx: src/lyx() [0x7e25e0]
( 13) src/lyx: src/lyx() [0x819474]
( 14) src/lyx: src/lyx() [0x79d36c]
( 15) src/lyx: src/lyx() [0x7acfd1]
( 16) src/lyx: src/lyx() [0x65e239]
( 17) src/lyx: src/lyx() [0x644f58]
( 18) src/lyx: src/lyx() [0x647093]
( 19) src/lyx: src/lyx() [0x4b07c4]
( 20) src/lyx: src/lyx() [0x54e75e]
( 21) src/lyx: src/lyx() [0x54f840]
( 22) src/lyx: src/lyx() [0x54f929]
( 23) src/lyx: src/lyx() [0x6b0cda]
( 24) src/lyx: src/lyx() [0x993d06]
( 25) src/lyx: src/lyx() [0x872032]
( 26) src/lyx: src/lyx() [0x53fd7e]
( 27) src/lyx: src/lyx() [0x9f260e]
( 28) src/lyx: src/lyx() [0xa0a048]
( 29) src/lyx: src/lyx() [0x9d603f]
( 30) src/lyx: src/lyx() [0x9c5eb5]
( 31) src/lyx: src/lyx() [0x5edd5a]
( 32) src/lyx: src/lyx() [0x9c83d3]
( 33) src/lyx: src/lyx() [0xa1dd25]
( 34) src/lyx: src/lyx() [0xa2073c]
( 35) /usr/lib/x86_64-linux-gnu/libQtGui.so.4: QWidget::event(QEvent*)
( 36) /usr/lib/x86_64-linux-gnu/libQtGui.so.4: QFrame::event(QEvent*)
( 37) /usr/lib/x86_64-linux-gnu/libQtGui.so.4: 
QAbstractScrollArea::event(QEvent*)

( 38) src/lyx: src/lyx() [0xa20f14]
( 39) /usr/lib/x86_64-linux-gnu/libQtGui.so.4: 
QApplicationPrivate::notify_helper(QObject*, QEvent*)
( 40) /usr/lib/x86_64-linux-gnu/libQtGui.so.4: 
QApplication::notify(QObject*, QEvent*)

( 41) src/lyx: src/lyx() [0x9d760a]
( 42) /usr/lib/x86_64-linux-gnu/libQtCore.so.4: 
QCoreApplication::notifyInternal(QObject*, QEvent*)
( 43) /usr/lib/x86_64-linux-gnu/libQtGui.so.4: 
/usr/lib/x86_64-linux-gnu/libQtGui.so.4(+0x26e023) [0x7f1525333023]
( 44) /usr/lib/x86_64-linux-gnu/libQtGui.so.4: 
/usr/lib/x86_64-linux-gnu/libQtGui.so.4(+0x26e4c2) [0x7f15253334c2]
( 45) /usr/lib/x86_64-linux-gnu/libQtGui.so.4: 
QApplication::x11ProcessEvent(_XEvent*)
( 46) /usr/lib/x86_64-linux-gnu/libQtGui.so.4: 
/usr/lib/x86_64-linux-gnu/libQtGui.so.4(+0x270ba2) [0x7f1525335ba2]
( 47) /lib/x86_64-linux-gnu/libglib-2.0.so.0: 
/lib/x86_64-linux-gnu/libglib-2.0.so.0(g_main_context_dispatch+0x24d) 
[0x7f1523b13c3d]
( 48) /lib/x86_64-linux-gnu/libglib-2.0.so.0: 
/lib/x86_64-linux-gnu/libglib-2.0.so.0(+0x49f20) [0x7f1523b13f20]
( 49) /lib/x86_64-linux-gnu/libglib-2.0.so.0: 
/lib/x86_64-linux-gnu/libglib-2.0.so.0(g_main_context_iteration+0x2c) 
[0x7f1523b13fcc]
( 50) /usr/lib/x86_64-linux-gnu/libQtCore.so.4: 
QEventDispatcherGlib::processEvents(QFlagsQEventLoop::ProcessEventsFlag)
( 51) /usr/lib/x86_64-linux-gnu/libQtGui.so.4: 
/usr/lib/x86_64-linux-gnu/libQtGui.so.4(+0x270c66) [0x7f1525335c66]
( 52) /usr/lib/x86_64-linux-gnu/libQtCore.so.4: 
QEventLoop::processEvents(QFlagsQEventLoop::ProcessEventsFlag)
( 53) /usr/lib/x86_64-linux-gnu/libQtCore.so.4: 
QEventLoop::exec(QFlagsQEventLoop::ProcessEventsFlag)

( 54) /usr/lib/x86_64-linux-gnu/libQtCore.so.4: QCoreApplication::exec()
( 55) src/lyx: src/lyx() [0x9c33a9]
( 56) src/lyx: src/lyx() [0x5f7e85]
( 57) src/lyx: src/lyx() [0x43a64e]
( 58) /lib/x86_64-linux-gnu/libc.so.6: 
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0) [0x7f1523ffda40]

( 59) src/lyx: src/lyx() [0x43a539]

Re: test of math previews

[Guillaume M-M]

Three bugs in the current stable (93c0512b). The third one has an easy 
fix (and also a very trivial partial fix if you want to move forward 
with the release). The first two could be postponed after the release, 
or maybe they are trivial as well to Enrico. Found the first two while 
testing my solution to the third one.


1) SIGSEGV with previews (no math macros involved). I believe this is 
the one I mentioned in a previous message. See the attached 
lyx-preview-sigsegv.lyx for instructions.



2) Closing a file whose previews are being generated results in an error 
dialog "Could not remove the temporary directory 
/tmp/lyx_tmpdir.LXSnBrW17125" and the following output on the terminal:


Warning: Could not remove temporary directory

Could not remove the temporary directory 
/tmp/lyx_tmpdir.LXSnBrW17125/lyx_tmpbuf0
Warning: Warning in extract_resolution! Unable to open 
"lyxpreviewQ17125_legacy.log"

Warning: ,IOError(2, 'No such file or directory')
Warning: check_latex_log: Unable to open "lyxpreviewQ17125_legacy.log"
Warning: ,IOError(2, 'No such file or directory')
Warning: Warning in legacy_extract_metrics_info! Unable to open 
"lyxpreviewQ17125_legacy.log"

Warning: ,IOError(2, 'No such file or directory')
Error: Failed to extract metrics info from lyxpreviewQ17125_legacy.log

So there's some additional cleanup to do now when closing a file.


3) Previews have the incorrect size with layouts that offer font sizes 
other than 1[0-2]pt (lyx-preview-fontsize.lyx). The legacy method offers 
a function that retrieves the actual font size and calculates the actual 
dpi. Attached is a patch (extract_resolution.diff) that imports this 
function in the new script.


(It's possible to just replace:
documentclass_re = re.compile("(documentclass\[)(1[012]pt,?)(.+)")
with:
documentclass_re = re.compile("(documentclass\[)([0-9]+pt,?)(.+)")
but this does not take into account custom classes, options, preambles, 
etc. If this path is retained, here's the result of $grep FontSize 
*.layout:


achemso.layout: FontSize10|11|12
acmsiggraph.layout:  FontSize   9|10|11|12
agutex.layout:  FontSize10|11|12
amsart.layout:  FontSize   8|9|10|11|12
amsbook.layout: FontSize   8|9|10|11|12
apa6.layout:FontSize  10|11|12
apa.layout: FontSize  6|8|10|12
broadway.layout:FontSize12
dtk.layout: FontSize  default # only 10pt in fact of A5
elsarticle.layout:  FontSize10|11|12
elsart.layout:#  FontSize "default"   # controlled by class
entcs.layout:   FontSize   11
extarticle.layout:  FontSize  8|9|10|11|12|14|17|20
extbook.layout: FontSize  8|9|10|11|12|14|17|20
extletter.layout:   FontSize  8|9|10|11|12|14|17|20
extreport.layout:   FontSize  8|9|10|11|12|14|17|20
foils.layout:   FontSize  17|20|25|30
hollywood.layout:   FontSize  12
IEEEtran-CompSoc.layout:  FontSize  12
IEEEtran.layout:  FontSize  9|10|11|12
iopart.layout:  FontSize10|12
jasatex.layout: FontSize10|11|12
latex8.layout:   FontSize 10
ltugboat.layout:FontSize  default # only 11pt
memoir.layout:  FontSize 9|10|11|12|14|17
powerdot.layout:  FontSize 
size=8|size=9|size=10|size=11|size=12|size=14|size=17|size=20

sciposter.layout:   FontSize  14|17|20|25|30|36
seminar.layout: FontSize8|9|10|11|12|14|17
siamltex.layout:FontSize   8|9|10|11|12
sigplanconf.layout:  FontSize   9|10|11
slides.layout:  FontSize   |
svmono.layout:  FontSize10
svmult.layout:  FontSize""

In particular, Powerdot has a special syntax, but the default font size 
is too big anyways (magnification=2000) so it's better to compute the 
font size from the log file, and this is precisely what 
extract_resolution already does.)


lyx-preview-sigsegv.lyx
Description: application/lyx


lyx-preview-fontsize.lyx
Description: application/lyx
diff --git a/lib/scripts/lyxpreview2bitmap.py b/lib/scripts/lyxpreview2bitmap.py
index 9775b0e..f9ee3b3 100755
--- a/lib/scripts/lyxpreview2bitmap.py
+++ b/lib/scripts/lyxpreview2bitmap.py
@@ -77,7 +77,7 @@
 
 import getopt, glob, os, re, shutil, string, sys
 
-from legacy_lyxpreview2ppm import legacy_conversion_step1
+from legacy_lyxpreview2ppm import legacy_conversion_step1, extract_resolution
 
 from lyxpreview_tools import bibtex_commands, check_latex_log, copyfileobj, \
  error, filter_pages, find_exe, find_exe_or_terminate, \
@@ -159,7 +159,6 @@ def extract_metrics_info(dvipng_stdout):
 
 
 def fix_latex_file(latex_file, pdf_output):
-documentclass_re = re.compile("(documentclass\[)(1[012]pt,?)(.+)")
 def_re = re.compile(r"(\\newcommandx|\\global\\long\\def)(\\[a-zA-Z]+)")
 
 tmp = mkstemp()
@@ -167,14 +166,9 @@ def fix_latex_file(latex_file, pdf_output):
 

Re: test of math previews

[Enrico Forestieri]

On Sat, Jun 20, 2015 at 02:11:53AM +0200, Enrico Forestieri wrote:
> On Fri, Jun 19, 2015 at 04:50:43PM +0100, Guillaume M-M wrote:
> 
> > Also, lyx-preview-macros-2-lassert.lyx no longer lasserts but on opening the
> > preview is not generated because \Coloneqq is not defined. This looks like
> > the bug that you are referring to in your message where packages for symbols
> > are not properly loaded. But, notice that the preview is correctly generated
> > on a second time, when entering and leaving the math inset. So maybe it's
> > easier to fix than it seemed. But indeed it's not a regression.
> 
> Yes, this has always been the case, seemingly. However, now one can obtain
> a preview after entering and exiting a math inset, while previously the
> preview was simply never generated. So, this is a step forward. As regards
> the reason it occurs, I have still to investigate the matter.

I had a closer look and this is a quite strange issue. It seems that this
occurs only for a lonely math inset. Enable instant preview for math and
then try loading the attached examples. Only the first one will fail.

I think that this is due to the fact that the MacroData of the global
macros defined in the lib/symbols file is set only after the first
call to updateMacros() and when there is only a math inset, that method
has no chance of being called and thus we miss the requirements.
This seems to be confirmed by the attached patch for master which allows
to preview all of the attached documents on loading.

The patch is quite simple but it seems to cure a very particular case,
so I am not sure it is worth applying. What do others think?

-- 
Enrico
diff --git a/src/mathed/MathMacro.cpp b/src/mathed/MathMacro.cpp
index 24ebe77..657856e 100644
--- a/src/mathed/MathMacro.cpp
+++ b/src/mathed/MathMacro.cpp
@@ -707,8 +707,21 @@ size_t MathMacro::appetite() const
 
 void MathMacro::validate(LaTeXFeatures & features) const
 {
+   // Immediately after a document is loaded, in some cases the MacroData
+   // of the global macros defined in the lib/symbols file may still not
+   // be known to the macro machinery because it will be set only after
+   // the first call to updateMacros(). This is not a problem unless
+   // instant preview is on for math, in which case we will be missing
+   // the corresponding requirements.
+   // In this case, we get the required info from the global macro table.
if (!d->requires_.empty())
features.require(d->requires_);
+   else if (!d->macro_) {
+   // Update requires for known global macros.
+   MacroData const * data = MacroTable::globalMacros().get(name());
+   if (data)
+   features.require(data->requires());
+   }
 
if (name() == "binom")
features.require("binom");
#LyX 2.1 created this file. For more info see http://www.lyx.org/
\lyxformat 474
\begin_document
\begin_header
\textclass article
\use_default_options false
\maintain_unincluded_children false
\language english
\language_package default
\inputencoding auto
\fontencoding global
\font_roman default
\font_sans default
\font_typewriter default
\font_math auto
\font_default_family default
\use_non_tex_fonts false
\font_sc false
\font_osf false
\font_sf_scale 100
\font_tt_scale 100
\graphics default
\default_output_format default
\output_sync 0
\bibtex_command default
\index_command default
\paperfontsize default
\use_hyperref false
\papersize default
\use_geometry false
\use_package amsmath 1
\use_package amssymb 1
\use_package cancel 1
\use_package esint 1
\use_package mathdots 0
\use_package mathtools 0
\use_package mhchem 1
\use_package stackrel 1
\use_package stmaryrd 1
\use_package undertilde 1
\cite_engine basic
\cite_engine_type default
\biblio_style plain
\use_bibtopic false
\use_indices false
\paperorientation portrait
\suppress_date false
\justification true
\use_refstyle 0
\index Index
\shortcut idx
\color #008000
\end_index
\secnumdepth 3
\tocdepth 3
\paragraph_separation indent
\paragraph_indentation default
\quotes_language english
\papercolumns 1
\papersides 1
\paperpagestyle default
\tracking_changes false
\output_changes false
\html_math_output 0
\html_css_as_file 0
\html_be_strict false
\end_header

\begin_body

\begin_layout Standard
\begin_inset Formula $\Coloneqq$
\end_inset

 
\begin_inset Note Note
status open

\begin_layout Plain Layout
This fails to preview on loading
\end_layout

\end_inset

 
\end_layout

\end_body
\end_document
#LyX 2.1 created this file. For more info see http://www.lyx.org/
\lyxformat 474
\begin_document
\begin_header
\textclass article
\use_default_options false
\maintain_unincluded_children false
\language english
\language_package default
\inputencoding auto
\fontencoding global
\font_roman default
\font_sans default
\font_typewriter default
\font_math auto
\font_default_family default
\use_non_tex_fonts false
\font_sc false
\font_osf false

Re: test of math previews

[Enrico Forestieri]

On Mon, Jun 22, 2015 at 11:32:17PM +0100, Guillaume M-M wrote:
> 
> 1) SIGSEGV with previews (no math macros involved). I believe this is the
> one I mentioned in a previous message. See the attached
> lyx-preview-sigsegv.lyx for instructions.

Thanks for catching this. I fixed this issue at cd046f0e. The other two
issues are not critical and I'll have a look later.

-- 
Enrico

Re: test of math previews

[Guillaume M-M]

Another segfault in the stable branch involving previews (and master as 
well). When Instant preview is on and one copies a formula, it randomly 
crashes. In the attached, select the math inset and hold Ctrl+C until it 
segfaults. Sometimes it happens on the first press. This is pretty 
annoying because it's not a corner case at all. There is a very simple 
math macro, but it does not crash with preview off.


The positive side is that I am getting a glimpse that having instant 
previews that actually work is very pleasing :)



Stack trace:

Stable:

0x76360a48 in std::basic_string::basic_string(std::basic_string const&) () from 
/usr/lib/x86_64-linux-gnu/libstdc++.so.6


Master:

0x0044e2e2 in ?? ()


lyx-preview-sigsegv3.lyx
Description: application/lyx

Re: test of math previews

[Guillaume M-M]

Le 23/06/2015 02:34, Guillaume M-M a écrit :

Another segfault in the stable branch involving previews (and master as
well). When Instant preview is on and one copies a formula, it randomly
crashes. In the attached, select the math inset and hold Ctrl+C until it
segfaults. Sometimes it happens on the first press. This is pretty
annoying because it's not a corner case at all. There is a very simple
math macro, but it does not crash with preview off.

The positive side is that I am getting a glimpse that having instant
previews that actually work is very pleasing :)


Stack trace:

Stable:

0x76360a48 in std::basic_string::basic_string(std::basic_string const&) () from
/usr/lib/x86_64-linux-gnu/libstdc++.so.6

Master:

0x0044e2e2 in ?? ()


Does not take cd046f0e into account. Let me test again.

Re: test of math previews

[Guillaume M-M]

Le 23/06/2015 02:36, Guillaume M-M a écrit :

Le 23/06/2015 02:34, Guillaume M-M a écrit :

Another segfault in the stable branch involving previews (and master as
well). When Instant preview is on and one copies a formula, it randomly
crashes. In the attached, select the math inset and hold Ctrl+C until it
segfaults. Sometimes it happens on the first press. This is pretty
annoying because it's not a corner case at all. There is a very simple
math macro, but it does not crash with preview off.

The positive side is that I am getting a glimpse that having instant
previews that actually work is very pleasing :)


Stack trace:

Stable:

0x76360a48 in std::basic_string::basic_string(std::basic_string const&) () from
/usr/lib/x86_64-linux-gnu/libstdc++.so.6

Master:

0x0044e2e2 in ?? ()


Does not take cd046f0e into account. Let me test again.





Still there at cd046f0e (master). But here's the trace I get when it 
crashes right at the first Ctrl+C:



(  1) src/lyx: src/lyx() [0x9c14e8]
(  2) src/lyx: src/lyx() [0x9c257b]
(  3) src/lyx: src/lyx() [0x9c2fc3]
(  4) src/lyx: src/lyx() [0xa2c091]
(  5) src/lyx: src/lyx() [0x9bfe57]
(  6) src/lyx: src/lyx() [0x5ef49a]
(  7) /lib/x86_64-linux-gnu/libc.so.6: 
/lib/x86_64-linux-gnu/libc.so.6(+0x352f0) [0x7f15240122f0]
(  8) src/lyx: std::basic_string::basic_string(std::basic_string const&)

(  9) src/lyx: src/lyx() [0x7fb7da]
( 10) src/lyx: src/lyx() [0x7f1f9e]
( 11) src/lyx: src/lyx() [0x819460]
( 12) src/lyx: src/lyx() [0x7e25e0]
( 13) src/lyx: src/lyx() [0x819474]
( 14) src/lyx: src/lyx() [0x79d36c]
( 15) src/lyx: src/lyx() [0x7acfd1]
( 16) src/lyx: src/lyx() [0x65e239]
( 17) src/lyx: src/lyx() [0x644f58]
( 18) src/lyx: src/lyx() [0x647093]
( 19) src/lyx: src/lyx() [0x4b07c4]
( 20) src/lyx: src/lyx() [0x54e75e]
( 21) src/lyx: src/lyx() [0x54f840]
( 22) src/lyx: src/lyx() [0x54f929]
( 23) src/lyx: src/lyx() [0x6b0cda]
( 24) src/lyx: src/lyx() [0x993d06]
( 25) src/lyx: src/lyx() [0x872032]
( 26) src/lyx: src/lyx() [0x53fd7e]
( 27) src/lyx: src/lyx() [0x9f260e]
( 28) src/lyx: src/lyx() [0xa0a048]
( 29) src/lyx: src/lyx() [0x9d603f]
( 30) src/lyx: src/lyx() [0x9c5eb5]
( 31) src/lyx: src/lyx() [0x5edd5a]
( 32) src/lyx: src/lyx() [0x9c83d3]
( 33) src/lyx: src/lyx() [0xa1dd25]
( 34) src/lyx: src/lyx() [0xa2073c]
( 35) /usr/lib/x86_64-linux-gnu/libQtGui.so.4: QWidget::event(QEvent*)
( 36) /usr/lib/x86_64-linux-gnu/libQtGui.so.4: QFrame::event(QEvent*)
( 37) /usr/lib/x86_64-linux-gnu/libQtGui.so.4: 
QAbstractScrollArea::event(QEvent*)

( 38) src/lyx: src/lyx() [0xa20f14]
( 39) /usr/lib/x86_64-linux-gnu/libQtGui.so.4: 
QApplicationPrivate::notify_helper(QObject*, QEvent*)
( 40) /usr/lib/x86_64-linux-gnu/libQtGui.so.4: 
QApplication::notify(QObject*, QEvent*)

( 41) src/lyx: src/lyx() [0x9d760a]
( 42) /usr/lib/x86_64-linux-gnu/libQtCore.so.4: 
QCoreApplication::notifyInternal(QObject*, QEvent*)
( 43) /usr/lib/x86_64-linux-gnu/libQtGui.so.4: 
/usr/lib/x86_64-linux-gnu/libQtGui.so.4(+0x26e023) [0x7f1525333023]
( 44) /usr/lib/x86_64-linux-gnu/libQtGui.so.4: 
/usr/lib/x86_64-linux-gnu/libQtGui.so.4(+0x26e4c2) [0x7f15253334c2]
( 45) /usr/lib/x86_64-linux-gnu/libQtGui.so.4: 
QApplication::x11ProcessEvent(_XEvent*)
( 46) /usr/lib/x86_64-linux-gnu/libQtGui.so.4: 
/usr/lib/x86_64-linux-gnu/libQtGui.so.4(+0x270ba2) [0x7f1525335ba2]
( 47) /lib/x86_64-linux-gnu/libglib-2.0.so.0: 
/lib/x86_64-linux-gnu/libglib-2.0.so.0(g_main_context_dispatch+0x24d) 
[0x7f1523b13c3d]
( 48) /lib/x86_64-linux-gnu/libglib-2.0.so.0: 
/lib/x86_64-linux-gnu/libglib-2.0.so.0(+0x49f20) [0x7f1523b13f20]
( 49) /lib/x86_64-linux-gnu/libglib-2.0.so.0: 
/lib/x86_64-linux-gnu/libglib-2.0.so.0(g_main_context_iteration+0x2c) 
[0x7f1523b13fcc]
( 50) /usr/lib/x86_64-linux-gnu/libQtCore.so.4: 
QEventDispatcherGlib::processEvents(QFlags)
( 51) /usr/lib/x86_64-linux-gnu/libQtGui.so.4: 
/usr/lib/x86_64-linux-gnu/libQtGui.so.4(+0x270c66) [0x7f1525335c66]
( 52) /usr/lib/x86_64-linux-gnu/libQtCore.so.4: 
QEventLoop::processEvents(QFlags)
( 53) /usr/lib/x86_64-linux-gnu/libQtCore.so.4: 
QEventLoop::exec(QFlags)

( 54) /usr/lib/x86_64-linux-gnu/libQtCore.so.4: QCoreApplication::exec()
( 55) src/lyx: src/lyx() [0x9c33a9]
( 56) src/lyx: src/lyx() [0x5f7e85]
( 57) src/lyx: src/lyx() [0x43a64e]
( 58) /lib/x86_64-linux-gnu/libc.so.6: 
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0) [0x7f1523ffda40]

( 59) src/lyx: src/lyx() [0x43a539]

Re: test of math previews

[Enrico Forestieri]

On Sat, Jun 20, 2015 at 09:13:40PM -0400, Richard Heck wrote:

 On 06/20/2015 08:45 PM, Scott Kostyshak wrote:
 On Sun, Jun 21, 2015 at 01:38:55AM +0100, Guillaume M-M wrote:
 
 Thank you for the convenience.
 
 I have no other issue to report for stable, Enrico wins :)
 LyX users across the globe win now :)
 
 Thanks to both of you!
 
 For stable, this is again your call, Enrico. I do not have any real sense
 for this code.
 But it's pretty clear that it has been tested much more extensively than
 most things
 that are committed to stable.

Yes, I also think that it was tested quite thoroughly. A big thank you
goes to Guillaume for his invaluable help in pinpointing special cases.

-- 
Enrico

Re: test of math previews

[Georg Baum]

Guillaume M-M wrote:

 Thank you very much, this is really quick now. Thanks also for the timer
 for generating previews. Also I only had 1 segmentation error, which I
 could not reproduce unfortunately. It happened after changing from
 Instant preview: On to No maths and zooming. But probably it's part
 of the bigger issues with math macros, and again I could not reproduce.

I'd guess so as well. I still want to work further on the math macro 
crashes. Unfortunately my time is limited as well, so this will still take 
some time.


Georg

Re: test of math previews

[Enrico Forestieri]

On Sat, Jun 20, 2015 at 09:13:40PM -0400, Richard Heck wrote:

> On 06/20/2015 08:45 PM, Scott Kostyshak wrote:
> >On Sun, Jun 21, 2015 at 01:38:55AM +0100, Guillaume M-M wrote:
> >
> >>Thank you for the convenience.
> >>
> >>I have no other issue to report for stable, Enrico wins :)
> >LyX users across the globe win now :)
> >
> >Thanks to both of you!
> 
> For stable, this is again your call, Enrico. I do not have any real sense
> for this code.
> But it's pretty clear that it has been tested much more extensively than
> most things
> that are committed to stable.

Yes, I also think that it was tested quite thoroughly. A big thank you
goes to Guillaume for his invaluable help in pinpointing special cases.

-- 
Enrico

Re: test of math previews

[Georg Baum]

Guillaume M-M wrote:

> Thank you very much, this is really quick now. Thanks also for the timer
> for generating previews. Also I only had 1 segmentation error, which I
> could not reproduce unfortunately. It happened after changing from
> "Instant preview: On" to "No maths" and zooming. But probably it's part
> of the bigger issues with math macros, and again I could not reproduce.

I'd guess so as well. I still want to work further on the math macro 
crashes. Unfortunately my time is limited as well, so this will still take 
some time.


Georg

Re: test of math previews

[Guillaume M-M]

Le 20/06/2015 01:11, Enrico Forestieri a écrit :

On Fri, Jun 19, 2015 at 04:50:43PM +0100, Guillaume M-M wrote:


Le 07/06/2015 12:49, Enrico Forestieri a écrit :

On Sat, Jun 06, 2015 at 08:07:12PM +0100, Guillaume M-M wrote:


lyx-preview-macros2-failure.lyx shows two cases where a macro is forgotten

from the list. The third inset works and is meant as a control. All 3 work

without the patch.


Here I was forgetting to also scan nested insets.


lyx-preview-macros2-lassert.lyx triggers an assertion when preview is
activated. Does not assert without the patch.


Here I did not account for the fact that also the macros defined in the
symbols file are spotted. However, they do not have a valid DataMacro
and this was causing the assert.

Note that there is another bug here, but it is not a regression as it
seems to be like that since ever (I checked with lyx versions since 1.3).
The bug is that when using a symbol defined in the symbols file and this
symbol requires a certain package, this requirement is not taken into
account when generating the tex file for preview and thus latex fails.


Notice that my thourough test is basically loading my current paper with
preview on.


I think that your paper is a very valid test case for macros ;)

The attached patch solves all of the above problems (and also the segfault
you report separately) for me. I am also attaching the patch for #9354
accordingly updated. Please test both and report the issues you find.
If your paper pass the tests, I think they are good for stable ;)



Hello again,


Hi Guillaume,


I tested the latest stable that seems to include the patches. As I already
told Enrico, sorry for the delay as I did not have much free time until now.

Thank you very much, this is really quick now. Thanks also for the timer for
generating previews. Also I only had 1 segmentation error, which I could not
reproduce unfortunately. It happened after changing from Instant preview:
On to No maths and zooming. But probably it's part of the bigger issues
with math macros, and again I could not reproduce.


Yes, I also think this is not specifically related to previews.


Here are two examples that still fail.

lyx-bug-undefined.lyx: the macro used in the argument of another macro is
not defined. I had many bugs of this style, so let's hope that this one is
representative, otherwise I'll come back with more errors.


Yes, I think it is. I was not taking into account that macros could also
occur in the arguments of other macros. Should be fixed in master now.


lyx-bug-renewcommandx4.lyx: here we have again an error where renewcommandx
is used while the macro is not defined beforehand. If it is really too error
prone to keep track of what has already been defined then it is easy to work
around by using \definecommandx:
   \def\definecommandx#1{\providecommand#1{}\renewcommandx#1}
This seems to work at least for the way newcommandx is used in lyx previews
(but maybe you have other reasons for distinguishing the cases newcommandx
and renewcommandx that I don't understand).


This was due to a stupid typo and should also be fixed in master.
As regards your proposal, I don't like defining commands only to immediately
redefining them. And the fix was quite straightforward.


Good. We are down to 14 failing previews. (examples at the end)




Lastly I see that you prevent loading the microtype package by redefining
\usepackage as you described earlier in the thread. While your redefinition
of \usepackage seems to be very careful, I still think that passing the
option draft to microtype is more reliable in case the user actually uses
macros from microtype (in addition there's no need to test whether the user
loads microtype).


I am concerned with the fact that in dvi mode the microtype package
considerably slows down the processing and prefer to not allow it.


Yes, and the draft option disables the microtype features completely 
according to the manual. That's also why I mentioned it. But it's your call.





I don't want to be too much of a PITA so here's a patch to
lyxpreview2bitmap.py that integrates my above two suggestions and solves the
above two problems. (The bug in lyx-bug-undefined.lyx has to be addressed
separately.) Parsing a file using regexps is always delicate so I think it's
good that with my patch there are fewer of these. I do not think that I
missed any subtlety in the existing code, but in any case you don't need to
accept it as is.


In this particular case the regexp was simply wrong.


Also, lyx-preview-macros-2-lassert.lyx no longer lasserts but on opening the
preview is not generated because \Coloneqq is not defined. This looks like
the bug that you are referring to in your message where packages for symbols
are not properly loaded. But, notice that the preview is correctly generated
on a second time, when entering and leaving the math inset. So maybe it's
easier to fix than it seemed. But indeed it's not a regression.


Yes, this has always been 

Re: test of math previews

[Enrico Forestieri]

On Sat, Jun 20, 2015 at 08:14:03AM +0100, Guillaume M-M wrote:
 Le 20/06/2015 01:11, Enrico Forestieri a écrit :
 On Fri, Jun 19, 2015 at 04:50:43PM +0100, Guillaume M-M wrote:
 
 Lastly I see that you prevent loading the microtype package by redefining
 \usepackage as you described earlier in the thread. While your redefinition
 of \usepackage seems to be very careful, I still think that passing the
 option draft to microtype is more reliable in case the user actually uses
 macros from microtype (in addition there's no need to test whether the user
 loads microtype).
 
 I am concerned with the fact that in dvi mode the microtype package
 considerably slows down the processing and prefer to not allow it.
 
 Yes, and the draft option disables the microtype features completely
 according to the manual. That's also why I mentioned it. But it's your call.

In this case I will reconsider. Please, let me perform some checks first.

 Please, try again with master and report back. The required changes to
 address these glitches are straightforward and maybe Richard will allow
 backporting them to stable.
 
 
 I simply applied your patch 98a5072a to stable directly (quicker to
 recompile).

You were missing the amendment at b610c13f that solves the issue with
lyx-bug-undefined5.lyx.

 lyx-bug-undefined3.lyx: macros inside nested macro definitions (!) are not
 taken into account.

I knew you would have been able to spot another corner case :)
This should also be solved at cabc7c4b.

 lyx-bug-undefined4.lyx: a combination of two bugs I think. In particular,
 our friend \renewcommandx again. More detailed comments in the files.

Here it was going as follows. The script detected a postscript special
in the second preview snippet and thus decided to use the legacy route
through dvips for obtaining the preview instead of directly using dvipng.
In order to do that, the affected snippets are extracted from the original
latex file. Now, in the original file the second snippet was correctly
using \renewcommandx because the macro was already defined in the first
snippet, but now it was incorrect, of course. Also solved at cabc7c4b.

 lyx-bug-undefined5.lyx: the arguments are only scanned on the first
 occurrence of a macro.

This had already been taken into account at b610c13f.

For your convenience I am attaching the overall set of patches for
the stable branch. Please, let me know if you are able to find some
other remaining glitches :)

-- 
Enrico
diff --git a/lib/scripts/lyxpreview2bitmap.py b/lib/scripts/lyxpreview2bitmap.py
index a7d7623..c2b94d4 100755
--- a/lib/scripts/lyxpreview2bitmap.py
+++ b/lib/scripts/lyxpreview2bitmap.py
@@ -160,7 +160,7 @@ def extract_metrics_info(dvipng_stdout):
 
 def fix_latex_file(latex_file, pdf_output):
 documentclass_re = re.compile((documentclass\[)(1[012]pt,?)(.+))
-def_re = re.compile(r(\\newcommandx|\\global\\long\\def)(\\[a-zA-Z])(.+))
+def_re = 
re.compile(r(\\newcommandx|\\global\\long\\def)(\\[a-zA-Z]+)(.+))
 usepackage_re = re.compile(usepackage)
 userpreamble_re = re.compile(User specified LaTeX commands)
 enduserpreamble_re = re.compile(makeatother)
diff --git a/lib/scripts/lyxpreview_tools.py b/lib/scripts/lyxpreview_tools.py
index 40ac711..7db524d 100644
--- a/lib/scripts/lyxpreview_tools.py
+++ b/lib/scripts/lyxpreview_tools.py
@@ -272,11 +272,13 @@ def write_metrics_info(metrics_info, metrics_file):
 # Reads a .tex files and create an identical file but only with
 # pages whose index is in pages_to_keep
 def filter_pages(source_path, destination_path, pages_to_keep):
+def_re = 
re.compile(r(\\newcommandx|\\renewcommandx|\\global\\long\\def)(\\[a-zA-Z]+)(.+))
 source_file = open(source_path, r)
 destination_file = open(destination_path, w)
 
 page_index = 0
 skip_page = False
+macros = []
 for line in source_file:
 # We found a new page
 if line.startswith(\\begin{preview}):
@@ -285,6 +287,14 @@ def filter_pages(source_path, destination_path, 
pages_to_keep):
 skip_page = page_index not in pages_to_keep
 
 if not skip_page:
+match = def_re.match(line)
+if match != None:
+definecmd = match.group(1)
+macroname = match.group(2)
+if not macroname in macros:
+macros.append(macroname)
+if definecmd == \\renewcommandx:
+line = line.replace(definecmd, \\newcommandx)
 destination_file.write(line)
 
 # End of a page, we reset the skip_page bool
diff --git a/src/mathed/InsetMathHull.cpp b/src/mathed/InsetMathHull.cpp
index 69643bb..0f62732 100644
--- a/src/mathed/InsetMathHull.cpp
+++ b/src/mathed/InsetMathHull.cpp
@@ -37,6 +37,7 @@
 #include LyXRC.h
 #include MacroTable.h
 #include MathMacro.h
+#include MathMacroTemplate.h
 #include output_xhtml.h
 #include Paragraph.h
 #include ParIterator.h
@@ -634,11 +635,15 @@ void 

Re: test of math previews

[Enrico Forestieri]

On Sat, Jun 20, 2015 at 04:58:35PM +0200, Enrico Forestieri wrote:
 On Sat, Jun 20, 2015 at 08:14:03AM +0100, Guillaume M-M wrote:
  Le 20/06/2015 01:11, Enrico Forestieri a écrit :
  On Fri, Jun 19, 2015 at 04:50:43PM +0100, Guillaume M-M wrote:
  
  Lastly I see that you prevent loading the microtype package by redefining
  \usepackage as you described earlier in the thread. While your 
  redefinition
  of \usepackage seems to be very careful, I still think that passing the
  option draft to microtype is more reliable in case the user actually uses
  macros from microtype (in addition there's no need to test whether the 
  user
  loads microtype).
  
  I am concerned with the fact that in dvi mode the microtype package
  considerably slows down the processing and prefer to not allow it.
  
  Yes, and the draft option disables the microtype features completely
  according to the manual. That's also why I mentioned it. But it's your call.
 
 In this case I will reconsider. Please, let me perform some checks first.

I am satisfied with the checks and committed this change at dd09a5ca.

-- 
Enrico

Re: test of math previews

[Guillaume M-M]

Le 20/06/2015 15:58, Enrico Forestieri a écrit :

On Sat, Jun 20, 2015 at 08:14:03AM +0100, Guillaume M-M wrote:

Le 20/06/2015 01:11, Enrico Forestieri a écrit :

On Fri, Jun 19, 2015 at 04:50:43PM +0100, Guillaume M-M wrote:


Lastly I see that you prevent loading the microtype package by redefining
\usepackage as you described earlier in the thread. While your redefinition
of \usepackage seems to be very careful, I still think that passing the
option draft to microtype is more reliable in case the user actually uses
macros from microtype (in addition there's no need to test whether the user
loads microtype).


I am concerned with the fact that in dvi mode the microtype package
considerably slows down the processing and prefer to not allow it.


Yes, and the draft option disables the microtype features completely
according to the manual. That's also why I mentioned it. But it's your call.


In this case I will reconsider. Please, let me perform some checks first.


Please, try again with master and report back. The required changes to
address these glitches are straightforward and maybe Richard will allow
backporting them to stable.



I simply applied your patch 98a5072a to stable directly (quicker to
recompile).


You were missing the amendment at b610c13f that solves the issue with
lyx-bug-undefined5.lyx.


Well yes unfortunately b610c13f did not exist when I pulled and 
recompiled. But I think I should also test master when I have more time.





lyx-bug-undefined3.lyx: macros inside nested macro definitions (!) are not
taken into account.


I knew you would have been able to spot another corner case :)
This should also be solved at cabc7c4b.


lyx-bug-undefined4.lyx: a combination of two bugs I think. In particular,
our friend \renewcommandx again. More detailed comments in the files.


Here it was going as follows. The script detected a postscript special
in the second preview snippet and thus decided to use the legacy route
through dvips for obtaining the preview instead of directly using dvipng.
In order to do that, the affected snippets are extracted from the original
latex file. Now, in the original file the second snippet was correctly
using \renewcommandx because the macro was already defined in the first
snippet, but now it was incorrect, of course. Also solved at cabc7c4b.


lyx-bug-undefined5.lyx: the arguments are only scanned on the first
occurrence of a macro.


This had already been taken into account at b610c13f.

For your convenience I am attaching the overall set of patches for
the stable branch. Please, let me know if you are able to find some
other remaining glitches :)



Thank you for the convenience.

I have no other issue to report for stable, Enrico wins :)

Re: test of math previews

[Scott Kostyshak]

On Sun, Jun 21, 2015 at 01:38:55AM +0100, Guillaume M-M wrote:

 Thank you for the convenience.
 
 I have no other issue to report for stable, Enrico wins :)

LyX users across the globe win now :)

Thanks to both of you!

Scott

Re: test of math previews

[Richard Heck]

On 06/20/2015 08:45 PM, Scott Kostyshak wrote:

On Sun, Jun 21, 2015 at 01:38:55AM +0100, Guillaume M-M wrote:


Thank you for the convenience.

I have no other issue to report for stable, Enrico wins :)

LyX users across the globe win now :)

Thanks to both of you!


For stable, this is again your call, Enrico. I do not have any real 
sense for this code.
But it's pretty clear that it has been tested much more extensively than 
most things

that are committed to stable.

Richard

Re: test of math previews

[Guillaume M-M]

Le 20/06/2015 01:11, Enrico Forestieri a écrit :

On Fri, Jun 19, 2015 at 04:50:43PM +0100, Guillaume M-M wrote:


Le 07/06/2015 12:49, Enrico Forestieri a écrit :

On Sat, Jun 06, 2015 at 08:07:12PM +0100, Guillaume M-M wrote:


lyx-preview-macros2-failure.lyx shows two cases where a macro is forgotten

>from the list. The third inset works and is meant as a control. All 3 work

without the patch.


Here I was forgetting to also scan nested insets.


lyx-preview-macros2-lassert.lyx triggers an assertion when preview is
activated. Does not assert without the patch.


Here I did not account for the fact that also the macros defined in the
symbols file are spotted. However, they do not have a valid DataMacro
and this was causing the assert.

Note that there is another bug here, but it is not a regression as it
seems to be like that since ever (I checked with lyx versions since 1.3).
The bug is that when using a symbol defined in the symbols file and this
symbol requires a certain package, this requirement is not taken into
account when generating the tex file for preview and thus latex fails.


Notice that my "thourough test" is basically loading my current paper with
preview on.


I think that your paper is a very valid test case for macros ;)

The attached patch solves all of the above problems (and also the segfault
you report separately) for me. I am also attaching the patch for #9354
accordingly updated. Please test both and report the issues you find.
If your paper pass the tests, I think they are good for stable ;)



Hello again,


Hi Guillaume,


I tested the latest stable that seems to include the patches. As I already
told Enrico, sorry for the delay as I did not have much free time until now.

Thank you very much, this is really quick now. Thanks also for the timer for
generating previews. Also I only had 1 segmentation error, which I could not
reproduce unfortunately. It happened after changing from "Instant preview:
On" to "No maths" and zooming. But probably it's part of the bigger issues
with math macros, and again I could not reproduce.


Yes, I also think this is not specifically related to previews.


Here are two examples that still fail.

lyx-bug-undefined.lyx: the macro used in the argument of another macro is
not defined. I had many bugs of this style, so let's hope that this one is
representative, otherwise I'll come back with more errors.


Yes, I think it is. I was not taking into account that macros could also
occur in the arguments of other macros. Should be fixed in master now.


lyx-bug-renewcommandx4.lyx: here we have again an error where renewcommandx
is used while the macro is not defined beforehand. If it is really too error
prone to keep track of what has already been defined then it is easy to work
around by using \definecommandx:
   \def\definecommandx#1{\providecommand#1{}\renewcommandx#1}
This seems to work at least for the way newcommandx is used in lyx previews
(but maybe you have other reasons for distinguishing the cases newcommandx
and renewcommandx that I don't understand).


This was due to a stupid typo and should also be fixed in master.
As regards your proposal, I don't like defining commands only to immediately
redefining them. And the fix was quite straightforward.


Good. We are down to 14 failing previews. (examples at the end)




Lastly I see that you prevent loading the microtype package by redefining
\usepackage as you described earlier in the thread. While your redefinition
of \usepackage seems to be very careful, I still think that passing the
option draft to microtype is more reliable in case the user actually uses
macros from microtype (in addition there's no need to test whether the user
loads microtype).


I am concerned with the fact that in dvi mode the microtype package
considerably slows down the processing and prefer to not allow it.


Yes, and the draft option disables the microtype features completely 
according to the manual. That's also why I mentioned it. But it's your call.





I don't want to be too much of a PITA so here's a patch to
lyxpreview2bitmap.py that integrates my above two suggestions and solves the
above two problems. (The bug in lyx-bug-undefined.lyx has to be addressed
separately.) Parsing a file using regexps is always delicate so I think it's
good that with my patch there are fewer of these. I do not think that I
missed any subtlety in the existing code, but in any case you don't need to
accept it as is.


In this particular case the regexp was simply wrong.


Also, lyx-preview-macros-2-lassert.lyx no longer lasserts but on opening the
preview is not generated because \Coloneqq is not defined. This looks like
the bug that you are referring to in your message where packages for symbols
are not properly loaded. But, notice that the preview is correctly generated
on a second time, when entering and leaving the math inset. So maybe it's
easier to fix than it seemed. But indeed it's not a regression.


Yes, this has 

Re: test of math previews

[Enrico Forestieri]

On Sat, Jun 20, 2015 at 08:14:03AM +0100, Guillaume M-M wrote:
> Le 20/06/2015 01:11, Enrico Forestieri a écrit :
> >On Fri, Jun 19, 2015 at 04:50:43PM +0100, Guillaume M-M wrote:
> >
> >>Lastly I see that you prevent loading the microtype package by redefining
> >>\usepackage as you described earlier in the thread. While your redefinition
> >>of \usepackage seems to be very careful, I still think that passing the
> >>option draft to microtype is more reliable in case the user actually uses
> >>macros from microtype (in addition there's no need to test whether the user
> >>loads microtype).
> >
> >I am concerned with the fact that in dvi mode the microtype package
> >considerably slows down the processing and prefer to not allow it.
> 
> Yes, and the draft option disables the microtype features completely
> according to the manual. That's also why I mentioned it. But it's your call.

In this case I will reconsider. Please, let me perform some checks first.

> >Please, try again with master and report back. The required changes to
> >address these glitches are straightforward and maybe Richard will allow
> >backporting them to stable.
> >
> 
> I simply applied your patch 98a5072a to stable directly (quicker to
> recompile).

You were missing the amendment at b610c13f that solves the issue with
lyx-bug-undefined5.lyx.

> lyx-bug-undefined3.lyx: macros inside nested macro definitions (!) are not
> taken into account.

I knew you would have been able to spot another corner case :)
This should also be solved at cabc7c4b.

> lyx-bug-undefined4.lyx: a combination of two bugs I think. In particular,
> our friend \renewcommandx again. More detailed comments in the files.

Here it was going as follows. The script detected a postscript special
in the second preview snippet and thus decided to use the legacy route
through dvips for obtaining the preview instead of directly using dvipng.
In order to do that, the affected snippets are extracted from the original
latex file. Now, in the original file the second snippet was correctly
using \renewcommandx because the macro was already defined in the first
snippet, but now it was incorrect, of course. Also solved at cabc7c4b.

> lyx-bug-undefined5.lyx: the arguments are only scanned on the first
> occurrence of a macro.

This had already been taken into account at b610c13f.

For your convenience I am attaching the overall set of patches for
the stable branch. Please, let me know if you are able to find some
other remaining glitches :)

-- 
Enrico
diff --git a/lib/scripts/lyxpreview2bitmap.py b/lib/scripts/lyxpreview2bitmap.py
index a7d7623..c2b94d4 100755
--- a/lib/scripts/lyxpreview2bitmap.py
+++ b/lib/scripts/lyxpreview2bitmap.py
@@ -160,7 +160,7 @@ def extract_metrics_info(dvipng_stdout):
 
 def fix_latex_file(latex_file, pdf_output):
 documentclass_re = re.compile("(documentclass\[)(1[012]pt,?)(.+)")
-def_re = re.compile(r"(\\newcommandx|\\global\\long\\def)(\\[a-zA-Z])(.+)")
+def_re = 
re.compile(r"(\\newcommandx|\\global\\long\\def)(\\[a-zA-Z]+)(.+)")
 usepackage_re = re.compile("usepackage")
 userpreamble_re = re.compile("User specified LaTeX commands")
 enduserpreamble_re = re.compile("makeatother")
diff --git a/lib/scripts/lyxpreview_tools.py b/lib/scripts/lyxpreview_tools.py
index 40ac711..7db524d 100644
--- a/lib/scripts/lyxpreview_tools.py
+++ b/lib/scripts/lyxpreview_tools.py
@@ -272,11 +272,13 @@ def write_metrics_info(metrics_info, metrics_file):
 # Reads a .tex files and create an identical file but only with
 # pages whose index is in pages_to_keep
 def filter_pages(source_path, destination_path, pages_to_keep):
+def_re = 
re.compile(r"(\\newcommandx|\\renewcommandx|\\global\\long\\def)(\\[a-zA-Z]+)(.+)")
 source_file = open(source_path, "r")
 destination_file = open(destination_path, "w")
 
 page_index = 0
 skip_page = False
+macros = []
 for line in source_file:
 # We found a new page
 if line.startswith("\\begin{preview}"):
@@ -285,6 +287,14 @@ def filter_pages(source_path, destination_path, 
pages_to_keep):
 skip_page = page_index not in pages_to_keep
 
 if not skip_page:
+match = def_re.match(line)
+if match != None:
+definecmd = match.group(1)
+macroname = match.group(2)
+if not macroname in macros:
+macros.append(macroname)
+if definecmd == "\\renewcommandx":
+line = line.replace(definecmd, "\\newcommandx")
 destination_file.write(line)
 
 # End of a page, we reset the skip_page bool
diff --git a/src/mathed/InsetMathHull.cpp b/src/mathed/InsetMathHull.cpp
index 69643bb..0f62732 100644
--- a/src/mathed/InsetMathHull.cpp
+++ b/src/mathed/InsetMathHull.cpp
@@ -37,6 +37,7 @@
 #include "LyXRC.h"
 #include "MacroTable.h"
 #include "MathMacro.h"
+#include "MathMacroTemplate.h"
 #include "output_xhtml.h"
 

Re: test of math previews

[Enrico Forestieri]

On Sat, Jun 20, 2015 at 04:58:35PM +0200, Enrico Forestieri wrote:
> On Sat, Jun 20, 2015 at 08:14:03AM +0100, Guillaume M-M wrote:
> > Le 20/06/2015 01:11, Enrico Forestieri a écrit :
> > >On Fri, Jun 19, 2015 at 04:50:43PM +0100, Guillaume M-M wrote:
> > >
> > >>Lastly I see that you prevent loading the microtype package by redefining
> > >>\usepackage as you described earlier in the thread. While your 
> > >>redefinition
> > >>of \usepackage seems to be very careful, I still think that passing the
> > >>option draft to microtype is more reliable in case the user actually uses
> > >>macros from microtype (in addition there's no need to test whether the 
> > >>user
> > >>loads microtype).
> > >
> > >I am concerned with the fact that in dvi mode the microtype package
> > >considerably slows down the processing and prefer to not allow it.
> > 
> > Yes, and the draft option disables the microtype features completely
> > according to the manual. That's also why I mentioned it. But it's your call.
> 
> In this case I will reconsider. Please, let me perform some checks first.

I am satisfied with the checks and committed this change at dd09a5ca.

-- 
Enrico

Re: test of math previews

[Guillaume M-M]

Le 20/06/2015 15:58, Enrico Forestieri a écrit :

On Sat, Jun 20, 2015 at 08:14:03AM +0100, Guillaume M-M wrote:

Le 20/06/2015 01:11, Enrico Forestieri a écrit :

On Fri, Jun 19, 2015 at 04:50:43PM +0100, Guillaume M-M wrote:


Lastly I see that you prevent loading the microtype package by redefining
\usepackage as you described earlier in the thread. While your redefinition
of \usepackage seems to be very careful, I still think that passing the
option draft to microtype is more reliable in case the user actually uses
macros from microtype (in addition there's no need to test whether the user
loads microtype).


I am concerned with the fact that in dvi mode the microtype package
considerably slows down the processing and prefer to not allow it.


Yes, and the draft option disables the microtype features completely
according to the manual. That's also why I mentioned it. But it's your call.


In this case I will reconsider. Please, let me perform some checks first.


Please, try again with master and report back. The required changes to
address these glitches are straightforward and maybe Richard will allow
backporting them to stable.



I simply applied your patch 98a5072a to stable directly (quicker to
recompile).


You were missing the amendment at b610c13f that solves the issue with
lyx-bug-undefined5.lyx.


Well yes unfortunately b610c13f did not exist when I pulled and 
recompiled. But I think I should also test master when I have more time.





lyx-bug-undefined3.lyx: macros inside nested macro definitions (!) are not
taken into account.


I knew you would have been able to spot another corner case :)
This should also be solved at cabc7c4b.


lyx-bug-undefined4.lyx: a combination of two bugs I think. In particular,
our friend \renewcommandx again. More detailed comments in the files.


Here it was going as follows. The script detected a postscript special
in the second preview snippet and thus decided to use the legacy route
through dvips for obtaining the preview instead of directly using dvipng.
In order to do that, the affected snippets are extracted from the original
latex file. Now, in the original file the second snippet was correctly
using \renewcommandx because the macro was already defined in the first
snippet, but now it was incorrect, of course. Also solved at cabc7c4b.


lyx-bug-undefined5.lyx: the arguments are only scanned on the first
occurrence of a macro.


This had already been taken into account at b610c13f.

For your convenience I am attaching the overall set of patches for
the stable branch. Please, let me know if you are able to find some
other remaining glitches :)



Thank you for the convenience.

I have no other issue to report for stable, Enrico wins :)

Re: test of math previews

[Scott Kostyshak]

On Sun, Jun 21, 2015 at 01:38:55AM +0100, Guillaume M-M wrote:

> Thank you for the convenience.
> 
> I have no other issue to report for stable, Enrico wins :)

LyX users across the globe win now :)

Thanks to both of you!

Scott

Re: test of math previews

[Richard Heck]

On 06/20/2015 08:45 PM, Scott Kostyshak wrote:

On Sun, Jun 21, 2015 at 01:38:55AM +0100, Guillaume M-M wrote:


Thank you for the convenience.

I have no other issue to report for stable, Enrico wins :)

LyX users across the globe win now :)

Thanks to both of you!


For stable, this is again your call, Enrico. I do not have any real 
sense for this code.
But it's pretty clear that it has been tested much more extensively than 
most things

that are committed to stable.

Richard

Re: test of math previews

[Enrico Forestieri]

On Fri, Jun 19, 2015 at 04:50:43PM +0100, Guillaume M-M wrote:

 Le 07/06/2015 12:49, Enrico Forestieri a écrit :
 On Sat, Jun 06, 2015 at 08:07:12PM +0100, Guillaume M-M wrote:
 
 lyx-preview-macros2-failure.lyx shows two cases where a macro is forgotten
 from the list. The third inset works and is meant as a control. All 3 work
 without the patch.
 
 Here I was forgetting to also scan nested insets.
 
 lyx-preview-macros2-lassert.lyx triggers an assertion when preview is
 activated. Does not assert without the patch.
 
 Here I did not account for the fact that also the macros defined in the
 symbols file are spotted. However, they do not have a valid DataMacro
 and this was causing the assert.
 
 Note that there is another bug here, but it is not a regression as it
 seems to be like that since ever (I checked with lyx versions since 1.3).
 The bug is that when using a symbol defined in the symbols file and this
 symbol requires a certain package, this requirement is not taken into
 account when generating the tex file for preview and thus latex fails.
 
 Notice that my thourough test is basically loading my current paper with
 preview on.
 
 I think that your paper is a very valid test case for macros ;)
 
 The attached patch solves all of the above problems (and also the segfault
 you report separately) for me. I am also attaching the patch for #9354
 accordingly updated. Please test both and report the issues you find.
 If your paper pass the tests, I think they are good for stable ;)
 
 
 Hello again,

Hi Guillaume,

 I tested the latest stable that seems to include the patches. As I already
 told Enrico, sorry for the delay as I did not have much free time until now.
 
 Thank you very much, this is really quick now. Thanks also for the timer for
 generating previews. Also I only had 1 segmentation error, which I could not
 reproduce unfortunately. It happened after changing from Instant preview:
 On to No maths and zooming. But probably it's part of the bigger issues
 with math macros, and again I could not reproduce.

Yes, I also think this is not specifically related to previews.

 Here are two examples that still fail.
 
 lyx-bug-undefined.lyx: the macro used in the argument of another macro is
 not defined. I had many bugs of this style, so let's hope that this one is
 representative, otherwise I'll come back with more errors.

Yes, I think it is. I was not taking into account that macros could also
occur in the arguments of other macros. Should be fixed in master now.

 lyx-bug-renewcommandx4.lyx: here we have again an error where renewcommandx
 is used while the macro is not defined beforehand. If it is really too error
 prone to keep track of what has already been defined then it is easy to work
 around by using \definecommandx:
   \def\definecommandx#1{\providecommand#1{}\renewcommandx#1}
 This seems to work at least for the way newcommandx is used in lyx previews
 (but maybe you have other reasons for distinguishing the cases newcommandx
 and renewcommandx that I don't understand).

This was due to a stupid typo and should also be fixed in master.
As regards your proposal, I don't like defining commands only to immediately
redefining them. And the fix was quite straightforward.

 Lastly I see that you prevent loading the microtype package by redefining
 \usepackage as you described earlier in the thread. While your redefinition
 of \usepackage seems to be very careful, I still think that passing the
 option draft to microtype is more reliable in case the user actually uses
 macros from microtype (in addition there's no need to test whether the user
 loads microtype).

I am concerned with the fact that in dvi mode the microtype package
considerably slows down the processing and prefer to not allow it.

 I don't want to be too much of a PITA so here's a patch to
 lyxpreview2bitmap.py that integrates my above two suggestions and solves the
 above two problems. (The bug in lyx-bug-undefined.lyx has to be addressed
 separately.) Parsing a file using regexps is always delicate so I think it's
 good that with my patch there are fewer of these. I do not think that I
 missed any subtlety in the existing code, but in any case you don't need to
 accept it as is.

In this particular case the regexp was simply wrong.

 Also, lyx-preview-macros-2-lassert.lyx no longer lasserts but on opening the
 preview is not generated because \Coloneqq is not defined. This looks like
 the bug that you are referring to in your message where packages for symbols
 are not properly loaded. But, notice that the preview is correctly generated
 on a second time, when entering and leaving the math inset. So maybe it's
 easier to fix than it seemed. But indeed it's not a regression.

Yes, this has always been the case, seemingly. However, now one can obtain
a preview after entering and exiting a math inset, while previously the
preview was simply never generated. So, this is a step forward. As regards
the reason it occurs, I 

Re: test of math previews

[Guillaume M-M]

Le 07/06/2015 12:49, Enrico Forestieri a écrit :

On Sat, Jun 06, 2015 at 08:07:12PM +0100, Guillaume M-M wrote:


lyx-preview-macros2-failure.lyx shows two cases where a macro is forgotten
from the list. The third inset works and is meant as a control. All 3 work
without the patch.


Here I was forgetting to also scan nested insets.


lyx-preview-macros2-lassert.lyx triggers an assertion when preview is
activated. Does not assert without the patch.


Here I did not account for the fact that also the macros defined in the
symbols file are spotted. However, they do not have a valid DataMacro
and this was causing the assert.

Note that there is another bug here, but it is not a regression as it
seems to be like that since ever (I checked with lyx versions since 1.3).
The bug is that when using a symbol defined in the symbols file and this
symbol requires a certain package, this requirement is not taken into
account when generating the tex file for preview and thus latex fails.


Notice that my thourough test is basically loading my current paper with
preview on.


I think that your paper is a very valid test case for macros ;)

The attached patch solves all of the above problems (and also the segfault
you report separately) for me. I am also attaching the patch for #9354
accordingly updated. Please test both and report the issues you find.
If your paper pass the tests, I think they are good for stable ;)



Hello again,


I tested the latest stable that seems to include the patches. As I 
already told Enrico, sorry for the delay as I did not have much free 
time until now.


Thank you very much, this is really quick now. Thanks also for the timer 
for generating previews. Also I only had 1 segmentation error, which I 
could not reproduce unfortunately. It happened after changing from 
Instant preview: On to No maths and zooming. But probably it's part 
of the bigger issues with math macros, and again I could not reproduce.


Here are two examples that still fail.

lyx-bug-undefined.lyx: the macro used in the argument of another macro 
is not defined. I had many bugs of this style, so let's hope that this 
one is representative, otherwise I'll come back with more errors.


lyx-bug-renewcommandx4.lyx: here we have again an error where 
renewcommandx is used while the macro is not defined beforehand. If it 
is really too error prone to keep track of what has already been defined 
then it is easy to work around by using \definecommandx:

  \def\definecommandx#1{\providecommand#1{}\renewcommandx#1}
This seems to work at least for the way newcommandx is used in lyx 
previews (but maybe you have other reasons for distinguishing the cases 
newcommandx and renewcommandx that I don't understand).


Lastly I see that you prevent loading the microtype package by 
redefining \usepackage as you described earlier in the thread. While 
your redefinition of \usepackage seems to be very careful, I still think 
that passing the option draft to microtype is more reliable in case the 
user actually uses macros from microtype (in addition there's no need to 
test whether the user loads microtype).


I don't want to be too much of a PITA so here's a patch to 
lyxpreview2bitmap.py that integrates my above two suggestions and solves 
the above two problems. (The bug in lyx-bug-undefined.lyx has to be 
addressed separately.) Parsing a file using regexps is always delicate 
so I think it's good that with my patch there are fewer of these. I do 
not think that I missed any subtlety in the existing code, but in any 
case you don't need to accept it as is.


Also, lyx-preview-macros-2-lassert.lyx no longer lasserts but on opening 
the preview is not generated because \Coloneqq is not defined. This 
looks like the bug that you are referring to in your message where 
packages for symbols are not properly loaded. But, notice that the 
preview is correctly generated on a second time, when entering and 
leaving the math inset. So maybe it's easier to fix than it seemed. But 
indeed it's not a regression.



Best regards
Guillaume



lyx-bug-undefined.lyx
Description: application/lyx


lyx-bug-renewcommandx4.lyx
Description: application/lyx
diff --git a/lib/scripts/lyxpreview2bitmap.py b/lib/scripts/lyxpreview2bitmap.py
index a7d7623..fe9bea1 100755
--- a/lib/scripts/lyxpreview2bitmap.py
+++ b/lib/scripts/lyxpreview2bitmap.py
@@ -159,65 +159,35 @@ def extract_metrics_info(dvipng_stdout):
 
 
 def fix_latex_file(latex_file, pdf_output):
-documentclass_re = re.compile((documentclass\[)(1[012]pt,?)(.+))
-def_re = re.compile(r(\\newcommandx|\\global\\long\\def)(\\[a-zA-Z])(.+))
-usepackage_re = re.compile(usepackage)
-userpreamble_re = re.compile(User specified LaTeX commands)
-enduserpreamble_re = re.compile(makeatother)
+documentclass_re = re.compile((documentclass))
+documentclass_pt_re = re.compile((documentclass\[)(1[012]pt,?)(.+))
+def_re = 

Re: test of math previews

[Guillaume M-M]

Le 07/06/2015 12:49, Enrico Forestieri a écrit :

On Sat, Jun 06, 2015 at 08:07:12PM +0100, Guillaume M-M wrote:


lyx-preview-macros2-failure.lyx shows two cases where a macro is forgotten
from the list. The third inset works and is meant as a control. All 3 work
without the patch.


Here I was forgetting to also scan nested insets.


lyx-preview-macros2-lassert.lyx triggers an assertion when preview is
activated. Does not assert without the patch.


Here I did not account for the fact that also the macros defined in the
symbols file are spotted. However, they do not have a valid DataMacro
and this was causing the assert.

Note that there is another bug here, but it is not a regression as it
seems to be like that since ever (I checked with lyx versions since 1.3).
The bug is that when using a symbol defined in the symbols file and this
symbol requires a certain package, this requirement is not taken into
account when generating the tex file for preview and thus latex fails.


Notice that my "thourough test" is basically loading my current paper with
preview on.


I think that your paper is a very valid test case for macros ;)

The attached patch solves all of the above problems (and also the segfault
you report separately) for me. I am also attaching the patch for #9354
accordingly updated. Please test both and report the issues you find.
If your paper pass the tests, I think they are good for stable ;)



Hello again,


I tested the latest stable that seems to include the patches. As I 
already told Enrico, sorry for the delay as I did not have much free 
time until now.


Thank you very much, this is really quick now. Thanks also for the timer 
for generating previews. Also I only had 1 segmentation error, which I 
could not reproduce unfortunately. It happened after changing from 
"Instant preview: On" to "No maths" and zooming. But probably it's part 
of the bigger issues with math macros, and again I could not reproduce.


Here are two examples that still fail.

lyx-bug-undefined.lyx: the macro used in the argument of another macro 
is not defined. I had many bugs of this style, so let's hope that this 
one is representative, otherwise I'll come back with more errors.


lyx-bug-renewcommandx4.lyx: here we have again an error where 
renewcommandx is used while the macro is not defined beforehand. If it 
is really too error prone to keep track of what has already been defined 
then it is easy to work around by using \definecommandx:

  \def\definecommandx#1{\providecommand#1{}\renewcommandx#1}
This seems to work at least for the way newcommandx is used in lyx 
previews (but maybe you have other reasons for distinguishing the cases 
newcommandx and renewcommandx that I don't understand).


Lastly I see that you prevent loading the microtype package by 
redefining \usepackage as you described earlier in the thread. While 
your redefinition of \usepackage seems to be very careful, I still think 
that passing the option draft to microtype is more reliable in case the 
user actually uses macros from microtype (in addition there's no need to 
test whether the user loads microtype).


I don't want to be too much of a PITA so here's a patch to 
lyxpreview2bitmap.py that integrates my above two suggestions and solves 
the above two problems. (The bug in lyx-bug-undefined.lyx has to be 
addressed separately.) Parsing a file using regexps is always delicate 
so I think it's good that with my patch there are fewer of these. I do 
not think that I missed any subtlety in the existing code, but in any 
case you don't need to accept it as is.


Also, lyx-preview-macros-2-lassert.lyx no longer lasserts but on opening 
the preview is not generated because \Coloneqq is not defined. This 
looks like the bug that you are referring to in your message where 
packages for symbols are not properly loaded. But, notice that the 
preview is correctly generated on a second time, when entering and 
leaving the math inset. So maybe it's easier to fix than it seemed. But 
indeed it's not a regression.



Best regards
Guillaume



lyx-bug-undefined.lyx
Description: application/lyx


lyx-bug-renewcommandx4.lyx
Description: application/lyx
diff --git a/lib/scripts/lyxpreview2bitmap.py b/lib/scripts/lyxpreview2bitmap.py
index a7d7623..fe9bea1 100755
--- a/lib/scripts/lyxpreview2bitmap.py
+++ b/lib/scripts/lyxpreview2bitmap.py
@@ -159,65 +159,35 @@ def extract_metrics_info(dvipng_stdout):
 
 
 def fix_latex_file(latex_file, pdf_output):
-documentclass_re = re.compile("(documentclass\[)(1[012]pt,?)(.+)")
-def_re = re.compile(r"(\\newcommandx|\\global\\long\\def)(\\[a-zA-Z])(.+)")
-usepackage_re = re.compile("usepackage")
-userpreamble_re = re.compile("User specified LaTeX commands")
-enduserpreamble_re = re.compile("makeatother")
+documentclass_re = re.compile("(documentclass)")
+documentclass_pt_re = re.compile("(documentclass\[)(1[012]pt,?)(.+)")
+def_re = 

Re: test of math previews

[Enrico Forestieri]

On Fri, Jun 19, 2015 at 04:50:43PM +0100, Guillaume M-M wrote:

> Le 07/06/2015 12:49, Enrico Forestieri a écrit :
> >On Sat, Jun 06, 2015 at 08:07:12PM +0100, Guillaume M-M wrote:
> >>
> >>lyx-preview-macros2-failure.lyx shows two cases where a macro is forgotten
> >>from the list. The third inset works and is meant as a control. All 3 work
> >>without the patch.
> >
> >Here I was forgetting to also scan nested insets.
> >
> >>lyx-preview-macros2-lassert.lyx triggers an assertion when preview is
> >>activated. Does not assert without the patch.
> >
> >Here I did not account for the fact that also the macros defined in the
> >symbols file are spotted. However, they do not have a valid DataMacro
> >and this was causing the assert.
> >
> >Note that there is another bug here, but it is not a regression as it
> >seems to be like that since ever (I checked with lyx versions since 1.3).
> >The bug is that when using a symbol defined in the symbols file and this
> >symbol requires a certain package, this requirement is not taken into
> >account when generating the tex file for preview and thus latex fails.
> >
> >>Notice that my "thourough test" is basically loading my current paper with
> >>preview on.
> >
> >I think that your paper is a very valid test case for macros ;)
> >
> >The attached patch solves all of the above problems (and also the segfault
> >you report separately) for me. I am also attaching the patch for #9354
> >accordingly updated. Please test both and report the issues you find.
> >If your paper pass the tests, I think they are good for stable ;)
> >
> 
> Hello again,

Hi Guillaume,

> I tested the latest stable that seems to include the patches. As I already
> told Enrico, sorry for the delay as I did not have much free time until now.
> 
> Thank you very much, this is really quick now. Thanks also for the timer for
> generating previews. Also I only had 1 segmentation error, which I could not
> reproduce unfortunately. It happened after changing from "Instant preview:
> On" to "No maths" and zooming. But probably it's part of the bigger issues
> with math macros, and again I could not reproduce.

Yes, I also think this is not specifically related to previews.

> Here are two examples that still fail.
> 
> lyx-bug-undefined.lyx: the macro used in the argument of another macro is
> not defined. I had many bugs of this style, so let's hope that this one is
> representative, otherwise I'll come back with more errors.

Yes, I think it is. I was not taking into account that macros could also
occur in the arguments of other macros. Should be fixed in master now.

> lyx-bug-renewcommandx4.lyx: here we have again an error where renewcommandx
> is used while the macro is not defined beforehand. If it is really too error
> prone to keep track of what has already been defined then it is easy to work
> around by using \definecommandx:
>   \def\definecommandx#1{\providecommand#1{}\renewcommandx#1}
> This seems to work at least for the way newcommandx is used in lyx previews
> (but maybe you have other reasons for distinguishing the cases newcommandx
> and renewcommandx that I don't understand).

This was due to a stupid typo and should also be fixed in master.
As regards your proposal, I don't like defining commands only to immediately
redefining them. And the fix was quite straightforward.

> Lastly I see that you prevent loading the microtype package by redefining
> \usepackage as you described earlier in the thread. While your redefinition
> of \usepackage seems to be very careful, I still think that passing the
> option draft to microtype is more reliable in case the user actually uses
> macros from microtype (in addition there's no need to test whether the user
> loads microtype).

I am concerned with the fact that in dvi mode the microtype package
considerably slows down the processing and prefer to not allow it.

> I don't want to be too much of a PITA so here's a patch to
> lyxpreview2bitmap.py that integrates my above two suggestions and solves the
> above two problems. (The bug in lyx-bug-undefined.lyx has to be addressed
> separately.) Parsing a file using regexps is always delicate so I think it's
> good that with my patch there are fewer of these. I do not think that I
> missed any subtlety in the existing code, but in any case you don't need to
> accept it as is.

In this particular case the regexp was simply wrong.

> Also, lyx-preview-macros-2-lassert.lyx no longer lasserts but on opening the
> preview is not generated because \Coloneqq is not defined. This looks like
> the bug that you are referring to in your message where packages for symbols
> are not properly loaded. But, notice that the preview is correctly generated
> on a second time, when entering and leaving the math inset. So maybe it's
> easier to fix than it seemed. But indeed it's not a regression.

Yes, this has always been the case, seemingly. However, now one can obtain
a preview after entering and exiting a math inset, 

Re: test of math previews

[Jürgen Spitzmüller]

2015-06-14 13:31 GMT+02:00 Enrico Forestieri for...@lyx.org:

 Jürgen, this is going to become complicated and fragile. What about
 following the same strategy used in stable?
 This implies reverting bc47054b.


Since I do not have time to investigate further, please do whatever you see
fit.

I would have preferred that we output proper code to begin with (with no
duplicated or wrong definitions that have to be fixed in a subsequent
step), but since I cannot invest more time to it, this is completely your
call.

Jürgen



 --
 Enrico

Re: test of math previews

[Jürgen Spitzmüller]

2015-06-14 16:04 GMT+02:00 Enrico Forestieri for...@lyx.org:

 I agree that this would have been better but, unfortunately, it doesn't
 fit the instant preview infrastructure. You wold have to output different
 definitions depending on whether you compile the whole set of snippets
 or only some of them. This would cause regenerating a preview simply
 because the cursor entered the inset. To correct this, a complicated
 strategy has to be followed. It is cleaner, safer and simpler taking care
 of the correct definition in the scripts.


I understand that you tried hard, so I am confident you know what to do.

Jürgen



 --
 Enrico

Re: test of math previews

[Enrico Forestieri]

On Thu, Jun 04, 2015 at 07:25:25PM +0200, Enrico Forestieri wrote:

 On Thu, Jun 04, 2015 at 06:56:46PM +0200, Jürgen Spitzmüller wrote:
  2015-06-04 18:39 GMT+02:00 Enrico Forestieri for...@lyx.org:
  
   Yes, I can reproduce in master. It does not occur in stable, where a
   different strategy is used, only involving the instant preview scripts.
   If I am not mistaken, it goes like this. When you enter and then leave
   a math inset, the generated preview snippet is compared with the cached
   ones. If it matches one of them, no further action is taken and the
   corresponding image is used. It is the way duplicated macros are avoided
   in master that causes this glitch, because the macro definitions are also
   cached. When the document is loaded, that snippet contains the macro
   definition because it was not appearing before. However, when you
   enter and then leave the math inset, the code thinks that the macro
   was already defined previously and does not include it anymore. Now the
   snippet does not match any of the cached ones because the macro is not
   there anymore. Hence, a new preview is attempted but it fails because of
   the missing macro definition.
  
   Fixing this requires a bit more work in master. First of all, the cached
   macros should be cleared after a preview cycle completed, such that next
   previews involving the same macro can succeed. However, given that the
   produced snippet would now be different from when it was generated the
   first time (no macro present at that time), the already cached image would
   not be used and a new one will be generated. This risks to be highly
   inefficient, of course, so something should be thought for avoiding it.
  
  
  
  Yes, I had a closer look at this as well. I am not sure about a good and
  efficient strategy. Maybe we should store for which insets definitions were
  cached and remove them from the general cache if this inset is modified. I
  do not yet have a brilliant idea, though.
 
 What about the following strategy?
 
 Always include the macro definition if it appears in the math inset and
 don't include it if its name is not there. To decide this, don't cache
 the whole macro but only its name. I mean, if the macro is defined by
 \global\long\def, include it as is. If not, check the cache for the
 name and accordingly use \newcommandx or \renewcommandx. When comparing
 cached snippets, treat \newcommandx and \renewcommandx as the same
 (maybe replacing \renew with \new before the comparison, or the way around).
 Or something like that...
 
 Of course, the cache should be cleared, but that's should be easy.
 See attached.

Jürgen, this is going to become complicated and fragile. What about
following the same strategy used in stable?
This implies reverting bc47054b.

-- 
Enrico

Re: test of math previews

[Enrico Forestieri]

On Sun, Jun 14, 2015 at 01:34:40PM +0200, Jürgen Spitzmüller wrote:

 2015-06-14 13:31 GMT+02:00 Enrico Forestieri for...@lyx.org:
 
  Jürgen, this is going to become complicated and fragile. What about
  following the same strategy used in stable?
  This implies reverting bc47054b.
 
 
 Since I do not have time to investigate further, please do whatever you see
 fit.
 
 I would have preferred that we output proper code to begin with (with no
 duplicated or wrong definitions that have to be fixed in a subsequent
 step), but since I cannot invest more time to it, this is completely your
 call.

I agree that this would have been better but, unfortunately, it doesn't
fit the instant preview infrastructure. You wold have to output different
definitions depending on whether you compile the whole set of snippets
or only some of them. This would cause regenerating a preview simply
because the cursor entered the inset. To correct this, a complicated
strategy has to be followed. It is cleaner, safer and simpler taking care
of the correct definition in the scripts.

-- 
Enrico

Re: test of math previews

[Enrico Forestieri]

On Thu, Jun 04, 2015 at 07:25:25PM +0200, Enrico Forestieri wrote:

> On Thu, Jun 04, 2015 at 06:56:46PM +0200, Jürgen Spitzmüller wrote:
> > 2015-06-04 18:39 GMT+02:00 Enrico Forestieri :
> > 
> > > Yes, I can reproduce in master. It does not occur in stable, where a
> > > different strategy is used, only involving the instant preview scripts.
> > > If I am not mistaken, it goes like this. When you enter and then leave
> > > a math inset, the generated preview snippet is compared with the cached
> > > ones. If it matches one of them, no further action is taken and the
> > > corresponding image is used. It is the way duplicated macros are avoided
> > > in master that causes this glitch, because the macro definitions are also
> > > cached. When the document is loaded, that snippet contains the macro
> > > definition because it was not appearing before. However, when you
> > > enter and then leave the math inset, the code thinks that the macro
> > > was already defined previously and does not include it anymore. Now the
> > > snippet does not match any of the cached ones because the macro is not
> > > there anymore. Hence, a new preview is attempted but it fails because of
> > > the missing macro definition.
> > >
> > > Fixing this requires a bit more work in master. First of all, the cached
> > > macros should be cleared after a preview cycle completed, such that next
> > > previews involving the same macro can succeed. However, given that the
> > > produced snippet would now be different from when it was generated the
> > > first time (no macro present at that time), the already cached image would
> > > not be used and a new one will be generated. This risks to be highly
> > > inefficient, of course, so something should be thought for avoiding it.
> > >
> > 
> > 
> > Yes, I had a closer look at this as well. I am not sure about a good and
> > efficient strategy. Maybe we should store for which insets definitions were
> > cached and remove them from the general cache if this inset is modified. I
> > do not yet have a brilliant idea, though.
> 
> What about the following strategy?
> 
> Always include the macro definition if it appears in the math inset and
> don't include it if its name is not there. To decide this, don't cache
> the whole macro but only its name. I mean, if the macro is defined by
> \global\long\def, include it as is. If not, check the cache for the
> name and accordingly use \newcommandx or \renewcommandx. When comparing
> cached snippets, treat \newcommandx and \renewcommandx as the same
> (maybe replacing \renew with \new before the comparison, or the way around).
> Or something like that...
> 
> Of course, the cache should be cleared, but that's should be easy.
> See attached.

Jürgen, this is going to become complicated and fragile. What about
following the same strategy used in stable?
This implies reverting bc47054b.

-- 
Enrico

Re: test of math previews

[Jürgen Spitzmüller]

2015-06-14 13:31 GMT+02:00 Enrico Forestieri :

> Jürgen, this is going to become complicated and fragile. What about
> following the same strategy used in stable?
> This implies reverting bc47054b.
>

Since I do not have time to investigate further, please do whatever you see
fit.

I would have preferred that we output proper code to begin with (with no
duplicated or wrong definitions that have to be fixed in a subsequent
step), but since I cannot invest more time to it, this is completely your
call.

Jürgen


>
> --
> Enrico
>

Re: test of math previews

[Enrico Forestieri]

On Sun, Jun 14, 2015 at 01:34:40PM +0200, Jürgen Spitzmüller wrote:

> 2015-06-14 13:31 GMT+02:00 Enrico Forestieri :
> 
> > Jürgen, this is going to become complicated and fragile. What about
> > following the same strategy used in stable?
> > This implies reverting bc47054b.
> >
> 
> Since I do not have time to investigate further, please do whatever you see
> fit.
> 
> I would have preferred that we output proper code to begin with (with no
> duplicated or wrong definitions that have to be fixed in a subsequent
> step), but since I cannot invest more time to it, this is completely your
> call.

I agree that this would have been better but, unfortunately, it doesn't
fit the instant preview infrastructure. You wold have to output different
definitions depending on whether you compile the whole set of snippets
or only some of them. This would cause regenerating a preview simply
because the cursor entered the inset. To correct this, a complicated
strategy has to be followed. It is cleaner, safer and simpler taking care
of the correct definition in the scripts.

-- 
Enrico

Re: test of math previews

[Jürgen Spitzmüller]

2015-06-14 16:04 GMT+02:00 Enrico Forestieri :

> I agree that this would have been better but, unfortunately, it doesn't
> fit the instant preview infrastructure. You wold have to output different
> definitions depending on whether you compile the whole set of snippets
> or only some of them. This would cause regenerating a preview simply
> because the cursor entered the inset. To correct this, a complicated
> strategy has to be followed. It is cleaner, safer and simpler taking care
> of the correct definition in the scripts.
>

I understand that you tried hard, so I am confident you know what to do.

Jürgen


>
> --
> Enrico
>

Re: test of math previews

[Richard Heck]

On 06/07/2015 07:49 AM, Enrico Forestieri wrote:
The attached patch solves all of the above problems (and also the 
segfault you report separately) for me. I am also attaching the patch 
for #9354 accordingly updated. Please test both and report the issues 
you find. If your paper pass the tests, I think they are good for 
stable ;) 


If you get to something that satisfies both you and Guillaume, please 
feel free to apply.


I'll plan to begin the 2.1.4 release process once this is resolved.

Richard

Re: test of math previews

[Richard Heck]

On 06/07/2015 07:49 AM, Enrico Forestieri wrote:
The attached patch solves all of the above problems (and also the 
segfault you report separately) for me. I am also attaching the patch 
for #9354 accordingly updated. Please test both and report the issues 
you find. If your paper pass the tests, I think they are good for 
stable ;) 


If you get to something that satisfies both you and Guillaume, please 
feel free to apply.


I'll plan to begin the 2.1.4 release process once this is resolved.

Richard

Re: test of math previews

[Enrico Forestieri]

On Sat, Jun 06, 2015 at 08:07:12PM +0100, Guillaume M-M wrote:
 
 lyx-preview-macros2-failure.lyx shows two cases where a macro is forgotten
 from the list. The third inset works and is meant as a control. All 3 work
 without the patch.

Here I was forgetting to also scan nested insets.

 lyx-preview-macros2-lassert.lyx triggers an assertion when preview is
 activated. Does not assert without the patch.

Here I did not account for the fact that also the macros defined in the
symbols file are spotted. However, they do not have a valid DataMacro
and this was causing the assert.

Note that there is another bug here, but it is not a regression as it
seems to be like that since ever (I checked with lyx versions since 1.3).
The bug is that when using a symbol defined in the symbols file and this
symbol requires a certain package, this requirement is not taken into
account when generating the tex file for preview and thus latex fails.

 Notice that my thourough test is basically loading my current paper with
 preview on.

I think that your paper is a very valid test case for macros ;)

The attached patch solves all of the above problems (and also the segfault
you report separately) for me. I am also attaching the patch for #9354
accordingly updated. Please test both and report the issues you find.
If your paper pass the tests, I think they are good for stable ;)

-- 
Enrico
diff --git a/src/mathed/InsetMath.h b/src/mathed/InsetMath.h
index 5e5f492..61f4a53 100644
--- a/src/mathed/InsetMath.h
+++ b/src/mathed/InsetMath.h
@@ -58,6 +58,7 @@ class InsetMathAMSArray;
 class InsetMathBrace;
 class InsetMathChar;
 class InsetMathDelim;
+class InsetMathFracBase;
 class InsetMathFrac;
 class InsetMathFont;
 class InsetMathGrid;
@@ -129,6 +130,8 @@ public:
virtual InsetMathChar const * asCharInset() const { return 0; }
virtual InsetMathDelim  * asDelimInset()  { return 0; }
virtual InsetMathDelim const* asDelimInset() const{ return 0; }
+   virtual InsetMathFracBase   * asFracBaseInset()   { return 0; }
+   virtual InsetMathFracBase const * asFracBaseInset() const { return 0; }
virtual InsetMathFrac   * asFracInset()   { return 0; }
virtual InsetMathFrac const * asFracInset() const { return 0; }
virtual InsetMathFont   * asFontInset()   { return 0; }
diff --git a/src/mathed/InsetMathFrac.h b/src/mathed/InsetMathFrac.h
index 5030730..c030c8a 100644
--- a/src/mathed/InsetMathFrac.h
+++ b/src/mathed/InsetMathFrac.h
@@ -30,6 +30,10 @@ public:
bool idxBackward(Cursor ) const { return false; }
///
bool idxForward(Cursor ) const { return false; }
+   ///
+   InsetMathFracBase * asFracBaseInset() { return this; }
+   ///
+   InsetMathFracBase const * asFracBaseInset() const { return this; }
 };
 
 
diff --git a/src/mathed/InsetMathHull.cpp b/src/mathed/InsetMathHull.cpp
index d4bdf87..69643bb 100644
--- a/src/mathed/InsetMathHull.cpp
+++ b/src/mathed/InsetMathHull.cpp
@@ -14,6 +14,10 @@
 
 #include InsetMathChar.h
 #include InsetMathColor.h
+#include InsetMathFrac.h
+#include InsetMathGrid.h
+#include InsetMathNest.h
+#include InsetMathScript.h
 #include MathExtern.h
 #include MathFactory.h
 #include MathStream.h
@@ -32,6 +36,7 @@
 #include LaTeXFeatures.h
 #include LyXRC.h
 #include MacroTable.h
+#include MathMacro.h
 #include output_xhtml.h
 #include Paragraph.h
 #include ParIterator.h
@@ -622,6 +627,55 @@ void InsetMathHull::addPreview(DocIterator const  
inset_pos,
 }
 
 
+void InsetMathHull::usedMacros(MathData const  md, DocIterator const  pos,
+   MacroNameSet  macros, MacroNameSet  defs) 
const
+{
+   MacroNameSet::iterator const end = macros.end();
+
+   for (size_t i = 0; i  md.size(); ++i) {
+   MathMacro const * mi = md[i].nucleus()-asMacro();
+   InsetMathScript const * si = md[i].nucleus()-asScriptInset();
+   InsetMathFracBase const * fi = 
md[i].nucleus()-asFracBaseInset();
+   InsetMathGrid const * gi = md[i].nucleus()-asGridInset();
+   InsetMathNest const * ni = md[i].nucleus()-asNestInset();
+   if (mi) {
+   // Make sure this is a macro defined in the document
+   // (as we also spot the macros in the symbols file)
+   // or that we have not already accounted for it.
+   docstring const name = mi-name();
+   if (macros.find(name) == end)
+   continue;
+   macros.erase(name);
+   MathData ar(pos.buffer());
+   MacroData const * data =
+   pos.buffer()-getMacro(name, pos, true);
+   if (data) {
+   odocstringstream macro_def;
+   

Re: test of math previews

[Scott Kostyshak]

On Fri, Jun 05, 2015 at 05:11:45PM -0400, Scott Kostyshak wrote:
 On Fri, Jun 5, 2015 at 3:40 AM, Jean-Marc Lasgouttes lasgout...@lyx.org 
 wrote:
  Le 05/06/2015 09:14, Enrico Forestieri a écrit :
 
  A possible way out would be outputting all the macros once at the
  beginning of the file, such that they are seen by all snippets, and then
  the specific one appearing in a math inset (as done in the patch I was
  proposing). I'll have a look whether this can be done in a simple way.
 
 
  Isn't it possible to output them as they are output by the normal latex
  export? Of course, this would mean that creating previews would be a very
  different process, more like a special version of our latex output code,
  instead of some registration done by the insets themselves.
 
 I am unsure which of the above messages relate to whether I should
 revert 73460423 and 6ac04e21. In any case, I am planning to revert
 because of JMarc's comment that the commits modify a full buffer just
 in the middle of a dispatch action. I leave a day for an objection
 (in case someone points out I misunderstood something) and then I will
 revert.

Reverted in stable at 9ea4fc6e and in master at 580947bf.

Scott

Re: test of math previews

[Enrico Forestieri]

On Sat, Jun 06, 2015 at 08:20:43PM +0100, Guillaume M-M wrote:
 
 Obvious one to test for

Thanks for spotting this. It is due to the recursion in the macro
definitions. The patch I sent takes this into account and lyx does
not segfaults anymore, but latex will be caught in an endless loop
and thus cannot generate a preview. You will have to kill it.

-- 
Enrico

Re: test of math previews

[Enrico Forestieri]

On Sat, Jun 06, 2015 at 08:18:12PM +0200, Kornel Benko wrote:
 
 But I wonder why there are preview entries in the lyxpreview*.tex
 ...
 \begin{document}
 \begin{preview}
 ${\displaystyle {#1}}$
 \end{preview}
 
 \begin{preview}
 $#1$
 \end{preview}
 
 \begin{preview}
 ${\scriptstyle {#1}}$
 \end{preview}
 
 \begin{preview}
 ${\scriptscriptstyle {#1}}$
 \end{preview}
 
 
 \end{document}

Because the macro definition contains those math insets, ie., $...$,
and the math insets cannot know that they will not be used.

-- 
Enrico

Re: test of math previews

[Enrico Forestieri]

On Sat, Jun 06, 2015 at 08:07:12PM +0100, Guillaume M-M wrote:
> 
> lyx-preview-macros2-failure.lyx shows two cases where a macro is forgotten
> from the list. The third inset works and is meant as a control. All 3 work
> without the patch.

Here I was forgetting to also scan nested insets.

> lyx-preview-macros2-lassert.lyx triggers an assertion when preview is
> activated. Does not assert without the patch.

Here I did not account for the fact that also the macros defined in the
symbols file are spotted. However, they do not have a valid DataMacro
and this was causing the assert.

Note that there is another bug here, but it is not a regression as it
seems to be like that since ever (I checked with lyx versions since 1.3).
The bug is that when using a symbol defined in the symbols file and this
symbol requires a certain package, this requirement is not taken into
account when generating the tex file for preview and thus latex fails.

> Notice that my "thourough test" is basically loading my current paper with
> preview on.

I think that your paper is a very valid test case for macros ;)

The attached patch solves all of the above problems (and also the segfault
you report separately) for me. I am also attaching the patch for #9354
accordingly updated. Please test both and report the issues you find.
If your paper pass the tests, I think they are good for stable ;)

-- 
Enrico
diff --git a/src/mathed/InsetMath.h b/src/mathed/InsetMath.h
index 5e5f492..61f4a53 100644
--- a/src/mathed/InsetMath.h
+++ b/src/mathed/InsetMath.h
@@ -58,6 +58,7 @@ class InsetMathAMSArray;
 class InsetMathBrace;
 class InsetMathChar;
 class InsetMathDelim;
+class InsetMathFracBase;
 class InsetMathFrac;
 class InsetMathFont;
 class InsetMathGrid;
@@ -129,6 +130,8 @@ public:
virtual InsetMathChar const * asCharInset() const { return 0; }
virtual InsetMathDelim  * asDelimInset()  { return 0; }
virtual InsetMathDelim const* asDelimInset() const{ return 0; }
+   virtual InsetMathFracBase   * asFracBaseInset()   { return 0; }
+   virtual InsetMathFracBase const * asFracBaseInset() const { return 0; }
virtual InsetMathFrac   * asFracInset()   { return 0; }
virtual InsetMathFrac const * asFracInset() const { return 0; }
virtual InsetMathFont   * asFontInset()   { return 0; }
diff --git a/src/mathed/InsetMathFrac.h b/src/mathed/InsetMathFrac.h
index 5030730..c030c8a 100644
--- a/src/mathed/InsetMathFrac.h
+++ b/src/mathed/InsetMathFrac.h
@@ -30,6 +30,10 @@ public:
bool idxBackward(Cursor &) const { return false; }
///
bool idxForward(Cursor &) const { return false; }
+   ///
+   InsetMathFracBase * asFracBaseInset() { return this; }
+   ///
+   InsetMathFracBase const * asFracBaseInset() const { return this; }
 };
 
 
diff --git a/src/mathed/InsetMathHull.cpp b/src/mathed/InsetMathHull.cpp
index d4bdf87..69643bb 100644
--- a/src/mathed/InsetMathHull.cpp
+++ b/src/mathed/InsetMathHull.cpp
@@ -14,6 +14,10 @@
 
 #include "InsetMathChar.h"
 #include "InsetMathColor.h"
+#include "InsetMathFrac.h"
+#include "InsetMathGrid.h"
+#include "InsetMathNest.h"
+#include "InsetMathScript.h"
 #include "MathExtern.h"
 #include "MathFactory.h"
 #include "MathStream.h"
@@ -32,6 +36,7 @@
 #include "LaTeXFeatures.h"
 #include "LyXRC.h"
 #include "MacroTable.h"
+#include "MathMacro.h"
 #include "output_xhtml.h"
 #include "Paragraph.h"
 #include "ParIterator.h"
@@ -622,6 +627,55 @@ void InsetMathHull::addPreview(DocIterator const & 
inset_pos,
 }
 
 
+void InsetMathHull::usedMacros(MathData const & md, DocIterator const & pos,
+   MacroNameSet & macros, MacroNameSet & defs) 
const
+{
+   MacroNameSet::iterator const end = macros.end();
+
+   for (size_t i = 0; i < md.size(); ++i) {
+   MathMacro const * mi = md[i].nucleus()->asMacro();
+   InsetMathScript const * si = md[i].nucleus()->asScriptInset();
+   InsetMathFracBase const * fi = 
md[i].nucleus()->asFracBaseInset();
+   InsetMathGrid const * gi = md[i].nucleus()->asGridInset();
+   InsetMathNest const * ni = md[i].nucleus()->asNestInset();
+   if (mi) {
+   // Make sure this is a macro defined in the document
+   // (as we also spot the macros in the symbols file)
+   // or that we have not already accounted for it.
+   docstring const name = mi->name();
+   if (macros.find(name) == end)
+   continue;
+   macros.erase(name);
+   MathData ar(pos.buffer());
+   MacroData const * data =
+   pos.buffer()->getMacro(name, pos, true);
+   if (data) {
+   odocstringstream 

Re: test of math previews

[Enrico Forestieri]

On Sat, Jun 06, 2015 at 08:20:43PM +0100, Guillaume M-M wrote:
> 
> Obvious one to test for

Thanks for spotting this. It is due to the recursion in the macro
definitions. The patch I sent takes this into account and lyx does
not segfaults anymore, but latex will be caught in an endless loop
and thus cannot generate a preview. You will have to kill it.

-- 
Enrico

Re: test of math previews

[Enrico Forestieri]

On Sat, Jun 06, 2015 at 08:18:12PM +0200, Kornel Benko wrote:
> 
> But I wonder why there are preview entries in the lyxpreview*.tex
> ...
> \begin{document}
> \begin{preview}
> ${\displaystyle {#1}}$
> \end{preview}
> 
> \begin{preview}
> $#1$
> \end{preview}
> 
> \begin{preview}
> ${\scriptstyle {#1}}$
> \end{preview}
> 
> \begin{preview}
> ${\scriptscriptstyle {#1}}$
> \end{preview}
> 
> 
> \end{document}

Because the macro definition contains those math insets, ie., $...$,
and the math insets cannot know that they will not be used.

-- 
Enrico

Re: test of math previews

[Scott Kostyshak]

On Fri, Jun 05, 2015 at 05:11:45PM -0400, Scott Kostyshak wrote:
> On Fri, Jun 5, 2015 at 3:40 AM, Jean-Marc Lasgouttes  
> wrote:
> > Le 05/06/2015 09:14, Enrico Forestieri a écrit :
> >>
> >> A possible way out would be outputting all the macros once at the
> >> beginning of the file, such that they are seen by all snippets, and then
> >> the specific one appearing in a math inset (as done in the patch I was
> >> proposing). I'll have a look whether this can be done in a simple way.
> >
> >
> > Isn't it possible to output them as they are output by the normal latex
> > export? Of course, this would mean that creating previews would be a very
> > different process, more like a special version of our latex output code,
> > instead of some registration done by the insets themselves.
> 
> I am unsure which of the above messages relate to whether I should
> revert 73460423 and 6ac04e21. In any case, I am planning to revert
> because of JMarc's comment that the commits "modify a full buffer just
> in the middle of a dispatch action". I leave a day for an objection
> (in case someone points out I misunderstood something) and then I will
> revert.

Reverted in stable at 9ea4fc6e and in master at 580947bf.

Scott

Re: test of math previews

[Enrico Forestieri]

On Fri, Jun 05, 2015 at 09:40:39AM +0200, Jean-Marc Lasgouttes wrote:

 Le 05/06/2015 09:14, Enrico Forestieri a écrit :
 A possible way out would be outputting all the macros once at the
 beginning of the file, such that they are seen by all snippets, and then
 the specific one appearing in a math inset (as done in the patch I was
 proposing). I'll have a look whether this can be done in a simple way.
 
 Isn't it possible to output them as they are output by the normal latex
 export? Of course, this would mean that creating previews would be a very
 different process, more like a special version of our latex output code,
 instead of some registration done by the insets themselves.

I don't think that is possible. The point is that the previews should be
updated whenever they change. Now, they can change even if the math inset
was not modified, because one could have modified a macro. Thus, the
snippets have also to contain the macros they use.

I tried to solve this problem by including in a snippet only the macros that
are actually used. I almost succeeded with the attached patch for stable
(master would require more surgery). I say almost because it seems that
only the macros used by the current macro are spotted, but not the ones
that, in turn, are used by them. I mean, after

\newcommand{\first}{pluto}
\newcommand{\second}{goofy\,and\,\first}
\newcommand{\third}{mickey,\,\second}
$\third$

the macros \second and \third are spotted but not \first. See also the
attached examples. After activating instant previews, when loading
lyx-preview-macros-2.lyx you will correctly see

Found: first
Found: second

but loading lyx-preview-macros-3.lyx produces

Found: second
Found: third
Warning: Failed to produce 1 preview snippet(s)

Essentially, the patch goes recursively through the definition of the
macros found in an inset, but it seems that the definition of second
level macros are empty? I am not so familiar with the code dealing
with math macros, but I thought that their definition in terms of a
LyX inset should be held in MacroData::definition_. However, it does
not seem to (always) be so?

Any help or suggestion will be appreciated.

-- 
Enrico
diff --git a/src/mathed/InsetMathHull.cpp b/src/mathed/InsetMathHull.cpp
index d4bdf87..a7db85f 100644
--- a/src/mathed/InsetMathHull.cpp
+++ b/src/mathed/InsetMathHull.cpp
@@ -31,6 +31,7 @@
 #include Language.h
 #include LaTeXFeatures.h
 #include LyXRC.h
+#include MathMacro.h
 #include MacroTable.h
 #include output_xhtml.h
 #include Paragraph.h
@@ -622,6 +623,18 @@ void InsetMathHull::addPreview(DocIterator const  
inset_pos,
 }
 
 
+void InsetMathHull::usedMacros(MathData const  md, MacroNameSet  macros) 
const
+{
+   for (size_t i = 0; i  md.size(); ++i) {
+   MathMacro * mi = const_castMathMacro 
*(md[i].nucleus()-asMacro());
+   if (mi) {
+   macros.insert(mi-name());
+   usedMacros(mi-definition(), macros);
+   }
+   }
+}
+
+
 void InsetMathHull::preparePreview(DocIterator const  pos,
bool forexport) const
 {
@@ -632,13 +645,17 @@ void InsetMathHull::preparePreview(DocIterator const  
pos,
 
Buffer const * buffer = pos.buffer();
 
-   // collect macros at this position
+   // collect macros used in this inset
MacroNameSet macros;
-   buffer-listMacroNames(macros);
+   for (row_type row = 0; row  nrows(); ++row)
+   for (col_type col = 0; col  ncols(); ++col)
+   usedMacros(cell(index(row, col)), macros);
+
MacroNameSet::iterator it = macros.begin();
MacroNameSet::iterator end = macros.end();
odocstringstream macro_preamble;
for (; it != end; ++it) {
+   lyxerr  Found:   *it  endl;
MacroData const * data = buffer-getMacro(*it, pos, true);
if (data) {
data-write(macro_preamble, true);
diff --git a/src/mathed/InsetMathHull.h b/src/mathed/InsetMathHull.h
index 0cd..65cb0ea 100644
--- a/src/mathed/InsetMathHull.h
+++ b/src/mathed/InsetMathHull.h
@@ -25,6 +25,7 @@ namespace lyx {
 class InsetLabel;
 class ParConstIterator;
 class RenderPreview;
+class MacroNameSet;
 
 
 /// This provides an interface between LyX insets and LyX math insets
@@ -190,6 +191,8 @@ private:
/// used by image export
void loadPreview(DocIterator const  pos) const;
///
+   void usedMacros(MathData const  md, MacroNameSet  macros) const;
+   ///
void setType(HullType type);
///
void validate1(LaTeXFeatures  features);
diff --git a/src/mathed/MathMacro.h b/src/mathed/MathMacro.h
index 239495e..18048d3 100644
--- a/src/mathed/MathMacro.h
+++ b/src/mathed/MathMacro.h
@@ -135,6 +135,7 @@ protected:
friend class MathData;
friend class ArgumentProxy;
friend class Cursor;
+   friend class InsetMathHull;
 
/// update the 

Re: test of math previews

[Enrico Forestieri]

On Fri, Jun 05, 2015 at 11:03:48PM +0100, Guillaume M-M wrote:

 Le 05/06/2015 08:14, Enrico Forestieri a écrit :
 On Thu, Jun 04, 2015 at 07:32:03PM -0400, Richard Heck wrote:
 On 06/04/2015 07:13 PM, Guillaume M-M wrote:
 
 Bad news, it seems to forget that macros can use macros. With the attached
 file the preview is as follows:
 
 \begin{preview}
 \global\long\def\b{\a}
 
 $\b$
 \end{preview}
 
 I suspect this is why all macros are included.
 
 Most probably. This leaves us pretty stuck with the problem caused by
 having hundreds of macros. I will not commit any change to stable until
 we find a good solution. After all, this is what already happens now
 and it is not a regression. I think the complaint is due to the fact
 that now also the previews get zoomed and this may cause relevant delays,
 
 Enrico mentions a complaint. There seems to be a misunderstanding. The
 freeze on zoom is a regression in stable, while the time explosion was
 already there. This is only a factual report of the issues trying to be the
 most helpful. The reports are the outcome of my tests of the preview
 mechanism following Enrico's request of having his patch at
 http://www.lyx.org/trac/ticket/9354 tested.
 
 In truth, I have no interest in full preview, only in the solution to #9354,
 and my tests concluded that there are no immediate issues with his patch,
 while I managed to find other issues---many thanks to Enrico and Jürgen for
 fixing them promptly. Fixing #9354 (preview inset does not work with math
 macros) in 2.1.4 would make the preview mechanism useful again in my
 context, avoiding the performance issues. (Especially because I cannot ask
 my collaborators to compile from git.) Apart from that, only this regression
 regarding the zoom/preview interaction remains regarding the stable branch
 (to summarise the thread, since you probably want to move forward with
 2.1.4).
 
 After this thread dies off, I may record the remaining important issues in
 the bug tracker, but again please do not regard such reports as complaints!

When you don't speak/write in your native language often you end up
conveying what you are able to and not what you actually mean.
And email doesn't help in this respect.

I didn't want to attach any negative meaning to the word complaint and,
morever, maybe I failed to actually properly acknowledge your help in this
matter. Indeed, you made a not so common testing work and helped discovering
issues that were escaping my (deemed) extensive testing.

That said, I was confident in applying all the changes involving only
touching the scripts because they solve some long standing issues without
the risk of introducing more severe issues. I think you tested the patch
for #9354 very well and I now feel confident that it is safe, but it is
the responsibility of the stable maintainer taking a decision about it.

Also take into account that, once the problem of the size explosion in the
presence of hundreds of macros is solved, it will have to be adapted, too.

-- 
Enrico

Re: test of math previews

[Kornel Benko]

Am Samstag, 6. Juni 2015 um 13:13:22, schrieb Enrico Forestieri for...@lyx.org
 +void InsetMathHull::usedMacros(MathData const  md, MacroNameSet  macros) 
 const
 +{
 +   for (size_t i = 0; i  md.size(); ++i) {
 +   MathMacro * mi = const_castMathMacro 
 *(md[i].nucleus()-asMacro());
 +   if (mi) {
 +   macros.insert(mi-name());
 +   usedMacros(mi-definition(), macros);

I would change the order, to get the used macro first.

 +   }
 +   }
 +}
 +

This may not help with empty second level macros though.

Kornel




signature.asc
Description: This is a digitally signed message part.

Re: test of math previews

[Enrico Forestieri]

On Sat, Jun 06, 2015 at 03:23:48PM +0200, Kornel Benko wrote:

 Am Samstag, 6. Juni 2015 um 13:13:22, schrieb Enrico Forestieri 
 for...@lyx.org
  +void InsetMathHull::usedMacros(MathData const  md, MacroNameSet  macros) 
  const
  +{
  +   for (size_t i = 0; i  md.size(); ++i) {
  +   MathMacro * mi = const_castMathMacro 
  *(md[i].nucleus()-asMacro());
  +   if (mi) {
  +   macros.insert(mi-name());
  +   usedMacros(mi-definition(), macros);
 
 I would change the order, to get the used macro first.

No, that is correct. The top level macro has to appear in the last position.

I failed to understand why that was not working but, most probably, it has
to do with the fact not all data is copied over to a cloned buffer.
However, I found a different way for collecting only the macros used
in a given math inset, so I did not investigate it further.

The attached patch for stable works for me and avoids the size explosion
when a very lot of macros are defined in a document. I count on Guillaume
to give it a thourough test. If he doesn't find glitches I will feel
confident about proposing to apply it for 2.1.4.

-- 
Enrico
diff --git a/src/mathed/InsetMathHull.cpp b/src/mathed/InsetMathHull.cpp
index d4bdf87..c23d2f4 100644
--- a/src/mathed/InsetMathHull.cpp
+++ b/src/mathed/InsetMathHull.cpp
@@ -31,6 +31,7 @@
 #include Language.h
 #include LaTeXFeatures.h
 #include LyXRC.h
+#include MathMacro.h
 #include MacroTable.h
 #include output_xhtml.h
 #include Paragraph.h
@@ -622,6 +623,28 @@ void InsetMathHull::addPreview(DocIterator const  
inset_pos,
 }
 
 
+void InsetMathHull::usedMacros(MathData const  md, DocIterator const  pos,
+   MacroNameSet  macros) const
+{
+   for (size_t i = 0; i  md.size(); ++i) {
+   MathMacro const * mi = md[i].nucleus()-asMacro();
+   if (mi) {
+   MathData ar(pos.buffer());
+   MacroData const * data =
+   pos.buffer()-getMacro(mi-name(), pos, true);
+   if (data) {
+   odocstringstream macro_def;
+   data-write(macro_def, true);
+   macro_def  endl;
+   macros.insert(macro_def.str());
+   asArray(data-definition(), ar);
+   }
+   usedMacros(ar, pos, macros);
+   }
+   }
+}
+
+
 void InsetMathHull::preparePreview(DocIterator const  pos,
bool forexport) const
 {
@@ -632,19 +655,17 @@ void InsetMathHull::preparePreview(DocIterator const  
pos,
 
Buffer const * buffer = pos.buffer();
 
-   // collect macros at this position
+   // collect macros used in this inset
MacroNameSet macros;
-   buffer-listMacroNames(macros);
+   for (row_type row = 0; row  nrows(); ++row)
+   for (col_type col = 0; col  ncols(); ++col)
+   usedMacros(cell(index(row, col)), pos, macros);
+
MacroNameSet::iterator it = macros.begin();
MacroNameSet::iterator end = macros.end();
-   odocstringstream macro_preamble;
-   for (; it != end; ++it) {
-   MacroData const * data = buffer-getMacro(*it, pos, true);
-   if (data) {
-   data-write(macro_preamble, true);
-   macro_preamble  endl;
-   }
-   }
+   docstring macro_preamble;
+   for (; it != end; ++it)
+   macro_preamble.append(*it);
 
docstring setcnt;
if (forexport  haveNumbers()) {
@@ -667,8 +688,7 @@ void InsetMathHull::preparePreview(DocIterator const  pos,
  '{' + convertdocstring(num) + '}';
}
}
-   docstring const snippet = macro_preamble.str() +
-   setcnt + latexString(*this);
+   docstring const snippet = macro_preamble + setcnt + latexString(*this);
LYXERR(Debug::MACROS, Preview snippet:   snippet);
preview_-addPreview(snippet, *buffer, forexport);
 }
diff --git a/src/mathed/InsetMathHull.h b/src/mathed/InsetMathHull.h
index 0cd..0b9e5fd 100644
--- a/src/mathed/InsetMathHull.h
+++ b/src/mathed/InsetMathHull.h
@@ -25,6 +25,7 @@ namespace lyx {
 class InsetLabel;
 class ParConstIterator;
 class RenderPreview;
+class MacroNameSet;
 
 
 /// This provides an interface between LyX insets and LyX math insets
@@ -152,6 +153,9 @@ public:
/// Recreates the preview if preview is enabled.
void reloadPreview(DocIterator const  pos) const;
///
+   void usedMacros(MathData const  md, DocIterator const  pos,
+   MacroNameSet  macros) const;
+   ///
void initUnicodeMath() const;
 
///

Re: test of math previews

[Guillaume M-M]

Le 06/06/2015 18:12, Enrico Forestieri a écrit :

On Sat, Jun 06, 2015 at 03:23:48PM +0200, Kornel Benko wrote:


Am Samstag, 6. Juni 2015 um 13:13:22, schrieb Enrico Forestieri for...@lyx.org

+void InsetMathHull::usedMacros(MathData const  md, MacroNameSet  macros) 
const
+{
+   for (size_t i = 0; i  md.size(); ++i) {
+   MathMacro * mi = const_castMathMacro 
*(md[i].nucleus()-asMacro());
+   if (mi) {
+   macros.insert(mi-name());
+   usedMacros(mi-definition(), macros);


I would change the order, to get the used macro first.


No, that is correct. The top level macro has to appear in the last position.

I failed to understand why that was not working but, most probably, it has
to do with the fact not all data is copied over to a cloned buffer.
However, I found a different way for collecting only the macros used
in a given math inset, so I did not investigate it further.

The attached patch for stable works for me and avoids the size explosion
when a very lot of macros are defined in a document. I count on Guillaume
to give it a thourough test. If he doesn't find glitches I will feel
confident about proposing to apply it for 2.1.4.



:)

lyx-preview-macros2-failure.lyx shows two cases where a macro is 
forgotten from the list. The third inset works and is meant as a 
control. All 3 work without the patch.


lyx-preview-macros2-lassert.lyx triggers an assertion when preview is 
activated. Does not assert without the patch.


Notice that my thourough test is basically loading my current paper 
with preview on.


lyx-preview-macros-2-failure.lyx
Description: application/lyx


lyx-preview-macros-2-lassert.lyx
Description: application/lyx

Re: test of math previews

[Guillaume M-M]

Le 06/06/2015 18:12, Enrico Forestieri a écrit :

On Sat, Jun 06, 2015 at 03:23:48PM +0200, Kornel Benko wrote:


Am Samstag, 6. Juni 2015 um 13:13:22, schrieb Enrico Forestieri for...@lyx.org

+void InsetMathHull::usedMacros(MathData const  md, MacroNameSet  macros) 
const
+{
+   for (size_t i = 0; i  md.size(); ++i) {
+   MathMacro * mi = const_castMathMacro 
*(md[i].nucleus()-asMacro());
+   if (mi) {
+   macros.insert(mi-name());
+   usedMacros(mi-definition(), macros);


I would change the order, to get the used macro first.


No, that is correct. The top level macro has to appear in the last position.

I failed to understand why that was not working but, most probably, it has
to do with the fact not all data is copied over to a cloned buffer.
However, I found a different way for collecting only the macros used
in a given math inset, so I did not investigate it further.

The attached patch for stable works for me and avoids the size explosion
when a very lot of macros are defined in a document. I count on Guillaume
to give it a thourough test. If he doesn't find glitches I will feel
confident about proposing to apply it for 2.1.4.



Obvious one to test for


lyx-preview-macros-2-sigsegv.lyx
Description: application/lyx

Re: test of math previews

[Kornel Benko]

Am Samstag, 6. Juni 2015 um 19:12:45, schrieb Enrico Forestieri for...@lyx.org
 On Sat, Jun 06, 2015 at 03:23:48PM +0200, Kornel Benko wrote:
 
  Am Samstag, 6. Juni 2015 um 13:13:22, schrieb Enrico Forestieri 
  for...@lyx.org
   +void InsetMathHull::usedMacros(MathData const  md, MacroNameSet  
   macros) const
   +{
   +   for (size_t i = 0; i  md.size(); ++i) {
   +   MathMacro * mi = const_castMathMacro 
   *(md[i].nucleus()-asMacro());
   +   if (mi) {
   +   macros.insert(mi-name());
   +   usedMacros(mi-definition(), macros);
  
  I would change the order, to get the used macro first.
 
 No, that is correct. The top level macro has to appear in the last position.

OK, I thought macros.insert() would insert at the end.

 I failed to understand why that was not working but, most probably, it has
 to do with the fact not all data is copied over to a cloned buffer.
 However, I found a different way for collecting only the macros used
 in a given math inset, so I did not investigate it further.
 
 The attached patch for stable works for me and avoids the size explosion
 when a very lot of macros are defined in a document. I count on Guillaume
 to give it a thourough test. If he doesn't find glitches I will feel
 confident about proposing to apply it for 2.1.4.
 

Works for me. That is, it does not crash with 
lyx-bug-lassert-preview-startup.lyx.

But I wonder why there are preview entries in the lyxpreview*.tex
...
\begin{document}
\begin{preview}
${\displaystyle {#1}}$
\end{preview}

\begin{preview}
$#1$
\end{preview}

\begin{preview}
${\scriptstyle {#1}}$
\end{preview}

\begin{preview}
${\scriptscriptstyle {#1}}$
\end{preview}


\end{document}
...

Kornel

signature.asc
Description: This is a digitally signed message part.

Re: test of math previews

[Enrico Forestieri]

On Fri, Jun 05, 2015 at 11:03:48PM +0100, Guillaume M-M wrote:

> Le 05/06/2015 08:14, Enrico Forestieri a écrit :
> >On Thu, Jun 04, 2015 at 07:32:03PM -0400, Richard Heck wrote:
> >>On 06/04/2015 07:13 PM, Guillaume M-M wrote:
> >>>
> >>>Bad news, it seems to forget that macros can use macros. With the attached
> >>>file the preview is as follows:
> >>>
> >>>\begin{preview}
> >>>\global\long\def\b{\a}
> >>>
> >>>$\b$
> >>>\end{preview}
> >>
> >>I suspect this is why all macros are included.
> >
> >Most probably. This leaves us pretty stuck with the problem caused by
> >having hundreds of macros. I will not commit any change to stable until
> >we find a good solution. After all, this is what already happens now
> >and it is not a regression. I think the complaint is due to the fact
> >that now also the previews get zoomed and this may cause relevant delays,
> 
> Enrico mentions a "complaint". There seems to be a misunderstanding. The
> freeze on zoom is a regression in stable, while the time explosion was
> already there. This is only a factual report of the issues trying to be the
> most helpful. The reports are the outcome of my tests of the preview
> mechanism following Enrico's request of having his patch at
>  tested.
> 
> In truth, I have no interest in full preview, only in the solution to #9354,
> and my tests concluded that there are no immediate issues with his patch,
> while I managed to find other issues---many thanks to Enrico and Jürgen for
> fixing them promptly. Fixing #9354 (preview inset does not work with math
> macros) in 2.1.4 would make the preview mechanism useful again in my
> context, avoiding the performance issues. (Especially because I cannot ask
> my collaborators to compile from git.) Apart from that, only this regression
> regarding the zoom/preview interaction remains regarding the stable branch
> (to summarise the thread, since you probably want to move forward with
> 2.1.4).
> 
> After this thread dies off, I may record the remaining important issues in
> the bug tracker, but again please do not regard such reports as complaints!

When you don't speak/write in your native language often you end up
conveying what you are able to and not what you actually mean.
And email doesn't help in this respect.

I didn't want to attach any negative meaning to the word "complaint" and,
morever, maybe I failed to actually properly acknowledge your help in this
matter. Indeed, you made a not so common testing work and helped discovering
issues that were escaping my (deemed) extensive testing.

That said, I was confident in applying all the changes involving only
touching the scripts because they solve some long standing issues without
the risk of introducing more severe issues. I think you tested the patch
for #9354 very well and I now feel confident that it is safe, but it is
the responsibility of the stable maintainer taking a decision about it.

Also take into account that, once the problem of the size explosion in the
presence of hundreds of macros is solved, it will have to be adapted, too.

-- 
Enrico