Community
Participate
Working Groups
I20051214-2000 We should try to reduce the calls to StyledText.setRedraw(boolean). For example when revealing a selection, we could check up-front whether scrolling will happen or not.
*** Bug 121140 has been marked as a duplicate of this bug. ***
From bug 117507 comment 12: One reason why we use setRedraw is to avoid the selection being cleared and repainted when doing a multi-edit. This is true for operations that perform multiple small edits and want to maintain a semantic (e.g. entire lines) selection, e.g. for the following operations: - shift left / right - correct indent - move lines For example, when not using setRedraw on/off, the "Correct Indentation" command becomes faster due to the performance impact of setRedraw, but it also starts to flicker. This is due to two reasons: - one can (almost) see every edit as the indent of each line is adjusted - when changing the editor contents, the selection is cleared and only restored after all edits have been performed. So, in order to not use setRedraw in those cases, we would require some other way to bracket multiple operations and tell StyledText to hold off repainting until we're done restoring the selection. - Note that StyledText::handleTextChanged calls Control:update in order to give immediate user feedback (see also bug 117507 comment 15 for why this is done). This is usually a good thing, but hurts us here where we only want to repaint when we're done. An idea might be to introduce new API in StyledText that allows us to bracket text and selection updates: StyledText::setEagerUpdate(boolean) While "eager updating" is off, no update() calls would be issued in StyledText::handleTextChanged. Felipe, does this make sense at all, or am I missing the point?
I quickly tested the idea outlined in comment 2 and it seems to work fine - "move lines" and "correct indentation" are noticeably smoother when only disabling the eager updating in StyledText, but not using StyledText::setRedraw and IDocumentAdapterExtension::start/stopForwardingDocumentChanges (which causes the entire text to be set when resuming). Both operations maintain a smooth selection without flickering due to the "disable eager updating" mechanism. This is on I20051220-0800 plus ZRH inputs from 20060103.
Created attachment 32525 [details] NoMoreSetRedraw.diff This is just a quick and dirty hack to test it out. - The "eagerUpdate" property should probably be reference counted (like setRedraw). - Places where setRedraw makes sense are now slower (e.g. unfolding everything in the editor). => we'd probably want to offer both versions of bracketing in IRewriteTargetExtensionX. - I also disabled using IDocumentAdapterExtension as this really hurts us with big documents - this is only justified for very large updates, such as unfolding everything.
(In reply to comment #4) > we'd probably want to offer both versions of bracketing in > IRewriteTargetExtensionX. To expand on that a little: I think we have two separate issues that we currently try to fix using TextViewer::setRedraw: a) bracketing multiple edits / selection changes to avoid flickering as described in comment 2. b) speeding up very large updates, such as expanding every folded region or formatting / shifting the entire document. The current TextViewer::setRedraw implementation does quite well for b) (where the user expects a small delay), but hurts us in case a) where we want to bracket a couple of small updates. The good thing is that we know which one to use. In some places we already differentiate based on how big a change is (for example, how many lines are shifted). Fixing this pr means to offer two different bracketing mechanisms to choose from, the current one for b) and one along the lines of the patch for a).
If you are successful, it should make things faster. Despite the fact that you can't see it in some widgets, setRedraw(false) causes the entire widget to be redrawn because it can't tell where the damage is. The redraw is hidden by SWT.DOUBLE_BUFFERED and SWT.NO_BACKGROUND.
I have removed setRedraw calls in those places: - DefaultUndoManager - TextViewerUndoManger - ProjectionViewer (for small updates only) - ConvertLineDelimitersAction The changes are tagged with te_{before,after}_removing_setRedraw. -- One problem remains: the recommended way for clients to bracket multi-operations is to call IDocumentExtension4::start/stopRewriteSession. Clients can select between UNRESTRICTED, SEQUENTIAL and STRICTLY_SEQUENTIAL rewrite session types, which may be used by document implementations to optimize their updating strategies. However, those types do not contain any information about the expected size of an update. TextViewer listens for document rewrite sessions and ensures the bracketing on the undo manager (begin/endCompoundChange) and also adds a pair of setRedraw calls around the rewrite session. This is often unneeded (for example when applying a simple text edit, see JavaModelUtil.applyEdit()), but may be absolutely crucial for the performance of very large updates (e.g. format all). I propose to add a fourth session type, UNRESTRICTED_SMALL, which can be used by clients that want to bracket a multi-edit but know that the size of the change is relatively small (e.g. up to 50 lines). TextViewer could then avoid calling setRedraw for small changes, while maintaining compatibility for the existing session types. Clients like JavaModelUtil can determine the size of a change by inspecting the edits.
+1 to add UNRESTRICTED_SMALL
fixed > 20060914
Some of this had to be reverted, see bug 158746 for details.
