There was a bit too much copying of dociterators gpoing on leading to an

avoidable ~5% overhead when loading the UserGuide. This is an attempt on
rectifying the situation.


git-svn-id: svn://svn.lyx.org/lyx/lyx-devel/trunk@22532 a592a061-630c-0410-9148-cb99ea01b6c8

src/Buffer.cpp

@@ -948,7 +948,6 @@ bool Buffer::write(ostream & ofs) const
 	    << "\\lyxformat " << LYX_FORMAT << "\n"
 	    << "\\begin_document\n";
 
-
 	/// For each author, set 'used' to true if there is a change
 	/// by this author in the document; otherwise set it to 'false'.
 	AuthorList::Authors::const_iterator a_it = params().authors().begin();
@@ -956,8 +955,8 @@ bool Buffer::write(ostream & ofs) const
 	for (; a_it != a_end; ++a_it)
 		a_it->second.setUsed(false);
 
-	ParIterator const end = par_iterator_end();
-	ParIterator it = par_iterator_begin();
+	ParIterator const end = const_cast<Buffer *>(this)->par_iterator_end();
+	ParIterator it = const_cast<Buffer *>(this)->par_iterator_begin();
 	for ( ; it != end; ++it)
 		it->checkAuthors(params().authors());
 
@@ -1487,7 +1486,7 @@ bool Buffer::isMultiLingual() const
 }
 
 
-ParIterator Buffer::getParFromID(int const id) const
+ParConstIterator Buffer::getParFromID(int const id) const
 {
 	ParConstIterator it = par_iterator_begin();
 	ParConstIterator const end = par_iterator_end();
@@ -1506,6 +1505,25 @@ ParIterator Buffer::getParFromID(int const id) const
 }
 
 
+ParIterator Buffer::getParFromID(int const id)
+{
+	ParIterator it = par_iterator_begin();
+	ParIterator const end = par_iterator_end();
+
+	if (id < 0) {
+		// John says this is called with id == -1 from undo
+		lyxerr << "getParFromID(), id: " << id << endl;
+		return end;
+	}
+
+	for (; it != end; ++it)
+		if (it->id() == id)
+			return it;
+
+	return end;
+}
+
+
 bool Buffer::hasParWithID(int const id) const
 {
 	ParConstIterator const it = getParFromID(id);
@@ -1989,9 +2007,10 @@ void Buffer::updateMacros() const
 
 void Buffer::updateMacroInstances() const
 {
-	LYXERR(Debug::MACROS, "updateMacroInstances for " << d->filename.onlyFileName());
-	ParIterator it = par_iterator_begin();
-	ParIterator end = par_iterator_end();
+	LYXERR(Debug::MACROS, "updateMacroInstances for "
+		<< d->filename.onlyFileName());
+	ParConstIterator it = par_iterator_begin();
+	ParConstIterator end = par_iterator_end();
 	for (; it != end; it.forwardPos()) {
 		// look for MathData cells in InsetMathNest insets
 		Inset * inset = it.nextInset();

src/Buffer.h

@@ -143,7 +143,9 @@ public:
 		pit_type &, pos_type &,
 		Font const &, docstring const &, bool);
 	///
-	ParIterator getParFromID(int id) const;
+	ParIterator getParFromID(int id);
+	///
+	ParConstIterator getParFromID(int id) const;
 	/// do we have a paragraph with this id?
 	bool hasParWithID(int id) const;
 

src/Cursor.cpp

@@ -14,25 +14,26 @@
 #include <config.h>
 
 #include "Bidi.h"
-#include "BufferView.h"
 #include "Buffer.h"
-#include "Cursor.h"
+#include "BufferView.h"
 #include "CoordCache.h"
+#include "Cursor.h"
 #include "CutAndPaste.h"
 #include "DispatchResult.h"
 #include "Encoding.h"
+#include "Font.h"
 #include "FuncRequest.h"
 #include "Language.h"
 #include "lfuns.h"
-#include "Font.h"
 #include "LyXFunc.h" // only for setMessage()
 #include "LyXRC.h"
+#include "paragraph_funcs.h"
+#include "Paragraph.h"
+#include "ParIterator.h"
 #include "Row.h"
 #include "Text.h"
-#include "Paragraph.h"
-#include "paragraph_funcs.h"
-#include "ParIterator.h"
 #include "TextMetrics.h"
+#include "TocBackend.h"
 
 #include "support/debug.h"
 #include "support/docstream.h"
@@ -1639,4 +1640,14 @@ void Cursor::recordUndoSelection()
 }
 
 
+void Cursor::checkBufferStructure()
+{
+	if (paragraph().layout()->toclevel == Layout::NOT_IN_TOC)
+		return;
+	Buffer const * master = buffer().masterBuffer();
+	master->tocBackend().updateItem(ParConstIterator(*this));
+	master->structureChanged();
+}
+
+
 } // namespace lyx

src/Cursor.h

@@ -217,6 +217,9 @@ public:
 	/// Convenience: prepare undo for the selected paragraphs
 	void recordUndoSelection();
 
+	///
+	void checkBufferStructure();
+
 public:
 	///
 	BufferView * bv_;

src/ParIterator.cpp

@@ -104,18 +104,6 @@ ParagraphList & ParIterator::plist() const
 }
 
 
-bool operator==(ParIterator const & iter1, ParIterator const & iter2)
-{
-	return DocIterator(iter1) == DocIterator(iter2);
-}
-
-
-bool operator!=(ParIterator const & iter1, ParIterator const & iter2)
-{
-	return !(iter1 == iter2);
-}
-
-
 DocIterator makeDocIterator(ParIterator const & par, pos_type pos)
 {
 	DocIterator dit(par);
@@ -164,9 +152,10 @@ ParagraphList const & ParConstIterator::plist() const
 	return text()->paragraphs();
 }
 
-
+#if 0
 bool operator==(ParConstIterator const & iter1, ParConstIterator const & iter2)
 {
+	// FIXME: this makes two full copies!
 	return DocIterator(iter1) == DocIterator(iter2);
 }
 
@@ -175,6 +164,7 @@ bool operator!=(ParConstIterator const & iter1, ParConstIterator const & iter2)
 {
 	return !(iter1 == iter2);
 }
+#endif
 
 
 // FIXME: const correctness!

src/ParIterator.h

@@ -45,7 +45,7 @@ public:
 	///
 	ParIterator(ParIterator const &);
 	///
-	ParIterator(DocIterator const &);
+	explicit ParIterator(DocIterator const &);
 
 	/// This really should be implemented...
 	//ParIterator & operator=(ParIterator const &);
@@ -79,15 +79,15 @@ ParIterator par_iterator_end(Inset & inset);
 
 
 ///
-bool operator==(ParIterator const & iter1, ParIterator const & iter2);
+//bool operator==(ParIterator const & it1, ParIterator const & it2);
 
-// FIXME: Unfortunately operator!=(ParIterator &, ParIterator &) is implemented with
-// operator!=(DocIterator &, DocIterator &) that gives false if the positions
-// are different, even if the pars are the same. So ultimately it's a bug in
-// operator!=(ParIterator &, ParIterator &) I'd say (nevertheless, I would be
-// reluctant to change it, because I fear that some part of the code could rely on
-// this "bug". --Alfredo
-bool operator!=(ParIterator const & iter1, ParIterator const & iter2);
+// FIXME: Unfortunately operator!=(ParIterator &, ParIterator &) is
+// implemented with operator!=(DocIterator &, DocIterator &) that gives
+// false if the positions are different, even if the pars are the same.
+// So ultimately it's a bug in operator!=(ParIterator &, ParIterator &)
+// I'd say (nevertheless, I would be reluctant to change it, because I
+// fear that some part of the code could rely on this "bug". --Alfredo
+//bool operator!=(ParIterator const & it1, ParIterator const & it2);
 
 
 class ParConstIterator : public std::iterator<std::forward_iterator_tag,
@@ -96,11 +96,11 @@ class ParConstIterator : public std::iterator<std::forward_iterator_tag,
 {
 public:
 	///
-	ParConstIterator(): DocIterator() {}
+	ParConstIterator() : DocIterator() {}
 	///
 	ParConstIterator(ParConstIterator const &);
 	///
-	ParConstIterator(DocIterator const &);
+	explicit ParConstIterator(DocIterator const &);
 	///
 
 	ParConstIterator & operator++();
@@ -114,11 +114,9 @@ public:
 	ParagraphList const & plist() const;
 };
 
-bool operator==(ParConstIterator const & iter1,
-		ParConstIterator const & iter2);
+//bool operator==(ParConstIterator const & it1, ParConstIterator const & it2);
 
-bool operator!=(ParConstIterator const & iter1,
-		ParConstIterator const & iter2);
+//bool operator!=(ParConstIterator const & it1, ParConstIterator const & it2);
 
 
 ParConstIterator par_const_iterator_begin(Inset const & inset);

src/Paragraph.cpp

@@ -250,15 +250,13 @@ bool Paragraph::isChanged(pos_type start, pos_type end) const
 }
 
 
-bool Paragraph::isMergedOnEndOfParDeletion(bool trackChanges) const {
+bool Paragraph::isMergedOnEndOfParDeletion(bool trackChanges) const
+{
 	// keep the logic here in sync with the logic of eraseChars()
-
-	if (!trackChanges) {
+	if (!trackChanges)
 		return true;
-	}
-
-	Change change = d->changes_.lookup(size());
 
+	Change const change = d->changes_.lookup(size());
 	return change.type == Change::INSERTED && change.author == 0;
 }
 
@@ -292,22 +290,17 @@ void Paragraph::setChange(Change const & change)
 void Paragraph::setChange(pos_type pos, Change const & change)
 {
 	BOOST_ASSERT(pos >= 0 && pos <= size());
-
 	d->changes_.set(change, pos);
 
 	// see comment in setChange(Change const &) above
-
-	if (change.type != Change::DELETED &&
-	    pos < size() && isInset(pos)) {
+	if (change.type != Change::DELETED && pos < size() && isInset(pos))
 		getInset(pos)->setChange(change);
-	}
 }
 
 
 Change const & Paragraph::lookupChange(pos_type pos) const
 {
 	BOOST_ASSERT(pos >= 0 && pos <= size());
-
 	return d->changes_.lookup(pos);
 }
 

src/Text.cpp

@@ -543,7 +543,7 @@ void Text::insertChar(Cursor & cur, char_type c)
 	}
 
 	par.insertChar(cur.pos(), c, cur.current_font, cur.buffer().params().trackChanges);
-	checkBufferStructure(cur.buffer(), cur);
+	cur.checkBufferStructure();
 
 //		cur.updateFlags(Update::Force);
 	setCursor(cur.top(), cur.pit(), cur.pos() + 1);
@@ -795,7 +795,7 @@ void Text::deleteWordForward(Cursor & cur)
 		cursorForwardOneWord(cur);
 		cur.setSelection();
 		cutSelection(cur, true, false);
-		checkBufferStructure(cur.buffer(), cur);
+		cur.checkBufferStructure();
 	}
 }
 
@@ -811,7 +811,7 @@ void Text::deleteWordBackward(Cursor & cur)
 		cursorBackwardOneWord(cur);
 		cur.setSelection();
 		cutSelection(cur, true, false);
-		checkBufferStructure(cur.buffer(), cur);
+		cur.checkBufferStructure();
 	}
 }
 
@@ -855,7 +855,7 @@ void Text::changeCase(Cursor & cur, TextCase action)
 	setCursor(cur, endPit, right);
 	cur.setSelection();
 
-	checkBufferStructure(cur.buffer(), cur);
+	cur.checkBufferStructure();
 }
 
 
@@ -905,7 +905,7 @@ bool Text::erase(Cursor & cur)
 			// the character has been logically deleted only => skip it
 			cur.top().forwardPos();
 		}
-		checkBufferStructure(cur.buffer(), cur);
+		cur.checkBufferStructure();
 		needsUpdate = true;
 	} else {
 		if (cur.pit() == cur.lastpit())
@@ -927,7 +927,7 @@ bool Text::erase(Cursor & cur)
 		// Make sure the cursor is correct. Is this really needed?
 		// No, not really... at least not here!
 		cur.text()->setCursor(cur.top(), cur.pit(), cur.pos());
-		checkBufferStructure(cur.buffer(), cur);
+		cur.checkBufferStructure();
 	}
 
 	return needsUpdate;
@@ -1015,7 +1015,7 @@ bool Text::backspace(Cursor & cur)
 		setCursorIntern(cur, cur.pit(), cur.pos() - 1,
 				false, cur.boundary());
 		cur.paragraph().eraseChar(cur.pos(), cur.buffer().params().trackChanges);
-		checkBufferStructure(cur.buffer(), cur);
+		cur.checkBufferStructure();
 	}
 
 	if (cur.pos() == cur.lastpos())
@@ -1384,7 +1384,7 @@ void Text::charsTranspose(Cursor & cur)
 	par.insertChar(pos1, char2, font2, trackChanges);
 	par.insertChar(pos2, char1, font1, trackChanges);
 
-	checkBufferStructure(cur.buffer(), cur);
+	cur.checkBufferStructure();
 
 	// After the transposition, move cursor to after the transposition.
 	setCursor(cur, cur.pit(), pos2);

src/TextMetrics.cpp

@@ -1641,7 +1641,7 @@ void TextMetrics::deleteLineForward(Cursor & cur)
 			text_->deleteWordForward(cur);
 		else
 			cap::cutSelection(cur, true, false);
-		checkBufferStructure(cur.buffer(), cur);
+		cur.checkBufferStructure();
 	}
 }
 

src/buffer_funcs.cpp

@@ -508,14 +508,4 @@ void updateLabels(Buffer const & buf, bool childonly)
 }
 
 
-void checkBufferStructure(Buffer & buffer, ParIterator const & par_it)
-{
-	if (par_it->layout()->toclevel != Layout::NOT_IN_TOC) {
-		Buffer const * master = buffer.masterBuffer();
-		master->tocBackend().updateItem(par_it);
-		master->structureChanged();
-	}
-}
-
-
 } // namespace lyx

src/buffer_funcs.h

@@ -53,9 +53,6 @@ void updateLabels(Buffer const &, bool childonly = false);
 ///
 void updateLabels(Buffer const &, ParIterator &);
 
-///
-void checkBufferStructure(Buffer &, ParIterator const &);
-
 } // namespace lyx
 
 #endif // BUFFER_FUNCS_H