Bug 532924 - [code mining] wrong redraw artifacts showing up
Summary: [code mining] wrong redraw artifacts showing up
Status: RESOLVED DUPLICATE of bug 534977
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.8   Edit
Hardware: PC All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Platform-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 534888 534977
Blocks: 529127
  Show dependency tree
 
Reported: 2018-03-27 02:08 EDT by Martin Lippert CLA
Modified: 2018-07-30 12:26 EDT (History)
11 users (show)

See Also:


Attachments
code mining redraw artifacts (20.82 MB, video/quicktime)
2018-03-27 02:09 EDT, Martin Lippert CLA
no flags Details
jdt code mining redraw issues - copy/paste (21.87 MB, video/quicktime)
2018-03-29 09:22 EDT, Martin Lippert CLA
no flags Details
pure jdt-codemining redraw issues (15.94 MB, video/quicktime)
2018-04-04 09:38 EDT, Martin Lippert CLA
no flags Details
CodeMiningDemo.java redraw artifact (53.82 KB, image/png)
2018-04-05 03:21 EDT, Martin Lippert CLA
no flags Details
InlinedAnnotationDemo.java screenshot on macOS (22.60 KB, image/png)
2018-04-05 03:23 EDT, Martin Lippert CLA
no flags Details
screenshot of code snippet before and after (32.12 KB, image/png)
2018-04-05 03:29 EDT, Martin Lippert CLA
no flags Details
redraw artifacts after show references got enabled (265.08 KB, image/png)
2018-04-05 03:58 EDT, Martin Lippert CLA
no flags Details
codemining redraw artifacts - 2018-05-03 - update1 (2.28 MB, video/quicktime)
2018-05-03 10:57 EDT, Martin Lippert CLA
no flags Details
codemining redraw artifacts - 2018-05-03 - update2 (2.89 MB, video/quicktime)
2018-05-03 10:58 EDT, Martin Lippert CLA
no flags Details
codemining redraw artifacts - 2018-05-03 - update3 (4.22 MB, video/quicktime)
2018-05-03 10:59 EDT, Martin Lippert CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Lippert CLA 2018-03-27 02:08:35 EDT
I open an editor, a language server is being started in the background, and after a while, code lenses show up and they appear in the text editor as code minings.

However, there also appear slightly wrong redraw artifacts (see the attached video).
Comment 1 Martin Lippert CLA 2018-03-27 02:09:24 EDT
Created attachment 273310 [details]
code mining redraw artifacts
Comment 2 Angelo ZERR CLA 2018-03-27 06:12:10 EDT
I cannot reproduce with JDT CodeMining. Are you using Platform Text from master?
Comment 3 Angelo ZERR CLA 2018-03-27 08:09:54 EDT
And I have forgotten to say you too that you must use master code of SWT, in other words:

 * org.eclipse.jface.text (codemining support)
 * org.eclipse.swt (StyledText which was improved)
Comment 4 Lars Vogel CLA 2018-03-27 08:16:18 EDT
(In reply to Angelo ZERR from comment #3)
> And I have forgotten to say you too that you must use master code of SWT, in
> other words:
> 
>  * org.eclipse.jface.text (codemining support)
>  * org.eclipse.swt (StyledText which was improved)

I think the latest I-Build should contain the changes. Get it from here: http://download.eclipse.org/eclipse/downloads/
Comment 5 Martin Lippert CLA 2018-03-29 05:33:54 EDT
I did my tests with:

- org.eclipse.jface.text_3.13.0.v20180323-1537
- org.eclipse.swt_3.107.0.v20180323-1902
- org.eclipse.swt.cocoa.macosx.x86_64_3.107.0.v20180323-1902

I can try a newer I-build from platform, lets see.
Comment 6 Martin Lippert CLA 2018-03-29 08:24:36 EDT
Same problem using the latest I-build:

- org.eclipse.jface.text_3.13.0.v20180327-1003
- org.eclipse.swt_3.107.0.v20180328-2159
- org.eclipse.swt.cocoa.macosx.x86_64_3.107.0.v20180328-2159
Comment 7 Angelo ZERR CLA 2018-03-29 08:31:29 EDT
@Martin have you the same problem with Java references from JDT CodeMining https://github.com/angelozerr/jdt-codemining?
Comment 8 Martin Lippert CLA 2018-03-29 09:05:38 EDT
Yes, I can reproduce the issue with your JDT code minings as well, but in a slightly different way... :-)

I enabled the references code mining and when opening a Java file, the reference code minings appear and show the same (or similar) redraw artifacts, but there seems to be a second redraw action that gets executed quickly after the references shown up, so that the wrong redraw artifacts quickly disappear again.

You can for sure reproduce some redrawing artifacts with your JDT code minings when doing this:

- select a few minings from the preferences
- open two Java files
- open the preference again and add or remove certain minings
- click "Apply" and close the preference dialog
- the open editor is updated correctly and looks nice
- then switch to the other open editor, the updated minings are not drawn correctly

When playing around with your JDT code minnings (which are a really awesome feature, by the way), the redrawing appears to be slow and sluggish when those minings appear and disappear. Stuff also quickly breaks the redrawing if you select a block of code, cut that out of the editor, paste stuff in, etc.

I can record another video and will attach that using your JST code minings to illustrate.

After playing with this in more detail, it looks like there quite a bit of bugfixing work ahead of us before this can go public... :-)
Comment 9 Martin Lippert CLA 2018-03-29 09:06:57 EDT
> I can record another video and will attach that using your JST code minings
> to illustrate.

JDT code minings, not JST code minings... Sorry for the typo...
Comment 10 Martin Lippert CLA 2018-03-29 09:22:18 EDT
Created attachment 273363 [details]
jdt code mining redraw issues - copy/paste
Comment 11 Eclipse Genie CLA 2018-03-30 17:03:47 EDT
New Gerrit change created: https://git.eclipse.org/r/120523
Comment 12 Angelo ZERR CLA 2018-03-30 17:05:16 EDT
@Martin, please review my gerrit patch https://git.eclipse.org/r/#/c/120523/ which should fix (only for the moment) jdt code mining redraw issues - copy/paste
Comment 13 Mickael Istria CLA 2018-04-01 15:44:41 EDT
Not sure whether it fixes the initial issue or not, but I've merged the patch as it improves the code anyway.
Comment 15 Angelo ZERR CLA 2018-04-02 10:11:20 EDT
>Not sure whether it fixes the initial issue or not, but I've merged the patch as it improves the code anyway.

Thanks Mickael. My gerrit patch should fix problem with copy/paste usecase that @Martin has explained.

For the original issue, I cannot reproduce but I have an idea to improve the draw. I will try to create a new gerrit path and I hope it will fixes problem of Martin.
Comment 16 Angelo ZERR CLA 2018-04-03 04:06:19 EDT
@Mickael I have a gerrit patch which I think should fix the problem of @Martin. But the big problem is that it changes the API of codemining.

At first let's me explain the problem. When inlined annotation is drawn (with InlinedAnnotationDrawingStrategy#draw), we draw the line content and line header annotation even if GlyphMetrics width (for line content annotation) and ascent (for line header annotation) is not updated. For instance with line header annotation, we have:

---------------------------------------------------------------
annotation.draw(gc, textWidget, offset, length, color, x, y); // draw is done
int height = annotation.getHeight();
if (height != 0){

} else {
  // here there is a problem because content of inlined annotation 
  // is drawn although GlyphMetrics is not updated and the label of mining
  // is drawn on the top of the line content without taking some place
}
...

---------------------------------------------------------------

To fix this problem, we should take care of that inlined annotation takes place. So my idea is to do that:

---------------------------------------------------------------

boolean toDrawn = style != null && style.metrics != null && style.metrics.ascent != 0;
annotation.draw(gc, textWidget, offset, length, color, x, toDrawn)
---------------------------------------------------------------

The inlined annotation must chech that toDrawn is true to drawn the content of inlined annotation in the gc, otherwise it returns just the well height/width.

Before creating a gerrit patch which changes the API, I would like to know if we can change this API. What do you think about that @Mickael?

If you are OK, it means that it will change too ICodeMining#draw method API to add toDrawn parameter.

I fear that it's not possible today to change the API -(
Comment 17 Mickael Istria CLA 2018-04-03 04:10:08 EDT
It's indeed too late for API changes. Moreover, adding a "toDraw" parameter to a method that's named "draw" wouldn't be considered as good enough API to be merged.
We need to figure out something that doesn't require API change.
Comment 18 Martin Lippert CLA 2018-04-03 06:34:37 EDT
I am testing this again with the latest for "org.eclipse.jface.text" from master and it doesn't improve the situation. My cut/paste scenario is still hugely broken with regards to redrawing... :-(
Comment 19 Angelo ZERR CLA 2018-04-03 06:42:55 EDT
> We need to figure out something that doesn't require API change.

It was my fear. I will try to find a solution for that...

But before that I would like to reproduce the problem of @Martin. After reading your comment it seems that you are using language server (so I think Codelens of lsp4e). I suspect that the problem is that there is 2 call of ISourceViewerExtension5#updateCodeMinings() whih is done:

  * by JDT with reonciling listener.
  * by lsp4e with the codemining strategy.

@Martin is it possible to see your code please? But I think you have not declared a codeMiningProvider extension point beause you are consuming the lsp4e codelens. Is that?
Comment 20 Martin Lippert CLA 2018-04-03 06:46:47 EDT
(In reply to Angelo ZERR from comment #19)
> @Martin is it possible to see your code please? But I think you have not
> declared a codeMiningProvider extension point beause you are consuming the
> lsp4e codelens. Is that?

I haven't implemented anything specific for the code lenses to show up. My language server is declared as an extension to LSP4E and for the java source content-type, that's it... :-)

(I have a few specific pieces for content-assist, hovers, etc., but nothing specific for code lenses)
Comment 21 Martin Lippert CLA 2018-04-03 06:53:10 EDT
(In reply to Angelo ZERR from comment #19)

If you want to try to reproduce my exact setting yourself, here are the instructions. You can go ahead and install our language server from here:

p2 repo:
http://dist.springsource.com/snapshot/TOOLS/sts4/nightly/e4.8

Select the "Spring Boot Language Server" feature only, you don't need anything else. That gives you all the language server goodness that we work on for Spring.

Now, as an example that produces code lenses, you could clone this repo:
https://github.com/bclozel/webflux-workshop

And import "stock-quotes" as an existing Maven project. When you then open the class "QuoteHandler", you should see the code lenses to appear after a short little while.

(these are all beta and CI versions, so if something goes wrong, let me know)
Comment 22 Angelo ZERR CLA 2018-04-03 12:59:58 EDT
Thanks @Martin for your information! I must find time to study this hard problem and if I can reproduce, I will try to discover the problem.
Comment 23 Martin Lippert CLA 2018-04-04 05:00:58 EDT
I double checked this with the generic editor instead of the java editor and it shows the same erroneous behavior... :-(
Comment 24 Angelo ZERR CLA 2018-04-04 05:05:01 EDT
@Martin it seems that with jdt-codemining (according your demo), you have not the problem (just a problem with copy/paste, but I cannot reproduce it -( ). It seems the problem comes from with language server. Is it a problem with LSP4e? I need time to play with a language server and I hope I can reproduce it.
Comment 25 Martin Lippert CLA 2018-04-04 05:30:19 EDT
(In reply to Angelo ZERR from comment #24)
> @Martin it seems that with jdt-codemining (according your demo), you have
> not the problem (just a problem with copy/paste, but I cannot reproduce it
> -( ). It seems the problem comes from with language server. Is it a problem
> with LSP4e? I need time to play with a language server and I hope I can
> reproduce it.

I think there is a fundamental redraw problem going on under the hood.
I can reproduce parts of this with your jdt-codemining, too. Try this:

- have jdt-codeminings installed, but everything disabled
- open a java editor with source code
- open preferences for jdt-codeminings
- enable everything
- click "apply"

The editor gets updated with code minings (while the preference dialog is still open), but there are wrong redraw artifacts showing up all over the place.

When I close the preference dialog, there is another redraw action going on that cleans everything up.
Comment 26 Martin Lippert CLA 2018-04-04 05:37:54 EDT
I also tried something else:

- have a java editor open with code
- open the jdt-codeminings preference page

then enable random things, click apply, disable random things, click apply, enable random things again, click apply, and so on (without closing the preference dialog).

This quickly turns the java editor into a huge mess of redraw artifacts, some of them don't even go away after closing the preference dialog and walking around with the cursor. Also walking the lines with the cursor is sluggish (at least feels sluggish to me, not the usual responsiveness).

Btw, I did all this with the language server disabled, so jdt-codeminings are the only minings that show up.
Comment 27 Martin Lippert CLA 2018-04-04 05:38:27 EDT
Ket me know if you need a video showing all this.
Comment 28 Angelo ZERR CLA 2018-04-04 05:48:49 EDT
Yes please!
Comment 29 Mickael Istria CLA 2018-04-04 05:57:49 EDT
Note that for such low-level drawing issues, it could be a platform-specific bug, such as MacOS drawing not cleaning previous locations by default or something like that.
Comment 30 Angelo ZERR CLA 2018-04-04 07:47:18 EDT
@Martin, I have tried to Apply mining with JDT preferences and I have no problem (I'm using Windows OS). I'm waiting for your demo, but as @Mickael said, perhaps it's a problem with MacOS? If it that, I fear that I cannot do nothing (I have not MacOS)
Comment 31 Martin Lippert CLA 2018-04-04 09:38:46 EDT
Created attachment 273420 [details]
pure jdt-codemining redraw issues

here is a new video, showing jdt-codeminings only (no language server involved) and several redraw issues on macOS
Comment 32 Martin Lippert CLA 2018-04-04 09:41:37 EDT
I don't have enough low-level knowledge for debugging this myself, but I am happy to test anything and provide as much feedback as possible.
Comment 33 Martin Lippert CLA 2018-04-04 09:43:06 EDT
btw, the same problems appear with the generic editor and code lenses provided from language servers, so it doesn't seem to be specific for jdt. Really looks like a deep down low-level drawing issue, independent from where the code mining is coming from or in which editor it is shown.
Comment 34 Dani Megert CLA 2018-04-04 10:07:09 EDT
(In reply to Martin Lippert from comment #33)
> Really looks like a deep down low-level drawing issue, independent from
> where the code mining is coming from or in which editor it is shown.

So, can you provide steps that do not involve code mining?
Comment 35 Martin Lippert CLA 2018-04-04 10:11:57 EDT
(In reply to Dani Megert from comment #34)
> (In reply to Martin Lippert from comment #33)
> > Really looks like a deep down low-level drawing issue, independent from
> > where the code mining is coming from or in which editor it is shown.
> 
> So, can you provide steps that do not involve code mining?

It happens with code mining only, but it looks like it is independent from where the code mining is coming from (jdt-codemining from Angelo, my language server, etc.).
Comment 36 Angelo ZERR CLA 2018-04-04 10:35:01 EDT
@Martin at first thanks for your demo, I have tested with StyledText.java and do the same usecase than you on Windows 64 bits and I confirm you that I have not your problem. It seemms that it's a bug with MacOS. 

@Mickael could you tell us if you have the same problem with your Linux OS?

If not, we can confirm that it's a bug with MacOS.

If you wish to debug, I think you should do step by step by enabling/disabling "Show references". The draw of this line header annotation is done in InlinedAnnotationDrawingStrategy#draw(LineHeaderAnnotation...)

The basic idea is:

 * try to draw the annotation
 * if height is > 0, check if GlyphMetrics ascent is the same than the waited height. If not, a StyleRange is set with the well GlyphMetrics and so in this case the redraw is done. 

Perhaps the bug is here when StyleRange with GlyphMetrics ascent is setted. I think we should start with this case with a simple main and UI which update StyleRange with GlyphMetrics. I will add a simple snippet with this case.
Comment 38 Angelo ZERR CLA 2018-04-05 01:35:52 EDT
@Martin, if you have problem with the 2 demos, could you test this simple snippet:

-----------------------------------------------------------------------------
import org.eclipse.swt.SWT;
import org.eclipse.swt.custom.StyleRange;
import org.eclipse.swt.custom.StyledText;
import org.eclipse.swt.events.SelectionAdapter;
import org.eclipse.swt.events.SelectionEvent;
import org.eclipse.swt.graphics.GlyphMetrics;
import org.eclipse.swt.layout.FillLayout;
import org.eclipse.swt.widgets.Button;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Shell;

public class GlyphMetricsDemo {

	public static void main(String[] args) {
		Display display = new Display();
		Shell shell = new Shell(display);
		shell.setSize(300, 200);
		shell.setLayout(new FillLayout());

		final StyledText text = new StyledText(shell, SWT.MULTI | SWT.BORDER | SWT.H_SCROLL | SWT.V_SCROLL);
		text.setText("aaaa\nbbbb\ncccc");
		
		final Button button = new Button(shell, SWT.PUSH);
		button.setText("Click Me");
		button.addSelectionListener(new SelectionAdapter() {

			public void widgetSelected(SelectionEvent event) {
				int offset = text.getCaretOffset();
				StyleRange style = new StyleRange();
				style.start = offset;
				style.length = 1;
				style.metrics = new GlyphMetrics(40, 0, 0);
				text.setStyleRange(style);
			}

		});
		shell.open();
		while (!shell.isDisposed()) {
			if (!display.readAndDispatch())
				display.sleep();
		}
		display.dispose();
	}
}
-----------------------------------------------------------------------------

Click on button, must replace the character offset style with GlyphMetrics ascent and line must take place (please note that the character will be not visible because GlyphMetrics replace the character). But with this snippet, we can check if you have the same problem than with Java Editor. Thanks for your feedback!
Comment 39 Martin Lippert CLA 2018-04-05 03:21:05 EDT
(In reply to Angelo ZERR from comment #37)
>  * inlined annotation demo:
> https://git.eclipse.org/c/gerrit/platform/eclipse.platform.text.git/tree/org.
> eclipse.jface.text.examples/src/org/eclipse/jface/text/examples/sources/
> inlined/InlinedAnnotationDemo.java

Will attach a screenshot. Not sure if that is showing the "correct" output or not.

>  * code mining demo:
> https://git.eclipse.org/c/gerrit/platform/eclipse.platform.text.git/tree/org.
> eclipse.jface.text.examples/src/org/eclipse/jface/text/examples/codemining/
> CodeMiningDemo.java

I tried this example and it does show a redraw artifact on the first character of the first mining. Will attach a screenshot, too.
Comment 40 Martin Lippert CLA 2018-04-05 03:21:48 EDT
Created attachment 273441 [details]
CodeMiningDemo.java redraw artifact
Comment 41 Martin Lippert CLA 2018-04-05 03:23:33 EDT
Created attachment 273442 [details]
InlinedAnnotationDemo.java screenshot on macOS
Comment 42 Martin Lippert CLA 2018-04-05 03:29:58 EDT
Created attachment 273443 [details]
screenshot of code snippet before and after

screenshot of your code snippet, before clicking the button (left) and after clicking the button (right)
Comment 43 Angelo ZERR CLA 2018-04-05 03:39:44 EDT
Thanks for your feedback @Martin. I would like to understand if the problem comes from the redraw of character. Could you:

 1) use eclipse.platform.text from master.
 2) Uncomment the redraw of characater for line header annotation https://github.com/eclipse/eclipse.platform.text/blob/master/org.eclipse.jface.text/src/org/eclipse/jface/text/source/inlined/InlinedAnnotationDrawingStrategy.java#L142
 3) play only with "Show references" with JDT Java Editor (enable/disable mining with preferences and click apply button like your demo).

Except the character which is not redrawn, have you the same problem ?
Comment 44 Martin Lippert CLA 2018-04-05 03:58:31 EDT
Created attachment 273444 [details]
redraw artifacts after show references got enabled

I used plastform.text from master (at least the jface.text bundle, or should I use more from platform.text?), added a line comment to that line you mentioned, opened a java editor (without "show references" enabled), then opened preference page, enabled "show references", then clicked "apply" -> screenshot.
Comment 45 Mickael Istria CLA 2018-04-05 05:23:54 EDT
(In reply to Martin Lippert from comment #31)
> Created attachment 273420 [details]
> pure jdt-codemining redraw issues
> 
> here is a new video, showing jdt-codeminings only (no language server
> involved) and several redraw issues on macOS

I tried this on Fedora 27 and couldn't see those issues.

It seems to me that the codemining API could be improved to prevent this bug from happening on macos. But on the other hand, if it works on the 2 other platform, it's probably better to use this as an opportunity to investigate the difference of behavior on lower level.
So my take on it is let's keep codeming as it and dig down into fixing macos drawing.
Comment 46 Eclipse Genie CLA 2018-04-05 07:01:03 EDT
New Gerrit change created: https://git.eclipse.org/r/120769
Comment 47 Angelo ZERR CLA 2018-04-05 07:06:44 EDT
> I used plastform.text from master (at least the jface.text bundle, or should I use more from platform.text?)

You have done the test correctly. In your screenshot you can see that "ublic" is displayed instead of displaying "public". Your screenshot show a problem for mining in class. You have a "p". This "p" comes from the old line before mining is drawn. So now the question is why we have this problem? 

I have discussed about this problem with Mickael, so it seems that problem should be fixed for MacOS (TextLayout#draw problem?)

But we have a little problem with mining draw (mining is drawn even if the GlyphMetrics is not updated and it draws the annotation content in the line content, but in Windows OS/Linux it seems that it's not a problem).

Could you please try my experimental patch at https://git.eclipse.org/r/#/c/120769/ and tell me if you have better result (it will work only with line header annotation like "Show references)? Thanks!
Comment 48 Martin Lippert CLA 2018-04-05 07:14:56 EDT
I tried the patch and it behaves "differently" now... :-)

The good news is: when opening a java editor and enabling the references code mining, it shows up just fine and correctly (no redraw artifacts).

The bad news is: when I disable the references again (via the preferences), they don't disappear in the editor, they stay. I have to close and re-open the editor itself to make them disappear.

But it looks like we are on the right path here... :-)
Comment 49 Martin Lippert CLA 2018-04-05 07:19:19 EDT
That was too early to announce victory... After a few more tries, the artifacts are back... :-(
Comment 50 Martin Lippert CLA 2018-04-05 07:39:19 EDT
The interesting part here is that the redraw artifacts disappear when I close the preference dialog. It looks like something goes wrong when code minings change (appear, disappear, get resolved, etc.) and the wrong pieces get redrawn (or only partially redrawn).
Comment 51 Angelo ZERR CLA 2018-04-05 07:53:08 EDT
I have an another idea, could you apply my new gerrit patch https://git.eclipse.org/r/120769 please.
Comment 52 Martin Lippert CLA 2018-04-05 08:05:40 EDT
Tried it, no difference... :-(

Can you point me at the code that calculates the redraw region if code minings appear (or disappear)? It looks to me like that redraw region is not correct.
Comment 53 Martin Lippert CLA 2018-04-05 08:12:42 EDT
Some wild guess... (I know, it is just a wild guess, but maybe worth a thought)... From the artifacts that show up on the screen it looks to me like redraws are executed in random order with wrong or outdated redraw regions. Something like "we need to redraw five code minings, lets go", then those code minings are redrawn in random order, but repaint just a specific piece of the editor and not the text that got moved due to code minings that already have been redrawn.

Since the wrong redraw artifacts are not always the same, but appear in random fashion, it really looks like some execution order problem with wrong redraw regions. Could that be? Or is this a totally stupid thought?
Comment 54 Angelo ZERR CLA 2018-04-05 08:32:55 EDT
> Tried it, no difference... :-(

-(

> Can you point me at the code that calculates the redraw region if code minings appear (or disappear)? 

Drawing of mining is done in InlinedAnnotionDrawingStrategy#draw. The basic idea is to draw only minings which are in the visible lines.

When mining must be drawn, you have 2 cases:

 * mining is resolved: the mining can be drawn.
 * mining is not resolved, the mining is not drawn and the redraw is done when mining is resolved (see redraw method of CodeMiningLineHeaderAnnotation which uses CompletableFuture (mining.resolve) and redraw the mining. This strategy gives the capability not to freeze the Eclipse IDE if mining is not resolved).

> It looks to me like that redraw region is not correct.

Please debug InlinedAnnotionDrawingStrategy#draw

> From the artifacts that show up on the screen it looks to me like redraws are executed in random order with wrong or outdated redraw regions. Something like "we need to redraw five code minings, lets go", 


draw of mining is executed by AnnotationPainter by checking if mining is in visible lines. If mining is resolved, mining is drawn, otherwise a redraw is executed once the mining is resolved (see my explanation below).

> then those code minings are redrawn in random order, 

Yes it depends on time of resolved mining.

> but repaint just a specific piece of the editor and not the text that got moved due to code minings that already have been redrawn.

mining is not the same classic annotation, you need it to redraw all mining which are visibles because a mining A can depend on any text (ex : references label is computed with search (like Ctrl+Shift+G). I mean if you have that:

------------------------------
[references 2]
void f();
------------------------------

[references 2] mining must be redrawn:

 * if you change the line void f() (like classic annotation)
 * if you change some other text which reference f().

> Since the wrong redraw artifacts are not always the same, but appear in random fashion, it really looks like some execution order problem with wrong redraw regions. Could that be? Or is this a totally stupid thought?

For me the problem comes from when GlyphMetrics which changed in several place  in MacOS. If you can debug the problem it should really fantastic! It's very hard for me to help you without debugging the problem as I have not MacOS. Don't hesitate to post your question and if code is bad, I'm totally aware to change it. Thanks!
Comment 55 Angelo ZERR CLA 2018-04-05 09:20:01 EDT
@Karsten, I think you have MacOS, no? Have you the same problem than @Martin?
Comment 56 Patrik Suzzi CLA 2018-04-20 03:10:35 EDT
I did a debug session, both on MacOS and Win10, to understand the different beahvior of CodeMining. I discovered that on Mac the paint event is called repeatedly, while on windows is called only once. I believe this difference in the repaint is related to the different behaviors in swt. Indeed I analyzed the loop: 

Display.sendEvent(EventTable, Event) 
-> EventTable.sendEvent(0)
-> TypedListener.handleEvent(Event)
-> AnnotationPainter.paintControl(PaintEvent)
-> InlinedAnnotationDrawingStrategy.draw(...)

And I did some stdout to gather information: 

As said, on Win10, when clicking on the TextEditor, the draw(LineHeaderAnnotation) is called once, with this output

[1] AnnotationPainter.paintControl() time: 464558389 hash: 630167544
[2] AnnotationPainter.drawDecoration() time: Decoration/96119738 hash: CodeMiningLineHeaderAnnotation/533825600
[3] InlinedAnnotationDrawingStrategy.draw(...) annotation: CodeMiningLineHeaderAnnotation/533825600 textWidget: StyledText/630167544

While, on MacOs, when clicking on the TextEditor, the draw(.) is called multiple times, in a continuous loop, while the Editor is active:

[1] AnnotationPainter.paintControl() time: 464558389 hash: 630167544
[2] AnnotationPainter.drawDecoration() time: Decoration/96119738 hash: CodeMiningLineHeaderAnnotation/533825600
[3] InlinedAnnotationDrawingStrategy.draw(...) annotation: CodeMiningLineHeaderAnnotation/533825600 textWidget: StyledText/630167544
[1] AnnotationPainter.paintControl() time: 464558799 hash: 630167544
[2] AnnotationPainter.drawDecoration() time: Decoration/96119738 hash: CodeMiningLineHeaderAnnotation/533825600
[3] InlinedAnnotationDrawingStrategy.draw(...) annotation: CodeMiningLineHeaderAnnotation/533825600 textWidget: StyledText/630167544
[1] AnnotationPainter.paintControl() time: 464620912 hash: 630167544
[2] AnnotationPainter.drawDecoration() time: Decoration/96119738 hash: CodeMiningLineHeaderAnnotation/533825600
[3] InlinedAnnotationDrawingStrategy.draw(...) annotation: CodeMiningLineHeaderAnnotation/533825600 textWidget: StyledText/630167544


During the debug, I get to some points, in SWT, where the code differs for the two cases. For example, the Display.sendEvent(EventTable, Event) on MacOs, is different than the one on Windows.

on windows:

void sendEvent (EventTable eventTable, Event event) {
	int type = event.type;
	sendPreEvent (type);
	try {
		eventTable.sendEvent (event);
	} finally {
		sendPostEvent (type);
	}
}

on Mac:

void sendEvent (EventTable table, Event event) {
	try {
		sendEventCount++;
		if (!filterEvent (event)) {
			if (table != null) {
				// HERE, SAME CODE AS IN WINDOWS
			}
		}
	} finally {
		sendEventCount--;
	}
} 

I think trying to fix this issue by doing SWT debugging is not the right choice. 
On the other side, I assume there should be some document or example showing the best practices. 

Hence, my question: Is there anyone who can point to the best practices for ensuring the annotation painting event is triggered only once in all the systems?
Comment 57 Angelo ZERR CLA 2018-04-20 05:37:10 EDT
> I did a debug session, both on MacOS and Win10, to understand the different beahvior of CodeMining.

Thank a lot Patrik !

> While, on MacOs, when clicking on the TextEditor, the draw(.) is called multiple times, in a continuous loop, while the Editor is active:

Perhaps it's a bug with InlinedAnnotationDrawingStrategy#draw (in MacOS)? Is it possiblz to debug it? When an annotation is drawn, several repaint must be done (the first time):

 * step1: gc is null and GlyphMetrics is not ready, textWidget.redrawRange is called. A repaint is done in this case.
 * step2: gc is not null and GlyphMetrics is not ready, textWidget.setStyleRange is called to update GlyphMetrics to take some spaces where annotation must be drawn in the next step. A repaint is done in this case.
 * step3: draw content of annotation.

The last case is when gc is null but the GlyphMetrics is ready, textWidget.redraw(int x, int y, int width, int height, boolean all) is called. It's perhaps where there is a problem?
Comment 58 Angelo ZERR CLA 2018-04-30 05:02:30 EDT
The bad news is that I can reproduce this problem in Windows OS by setting the Theme as Dark (which generate more StyleRange than Light Theme). It's a good news for MacOS! So I move this bug to All OS.

It's very hard problem, because I cannot reproduce everytime, but it seems the problem comes from the StyledText#setStyleRanges(int start, int length, int[] ranges, StyleRange[] styles, boolean reset). When I set top to 0, it seems working. I know it's not a good fix, but it's an idea (is it a problem with compute of top).

@Martin, could you update StyledText L10109:

------------------------------------------------------------
super.redraw(0, top, clientAreaWidth, bottom - top, false);
------------------------------------------------------------

with:

------------------------------------------------------------
top = 0;
super.redraw(0, top, clientAreaWidth, bottom - top, false);
------------------------------------------------------------

and tell us it fixes your problem in MacOS? Thanks!
Comment 59 Angelo ZERR CLA 2018-05-02 05:56:31 EDT
After debugging more the problem it seems problem comes from  the StyledTextPaintListener#paintControl of org.eclipse.e4.ui.internal.css.swt.dom.scrollbar#StyledTextThemedScrollBarAdapter.

@Martin, could you comment the body of StyledTextPaintListener#paintControl and tell me if it fixes your problem?

Thanks!
Comment 60 Angelo ZERR CLA 2018-05-02 13:46:14 EDT
@Martin please test my gerrit patch of th ebuig https://bugs.eclipse.org/bugs/show_bug.cgi?id=534276 

Perhaps it is the same problem than you.
Comment 61 Martin Lippert CLA 2018-05-03 08:03:02 EDT
(In reply to Angelo ZERR from comment #60)
> @Martin please test my gerrit patch of th ebuig
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=534276 
> 
> Perhaps it is the same problem than you.

I tried with latest from master (jface.text) and platform integration build from I20180502-0100.

It looks a lot better and seems to work more reliable, but redraw artifacts still show up, for example, when doing copy/paste of several lines, for example. The redraw artifacts only occur as a direct result of an action that modifies the text and disappear as soon as the specific region got redrawn.

Do you need another video?
Comment 62 Angelo ZERR CLA 2018-05-03 08:19:00 EDT
> but redraw artifacts still show up

-(

> Do you need another video?

Yes please.
Comment 63 Martin Lippert CLA 2018-05-03 10:57:46 EDT
Created attachment 273911 [details]
codemining redraw artifacts - 2018-05-03 - update1
Comment 64 Martin Lippert CLA 2018-05-03 10:58:13 EDT
Created attachment 273912 [details]
codemining redraw artifacts - 2018-05-03 - update2
Comment 65 Martin Lippert CLA 2018-05-03 10:59:09 EDT
Created attachment 273913 [details]
codemining redraw artifacts - 2018-05-03 - update3

added three quick screencasts showing various redraw artifacts using the latest I-build and jface.text from master (2018-05-03)
Comment 66 Angelo ZERR CLA 2018-05-03 11:33:52 EDT
Thanks a lot @Martin for your video. In other words, there is again the problem with MacOS.

Could you just try with line content annotation and not with line header annotation (disable Show references mining which draws mining in the header of the line). Have you problem?
Comment 67 Angelo ZERR CLA 2018-05-04 01:45:32 EDT
According your screenshot with InlinedAnnotationDemo https://bugs.eclipse.org/bugs/attachment.cgi?id=273442 it seems that you have not problem. Could you play with this demo (by adding, removing some text with color: etc). Have you the problem? If you have not a problem, could you test with CodeMiningDemo?

If you have not problem with InlinedAnnotationDemo but with CodeMiningDemo, it means that inlined annotation works with MacOS, but not with a second redraw done in a Thread.
Comment 68 Martin Lippert CLA 2018-05-04 07:53:00 EDT
> Could you just try with line content annotation and not with line header
> annotation (disable Show references mining which draws mining in the header
> of the line). Have you problem?

I enabled parameter names and parameter types annotations only and tried again. When I open the Java editor for the first time, it doesn't render parts of the line correctly. When I close the editor and open it again, it renders the bits correctly.

It only happens when I open the editor for the first time (for my workspace session). And this first time the redraw is significantly slower than for the second time or when opening additional editors.

Could indeed be related to threading and race conditions.
Comment 69 Angelo ZERR CLA 2018-05-17 08:03:27 EDT
> I enabled parameter names and parameter types annotations only and tried again. When I open the Java editor for the first time, it doesn't render parts of the line correctly. When I close the editor and open it again, it renders the bits correctly.

It only happens when I open the editor for the first time (for my workspace session). And this first time the redraw is significantly slower than for the second time or when opening additional editors.

Could indeed be related to threading and race conditions.

The Java mining uses the CompilationUnit AST and visitor friends. I think it's slow teh first time because build of CompilationUnit takes time the first time.

@Lars have the same problem with Linux OS https://github.com/angelozerr/jdt-codemining/issues/27
Comment 70 Angelo ZERR CLA 2018-05-17 09:33:36 EDT
@Martin, if you play with InlinedAnnotationDemo with a lot of copy/paste, remove texte (by typing/cpying text  "color: rgb(0,1,1)" ) have you the problem?
Comment 71 Mickael Istria CLA 2018-05-22 11:29:39 EDT
@Angelo: any chance this one can be fixed before Photon? If not, I believe it will delay ability for JDT to adopt code-minings in a near future.
Comment 72 Angelo ZERR CLA 2018-05-22 11:36:38 EDT
@Mickael, I'm in contact with @Martin and we have found a simple snippet without inlined annotation which causes problem (just call setStyleRange in paint even and draw content with gc). There is a problem with MacOS and not with Windows.

I will create a new bug for that. I have some idea to fix the problem but not sure it will work. So please keep open this bug and if I find a fix which works with @Martin, I will create a gerrit patch. Thanks!
Comment 73 Till Brychcy CLA 2018-05-23 15:17:52 EDT
I can reproduce the behaviour in "CodeMiningDemo.java redraw artifact", where it looks like the grey "2" is drawn over a black "c"

"InlinedAnnotationDemo.java screenshot on macOS" looks the same for me, too, and I assume this is as expected
Comment 74 Angelo ZERR CLA 2018-05-24 03:24:37 EDT
> I can reproduce the behaviour in "CodeMiningDemo.java redraw artifact", where it looks like the grey "2" is drawn over a black "c"

I think https://bugs.eclipse.org/bugs/show_bug.cgi?id=534888 could improve that but it requires change of API, so it will not available for Photon.

> "InlinedAnnotationDemo.java screenshot on macOS" looks the same for me, too, and I assume this is as expected

Thanks @Till for your feedback. To be honnest with you, I'm a little lost. It seems that with the same Platform Text, SWT version and MacOS version we have 2 behaviours:

 * @Till have no problem with InlinedAnnotationDemo and snippet https://bugs.eclipse.org/bugs/show_bug.cgi?id=534977 but have problem with CodeMiningDemo
 * @Martin have problem with InlinedAnnotationDemo, snippet and CodeMiningDemo.

@Till could you just change in InlinedAnnotationSupport#runInUIThread L560, 

----------------------------------
display.asyncExec(() -> {
----------------------------------

with 

----------------------------------
display.syncExec(() -> {
----------------------------------

and tell me if you have better draw with CodeMiningDemo please.
Comment 75 Till Brychcy CLA 2018-05-24 05:35:50 EDT
(In reply to Angelo ZERR from comment #74)
> @Till could you just change in InlinedAnnotationSupport#runInUIThread L560, 
> 
> ----------------------------------
> display.asyncExec(() -> {
> ----------------------------------
> 
> with 
> 
> ----------------------------------
> display.syncExec(() -> {
> ----------------------------------
> 
> and tell me if you have better draw with CodeMiningDemo please.

Doesn't make a difference.
Comment 76 Angelo ZERR CLA 2018-05-24 05:58:37 EDT
@Till could you try my gerrit patch https://git.eclipse.org/r/#/c/123245/ please and tell me if it improves somethings. Thanks!
Comment 77 Till Brychcy CLA 2018-05-24 06:57:18 EDT
(In reply to Angelo ZERR from comment #76)
> @Till could you try my gerrit patch https://git.eclipse.org/r/#/c/123245/
> please and tell me if it improves somethings. Thanks!

This patch leaves the result of CodeMiningDemo unchanged.
Comment 78 Till Brychcy CLA 2018-05-24 07:14:16 EDT
I don't have time to debug this now, but a breakpoint at draw reveals:
- This breakpoint is hit 8 times before the annotations is drawn (and then the 2 is drawn over c)
- There are different stack traces:

org.eclipse.jface.text.source.inlined.InlinedAnnotationDrawingStrategy.draw(org.eclipse.jface.text.source.inlined.LineHeaderAnnotation, org.eclipse.swt.graphics.GC, org.eclipse.swt.custom.StyledText, int, int, org.eclipse.swt.graphics.Color) line: 76	
org.eclipse.jface.text.source.inlined.InlinedAnnotationDrawingStrategy.draw(org.eclipse.jface.text.source.inlined.AbstractInlinedAnnotation, org.eclipse.swt.graphics.GC, org.eclipse.swt.custom.StyledText, int, int, org.eclipse.swt.graphics.Color) line: 58	
org.eclipse.jface.text.source.inlined.InlinedAnnotationDrawingStrategy.draw(org.eclipse.jface.text.source.Annotation, org.eclipse.swt.graphics.GC, org.eclipse.swt.custom.StyledText, int, int, org.eclipse.swt.graphics.Color) line: 41	
org.eclipse.jface.text.source.AnnotationPainter.drawDecoration(org.eclipse.jface.text.source.AnnotationPainter$Decoration, org.eclipse.swt.graphics.GC, org.eclipse.jface.text.source.Annotation, org.eclipse.jface.text.IRegion, org.eclipse.jface.text.IDocument) line: 1421	
org.eclipse.jface.text.source.AnnotationPainter.handleDrawRequest(org.eclipse.swt.events.PaintEvent) line: 1387	
org.eclipse.jface.text.source.AnnotationPainter.enablePainting() line: 481	
org.eclipse.jface.text.source.AnnotationPainter.updatePainting(org.eclipse.jface.text.source.AnnotationModelEvent) line: 951	
org.eclipse.jface.text.source.AnnotationPainter.access$1(org.eclipse.jface.text.source.AnnotationPainter, org.eclipse.jface.text.source.AnnotationModelEvent) line: 943	
org.eclipse.jface.text.source.AnnotationPainter$1.run() line: 1074	
org.eclipse.swt.widgets.RunnableLock.run(org.eclipse.swt.widgets.Display) line: 37	
org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(boolean) line: 182	
org.eclipse.swt.widgets.Display.runAsyncMessages(boolean) line: 4043	
org.eclipse.swt.widgets.Display.readAndDispatch() line: 3714	
org.eclipse.jface.text.examples.codemining.CodeMiningDemo.main(java.lang.String[]) line: 79	

and

org.eclipse.jface.text.source.inlined.InlinedAnnotationDrawingStrategy.draw(org.eclipse.jface.text.source.inlined.LineHeaderAnnotation, org.eclipse.swt.graphics.GC, org.eclipse.swt.custom.StyledText, int, int, org.eclipse.swt.graphics.Color) line: 76	
org.eclipse.jface.text.source.inlined.InlinedAnnotationDrawingStrategy.draw(org.eclipse.jface.text.source.inlined.AbstractInlinedAnnotation, org.eclipse.swt.graphics.GC, org.eclipse.swt.custom.StyledText, int, int, org.eclipse.swt.graphics.Color) line: 58	
org.eclipse.jface.text.source.inlined.InlinedAnnotationDrawingStrategy.draw(org.eclipse.jface.text.source.Annotation, org.eclipse.swt.graphics.GC, org.eclipse.swt.custom.StyledText, int, int, org.eclipse.swt.graphics.Color) line: 41	
org.eclipse.jface.text.source.AnnotationPainter.drawDecoration(org.eclipse.jface.text.source.AnnotationPainter$Decoration, org.eclipse.swt.graphics.GC, org.eclipse.jface.text.source.Annotation, org.eclipse.jface.text.IRegion, org.eclipse.jface.text.IDocument) line: 1421	
org.eclipse.jface.text.source.AnnotationPainter.handleDrawRequest(org.eclipse.swt.events.PaintEvent) line: 1387	
org.eclipse.jface.text.source.AnnotationPainter.paintControl(org.eclipse.swt.events.PaintEvent) line: 1335	
org.eclipse.swt.widgets.TypedListener.handleEvent(org.eclipse.swt.widgets.Event) line: 231	
org.eclipse.swt.widgets.EventTable.sendEvent(org.eclipse.swt.widgets.Event) line: 86	
org.eclipse.swt.widgets.Display.sendEvent(org.eclipse.swt.widgets.EventTable, org.eclipse.swt.widgets.Event) line: 4247	
org.eclipse.swt.custom.StyledText(org.eclipse.swt.widgets.Widget).sendEvent(org.eclipse.swt.widgets.Event) line: 1508	
org.eclipse.swt.custom.StyledText(org.eclipse.swt.widgets.Widget).sendEvent(int, org.eclipse.swt.widgets.Event, boolean) line: 1531	
org.eclipse.swt.custom.StyledText(org.eclipse.swt.widgets.Widget).sendEvent(int, org.eclipse.swt.widgets.Event) line: 1516	
org.eclipse.swt.custom.StyledText(org.eclipse.swt.widgets.Control).drawWidget(long, org.eclipse.swt.internal.cocoa.NSGraphicsContext, org.eclipse.swt.internal.cocoa.NSRect) line: 1277	
org.eclipse.swt.custom.StyledText(org.eclipse.swt.widgets.Canvas).drawWidget(long, org.eclipse.swt.internal.cocoa.NSGraphicsContext, org.eclipse.swt.internal.cocoa.NSRect) line: 172	
org.eclipse.swt.custom.StyledText(org.eclipse.swt.widgets.Widget).drawRect(long, long, org.eclipse.swt.internal.cocoa.NSRect) line: 772	
org.eclipse.swt.custom.StyledText(org.eclipse.swt.widgets.Canvas).drawRect(long, long, org.eclipse.swt.internal.cocoa.NSRect) line: 166	
org.eclipse.swt.widgets.Display.windowProc(long, long, long) line: 5884	
org.eclipse.swt.internal.cocoa.OS.objc_msgSendSuper(org.eclipse.swt.internal.cocoa.objc_super, long, long, long, long, boolean) line: not available [native method]	
org.eclipse.swt.widgets.Display.applicationNextEventMatchingMask(long, long, long, long, long, long) line: 5138	
org.eclipse.swt.widgets.Display.applicationProc(long, long, long, long, long, long) line: 5562	
org.eclipse.swt.internal.cocoa.OS.objc_msgSend(long, long, long, long, long, boolean) line: not available [native method]	
org.eclipse.swt.internal.cocoa.NSApplication.nextEventMatchingMask(long, org.eclipse.swt.internal.cocoa.NSDate, org.eclipse.swt.internal.cocoa.NSString, boolean) line: 94	
org.eclipse.swt.widgets.Display.readAndDispatch() line: 3707	
org.eclipse.jface.text.examples.codemining.CodeMiningDemo.main(java.lang.String[]) line: 79
Comment 79 Till Brychcy CLA 2018-05-24 07:17:04 EDT
(In reply to Till Brychcy from comment #78)
> I don't have time to debug this now, but a breakpoint at draw reveals:
> - This breakpoint is hit 8 times before the annotations is drawn (and the

"draw" is org.eclipse.jface.text.source.inlined.InlinedAnnotationDrawingStrategy.draw(LineHeaderAnnotation, GC, StyledText, int, int, Color).

Tested with the master, not the gerrit.
Comment 80 Eclipse Genie CLA 2018-05-28 08:12:39 EDT
New Gerrit change created: https://git.eclipse.org/r/123440
Comment 81 Angelo ZERR CLA 2018-07-30 04:56:02 EDT
I think this bug is the same bug than https://bugs.eclipse.org/bugs/show_bug.cgi?id=534977 

I think we can close it since https://bugs.eclipse.org/bugs/show_bug.cgi?id=534977 is fixed for MacOS, no?

@Martin could you confirm that?
Comment 82 Martin Lippert CLA 2018-07-30 09:15:29 EDT
Yes, looks like we can close this one, too.
Great to see this working now without the redraw artifacts... :-)

Thanks so much for working on this!!!
Comment 83 Angelo ZERR CLA 2018-07-30 12:26:31 EDT

*** This bug has been marked as a duplicate of bug 534977 ***