Community
Participate
Working Groups
Created attachment 288142 [details] Gif-Animation with extending and reducing selection by keyboard bindings From: https://bugs.eclipse.org/bugs/show_bug.cgi?id=576377#c9 > This is a further extension of the multi-selection-commands, that now support > expanding the selection, reducing the selection, and revoking the selection. > I just prepared some commands without key binding defaults so far. > The key issue here is the missing cursor in the first line. It even disappears > sometimes after revoking all selections. I think that needs to be fixed > elsewhere. The issue is not dependent on the currently newly developed key-bindings, but also happens with Block selection -> To Multi-selection.
Created attachment 288168 [details] Gif-Animiation with multiple carets with third caret blinking The cause for the caret sometimes not showing is to be found in StyledText.handlePaint(): There's an assumption, that it's always the first caret, that is drawn: for (int i = 1; i < carets.length; i++) { //skip 1st caret that's already drawn Actually that's not necessarily the case: sometimes (with e.g. three carets) it might be the third caret that is already drawn. This becomes apparent when adding a line in setCaretLocations(), that seems to be missing: carets[i].setSize(firstCaret.getSize()); In the original code the sibling carets don't get initialized with the size of the first caret (and thus also miss showing the insert/overwrite status), and that somehow results in the blinking caret being hidden by the drawing over it. When the sibling carets get initilized with the original caret's size this becomes apparent, as the blinking caret might overlap with e.g. the third, and the position starts to blink. As a first approximation to a solution I propose to always draw all carets in handlePaint() and add the setSize()-line in setCaretLocations(). The only drawback is that the blinking caret doesn't go away completely (but reveals the sibling's drawing underneath), but maybe that's even a feature ;-) ... If someone could tell me where the actual blinking and drawing of the original caret is done, I could see if I can refine this, but so far I couldn't find anything about it. I've added an attachment making the modified behaviour visible. It also shows quite clearly that it's not always the first caret that's blinking.
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/191516
Note that this issue is not existing on Linux (tried the MANIFEST editor and some other ones). The caret on 1st line is present and blinking (unlike other ones as described in bug 574617). So the patch you submitted is not desired under Linux at least as it's more a workaroud. We should instead investigate why Caret is not visible on your machine. I imagine it could be that it's better to draw/instantiate *the* native caret last or something of that form?
(In reply to Mickael Istria from comment #3) > Note that this issue is not existing on Linux (tried the MANIFEST editor and > some other ones). The caret on 1st line is present and blinking (unlike > other ones as described in bug 574617). So the patch you submitted is not > desired under Linux at least as it's more a workaroud. We should instead > investigate why Caret is not visible on your machine. I imagine it could be > that it's better to draw/instantiate *the* native caret last or something of > that form? Yes, it's a workaround... I did some searching but couldn't find the place where the native blinking caret is drawn. Anyhow: it seems to be drawn on the last line in Windows. I tried it with the Java Editor. Besides that the issue with the missing size assignment for the sibling carets should also appear under linux. If you give me a hint where to search I'm willing to do that. If we want to drive the multicaret forward we need to find a solution - either a real one or a workaround.
(In reply to Dirk Steinkamp from comment #4) > Yes, it's a workaround... I did some searching but couldn't find the place > where the native blinking caret is drawn. It could be native to the graphical toolkit (gtk, win32). > Anyhow: it seems to be drawn on > the last line in Windows. I tried it with the Java Editor. OK, so GTK seems to treat the first instantiated caret as *the* native caret, while Windows will use the last one I guess. If so, we need workarounds in the code to treat those cases differently, unless we can have it supported natively (I have doubts) > Besides that the issue with the missing size assignment for the sibling > carets should also appear under linux. Can you please create a dedicated patch for that? > If you give me a hint where to search I'm willing to do that. If we want to > drive the multicaret forward we need to find a solution - either a real one > or a workaround. I don't really have additional hints to give, you've already got as deep as what I'm familiar with... It's now all about investigating.
(In reply to Mickael Istria from comment #5) > (In reply to Dirk Steinkamp from comment #4) > > Besides that the issue with the missing size assignment for the sibling > > carets should also appear under linux. > > Can you please create a dedicated patch for that? That's already included in the first patch. How to handle this best?
(In reply to Mickael Istria from comment #5) > (In reply to Dirk Steinkamp from comment #4) > > Yes, it's a workaround... I did some searching but couldn't find the place > > where the native blinking caret is drawn. > > It could be native to the graphical toolkit (gtk, win32). > > > Anyhow: it seems to be drawn on > > the last line in Windows. I tried it with the Java Editor. > > OK, so GTK seems to treat the first instantiated caret as *the* native > caret, while Windows will use the last one I guess. If so, we need > workarounds in the code to treat those cases differently, unless we can have > it supported natively (I have doubts) Relating to win32: The issue seems to be related to what is happening in OS.SetCaretPos() - which is a native implementation. It's called by Caret.move(), which is called by StyledText.setCaretLocations(). And there's the issue in Windows: the last call of SetCaretPos() is where the blinking cursor ends up. Considering this a change in setCaretLocations() is a remedy: change for (int i = 0; i < Math.min(caretOffsets.length, locations.length); i++) { to for (int i = Math.min(caretOffsets.length, locations.length)-1; i>=0; i--) { This will traverse the carets in opposite direction and result in a proper display on windows. Could you try that on Linux and see if it helps there, too? While investigating the mechanics I stumbled upon another side effect of all this: When an editor loses focus and then gains it again, only the "blinking OS caret" is drawn, but the other virtual carets are not updated/drawn until the caret moves. This is because Composite.setFocus() -> ... -> Caret.setFocus() doesn't know of extra carets, and thus only shows one caret. A remedy would be to add an implementation for StyledText.setFocus() like this: @Override public boolean setFocus() { boolean b = super.setFocus(); setCaretLocations(); return b; } This takes care of the drawing of the extra carets.
(In reply to Dirk Steinkamp from comment #6) > That's already included in the first patch. How to handle this best? checkout master, make the change, commit and push as a new patch set. > change > for (int i = 0; i < Math.min(caretOffsets.length, locations.length); i++) { > to > for (int i = Math.min(caretOffsets.length, locations.length)-1; i>=0; i--) { This seems to work well on Linux, if I tested it correctly ;) But I don't know how it could behave on a Mac... Anyway, Win is more important than Mac in popularity, and I personally don't care about Mac at all, so if we get more happy users on Windows than unhappy users on Mac, it seems profitable. > A remedy would be to add an implementation for StyledText.setFocus() like this: Can you please a separate issue about this, and create a separate patch?
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/191516 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=b7a0577863554d9e7048a4a63388333d8cba0b71
Thanks Dirk!
