From 9387dcdaef19289af87f24639bf7336526d027f5 Mon Sep 17 00:00:00 2001 From: Christophe Meriaux Date: Thu, 1 Nov 2018 15:05:30 +0100 Subject: [PATCH] Fix restoring line position issue while document is wrapped If you switch between tabs while wrap mode is enable, text jump to another lines. It's an old bug, fixed in v7.5.9, but it had performance regression so it was reverted in v6.0.0. It's been one year, and I'm back. This works whatever the size of the file. There isn't any performance regression because we don't use SCI_ENSUREVISIBLE scintilla command. In case wrap option on, The restore position function is done in **twice** steps. - First step: set selection, set anchor, set xoffset... - Second step: once Scintilla has send the notification SCN_PAINTED, we can scroll several lines to set the first visible line to the correct wrapped line. Keep in mind that Line wrapping is a background activity that takes time, specially for huge file. Fix #2078, fix #2576, fix #3570, fix #4825, fix #4881, close #7781 --- PowerEditor/src/Notepad_plus.cpp | 4 +- PowerEditor/src/NppCommands.cpp | 2 +- PowerEditor/src/NppIO.cpp | 8 +- PowerEditor/src/NppNotification.cpp | 15 +++- PowerEditor/src/Parameters.cpp | 4 + PowerEditor/src/Parameters.h | 2 + .../ScitillaComponent/ScintillaEditView.cpp | 74 +++++++++++++++++-- .../src/ScitillaComponent/ScintillaEditView.h | 7 +- 8 files changed, 99 insertions(+), 17 deletions(-) diff --git a/PowerEditor/src/Notepad_plus.cpp b/PowerEditor/src/Notepad_plus.cpp index 329003ddd..17a5682cc 100644 --- a/PowerEditor/src/Notepad_plus.cpp +++ b/PowerEditor/src/Notepad_plus.cpp @@ -2821,7 +2821,7 @@ void Notepad_plus::setLanguage(LangType langType) if (reset) { _subEditView.execute(SCI_SETDOCPOINTER, 0, prev); - _subEditView.restoreCurrentPos(); + _subEditView.restoreCurrentPosPreStep(); } }; @@ -3739,7 +3739,7 @@ void Notepad_plus::docGotoAnotherEditView(FileTransferMode mode) Buffer *buf = MainFileManager.getBufferByID(current); _pEditView->saveCurrentPos(); //allow copying of position buf->setPosition(buf->getPosition(_pEditView), _pNonEditView); - _pNonEditView->restoreCurrentPos(); //set position + _pNonEditView->restoreCurrentPosPreStep(); //set position activateBuffer(current, viewToGo); } diff --git a/PowerEditor/src/NppCommands.cpp b/PowerEditor/src/NppCommands.cpp index c43a196fb..f4f1071c7 100644 --- a/PowerEditor/src/NppCommands.cpp +++ b/PowerEditor/src/NppCommands.cpp @@ -2504,7 +2504,7 @@ void Notepad_plus::command(int id) // Paste the texte, restore buffer status _pEditView->execute(SCI_PASTE); - _pEditView->restoreCurrentPos(); + _pEditView->restoreCurrentPosPreStep(); // Restore the previous clipboard data ::OpenClipboard(_pPublicInterface->getHSelf()); diff --git a/PowerEditor/src/NppIO.cpp b/PowerEditor/src/NppIO.cpp index 8b0e68c54..a18811b04 100644 --- a/PowerEditor/src/NppIO.cpp +++ b/PowerEditor/src/NppIO.cpp @@ -462,13 +462,13 @@ bool Notepad_plus::doReload(BufferID id, bool alert) if (mainVisisble) { _mainEditView.execute(SCI_SETDOCPOINTER, 0, pBuf->getDocument()); - _mainEditView.restoreCurrentPos(); + _mainEditView.restoreCurrentPosPreStep(); } if (subVisisble) { _subEditView.execute(SCI_SETDOCPOINTER, 0, pBuf->getDocument()); - _subEditView.restoreCurrentPos(); + _subEditView.restoreCurrentPosPreStep(); } // Once reload is complete, activate buffer which will take care of @@ -2057,8 +2057,8 @@ bool Notepad_plus::loadSession(Session & session, bool isSnapshotMode) _isFolding = false; } - _mainEditView.restoreCurrentPos(); - _subEditView.restoreCurrentPos(); + _mainEditView.restoreCurrentPosPreStep(); + _subEditView.restoreCurrentPosPreStep(); if (session._activeMainIndex < _mainDocTab.nbItem())//session.nbMainFiles()) activateBuffer(_mainDocTab.getBufferByIndex(session._activeMainIndex), MAIN_VIEW); diff --git a/PowerEditor/src/NppNotification.cpp b/PowerEditor/src/NppNotification.cpp index 335de71c5..6836b290c 100644 --- a/PowerEditor/src/NppNotification.cpp +++ b/PowerEditor/src/NppNotification.cpp @@ -946,17 +946,28 @@ BOOL Notepad_plus::notify(SCNotification *notification) if (not notifyView) return FALSE; + // Check if a restore position is needed. + // Restoring a position must done after SCN_PAINTED notification so that it works in every circumstances (including wrapped large file) + if (_mainEditView.isPositionRestoreNeeded()) + { + _mainEditView.restoreCurrentPosPostStep(); + } + if (_subEditView.isPositionRestoreNeeded()) + { + _subEditView.restoreCurrentPosPostStep(); + } + // ViewMoveAtWrappingDisableFix: Disable wrapping messes up visible lines. // Therefore save view position before in IDM_VIEW_WRAP and restore after SCN_PAINTED, as doc. says if (_mainEditView.isWrapRestoreNeeded()) { - _mainEditView.restoreCurrentPos(); + _mainEditView.restoreCurrentPosPreStep(); _mainEditView.setWrapRestoreNeeded(false); } if (_subEditView.isWrapRestoreNeeded()) { - _subEditView.restoreCurrentPos(); + _subEditView.restoreCurrentPosPreStep(); _subEditView.setWrapRestoreNeeded(false); } notifyView->updateLineNumberWidth(); diff --git a/PowerEditor/src/Parameters.cpp b/PowerEditor/src/Parameters.cpp index f3ee701c1..5a86f28fc 100644 --- a/PowerEditor/src/Parameters.cpp +++ b/PowerEditor/src/Parameters.cpp @@ -2076,6 +2076,8 @@ bool NppParameters::getSessionFromXmlTree(TiXmlDocument *pSessionDoc, Session *p (childNode->ToElement())->Attribute(TEXT("endPos"), &position._endPos); (childNode->ToElement())->Attribute(TEXT("selMode"), &position._selMode); (childNode->ToElement())->Attribute(TEXT("scrollWidth"), &position._scrollWidth); + (childNode->ToElement())->Attribute(TEXT("offset"), &position._offset); + (childNode->ToElement())->Attribute(TEXT("wrapCount"), &position._wrapCount); MapPosition mapPosition; int32_t mapPosVal; const TCHAR *mapPosStr = (childNode->ToElement())->Attribute(TEXT("mapFirstVisibleDisplayLine"), &mapPosVal); @@ -3120,6 +3122,8 @@ void NppParameters::writeSession(const Session & session, const TCHAR *fileName) (fileNameNode->ToElement())->SetAttribute(TEXT("startPos"), viewSessionFiles[i]._startPos); (fileNameNode->ToElement())->SetAttribute(TEXT("endPos"), viewSessionFiles[i]._endPos); (fileNameNode->ToElement())->SetAttribute(TEXT("selMode"), viewSessionFiles[i]._selMode); + (fileNameNode->ToElement())->SetAttribute(TEXT("offset"), viewSessionFiles[i]._offset); + (fileNameNode->ToElement())->SetAttribute(TEXT("wrapCount"), viewSessionFiles[i]._wrapCount); (fileNameNode->ToElement())->SetAttribute(TEXT("lang"), viewSessionFiles[i]._langName.c_str()); (fileNameNode->ToElement())->SetAttribute(TEXT("encoding"), viewSessionFiles[i]._encoding); (fileNameNode->ToElement())->SetAttribute(TEXT("userReadOnly"), (viewSessionFiles[i]._isUserReadOnly && !viewSessionFiles[i]._isMonitoring) ? TEXT("yes") : TEXT("no")); diff --git a/PowerEditor/src/Parameters.h b/PowerEditor/src/Parameters.h index ff0c94afd..d1cc9c73d 100644 --- a/PowerEditor/src/Parameters.h +++ b/PowerEditor/src/Parameters.h @@ -136,6 +136,8 @@ struct Position int _xOffset = 0; int _selMode = 0; int _scrollWidth = 1; + int _offset = 0; + int _wrapCount = 0; }; diff --git a/PowerEditor/src/ScitillaComponent/ScintillaEditView.cpp b/PowerEditor/src/ScitillaComponent/ScintillaEditView.cpp index f48d6828b..c36a6de9d 100644 --- a/PowerEditor/src/ScitillaComponent/ScintillaEditView.cpp +++ b/PowerEditor/src/ScitillaComponent/ScintillaEditView.cpp @@ -1792,7 +1792,8 @@ void ScintillaEditView::saveCurrentPos() //Save data so, that the current topline becomes visible again after restoring. int32_t displayedLine = static_cast(execute(SCI_GETFIRSTVISIBLELINE)); int32_t docLine = static_cast(execute(SCI_DOCLINEFROMVISIBLE, displayedLine)); //linenumber of the line displayed in the top - //int offset = displayedLine - execute(SCI_VISIBLEFROMDOCLINE, docLine); //use this to calc offset of wrap. If no wrap this should be zero + int32_t offset = displayedLine - static_cast(execute(SCI_VISIBLEFROMDOCLINE, docLine)); //use this to calc offset of wrap. If no wrap this should be zero + int wrapCount = static_cast(execute(SCI_WRAPCOUNT, docLine)); Buffer * buf = MainFileManager.getBufferByID(_currentBufferID); @@ -1804,17 +1805,21 @@ void ScintillaEditView::saveCurrentPos() pos._xOffset = static_cast(execute(SCI_GETXOFFSET)); pos._selMode = static_cast(execute(SCI_GETSELECTIONMODE)); pos._scrollWidth = static_cast(execute(SCI_GETSCROLLWIDTH)); + pos._offset = offset; + pos._wrapCount = wrapCount; buf->setPosition(pos, this); } -void ScintillaEditView::restoreCurrentPos() +void ScintillaEditView::restoreCurrentPosPreStep() { + // restore current position is executed in two steps: + // - pre step : restoreCurrentPosPreStep (this function) + // - post step : function restoreCurrentPosPreStep that will be executed after scintilla + Buffer * buf = MainFileManager.getBufferByID(_currentBufferID); Position & pos = buf->getPosition(this); - execute(SCI_GOTOPOS, 0); //make sure first line visible by setting caret there, will scroll to top of document - execute(SCI_SETSELECTIONMODE, pos._selMode); //enable execute(SCI_SETANCHOR, pos._startPos); execute(SCI_SETCURRENTPOS, pos._endPos); @@ -1825,9 +1830,64 @@ void ScintillaEditView::restoreCurrentPos() execute(SCI_SETXOFFSET, pos._xOffset); } execute(SCI_CHOOSECARETX); // choose current x position - int lineToShow = static_cast(execute(SCI_VISIBLEFROMDOCLINE, pos._firstVisibleLine)); - scroll(0, lineToShow); + execute(SCI_SETFIRSTVISIBLELINE, lineToShow); + if (isWrap()) + { + // Enable flag 'positionRestoreNeeded' so that function restoreCurrentPosPostStep get called + // once scintilla send SCN_PAITED notification + _positionRestoreNeeded = true; + } + _restorePositionRetryCount = 0; + +} + +void ScintillaEditView::restoreCurrentPosPostStep() +{ + // scintilla can send several SCN_PAINTED notifications before the buffer is ready to be displayed. + // this post step function is therefore iterated several times in a maximum of 8 iterations. + // 8 is an arbitrary number. 2 is a minimum. Maximum value is unknown. + static int32_t restoreDone = 0; + Buffer * buf = MainFileManager.getBufferByID(_currentBufferID); + Position & pos = buf->getPosition(this); + + ++_restorePositionRetryCount; + + if (_restorePositionRetryCount > 8) + { + // Abort the position restoring process. Buffer topology may have changed + _positionRestoreNeeded = false; + return; + } + + int32_t displayedLine = static_cast(execute(SCI_GETFIRSTVISIBLELINE)); + int32_t docLine = static_cast(execute(SCI_DOCLINEFROMVISIBLE, displayedLine)); //linenumber of the line displayed in the + + + // check docLine must equals saved position + if (docLine != pos._firstVisibleLine) + { + + // Scintilla has paint the buffer but the position is not correct. + int lineToShow = static_cast(execute(SCI_VISIBLEFROMDOCLINE, pos._firstVisibleLine)); + execute(SCI_SETFIRSTVISIBLELINE, lineToShow); + } + else if (pos._offset > 0) + { + // don't scroll anything if the wrap count is different than the saved one. + // Buffer update may be in progress (in case wrap is enabled) + int wrapCount = static_cast(execute(SCI_WRAPCOUNT, docLine)); + if (wrapCount == pos._wrapCount) + { + scroll(0, pos._offset); + _positionRestoreNeeded = false; + } + } + else + { + // Buffer position is correct, and there is no scroll to apply + _positionRestoreNeeded = false; + } } void ScintillaEditView::restyleBuffer() { @@ -1882,7 +1942,7 @@ void ScintillaEditView::activateBuffer(BufferID buffer) const std::vector & lineStateVectorNew = newBuf->getHeaderLineState(this); syncFoldStateWith(lineStateVectorNew); - restoreCurrentPos(); + restoreCurrentPosPreStep(); bufferUpdated(_currentBuffer, (BufferChangeMask & ~BufferChangeLanguage)); //everything should be updated, but the language (which undoes some operations done here like folding) diff --git a/PowerEditor/src/ScitillaComponent/ScintillaEditView.h b/PowerEditor/src/ScitillaComponent/ScintillaEditView.h index d9abf475e..2f4b4b539 100644 --- a/PowerEditor/src/ScitillaComponent/ScintillaEditView.h +++ b/PowerEditor/src/ScitillaComponent/ScintillaEditView.h @@ -273,7 +273,8 @@ public: void insertNewLineBelowCurrentLine(); void saveCurrentPos(); - void restoreCurrentPos(); + void restoreCurrentPosPreStep(); + void restoreCurrentPosPostStep(); void beginOrEndSelect(); bool beginEndSelectedIsStarted() const { @@ -632,6 +633,8 @@ public: void setTabSettings(Lang *lang); bool isWrapRestoreNeeded() const {return _wrapRestoreNeeded;}; void setWrapRestoreNeeded(bool isWrapRestoredNeeded) {_wrapRestoreNeeded = isWrapRestoredNeeded;}; + bool isPositionRestoreNeeded() const {return _positionRestoreNeeded;}; + bool isCJK() const { return ((_codepage == CP_CHINESE_TRADITIONAL) || (_codepage == CP_CHINESE_SIMPLIFIED) || @@ -668,6 +671,8 @@ protected: int _codepage = CP_ACP; bool _lineNumbersShown = false; bool _wrapRestoreNeeded = false; + bool _positionRestoreNeeded = false; + uint32_t _restorePositionRetryCount = 0; typedef std::unordered_map StyleMap; typedef std::unordered_map BufferStyleMap;