Re: Beta1 is slow on undo
Am Sonntag, den 15.10.2017, 18:41 -0400 schrieb Richard Heck:
> No, it comes whenever a document contains a BibTeX inset. A better
> patch
> would make it happen
> only when a BibTeX inset was involved. Then the delay is no surprise:
> Of
> course we have to reread
> the files.
The attached patch does not address this very bug, but I have noted
that we unnecessarily read bibtex files multiple time (e.g. with
multiple bibtex insets in master/child constellations or with
subdivided bibliographies/chapterbib).
The attached patch assures each file is only parsed once within a
single collectBibkey() process. It will slightly speed up the process
in some circumstances.
Is it worth it?
Jürgen
>
> Richard
> diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index 9c42b07e7f..8182893a1a 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -2446,15 +2446,16 @@ void Buffer::reloadBibInfoCache() const
return;
d->bibinfo_.clear();
- collectBibKeys();
+ FileNameList checkedFiles;
+ collectBibKeys(checkedFiles);
d->bibinfo_cache_valid_ = true;
}
-void Buffer::collectBibKeys() const
+void Buffer::collectBibKeys(FileNameList & checkedFiles) const
{
for (InsetIterator it = inset_iterator_begin(inset()); it; ++it)
- it->collectBibKeys(it);
+ it->collectBibKeys(it, checkedFiles);
}
diff --git a/src/Buffer.h b/src/Buffer.h
index 165f5d8795..35e7632973 100644
--- a/src/Buffer.h
+++ b/src/Buffer.h
@@ -17,6 +17,7 @@
#include "support/unique_ptr.h"
#include "support/strfwd.h"
#include "support/types.h"
+#include "support/FileNameList.h"
#include
#include
@@ -520,7 +521,7 @@ public:
/// or just for it, if it isn't a child.
BiblioInfo const & masterBibInfo() const;
/// collect bibliography info from the various insets in this buffer.
- void collectBibKeys() const;
+ void collectBibKeys(support::FileNameList &) const;
/// add some BiblioInfo to our cache
void addBiblioInfo(BiblioInfo const & bi) const;
/// add a single piece of bibliography info to our cache
diff --git a/src/insets/Inset.h b/src/insets/Inset.h
index f10ab3f883..ce17c8193a 100644
--- a/src/insets/Inset.h
+++ b/src/insets/Inset.h
@@ -23,6 +23,7 @@
#include "support/strfwd.h"
#include "support/types.h"
+#include "support/FileNameList.h"
#include
@@ -529,7 +530,7 @@ public:
UpdateType /* utype*/,
TocBackend & /* tocbackend */) const {}
/// Collect BibTeX information
- virtual void collectBibKeys(InsetIterator const &) const {}
+ virtual void collectBibKeys(InsetIterator const &, support::FileNameList &) const {}
/// Update the counters of this inset and of its contents.
/// The boolean indicates whether we are preparing for output, e.g.,
/// of XHTML.
diff --git a/src/insets/InsetBibitem.cpp b/src/insets/InsetBibitem.cpp
index afc30ae37d..d5ebaf7640 100644
--- a/src/insets/InsetBibitem.cpp
+++ b/src/insets/InsetBibitem.cpp
@@ -285,7 +285,7 @@ docstring bibitemWidest(Buffer const & buffer, OutputParams const & runparams)
}
-void InsetBibitem::collectBibKeys(InsetIterator const & it) const
+void InsetBibitem::collectBibKeys(InsetIterator const & it, FileNameList & /*checkedFiles*/) const
{
docstring const key = getParam("key");
docstring const label = getParam("label");
diff --git a/src/insets/InsetBibitem.h b/src/insets/InsetBibitem.h
index 41497e1849..ba33732ad0 100644
--- a/src/insets/InsetBibitem.h
+++ b/src/insets/InsetBibitem.h
@@ -60,7 +60,7 @@ public:
///
docstring xhtml(XHTMLStream &, OutputParams const &) const;
///
- void collectBibKeys(InsetIterator const &) const;
+ void collectBibKeys(InsetIterator const &, support::FileNameList &) const;
/// update the counter of this inset
void updateBuffer(ParIterator const &, UpdateType);
///@}
diff --git a/src/insets/InsetBibtex.cpp b/src/insets/InsetBibtex.cpp
index a905110773..30b169acb0 100644
--- a/src/insets/InsetBibtex.cpp
+++ b/src/insets/InsetBibtex.cpp
@@ -645,13 +645,13 @@ namespace {
} // namespace
-void InsetBibtex::collectBibKeys(InsetIterator const & /*di*/) const
+void InsetBibtex::collectBibKeys(InsetIterator const & /*di*/, FileNameList & checkedFiles) const
{
- parseBibTeXFiles();
+ parseBibTeXFiles(checkedFiles);
}
-void InsetBibtex::parseBibTeXFiles() const
+void InsetBibtex::parseBibTeXFiles(FileNameList & checkedFiles) const
{
// This bibtex parser is a first step to parse bibtex files
// more precisely.
@@ -678,7 +678,14 @@ void InsetBibtex::parseBibTeXFiles() const
support::FileNamePairList::const_iterator it = files.begin();
support::FileNamePairList::const_iterator en = files.end();
for (; it != en; ++ it) {
- ifdocstream ifs(it->second.toFilesystemEncoding().c_str(),
+ FileName const bibfile = it->second;
+ if (find(checkedFiles.begin(), checkedFiles.end(), bibfile) != checkedFiles.end())
+ // already checked this one. Skip.
+ continue;
+ else
+ // record that we check this.
+ checkedFiles.push_back(bibfile);
+ ifdocstream ifs(bib
Re: Fix How Buffers are Set After Undo/Redo
On 10/15/2017 11:01 PM, Richard Heck wrote:
> On 10/15/2017 10:46 PM, Richard Heck wrote:
>> In the existing codebase, there is a problem about how Insets get their
>> Buffers set after undo/redo. What we do at the moment is go through the
>> *entire Buffer* and set the Buffer for every single inset. This seems
>> like overkill. The attached patch fixes this by just setting the Buffers
>> for the newly created insets, i.e., those in the paragraphs we just pasted.
>>
>> I'm proposing this at the moment for master only, but would appreciate
>> comments first.
> Well, not so fast, I guess. I'm seeing crashes now. But something like
> this must be right.
OK, this one is better. I'm not sure I understand why I need to reset
first this way, but it seems I do.
Richard
>From efea49e939d8d20b08897b8089e659f013d4cf2e Mon Sep 17 00:00:00 2001
From: Richard Heck
Date: Sun, 15 Oct 2017 22:22:22 -0400
Subject: [PATCH 1/2] Fix the way that the Buffer gets set on undo.
Previously, we went through the entire Buffer and set it for every
single inset. Now we just do it for the insets we pasted.
---
src/Buffer.cpp | 6 --
src/Buffer.h | 4
src/Undo.cpp | 12 +---
3 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index c4ce8ea..89a76b5 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -4699,12 +4699,6 @@ void Buffer::bufferErrors(TeXErrors const & terr, ErrorList & errorList) const
}
-void Buffer::setBuffersForInsets() const
-{
- inset().setBuffer(const_cast(*this));
-}
-
-
void Buffer::updateBuffer(UpdateScope scope, UpdateType utype) const
{
LBUFERR(!text().paragraphs().empty());
diff --git a/src/Buffer.h b/src/Buffer.h
index 165f5d8..6d50e70 100644
--- a/src/Buffer.h
+++ b/src/Buffer.h
@@ -719,10 +719,6 @@ public:
/// return a list of all used branches (also in children)
void getUsedBranches(std::list &, bool const from_master = false) const;
- /// sets the buffer_ member for every inset in this buffer.
- // FIXME This really shouldn't be needed, but at the moment it's not
- // clear how to do it just for the individual pieces we need.
- void setBuffersForInsets() const;
/// Updates screen labels and some other information associated with
/// insets and paragraphs. Actually, it's more like a general "recurse
/// through the Buffer" routine, that visits all the insets and paragraphs.
diff --git a/src/Undo.cpp b/src/Undo.cpp
index a479728..31072ba 100644
--- a/src/Undo.cpp
+++ b/src/Undo.cpp
@@ -497,6 +497,15 @@ void Undo::Private::doTextUndoOrRedo(CursorData & cur, UndoElementStack & stack,
for (; pit != end; ++pit)
pit->setInsetOwner(dit.realInset());
plist.insert(first, undo.pars->begin(), undo.pars->end());
+
+ // set the buffers for insets we created
+ ParagraphList::iterator fpit = plist.begin();
+ advance(fpit, undo.from);
+ ParagraphList::iterator fend = fpit;
+ advance(fend, undo.pars->size());
+ for (; fpit != fend; ++fpit)
+ fpit->setBuffer(buffer_);
+
delete undo.pars;
undo.pars = 0;
}
@@ -531,9 +540,6 @@ bool Undo::Private::textUndoOrRedo(CursorData & cur, bool isUndoOperation)
const size_t gid = stack.top().group_id;
while (!stack.empty() && stack.top().group_id == gid)
doTextUndoOrRedo(cur, stack, otherstack);
-
- // Adapt the new material to current buffer.
- buffer_.setBuffersForInsets(); // FIXME This shouldn't be here.
return true;
}
--
2.9.5
Re: Fix How Buffers are Set After Undo/Redo
On 10/15/2017 10:46 PM, Richard Heck wrote: > In the existing codebase, there is a problem about how Insets get their > Buffers set after undo/redo. What we do at the moment is go through the > *entire Buffer* and set the Buffer for every single inset. This seems > like overkill. The attached patch fixes this by just setting the Buffers > for the newly created insets, i.e., those in the paragraphs we just pasted. > > I'm proposing this at the moment for master only, but would appreciate > comments first. Well, not so fast, I guess. I'm seeing crashes now. But something like this must be right. Richard
Fix How Buffers are Set After Undo/Redo
In the existing codebase, there is a problem about how Insets get their
Buffers set after undo/redo. What we do at the moment is go through the
*entire Buffer* and set the Buffer for every single inset. This seems
like overkill. The attached patch fixes this by just setting the Buffers
for the newly created insets, i.e., those in the paragraphs we just pasted.
I'm proposing this at the moment for master only, but would appreciate
comments first.
The second patch is just a renaming, from Paragraph::setBuffer to
Paragraph::setInsetBuffers. The former is misleading as to what the
method does. I'd like to commit this to master and 2.3.x, the latter
simply to avoid conflicts when cherry-picking.
Richard
>From 2d77bd1c027b26efcb31691bb640d124e65a63c8 Mon Sep 17 00:00:00 2001
From: Richard Heck
Date: Sun, 15 Oct 2017 22:22:22 -0400
Subject: [PATCH 1/2] Fix the way that the Buffer gets set on undo.
Previously, we went through the entire Buffer and set it for every
single inset. Now we just do it for the insets we pasted.
---
src/Buffer.cpp | 6 --
src/Buffer.h | 4
src/Undo.cpp | 15 ---
3 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index c4ce8ea..89a76b5 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -4699,12 +4699,6 @@ void Buffer::bufferErrors(TeXErrors const & terr, ErrorList & errorList) const
}
-void Buffer::setBuffersForInsets() const
-{
- inset().setBuffer(const_cast(*this));
-}
-
-
void Buffer::updateBuffer(UpdateScope scope, UpdateType utype) const
{
LBUFERR(!text().paragraphs().empty());
diff --git a/src/Buffer.h b/src/Buffer.h
index 165f5d8..6d50e70 100644
--- a/src/Buffer.h
+++ b/src/Buffer.h
@@ -719,10 +719,6 @@ public:
/// return a list of all used branches (also in children)
void getUsedBranches(std::list &, bool const from_master = false) const;
- /// sets the buffer_ member for every inset in this buffer.
- // FIXME This really shouldn't be needed, but at the moment it's not
- // clear how to do it just for the individual pieces we need.
- void setBuffersForInsets() const;
/// Updates screen labels and some other information associated with
/// insets and paragraphs. Actually, it's more like a general "recurse
/// through the Buffer" routine, that visits all the insets and paragraphs.
diff --git a/src/Undo.cpp b/src/Undo.cpp
index a479728..4798737 100644
--- a/src/Undo.cpp
+++ b/src/Undo.cpp
@@ -497,6 +497,18 @@ void Undo::Private::doTextUndoOrRedo(CursorData & cur, UndoElementStack & stack,
for (; pit != end; ++pit)
pit->setInsetOwner(dit.realInset());
plist.insert(first, undo.pars->begin(), undo.pars->end());
+
+ // set the buffers for insets we created
+ // pargraphs were inserted before first, so first paragraph
+ // inserted is first - 1
+ ParagraphList::iterator fpit = first;
+ advance(fpit, -1);
+ // one more than last paragraph we inserted
+ ParagraphList::iterator fend = fpit;
+ advance(fend, undo.pars->size());
+ for (; fpit != fend; ++fpit)
+ fpit->setBuffer(buffer_);
+
delete undo.pars;
undo.pars = 0;
}
@@ -531,9 +543,6 @@ bool Undo::Private::textUndoOrRedo(CursorData & cur, bool isUndoOperation)
const size_t gid = stack.top().group_id;
while (!stack.empty() && stack.top().group_id == gid)
doTextUndoOrRedo(cur, stack, otherstack);
-
- // Adapt the new material to current buffer.
- buffer_.setBuffersForInsets(); // FIXME This shouldn't be here.
return true;
}
--
2.9.5
>From ff21a3cc972ce8b874f8023e1afac9544a7611c3 Mon Sep 17 00:00:00 2001
From: Richard Heck
Date: Sun, 15 Oct 2017 22:14:08 -0400
Subject: [PATCH 2/2] Rename Paragraph::setBuffer to
Paragraph::setInsetBuffers, to avoid confusion about what this routine does.
---
src/Compare.cpp | 2 +-
src/CutAndPaste.cpp | 6 +++---
src/Paragraph.cpp| 2 +-
src/Paragraph.h | 2 +-
src/Undo.cpp | 2 +-
src/insets/InsetText.cpp | 2 +-
6 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/Compare.cpp b/src/Compare.cpp
index 796eb19..214f25c 100644
--- a/src/Compare.cpp
+++ b/src/Compare.cpp
@@ -736,7 +736,7 @@ bool Compare::Impl::diff(Buffer const * new_buf, Buffer const * old_buf,
diff_i(rp_new);
for (pit_type p = 0; p < (pit_type)dest_pars_->size(); ++p) {
- (*dest_pars_)[p].setBuffer(const_cast(*dest_buf));
+ (*dest_pars_)[p].setInsetBuffers(const_cast(*dest_buf));
(*dest_pars_)[p].setInsetOwner(&dest_buf_->inset());
}
diff --git a/src/CutAndPaste.cpp b/src/CutAndPaste.cpp
index 05f9b90..b649d8f 100644
--- a/src/CutAndPaste.cpp
+++ b/src/CutAndPaste.cpp
@@ -422,7 +422,7 @@ pasteSelectionHelper(DocIterator const & cur, ParagraphList const & parlist,
// Set paragraph buffers. It's important to do this right away
// before something calls Inset::buffer() and causes a crash.
for (pit_type p = startpit; p <= pit; ++p)
- pars[p].setBuffer(const_cast(buffer));
+ pars[p].setInsetBuffers(const_cast(buffer));
Re: Beta1 is slow on undo
On 10/15/2017 06:52 PM, Scott Kostyshak wrote: > On Sun, Oct 15, 2017 at 10:41:52PM +, Richard Heck wrote: >> On 10/15/2017 03:02 PM, Scott Kostyshak wrote: >>> On Sun, Oct 15, 2017 at 05:27:26PM +, Richard Heck wrote: >>> I propose to revert that commit in all branches pending a better solution. I don't think that will be too difficult to find. It's just a matter of doing the work. >>> Reverting in 2.2.x makes sense to me to be extra safe. As for 2.3.x, I >>> don't have a strong preference. It's up to you to decide the tradeoff >>> between the cost of a small delay and the cost of #9185 (both costs seem >>> small to me). From what I understand, the delay in undo only comes when >>> a BibTeX inset is involved, which I imagine is not very often. >> No, it comes whenever a document contains a BibTeX inset. A better patch >> would make it happen >> only when a BibTeX inset was involved. Then the delay is no surprise: Of >> course we have to reread >> the files. > Ah I see. I agree then that reverting in the other branches as well is a > good idea. I have reverted all of them and added some comments to the bug, which is actually #9158 https://www.lyx.org/trac/ticket/9158 Richard
Re: compilation of LyX 2.3 fails with Python 3.6.2
El 15.10.2017 a las 23:07, Kornel Benko escribió: As I think I proposed in the attached part in other mail. Essentially, there is the change of open(file) to codecs.open(file, 'r', 'UTF-8') Thanks Kornel, unfortunately this does not work. Changing in ReplaceValues.py for line in open(InFile) into for line in codecs.open(InFile, 'r', 'UTF-8'): still gives me: File "C:\Program Files (x86)\Python35-32\lib\encodings\cp1252.py", line 19, in encode return codecs.charmap_encode(input,self.errors,encoding_table)[0] UnicodeEncodeError: 'charmap' codec can't encode character '\u014d' in position 1: character maps to regards Uwe
Re: Beta1 is slow on undo
On Sun, Oct 15, 2017 at 10:41:52PM +, Richard Heck wrote: > On 10/15/2017 03:02 PM, Scott Kostyshak wrote: > > On Sun, Oct 15, 2017 at 05:27:26PM +, Richard Heck wrote: > > > >> I propose to revert that commit in all branches pending a better > >> solution. I don't think that will be > >> too difficult to find. It's just a matter of doing the work. > > Reverting in 2.2.x makes sense to me to be extra safe. As for 2.3.x, I > > don't have a strong preference. It's up to you to decide the tradeoff > > between the cost of a small delay and the cost of #9185 (both costs seem > > small to me). From what I understand, the delay in undo only comes when > > a BibTeX inset is involved, which I imagine is not very often. > > No, it comes whenever a document contains a BibTeX inset. A better patch > would make it happen > only when a BibTeX inset was involved. Then the delay is no surprise: Of > course we have to reread > the files. Ah I see. I agree then that reverting in the other branches as well is a good idea. Scott signature.asc Description: PGP signature
Re: Beta1 is slow on undo
On 10/15/2017 03:02 PM, Scott Kostyshak wrote: > On Sun, Oct 15, 2017 at 05:27:26PM +, Richard Heck wrote: > >> I propose to revert that commit in all branches pending a better >> solution. I don't think that will be >> too difficult to find. It's just a matter of doing the work. > Reverting in 2.2.x makes sense to me to be extra safe. As for 2.3.x, I > don't have a strong preference. It's up to you to decide the tradeoff > between the cost of a small delay and the cost of #9185 (both costs seem > small to me). From what I understand, the delay in undo only comes when > a BibTeX inset is involved, which I imagine is not very often. No, it comes whenever a document contains a BibTeX inset. A better patch would make it happen only when a BibTeX inset was involved. Then the delay is no surprise: Of course we have to reread the files. Richard
Re: compilation of LyX 2.3 fails with Python 3.6.2
Am Sonntag, 15. Oktober 2017 um 19:38:45, schrieb Uwe Stöhr > El 14.10.2017 a las 21:09, Kornel Benko escribió: > > > What is the exact directory name of Additional.lyx on your platform? > > D:\LyXGit\2.3.x\lib\doc > (like all other doc files) > > > Check also \origin part of Additional.lyx. > > it is correctly this: > \origin /systemlyxdir/doc/ > > > Can it be that python looks for wrong encoding (e.g. cp1252) which does > > _not_ contain 0x9d char. > > I explicitly saved the file now using UTF8 encoding. But since Python > uses nevertheless CP1252 I need a method how to force Python to use > utf8. How can I do this on the file ReplaceValues.py ? > > That is definitely the bug: Python expects CP1252 while it is not. For > example In additional.lyx I get this error when saving in CP1252: > > D:\LyXGit\2.3.x\lib\doc\Additional.lyx: > Cannot save: java.io.CharConversionException: Failed to encode the character > '�' (U+FFFD) at column 4 in line 24002 with the encoding "Cp1252". > > This is a quate character which does not exists in CP1252. When I delete > this character ReplaceValues.py works fine. > We have many more uft8-only chars in our other doc files, so the only > solution is to force Python to expect uft8. As I think I proposed in the attached part in other mail. Essentially, there is the change of open(file) to codecs.open(file, 'r', 'UTF-8') > thanks and regards > Uwe Kornel signature.asc Description: This is a digitally signed message part.
Re: Colour schemes
On 16/10/2017 5:22 a.m., Richard Heck wrote: On 10/14/2017 05:15 PM, Andrew Parsloe wrote: Recent eye trouble made it difficult to read black text on a white-ish screen and I found myself exploring colour schemes with dark backgrounds. There are some examples on the LyX wiki at /Tips/ColorSchemes. White text against a blue background has proved helpful. But there is no built-in way to invoke such schemes. The user should not be asked to change all the colour settings displayed at Tools > Preferences > Look & Feel > Colors, and copying from the wiki and pasting into preferences (which obviously is what I did) means ignoring the warning at the start of the preferences file, "If you want to make your own modifications you should do them from inside LyX and save". Could a drop-down list box with the names of schemes be added to the dialogue at Tools > Preferences > Look & Feel > Colors? I understand the reluctance to ship LyX with half-baked attempts, so the box would initially be populated only with the standard scheme that comes with LyX. But perhaps it could search the user's ui folder for files with some standard extension like .scheme and add those to the list (after reconfiguring). The user would then be able to test them at a click (to the Apply button), tweak them as desired, and save them back to that folder. LyX itself wouldn't be responsible for such schemes but in time, given a facility like this allowing easy testing, tweaking and saving, a scheme might mature to the point where it could be considered for distribution with LyX. In particular, it would be good for LyX to ship with at least one dark background scheme. This seems like a reasonable request, and probably not one that is very difficult to implement. But it's certainly too late for 2.3.0. I'd suggest filing an enhancement request. Please cc me. Richard Thank you for the interest. Ticket #10774. It's not something that has concerned me before but it has become relevant. Andrew --- This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus
Re: Beta1 is slow on undo
On Sun, Oct 15, 2017 at 05:27:26PM +, Richard Heck wrote: > I propose to revert that commit in all branches pending a better > solution. I don't think that will be > too difficult to find. It's just a matter of doing the work. Reverting in 2.2.x makes sense to me to be extra safe. As for 2.3.x, I don't have a strong preference. It's up to you to decide the tradeoff between the cost of a small delay and the cost of #9185 (both costs seem small to me). From what I understand, the delay in undo only comes when a BibTeX inset is involved, which I imagine is not very often. Thanks, Scott signature.asc Description: PGP signature
Re: compilation of LyX 2.3 fails with Python 3.6.2
El 14.10.2017 a las 21:09, Kornel Benko escribió: What is the exact directory name of Additional.lyx on your platform? D:\LyXGit\2.3.x\lib\doc (like all other doc files) Check also \origin part of Additional.lyx. it is correctly this: \origin /systemlyxdir/doc/ Can it be that python looks for wrong encoding (e.g. cp1252) which does _not_ contain 0x9d char. I explicitly saved the file now using UTF8 encoding. But since Python uses nevertheless CP1252 I need a method how to force Python to use utf8. How can I do this on the file ReplaceValues.py ? That is definitely the bug: Python expects CP1252 while it is not. For example In additional.lyx I get this error when saving in CP1252: D:\LyXGit\2.3.x\lib\doc\Additional.lyx: Cannot save: java.io.CharConversionException: Failed to encode the character '�' (U+FFFD) at column 4 in line 24002 with the encoding "Cp1252". This is a quate character which does not exists in CP1252. When I delete this character ReplaceValues.py works fine. We have many more uft8-only chars in our other doc files, so the only solution is to force Python to expect uft8. thanks and regards Uwe
Re: Beta1 is slow on undo
On 10/15/2017 12:58 PM, Scott Kostyshak wrote: > On Sun, Oct 08, 2017 at 02:19:19PM +, racoon wrote: >> On 08.10.2017 00:19, Scott Kostyshak wrote: >>> On Sat, Oct 07, 2017 at 05:57:48PM +, Richard Heck wrote: On 10/07/2017 12:09 PM, racoon wrote: > Hi, > > I noticed that beta1 shows some slowness when undoing with some of my > documents. While undoing is instantaneous in 2.2.3, 2.3 takes about a > second to respond to it. > > Did anyone notice this? I don't have a very fast computer though. Yes, I have also seen that, and my machines are pretty fast. >>> I can't reproduce. My test case was to open the user guide, selected >>> all, delete, and undo. Both were instantaneous for me. Do you have a >>> better test case for me? >> Okay, I think I have narrowed it down to the inclusion of any bibliography. >> Test case attached. > Uwe, can you check the file that racoon attached at [1] to see if you > can reproduce. I can reproduce but only with a very small delay, not the > one second delay racoon found. So perhaps it is platform-dependent. To > reproduce: > > 1. Open the file at [1]. > 2. Delete the BibTeX inset. > 3. Undo. > > Right after (3) do you see a small delay, compared with when you do the > above on 2.2.3? I am confident that Scott's identification of 02847641 as the culprit is correct. That forces a re-read of all BibTeX files on every undo or redo. How long that takes will depend upon how big the files are, how fast the disk access is, etc. I propose to revert that commit in all branches pending a better solution. I don't think that will be too difficult to find. It's just a matter of doing the work. Richard
Re: [LyX/2.2.x] Fix bug #8782.
On 10/15/2017 12:52 PM, Scott Kostyshak wrote: > On Sun, Oct 15, 2017 at 04:17:21PM +, Richard Heck wrote: >> commit de2a58d986739bccb83615e8515d715ebf1b5821 >> Author: Richard Heck >> Date: Thu Jun 30 14:53:19 2016 -0400 >> >> Fix bug #8782. >> >> We need to invalidate the BibTeX cache when undoing or redoing. I do >> not like having to do it for every undo or redo. We should only have >> to do it if we restored or deleted an InsetBibTeX. But there is no >> way, so far as I can see, to do it that way. I tried. >> >> (cherry picked from commit 02847641a8b97a57d1866120bb446427e0bda7a4) >> --- > This is the commit that bisect lead me to when experiencing a very > slight delay after an undo [1]. The delay is very small for me, but we > are not sure whether this commit is also why racoon experiences a > one-second delay. Perhaps this issue is important enough that we would > accept a one-second delay. I just wanted to mention this though. > > I will CC Uwe on the main thread [1]. Hopefully he can reproduce the > issue racoon sees so we can figure out the root problem. Oh, yes! Sorry, I missed that other message. This would do it, and it is exactly the kind of change I was thinking might cause this. It does force a re-read of the BibTeX files, so how long a delay you see will depend upon how big the files are, etc. Richard
Re: Beta1 is slow on undo
On Sun, Oct 08, 2017 at 02:19:19PM +, racoon wrote: > On 08.10.2017 00:19, Scott Kostyshak wrote: > > On Sat, Oct 07, 2017 at 05:57:48PM +, Richard Heck wrote: > > > On 10/07/2017 12:09 PM, racoon wrote: > > > > Hi, > > > > > > > > I noticed that beta1 shows some slowness when undoing with some of my > > > > documents. While undoing is instantaneous in 2.2.3, 2.3 takes about a > > > > second to respond to it. > > > > > > > > Did anyone notice this? I don't have a very fast computer though. > > > > > > Yes, I have also seen that, and my machines are pretty fast. > > > > I can't reproduce. My test case was to open the user guide, selected > > all, delete, and undo. Both were instantaneous for me. Do you have a > > better test case for me? > > Okay, I think I have narrowed it down to the inclusion of any bibliography. > Test case attached. Uwe, can you check the file that racoon attached at [1] to see if you can reproduce. I can reproduce but only with a very small delay, not the one second delay racoon found. So perhaps it is platform-dependent. To reproduce: 1. Open the file at [1]. 2. Delete the BibTeX inset. 3. Undo. Right after (3) do you see a small delay, compared with when you do the above on 2.2.3? Scott [1] https://www.mail-archive.com/search?l=mid&q=ordc32%24s9j%241%40blaine.gmane.org signature.asc Description: PGP signature
Re: Beta1 is slow on undo
On Wed, Oct 11, 2017 at 03:35:34PM +, Richard Heck wrote: > On 10/08/2017 10:19 AM, racoon wrote: > > On 08.10.2017 00:19, Scott Kostyshak wrote: > >> On Sat, Oct 07, 2017 at 05:57:48PM +, Richard Heck wrote: > >>> On 10/07/2017 12:09 PM, racoon wrote: > Hi, > > I noticed that beta1 shows some slowness when undoing with some of my > documents. While undoing is instantaneous in 2.2.3, 2.3 takes about a > second to respond to it. > > Did anyone notice this? I don't have a very fast computer though. > >>> > >>> Yes, I have also seen that, and my machines are pretty fast. > >> > >> I can't reproduce. My test case was to open the user guide, selected > >> all, delete, and undo. Both were instantaneous for me. Do you have a > >> better test case for me? > > > > Okay, I think I have narrowed it down to the inclusion of any > > bibliography. Test case attached. > > In investigating this, I typed a bunch of short lines: a, b, c, d, etc, > all alone on their own lines. I did not see slowness on undo until I had > typed quite a few, and then suddenly there it was. But then it was gone > again. I'll try again later to figure out how to reproduce it reliably. > > What is clear is that undo is a lot slower when entire lines are being > removed or replaced, as opposed merely to part of a single line, > especially the last character being restored. I would guess this is > because we do a full screen redraw in the slower case. Richard, is the above slowness a change with respect to 2.2.x? Scott signature.asc Description: PGP signature
Re: [LyX/2.2.x] Fix bug #8782.
On Sun, Oct 15, 2017 at 04:17:21PM +, Richard Heck wrote: > commit de2a58d986739bccb83615e8515d715ebf1b5821 > Author: Richard Heck > Date: Thu Jun 30 14:53:19 2016 -0400 > > Fix bug #8782. > > We need to invalidate the BibTeX cache when undoing or redoing. I do > not like having to do it for every undo or redo. We should only have > to do it if we restored or deleted an InsetBibTeX. But there is no > way, so far as I can see, to do it that way. I tried. > > (cherry picked from commit 02847641a8b97a57d1866120bb446427e0bda7a4) > --- This is the commit that bisect lead me to when experiencing a very slight delay after an undo [1]. The delay is very small for me, but we are not sure whether this commit is also why racoon experiences a one-second delay. Perhaps this issue is important enough that we would accept a one-second delay. I just wanted to mention this though. I will CC Uwe on the main thread [1]. Hopefully he can reproduce the issue racoon sees so we can figure out the root problem. Scott [1] https://www.mail-archive.com/search?l=mid&q=20171009052835.23srn2675xenshtu%40steph signature.asc Description: PGP signature
Re: Colour schemes
On 10/14/2017 05:15 PM, Andrew Parsloe wrote: > Recent eye trouble made it difficult to read black text on a white-ish > screen and I found myself exploring colour schemes with dark > backgrounds. There are some examples on the LyX wiki at > /Tips/ColorSchemes. White text against a blue background has proved > helpful. > > But there is no built-in way to invoke such schemes. The user should > not be asked to change all the colour settings displayed at Tools > > Preferences > Look & Feel > Colors, and copying from the wiki and > pasting into preferences (which obviously is what I did) means > ignoring the warning at the start of the preferences file, "If you > want to make your own modifications you should do them from inside LyX > and save". > > Could a drop-down list box with the names of schemes be added to the > dialogue at Tools > Preferences > Look & Feel > Colors? I understand > the reluctance to ship LyX with half-baked attempts, so the box would > initially be populated only with the standard scheme that comes with > LyX. But perhaps it could search the user's ui folder for files with > some standard extension like .scheme and add those to the list (after > reconfiguring). The user would then be able to test them at a click > (to the Apply button), tweak them as desired, and save them back to > that folder. LyX itself wouldn't be responsible for such schemes but > in time, given a facility like this allowing easy testing, tweaking > and saving, a scheme might mature to the point where it could be > considered for distribution with LyX. In particular, it would be good > for LyX to ship with at least one dark background scheme. This seems like a reasonable request, and probably not one that is very difficult to implement. But it's certainly too late for 2.3.0. I'd suggest filing an enhancement request. Please cc me. Richard
Re: Update on the 2.3.0rc1 situation
On Sun, Oct 15, 2017 at 11:35:21AM +0200, Jürgen Spitzmüller wrote: > Am Sonntag, den 15.10.2017, 10:52 +0200 schrieb Enrico Forestieri: > > I have done some more experiments and I think I found a more > > reasonable > > explanation of what I still think might be a compiler bug. Seemingly, > > when > > the first parameter passed to regex_match() is afterward changed, the > > second > > one (holding the matches) gets corrupted. According to this > > explanation, > > I devised the attached patch that still fixes the issue for me. > > Yes, this looks better. +1 from me. Thanks, I committed it. -- Enrico
Re: Update on the 2.3.0rc1 situation
Am Sonntag, den 15.10.2017, 10:52 +0200 schrieb Enrico Forestieri: > I have done some more experiments and I think I found a more > reasonable > explanation of what I still think might be a compiler bug. Seemingly, > when > the first parameter passed to regex_match() is afterward changed, the > second > one (holding the matches) gets corrupted. According to this > explanation, > I devised the attached patch that still fixes the issue for me. Yes, this looks better. +1 from me. Jürgen signature.asc Description: This is a digitally signed message part
Re: Update on the 2.3.0rc1 situation
On Sat, Oct 14, 2017 at 09:43:50PM +0200, Pavel Sanda wrote: > Enrico Forestieri wrote: > > This is with gcc 6.4.0. Maybe other versions don't have the issue. > > If you disable compiler optimization is the bug still there? P Yes, it does not seem to depend on the optimization level. -- Enrico
Re: Update on the 2.3.0rc1 situation
On Sat, Oct 14, 2017 at 03:39:47PM -0400, Richard Heck wrote:
> On 10/14/2017 01:51 PM, Enrico Forestieri wrote:
> > On Sat, Oct 14, 2017 at 07:30:29PM +0200, Jürgen Spitzmüller wrote:
> >> Am Samstag, den 14.10.2017, 19:15 +0200 schrieb Enrico Forestieri:
> >>> On Sat, Oct 14, 2017 at 06:51:31PM +0200, Jürgen Spitzmüller wrote:
> What happens if you do
>
> LYXERR0("test 1: " << sub.str(5));
> string const test = sub.str(5);
> LYXERR0("test 2: " << test);
> >>> The result is:
> >>>
> >>> BiblioInfo.cpp (252): test 1: %surname%
> >>> BiblioInfo.cpp (254): test 2: %surname%
> >>>
> >>> but later I get again "%prename% %surnamP". However, the attached
> >>> patch
> >>> seems to fix this issue, even if it is a mystery why it does.
> >>> This must be a compiler bug.
> >> Mysterious indeed. However, if it really fixes the bug, commit it (with
> >> a comment explaining why we use this temporary string here). After all,
> >> this change cannot harm.
> >>
> >> (But of course it would be better if we knew what is going on here).
> > Note that this seems to be very fragile. For example, moving the
> > line defining the match5 variable down, it does not work anymore.
> > It is as if the various matches must be accessed in reverse order,
> > otherwise the last one gets corrupted.
> >
> > There are three instances where this is done, so a more exhaustive
> > patch would be the attached one, which still fixes the issue for me.
> >
> > This is with gcc 6.4.0. Maybe other versions don't have the issue.
>
> Very, very weird. As you said, must be a compiler bug. Nice work to find
> a workaround.
>
> I'll give +1 to commit it. Can't possibly do any harm, as Jürgen said
> first
I have done some more experiments and I think I found a more reasonable
explanation of what I still think might be a compiler bug. Seemingly, when
the first parameter passed to regex_match() is afterward changed, the second
one (holding the matches) gets corrupted. According to this explanation,
I devised the attached patch that still fixes the issue for me.
--
Enrico
diff --git a/src/BiblioInfo.cpp b/src/BiblioInfo.cpp
index 87245d3824..bb7c64cda2 100644
--- a/src/BiblioInfo.cpp
+++ b/src/BiblioInfo.cpp
@@ -236,6 +236,8 @@ docstring constructName(docstring const & name, string
const scheme)
static regex const
reg2("(.*)(\\{%suffix%\\[\\[)([^\\]]+)(\\]\\]\\})(.*)");
static regex const
reg3("(.*)(\\{%prefix%\\[\\[)([^\\]]+)(\\]\\]\\})(.*)");
smatch sub;
+ // Changing the first parameter of regex_match() may corrupt the
+ // second one. In this case we use the temporary string tmp.
if (regex_match(scheme, sub, reg1)) {
res = sub.str(1);
if (!prename.empty())
@@ -243,16 +245,16 @@ docstring constructName(docstring const & name, string
const scheme)
res += sub.str(5);
}
if (regex_match(res, sub, reg2)) {
- res = sub.str(1);
+ string tmp = sub.str(1);
if (!suffix.empty())
- res += sub.str(3);
- res += sub.str(5);
+ tmp += sub.str(3);
+ res = tmp + sub.str(5);
}
if (regex_match(res, sub, reg3)) {
- res = sub.str(1);
+ string tmp = sub.str(1);
if (!prefix.empty())
- res += sub.str(3);
- res += sub.str(5);
+ tmp += sub.str(3);
+ res = tmp + sub.str(5);
}
docstring result = from_ascii(res);
result = subst(result, from_ascii("%prename%"), prename);
Re: LyX-Workarea: Background not shown correctly
I’ve done some more debugging and I believe I’ve found the Qt code that turns the background black or white. Om macos (and probably also on windows) Qt uses a QBackingStore for painting. I couldn’t find a QPlatformBackingStore for linux so I assume the implementation is different on linux, which would explain the different behaviour. When issuing an viewport()->update() the region (the complete viewport if no smaller area is specified) is marked as dirty and at paint time is then repainted, via QWidgetBackingStore::doSync(). In QRasterBackingStore::beginPaint() the region is initially filled with a color Qt::transparent which has alpha=r=g=b=0 which will leave the screen black if not overwritten. When WA_OpaquePaintEvent is not set this is followed by a paintBackground() call which fills the region with the autoFillBrush-color which has all ones. This will leave the screen white if not overwritten. Depending on the update_strategy_ only a part of the screen is overwritten by lyx, leaving the rest black or white. I assume that in the tests I’ve described previously all actions restoring the normal view somehow trigger a FullScreenUpdate. pdv > On 12 Oct 2017, at 21:30, Jean-Marc Lasgouttes wrote: > > Le 08/10/2017 à 18:57, pdv a écrit : >> The new code contains twice a viewport()->update() without specification of >> a rectangle. Since > Qt normally erases the widget's area before the >> paintEvent() call >> I guess that the whole viewport is erased. Subsequently only part of it is >> redrawn except for the case FullScreenUpdate and in that case there is no >> problem. > > Actually, we use > viewport()->setAttribute(Qt::WA_OpaquePaintEvent); > to ask Qt not to overwrite it. It seems to work on Linux. > > Please feel free to look at the code and docs (QWidget and > QAbstractScrollArea) and tell us what I may have missed. I just saw that we > should override viewportEvent() instead of event(), but I am not sure that it > would make a difference. > > Can you compile master? Do you see the issue? > > JMarc smime.p7s Description: S/MIME cryptographic signature
