Bug 531667 - [GTK3] Cannot draw Canvas with Control.print(GC)
Summary: [GTK3] Cannot draw Canvas with Control.print(GC)
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 4.7.2   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 4.8 M7   Edit
Assignee: Simeon Andreev CLA
QA Contact: Andrey Loskutov CLA
URL:
Whiteboard:
Keywords:
: 533131 (view as bug list)
Depends on: 534417
Blocks: 525804 533241
  Show dependency tree
 
Reported: 2018-02-26 07:11 EST by Simeon Andreev CLA
Modified: 2018-11-08 04:54 EST (History)
8 users (show)

See Also:


Attachments
A snippet with a canvas to reproduce the problem. (2.13 KB, text/x-java)
2018-02-26 07:11 EST, Simeon Andreev CLA
no flags Details
Shell from snippet with SWT_GTK3=0. (7.11 KB, image/png)
2018-02-26 10:20 EST, Simeon Andreev CLA
no flags Details
Shell from snippet with SWT_GTK3=1. (7.12 KB, image/png)
2018-02-26 10:21 EST, Simeon Andreev CLA
no flags Details
Image drawn by snippet with SWT_GTK3=0. (3.01 KB, image/png)
2018-02-26 10:21 EST, Simeon Andreev CLA
no flags Details
Image drawn by snippet with SWT_GTK3=1. (176 bytes, image/png)
2018-02-26 10:21 EST, Simeon Andreev CLA
no flags Details
crazy_ctabfolder_painting (102.09 KB, image/png)
2018-02-27 11:14 EST, Andrey Loskutov CLA
no flags Details
Removing transformations from GC.getClipping results in OK behaviour. (2.50 KB, patch)
2018-02-28 11:45 EST, Simeon Andreev CLA
no flags Details | Diff
Snippet which showcases GC.getClipping behaviour for Bug 421127. (5.71 KB, text/x-java)
2018-03-01 05:13 EST, Simeon Andreev CLA
no flags Details
Snippet showing regression with commited patch. (1.46 KB, text/x-java)
2018-03-20 14:25 EDT, Simeon Andreev CLA
no flags Details
Sample project to reproduce the regression on a GMF-based editor (10.51 KB, application/zip)
2018-03-21 05:46 EDT, Pierre-Charles David CLA
no flags Details
How the GMF example modeler looks after the scenario (162.30 KB, image/png)
2018-03-21 05:48 EDT, Pierre-Charles David CLA
no flags Details
GMF editor drawing artefacts after latest patch. (36.95 KB, image/png)
2018-03-27 10:40 EDT, Simeon Andreev CLA
no flags Details
Validation decorators - good (1.40 KB, image/png)
2018-03-28 16:37 EDT, Daniel Wille CLA
no flags Details
Validation decorators - bad (1.73 KB, image/png)
2018-03-28 16:38 EDT, Daniel Wille CLA
no flags Details
Example plug-in showing validation errors (2.31 KB, application/gzip)
2018-03-30 11:08 EDT, Daniel Wille CLA
no flags Details
my workbench frozen on double click in staging view (218.71 KB, image/png)
2018-04-10 11:19 EDT, Andrey Loskutov CLA
no flags Details
ghost button above "default value" (33.36 KB, image/png)
2018-04-10 11:45 EDT, Ansgar Radermacher CLA
no flags Details
Clipping issues on HiDPI. (25.28 KB, image/png)
2018-05-03 15:50 EDT, Peter Severin CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Simeon Andreev CLA 2018-02-26 07:11:14 EST
Created attachment 272873 [details]
A snippet with a canvas to reproduce the problem.

Steps to reproduce:

1. Run attached snippet.
2. Observe that image "~/some_canvas.png" is empty.
3. Run attached snippet with ENV variable SWT_GTK3=0.
4. Observe that image "~/some_canvas.png" is the same as the shown shell.

The canvas is defined as follows:

Composite composite = new Composite(shell, SWT.NONE);
composite.setLayout(new FillLayout());
Canvas canvas = new Canvas(composite, SWT.NONE);
canvas.setBackground(display.getSystemColor(SWT.COLOR_BLACK));
Color white = display.getSystemColor(SWT.COLOR_WHITE);

canvas.addPaintListener(new PaintListener() {
	public void paintControl(PaintEvent e) {
		Rectangle clientArea = canvas.getClientArea();
		e.gc.setBackground(white);
		e.gc.fillOval(0, 0, clientArea.width, clientArea.height);
	}
});

The example is more or less taken from:

http://www.eclipse.org/articles/Article-SWT-graphics/SWT_graphics.html#Canvas


Environment:

Eclipse SDK
Version: Photon (4.8)
Build id: I20171217-2000

RHEL 7.2
gtk3-3.14.13

Also observed with:

RHEL 7.4
gtk3-3.22.10
Comment 1 Simeon Andreev CLA 2018-02-26 07:18:46 EST
I've observed that: 

1. Control.print does trigger an onPaint event on the canvas.
2. The GC that is passed to Control.print is not the same as the GC on which the onPaint event arrives.

I'm not sure if the two are supposed to be merged somehow.

I tried defining Control.print in our custom Canvas, however Control.print doesn't seem to be called on the children of the control. So that doesn't help.
Comment 2 Andrey Loskutov CLA 2018-02-26 09:22:37 EST
Interesting, there was a change on 4.8 for printing (see bug 530969) but this fix seem not to work anymore on GTK 3.14+.
Comment 3 Eric Williams CLA 2018-02-26 10:08:52 EST
Please attach the image you are using so I can run it with the snippet.
Comment 4 Simeon Andreev CLA 2018-02-26 10:20:45 EST
Created attachment 272877 [details]
Shell from snippet with SWT_GTK3=0.
Comment 5 Simeon Andreev CLA 2018-02-26 10:21:02 EST
Created attachment 272878 [details]
Shell from snippet with SWT_GTK3=1.
Comment 6 Simeon Andreev CLA 2018-02-26 10:21:23 EST
Created attachment 272879 [details]
Image drawn by snippet with SWT_GTK3=0.
Comment 7 Simeon Andreev CLA 2018-02-26 10:21:39 EST
Created attachment 272880 [details]
Image drawn by snippet with SWT_GTK3=1.
Comment 8 Simeon Andreev CLA 2018-02-26 10:23:08 EST
Hi Eric,

I'm not sure which image you are requesting, so I attached:

* shell with GTK2 (OK)
* image drawn with GTK2 (OK)
* shell with GTK3 (OK)
* image drawn with GTK3 (not OK)

You can specify where the image is saved in the first line of the main method. Currently this is directly in the home directory of the current user as "some_canvas.png".

Best regards,
Simeon
Comment 9 Eric Williams CLA 2018-02-26 10:31:49 EST
(In reply to Simeon Andreev from comment #8)
> Hi Eric,
> 
> I'm not sure which image you are requesting, so I attached:
> 
> * shell with GTK2 (OK)
> * image drawn with GTK2 (OK)
> * shell with GTK3 (OK)
> * image drawn with GTK3 (not OK)
> 
> You can specify where the image is saved in the first line of the main
> method. Currently this is directly in the home directory of the current user
> as "some_canvas.png".
> 
> Best regards,
> Simeon

Sorry I misread what the snippet does, I understand now. I can reproduce the issue as you have described. I'll add this one to my list.
Comment 10 Simeon Andreev CLA 2018-02-27 08:56:08 EST
After checking out what Control.gtk_draw(long, long) does on the stack trace from Control.print(GC), its rather obvious what is missing:

if (GTK.GTK_VERSION <= OS.VERSION (3, 9, 0)) {
	data.cairo = cairo;
}

This ensures the GC draws into the correct area.

The version check is coming from the following commit:

https://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=a8f2f3ce06e6b080f57995fbe883c8c15e3b7c54

The commit message states: "fixing cairo clipping"
Comment 11 Simeon Andreev CLA 2018-02-27 09:04:37 EST
Hi Arun Thondapu,

could you explain what commit a8f2f3ce fixes? See Comment #10.

Best regards and thanks,
Simeon Andreev
Comment 12 Andrey Loskutov CLA 2018-02-27 10:00:34 EST
(In reply to Simeon Andreev from comment #11)
> Hi Arun Thondapu,
> 
> could you explain what commit a8f2f3ce fixes? See Comment #10.
> 
> Best regards and thanks,
> Simeon Andreev

This is coming from bug 421127 comment 55, see patch https://git.eclipse.org/r/25290.

This was explained to be needed for:

###############
1) If you try CustomControlExample with GTK >= 3.10, the tabs in the CTabFolder group won't be shown.

I suppose the same issue is described in bug 421127 comment 14.

The issue occurs because GTK >= 3.10 adds the same cairo object to all container's children. SWT invalidates it.
Solution: forcing SWT to create a new cairo whenever getting a draw signal (Control.gtk_draw).
###############
Comment 13 Andrey Loskutov CLA 2018-02-27 11:14:32 EST
Created attachment 272912 [details]
crazy_ctabfolder_painting

(In reply to Andrey Loskutov from comment #12)
> This is coming from bug 421127 comment 55, see patch
> https://git.eclipse.org/r/25290.
> 
> This was explained to be needed for:
> 
> ###############
> 1) If you try CustomControlExample with GTK >= 3.10, the tabs in the
> CTabFolder group won't be shown.
> 
> I suppose the same issue is described in bug 421127 comment 14.
> 
> The issue occurs because GTK >= 3.10 adds the same cairo object to all
> container's children. SWT invalidates it.
> Solution: forcing SWT to create a new cairo whenever getting a draw signal
> (Control.gtk_draw).
> ###############


If we revert the change from patch https://git.eclipse.org/r/25290: 

// if (GTK.GTK_VERSION <= OS.VERSION (3, 9, 0)) {
	data.cairo = cairo;
// }

we observe that printing starts to work, but CTabfolder items start to behave really surprising (on GTK 3.14): they are *not always* invisible as stated in bug 421127 comment 55! They are only invisible if they are placed in the right 50% of the parent composite or in the lower 50% !?!?!

In the attached screenshot editor tabs starting from A6.java aren't painted, same as the two invisible tabs in the outline view area and same as all tabs in the problems/javadoc stack area on the bottom.

If one resizes the stacks and the Javadoc/Problems tabs are moved to the upper   part, they re-appear again!!! Same with editor tabs.

So it sounds like a crazy bounds/location/clipping bug in CTabFolder/CTabItem, not like a bug in GTK or Control. As if someone halves the width/height in some computation and allows to paint only if the controls are in the upper left half only.
Comment 14 Simeon Andreev CLA 2018-02-28 04:08:23 EST
Actually the reason that the problem is seen only for CTabFolder is that nobody else seems to use GC.getClipping. The callers are:

org.eclipse.swt.custom.CTabFolderRenderer.drawSelected(int, GC, Rectangle, int)
org.eclipse.swt.custom.CTabFolderRenderer.drawUnselected(int, GC, Rectangle, int)
org.eclipse.swt.widgets.CoolBar.onPaint(Event)

When the tabs are not seen, its because the clipping region returned by the method is [0, 0, 0, 0].

I'm trying to find out where the clipping comes from next, since it doesn't come from the draw method itself.
Comment 15 Simeon Andreev CLA 2018-02-28 10:55:10 EST
This is the reason as far as I can tell: https://bugs.eclipse.org/bugs/show_bug.cgi?id=421127#c14

As far as I can tell, https://git.eclipse.org/r/25290 is a round-about way of ensuring the cairo matrices are always the identity matrix. 

I'm currently trying to understand the transformations done on widget bounds and their clipping areas. At least in some cases I see that GCData.clippingTransform is set with the Cairo user space matrix.

Unfortunately I'm not sure what the "user" and "device" spaces are. Hints on this would be greatly appreciated.
Comment 16 Simeon Andreev CLA 2018-02-28 11:28:52 EST
This is what GC.getClipping does:

1. Transform used clipping rectangle with the Cairo matrix.
2. Intersect the non-transformed component bounds (?) with the transformed clipping.
3. Transform the intersected area, with the inverted (?) Cairo matrix.

I'm not exactly sure what this code is meant to do. As soon as the Cairo transformation matrix has unfortunate values, step 2. yields an empty area and getClipping returns {0, 0, 0, 0}. Then the CTabs tabs don't paint.

I mainly don't understand:

a) Why is step 2. intersecting polygons in different spaces? I would have expected 3. to take place before 2.
b) Why is step 3. using the inverted Cairo transformation matrix? If this is important, why doesn't step 2. also use the inverted one?
Comment 17 Simeon Andreev CLA 2018-02-28 11:34:02 EST
(In reply to Andrey Loskutov from comment #13)
> If one resizes the stacks and the Javadoc/Problems tabs are moved to the
> upper   part, they re-appear again!!! Same with editor tabs.

I'm not 100% sure at the moment but I think this results due to rounding problems. They should never be visible from what I can tell.
Comment 18 Simeon Andreev CLA 2018-02-28 11:45:00 EST
Created attachment 272939 [details]
Removing transformations from GC.getClipping results in OK behaviour.

Its rather unfortunate that if I remove all the matrix transformations from GC.getClipping, as well as changes from https://git.eclipse.org/r/25290, everything looks fine.

I don't know what to think here. Were the transformations some sort of preparation for future changes that never came / have not come yet? Or did they become obsolete at some point?

Maybe if we run this with an older GTK nothing works?
Comment 19 Eric Williams CLA 2018-02-28 11:57:26 EST
I wish I could be of more help but Cairo/clippings are not my area of expertise. All I can say is that we should definitely remove the GTK3.9 check (see comment 10) and replace it with a better fix.

I'm currently working on the context menu bug so I won't be investigating any major issues until that is fixed.
Comment 20 Simeon Andreev CLA 2018-03-01 05:13:59 EST
Created attachment 272952 [details]
Snippet which showcases GC.getClipping behaviour for Bug 421127.
Comment 21 Eclipse Genie CLA 2018-03-01 05:53:06 EST
New Gerrit change created: https://git.eclipse.org/r/118410
Comment 22 Simeon Andreev CLA 2018-03-01 05:55:59 EST
Uploaded patch suggestion 1., which is not very invasive in respect to code changes.

I can also upload a suggestion where I comment out the transformations done on the clipping and on the GC region, if GTK 3.14 or above. That would make more sense from my perspective but would mean changing 2 places in GC.getClippingInPixels and would make the code harder to understand than suggestion 1.
Comment 23 Simeon Andreev CLA 2018-03-01 06:50:53 EST
Compared GC.getClipping results with SWT_GTK3=1, with the patch they are the same, although the Cairo matrices are different.

Without the patch they are also the same, due to the fix for Bug 421127.
Comment 24 Simeon Andreev CLA 2018-03-01 07:17:18 EST
(In reply to Simeon Andreev from comment #23)
> Compared GC.getClipping results with SWT_GTK3=1, with the patch they are the
> same, although the Cairo matrices are different.
> 
> Without the patch they are also the same, due to the fix for Bug 421127.

Sorry, I meant SWT_GTK3=0.
Comment 26 Eric Williams CLA 2018-03-05 10:26:51 EST
(In reply to Eclipse Genie from comment #25)
> Gerrit change https://git.eclipse.org/r/118410 was merged to [master].
> Commit:
> http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/
> ?id=e78489ee67784eb3704ba210de1effcb777e0126

Patch in master now. Many thanks for the investigation time spent to fix this one, Simeon!
Comment 27 Simeon Andreev CLA 2018-03-05 11:22:24 EST
Thanks for merging!
Comment 28 Lakshmi P Shanmugam CLA 2018-03-07 03:17:05 EST
Eric, can you please verify this one with the latest I-build?
Comment 29 Eric Williams CLA 2018-03-07 12:46:32 EST
(In reply to Lakshmi Shanmugam from comment #28)
> Eric, can you please verify this one with the latest I-build?

Verified in I20180305-2000.
Comment 30 Pierre-Charles David CLA 2018-03-20 12:00:15 EDT
I'm reopening this, it caused regressions in Sirius (and possible any GMF or even GEF-based editors). See https://bugs.eclipse.org/bugs/show_bug.cgi?id=525804#c10 and the following comments for the details, but the tl;dr is that with e78489ee67784eb3704ba210de1effcb777e0126, the contents of a Sirius editor can be drawn outside of the editor area, and coordinates are broken. See https://bugs.eclipse.org/bugs/attachment.cgi?id=273224 for what this looks like (this is with Gtk 3.22.11 and also 3.14.5).

Reverting https://git.eclipse.org/r/c/118410/ (e78489ee67784eb3704ba210de1effcb777e0126) seems to fix the issue in our case (see https://bugs.eclipse.org/bugs/show_bug.cgi?id=525804#c15). Of course I have no idea on the possible negative impacts of a complete revert, but this seems to confirm that particular change is the cause of the regression on our side.
Comment 31 Pierre-Charles David CLA 2018-03-20 12:11:42 EDT
Actually it seems the source of the regression is the change in https://git.eclipse.org/r/#/c/118410/11/bundles/org.eclipse.swt/Eclipse+SWT/gtk/org/eclipse/swt/widgets/Control.java. Reverting this particular change while keeping the other one in GC.java seems to be enough to fix the regression on our side.
Comment 32 Andrey Loskutov CLA 2018-03-20 12:21:29 EDT
(In reply to Pierre-Charles David from comment #30)
> I'm reopening this, it caused regressions in Sirius (and possible any GMF or
> even GEF-based editors). See
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=525804#c10 and the following
> comments for the details, but the tl;dr is that with
> e78489ee67784eb3704ba210de1effcb777e0126, the contents of a Sirius editor
> can be drawn outside of the editor area, and coordinates are broken. See
> https://bugs.eclipse.org/bugs/attachment.cgi?id=273224 for what this looks
> like (this is with Gtk 3.22.11 and also 3.14.5).
> 
> Reverting https://git.eclipse.org/r/c/118410/
> (e78489ee67784eb3704ba210de1effcb777e0126) seems to fix the issue in our
> case (see https://bugs.eclipse.org/bugs/show_bug.cgi?id=525804#c15). Of
> course I have no idea on the possible negative impacts of a complete revert,
> but this seems to confirm that particular change is the cause of the
> regression on our side.

Can you please provide steps to reproduce? Bug 525804 does not contain them.
You also mentioned that this affects "simple" GEF diagrams - can you please provide as small as possible example where the problem can be reproduced?
Comment 33 Andrey Loskutov CLA 2018-03-20 12:29:00 EDT
(In reply to Pierre-Charles David from comment #31)
> Actually it seems the source of the regression is the change in
> https://git.eclipse.org/r/#/c/118410/11/bundles/org.eclipse.swt/Eclipse+SWT/
> gtk/org/eclipse/swt/widgets/Control.java. Reverting this particular change
> while keeping the other one in GC.java seems to be enough to fix the
> regression on our side.

This line basically "undoes" the changes in GC too. 
I guess the matrix transformation in GC is flawed, as soon as the x,y-offsets of the client area are > 0. Something like we need translate it to 0,0 before or after manipulation.
Comment 34 Simeon Andreev CLA 2018-03-20 14:25:48 EDT
Created attachment 273229 [details]
Snippet showing regression with commited patch.

I've added some debug info to GC.getClipping, they seem to return correct values.

I've also disabled the clipping checks in CTabFolderRenderer, no improvement.

I'll look into this.
Comment 35 Eclipse Genie CLA 2018-03-20 18:40:33 EDT
New Gerrit change created: https://git.eclipse.org/r/119832
Comment 36 Simeon Andreev CLA 2018-03-20 18:42:43 EDT
After a cursory glance in GC, I feel like something is missing in org.eclipse.swt.graphics.GC.identity(). Please check the patch and see if it helps. If not please provide GEF based snippets that I can check with.

Andrey, the patch should fix our problem. Probably for the best if we check whether it breaks something else though.
Comment 37 Pierre-Charles David CLA 2018-03-21 05:46:42 EDT
Created attachment 273235 [details]
Sample project to reproduce the regression on a GMF-based editor

I was not able to reproduce the regression on a plain GEF editor, the "GEF 3.x (Legacy)" example editors have been broken for a while.

It is reproducible using the GMF Runtime examples though (so not specific to Sirius):
1. Install a fresh Photon M6 commiters package
2. Help > Install new software: select Photon repo and install 'Graphical Modeling Framework SDK'.
3. Import the attached project.
4. Open the logicdiagram.logic2 file.
5. Using the outline view, scroll the editor's viewport to the middle of the diagram. Notice already that the lower right part of the diagram is not redrawn inside the canvas.
6. Close and reopen logicdiagram.logic2 (GMF stores the scroll location in the workspace's .metadata so the previous scroll position is restored): parts of the diagram is drawn on top of the main Eclipse UI (menubar, toolbar, editor's title area...).
Comment 38 Pierre-Charles David CLA 2018-03-21 05:48:25 EDT
Created attachment 273236 [details]
How the GMF example modeler looks after the scenario

This is how the GMF editor looks at the end of the scenario described above.
Comment 39 Andrey Loskutov CLA 2018-03-21 09:33:32 EDT
(In reply to Pierre-Charles David from comment #38)
> Created attachment 273236 [details]
> How the GMF example modeler looks after the scenario
> 
> This is how the GMF editor looks at the end of the scenario described above.

Can you please give some steps how to get all this editors/projects as code inside a plain SDK workspace? Similar to Simeon, I can run the example but I can't see any sources installed along with the GMF bits. A git repo would be good. 

If I see it right, GMF does not use GEF, and GEF examples (or all GEF) seem to be broken for a different reason anyway?

So we are trying to understand G*M*F regression here, not G*E*F? I'm really confused, because the project main page says it IS GEF based? https://www.eclipse.org/modeling/gmp/.

Beside this, if you have some code which does transformations with SWT Transform object *on paint*, can you please check if this small change fixes rendering, here example code of the paint listener:

Transform transform = new Transform(display);

// the line below is the possible "fix":
gc.getTransform(transform);

// some transform code ...
// transform.translate(0, 50);
// transform.rotate(-90);
gc.setTransform(transform);
Comment 40 Pierre-Charles David CLA 2018-03-21 10:15:37 EDT
(In reply to Andrey Loskutov from comment #39)
> (In reply to Pierre-Charles David from comment #38)
> > Created attachment 273236 [details]
> > How the GMF example modeler looks after the scenario
> > 
> > This is how the GMF editor looks at the end of the scenario described above.
> 
> Can you please give some steps how to get all this editors/projects as code
> inside a plain SDK workspace? Similar to Simeon, I can run the example but I
> can't see any sources installed along with the GMF bits. A git repo would be
> good.

I'll try to find more complete instructions, but the relevant project's sources are in https://github.com/eclipse/gef-legacy.git and in git://git.eclipse.org/gitroot/gmf-runtime/org.eclipse.gmf-runtime.git.

> If I see it right, GMF does not use GEF, and GEF examples (or all GEF) seem
> to be broken for a different reason anyway?
>
> So we are trying to understand G*M*F regression here, not G*E*F? I'm really
> confused, because the project main page says it IS GEF based?
> https://www.eclipse.org/modeling/gmp/.

We first saw this in Sirius, which is based on GMF Runtime, which itself is based on GEF 3.x. The thing is, all of these are frameworks to create graphical modelers, not end-user apps that can be tested directly or even exercised with a short snippet. GEF 3.x had example modelers, but their packaging has been broken for a while (3.x is an unmaintained branch), so I have not been able to reproduce this at the GEF level (yet). GMF Runtime has example modelers too, and these work enough to show the issue, see my scenario from above comment.

So, the regression affects at the very least GMF Runtime and everything above it. I strongly suspect that this is low-level enough that it affects GEF itself (and everything above), but I've not yet been able to confirm that.

> Beside this, if you have some code which does transformations with SWT
> Transform object *on paint*, can you please check if this small change fixes
> rendering, here example code of the paint listener:
> 
> Transform transform = new Transform(display);
> 
> // the line below is the possible "fix":
> gc.getTransform(transform);
> 
> // some transform code ...
> // transform.translate(0, 50);
> // transform.rotate(-90);
> gc.setTransform(transform);

We (in Sirius, and even GMF Runtime) delegate most of this low-level rendering stuff to GEF, and more precisely to its Draw2D part. I'll try to see were this could fit (maybe somewhere in https://github.com/eclipse/gef-legacy/blob/master/org.eclipse.draw2d/src/org/eclipse/draw2d/SWTGraphics.java or https://github.com/eclipse/gef-legacy/blob/master/org.eclipse.draw2d/src/org/eclipse/draw2d/Figure.java#L1188), but I'm really no expert on that part of the stack.
Comment 41 Pierre-Charles David CLA 2018-03-21 10:55:29 EDT
(In reply to Pierre-Charles David from comment #40)
> I strongly suspect that this is low-level enough that it affects
> GEF itself (and everything above), but I've not yet been able to confirm
> that.

Actually, I was not able to reproduce the problem with the pure GEF examples, using the following steps:
1. Start from a fresh Photon M6 Commiters package.
2. Clone https://github.com/eclipse/gef-legacy.git and import the projects into the workspace. You can ignore/close the org.eclipse.zest* projects. There are some "compilation" errors but they are actually PDE checks that fail; the code itself runs fine.
3. Launch a runtime
4. Create a plain project, and inside of if a file named test.shapes (the extension is important). Close the automatically opened text editor and re-open the file with the "GEF Shapes Examples" editor.
5. Create some shapes, connections between them, scroll the editor => everything seems to draw correctly.

The "GEF Shape Example" is really basic, so I'm not sure what to conclude from this. Either the issue is at the GMF level, or this particular example is too simple to trigger it.

> > Beside this, if you have some code which does transformations with SWT
> > Transform object *on paint*, can you please check if this small change fixes
> > rendering, here example code of the paint listener:
> > 
> > Transform transform = new Transform(display);
> > 
> > // the line below is the possible "fix":
> > gc.getTransform(transform);
> > 
> > // some transform code ...
> > // transform.translate(0, 50);
> > // transform.rotate(-90);
> > gc.setTransform(transform);

In the more complete context where I reproduce the bug (using a GMF modeler this time), I imported the sources of GEF in my workspace and added  gc.getTransform(transform) here [1], which is the only place in GEF (that I could find) that uses org.eclipse.swt.graphics.Transform. Not sure if this is "*on paint*" as you mention, but it does not seem to have any effect in my case.

[1] https://github.com/eclipse/gef-legacy/blob/master/org.eclipse.draw2d/src/org/eclipse/draw2d/SWTGraphics.java#L814
Comment 42 Simeon Andreev CLA 2018-03-21 11:15:43 EDT
From my perspective, safest would be to revert the patch here. It obviously needs more work.

As Andrey points out, working Control.print is not very useful if the widget doesn't draw itself correctly in the first place.
Comment 43 Andrey Loskutov CLA 2018-03-21 12:34:53 EDT
(In reply to Simeon Andreev from comment #42)
> From my perspective, safest would be to revert the patch here. It obviously
> needs more work.
> 
> As Andrey points out, working Control.print is not very useful if the widget
> doesn't draw itself correctly in the first place.

We should try to understand the problem here, revert is easy :-)

What we saw so far:

1 CTabFolder with RIGHT_TO_LEFT doesn't work => we have a patch, was "easy"

2 GC.setTransform() needs some love (Cairo.cairo_set_matrix -> Cairo.cairo_transform) because we need the x,y offsets now (they are not 0,0). I think this is OK for the patch.

3 What leads to broken GMF (but not GEF?)??? Some GTK "workaround" in GMF? Or is this also in GEF / Draw2D?

4 Something else?

The problem with 3 is that this is not part of the platform and we can't add any tests for it. So we need to identify root cause and reduce it to the minimal SWT only example. 
@Pierre-Charles: I would really appreciate if you could create some minimal example based on "smallest possible" set of dependencies ("pure Draw2D" or "pure GEF" would be very helpful).

For 4 we need the analysis from experts (Eric?) after the fix of 3.

So let investigate what broke GMF, and we will see which way we should go (revert the patch or improve it), and hopefully SWT team could help here too.
Comment 44 Eric Williams CLA 2018-03-21 16:58:18 EDT
(In reply to Andrey Loskutov from comment #43)
> 2 GC.setTransform() needs some love (Cairo.cairo_set_matrix ->
> Cairo.cairo_transform) because we need the x,y offsets now (they are not
> 0,0). I think this is OK for the patch.

What do you mean exactly by the offsets?

> 3 What leads to broken GMF (but not GEF?)??? Some GTK "workaround" in GMF?
> Or is this also in GEF / Draw2D?
> 
> 4 Something else?
> 
> The problem with 3 is that this is not part of the platform and we can't add
> any tests for it. So we need to identify root cause and reduce it to the
> minimal SWT only example. 
> @Pierre-Charles: I would really appreciate if you could create some minimal
> example based on "smallest possible" set of dependencies ("pure Draw2D" or
> "pure GEF" would be very helpful).
> 
> For 4 we need the analysis from experts (Eric?) after the fix of 3.
> 
> So let investigate what broke GMF, and we will see which way we should go
> (revert the patch or improve it), and hopefully SWT team could help here too.

I can't speak much about this issue as I am not a GC expert by any stretch. However we have some time to investigate, as M7 is still relatively far away. If nothing is found by then, we can revert the change and try again for 4.9.
Comment 45 Pierre-Charles David CLA 2018-03-22 08:52:04 EDT
(In reply to Andrey Loskutov from comment #43)
> @Pierre-Charles: I would really appreciate if you could create some minimal
> example based on "smallest possible" set of dependencies ("pure Draw2D" or
> "pure GEF" would be very helpful).

I understand, and I'd like to help, but unfortunately I don't think I'll be able to spend more time on this at the moment. Maybe later, but I can't guarantee it. If there are new patches or proposed changes though, I should be able to test them manually in my environment and report the results.
Comment 46 Simeon Andreev CLA 2018-03-27 10:40:08 EDT
Created attachment 273319 [details]
GMF editor drawing artefacts after latest patch.

OK, with the current patch we have the following state:

1. Mirrored items (e.g. right to left tabs) are drawn correctly.
2. Calls to GC.setTransform have the expected behaviour.
3. Calls to GC.getTransform return the wrong transformation.

We still need to figure out how to adjust GC to satisfy the GC.getTransform API, but we are mostly done with the analysis of what broke for what reason.


The following problem remains though:

The attached project with the LogicNotationEditor from GMF examples renders as expected, but following problem is seen e.g. for TopTerminalFigure / BottomTerminalFigure:

In the example, the blue rectangles (of e.g. a LEDFigure) draw over other part stacks, when resizing the part stacks so that the blue rectangles land *over* the other part stack. See attached screenshot. Sometimes this is also seen when scrolling in the editor. 

After some debugging, it looks like when painting itself e.g. BottomTerminalFigure will call org.eclipse.draw2d.Graphics.translate(Point). This in turn will set GC clipping. If the BottomTerminalFigure is obscured by another part stack, this results in a 0 area clipping region for GC. Instead of resulting into nothing drawn, it results in the rectangle being drawn over other widgets.

This looks like another SWT bug to me, so far I don't know what to do with this. I would expect that if the clipping region has 0 area, nothing is drawn. As opposed to everything is drawn...
Comment 47 Eric Williams CLA 2018-03-27 12:33:58 EDT
(In reply to Simeon Andreev from comment #46)
> This looks like another SWT bug to me, so far I don't know what to do with
> this. I would expect that if the clipping region has 0 area, nothing is
> drawn. As opposed to everything is drawn...

Is this expected? I thought a clipping's purpose is to restrict the drawing region: a clipping of 0 in theory should mean all "dirty" areas are redrawn, as in there is no restriction.

I could be totally wrong but this is what I understood from: http://www.eclipse.org/articles/Article-SWT-graphics/SWT_graphics.html#Clipping
Comment 48 Simeon Andreev CLA 2018-03-28 05:20:35 EDT
(In reply to Eric Williams from comment #47)
> I thought a clipping's purpose is to restrict the drawing
> region: a clipping of 0 in theory should mean all "dirty" areas are redrawn,
> as in there is no restriction.

I don't know, the GMF editor sets a clipping which is outside of the widget's clipping area. This results in a 0 clipping area, allowing the GMF editor to draw outside of its bounds. Doesn't look right to me; either its a client code problem or an SWT problem.
Comment 49 Pierre-Charles David CLA 2018-03-28 06:54:05 EDT
(In reply to Eric Williams from comment #47)
> I thought a clipping's purpose is to restrict the drawing region: a clipping of 0 in theory should mean all "dirty" areas are redrawn, as in there is no restriction.
> I could be totally wrong but this is what I understood from:
> http://www.eclipse.org/articles/Article-SWT-graphics/SWT_graphics.
> html#Clipping

I understood the opposite :) Notably, the first sentence says "The clipping area of a GC is the portion onto which visible drawing occurs", from which I inferred that an empty clipping area would mean that no drawing should be visible.

However, the Javadoc for org.eclipse.swt.graphics.GC.setClipping(Rectangle rect) (and related) mentions that "Specifying <code>null</code> for the rectangle reverts the receiver's clipping area to its original value.", which matches your interpretation. Could it be that a non-null but 0-area clipping region computed at one point becomes converted to/treated as null later on, thus cancelling any actual clipping?
Comment 50 Eric Williams CLA 2018-03-28 10:38:26 EDT
(In reply to Simeon Andreev from comment #48)
> I don't know, the GMF editor sets a clipping which is outside of the
> widget's clipping area. This results in a 0 clipping area, allowing the GMF
> editor to draw outside of its bounds. Doesn't look right to me; either its a
> client code problem or an SWT problem.

Was this the case before the patch as well? Maybe the "getting" of the clipping is wrong and that's the issue here, and the reason we didn't notice it before was that GC.setClipping wasn't working properly? I have seen this sort of case before.


(In reply to Pierre-Charles David from comment #49)
> However, the Javadoc for org.eclipse.swt.graphics.GC.setClipping(Rectangle
> rect) (and related) mentions that "Specifying <code>null</code> for the
> rectangle reverts the receiver's clipping area to its original value.",
> which matches your interpretation. Could it be that a non-null but 0-area
> clipping region computed at one point becomes converted to/treated as null
> later on, thus cancelling any actual clipping?

Looking at GC.setClippingInPixels(Rectangle rect), I see that if the rectangle passed in is null, then "setClipping(0)" is called. Seems to me that setClipping(null) and setClipping(0) are synonymous, then.
Comment 51 Daniel Wille CLA 2018-03-28 16:37:03 EDT
Much like Pierre-Charles David, I began noticing some strange issues starting with M6. However I'm seeing mine with error decorators from a data binding validation. I traced the start of the issue to this commit:

* e78489ee67 Bug 531667 - [GTK3] Cannot draw Canvas with Control.print(GC)

which is how I found this bug.

I'm going to attach some screenshots in the hope that it's helpful to diagnose. You can see in the good image that there are 3 validation errors (on each of the 3 text boxes), and in the bad one there are twice as many that display.

I'd be happy to help test any fixes that are committed.
Comment 52 Daniel Wille CLA 2018-03-28 16:37:40 EDT
Created attachment 273352 [details]
Validation decorators - good
Comment 53 Daniel Wille CLA 2018-03-28 16:38:00 EDT
Created attachment 273353 [details]
Validation decorators - bad
Comment 54 Andrey Loskutov CLA 2018-03-28 16:39:23 EDT
(In reply to Daniel Wille from comment #51)
> I'd be happy to help test any fixes that are committed.

Please check linked gerrit patch: https://git.eclipse.org/r/119832
Comment 55 Daniel Wille CLA 2018-03-29 16:28:38 EDT
Yeah, I'm still seeing the behavior even with patch 9 (1480b2522e).
Comment 56 Andrey Loskutov CLA 2018-03-30 06:30:06 EDT
(In reply to Daniel Wille from comment #55)
> Yeah, I'm still seeing the behavior even with patch 9 (1480b2522e).

Daniel, can you please attach simplest possible project to reproduce the issue?
Comment 57 Daniel Wille CLA 2018-03-30 11:08:42 EDT
Created attachment 273386 [details]
Example plug-in showing validation errors

Here's an example plugin you can do Run As -> Eclipse Application with. It opens a very simple wizard that will show a whole bunch of error decorators, even though there should just be two.

Interestingly, I found that a key to reproducing the error was to place the labels/text boxes *inside a group*.
Comment 58 Simeon Andreev CLA 2018-04-01 02:56:42 EDT
From Andrey:

Possibly related Bug 201589 with https://git.eclipse.org/r/#/c/120384/.


I'll check if this helps, hopefully this week. I didn't look at the review so far.
Comment 59 Simeon Andreev CLA 2018-04-01 02:57:28 EDT
Sorry, disregard comment 58, this was the wrong ticket.
Comment 60 Simeon Andreev CLA 2018-04-01 05:31:06 EDT
(In reply to Daniel Wille from comment #57)
> Here's an example plugin you can do Run As -> Eclipse Application with. It
> opens a very simple wizard that will show a whole bunch of error decorators,
> even though there should just be two.

OK, on GTK 3.14 the JFace error decorators are not seen at all before the first patch here (I went back to I20180305-0800, v4860b). After the patch, one of the decorators is seen at the top of the group. From my perspective, with or without said patch, behaviour is equally broken. We've been told time and again that GTK 3.14 is no longer supported, so I don't know if this needs a ticket or not (and whether a ticket will do any good).

On GTK 3.22, I see the problem as described. Here is a minimal snippet to reproduce the problem, using only SWT and ControlDecoration from JFace:


Display display = new Display();
Shell shell = new Shell(display);
shell.setLayout(new FillLayout());
shell.setSize(300, 80);
shell.setText("ControlDecoration in Group");

Group g = new Group(shell, SWT.NONE);
g.setText("group"); // comment this out
g.setLayout(new GridLayout(2, false));

new Label(g, SWT.NONE).setText("some label");
Text t = new Text(g, SWT.NONE);
t.setText("some text");
ControlDecoration d = new ControlDecoration(t, SWT.TOP | SWT.LEFT, null);

Image image = new Image(display, 8, 8);
GC gc = new GC(image);
gc.setBackground(display.getSystemColor(SWT.COLOR_DARK_RED));
gc.fillRectangle(0, 0, 16, 16);
gc.dispose();
d.setImage(image);

shell.open();

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


As hinted, the group seems to be what causes the problem. If I disable its text, the painting artefact "lessens". There are still 2 decoration rectangles per text field, but they mostly overlap. Note that one of them doesn't seem to "go away".

If I had to guess, the SWT group bounds are wrong, if a group header / group border is present (possibly not all the time). I'll check why next.
Comment 61 Simeon Andreev CLA 2018-04-01 07:35:07 EDT
After some debugging, here is what JFace ControlDecoration does:

Attach a paint listener on every parent of the control which it decorates.

The paint listener will use ControlDecoration.getDecorationRectangle. This delegates to SWT Control.toControl(Point).

Here is how this goes:

painting Shell
global point : (77, 45)
local point  : (73, 21)

painting Group
global point : (77, 45)
local point  : (72, 5)

I've checked the values also with Composite with a Text in it, above the group. I observe that the paint for the composite also has the correct local coordinates.

So more or less, the group provides wrong result from Control.toControl(Point). The group paints its components at a different location, taking the text header and the border into account. However, its toControl() method, does not. I'll need to check why.

I'm not sure how the patch here affects this; the global/local coordinates are the same before and after the patch. I'm thinking the Group paint doesn't actually draw anything without the patch, and so the drawing artefact is not seen. I didn't debug the native code to validate this though.
Comment 62 Andrey Loskutov CLA 2018-04-01 11:30:50 EDT
*** Bug 533131 has been marked as a duplicate of this bug. ***
Comment 63 Ansgar Radermacher CLA 2018-04-03 05:11:12 EDT
Steps to reproduce that Papyrus (as an example of other GEFU/GMF based editors) draws outside its normal area:

* Download Papyrus (e.g. via its nightly update site http://download.eclipse.org/modeling/mdt/papyrus/updates/nightly/photon)

* Create a new Papyrus project / model containing a composite structure diagram

* Create a Class inside the diagram, add a port

* Zoom/move, so that the port is outside the visible area

* Bring another window into foreground that hides the Eclipse window.

* Now, bring the Eclipse window into foreground again. Result: the port is drawn outside the drawing area.

Reproduced with Kubuntu 16.04, gtk version 3.18.9, SWT from master (updated today) + the 2nd gerrit patch applied.
Comment 64 Andrey Loskutov CLA 2018-04-03 10:06:58 EDT
(In reply to Ansgar Radermacher from comment #63)
> Steps to reproduce that Papyrus (as an example of other GEFU/GMF based
> editors) draws outside its normal area:
> 
> * Download Papyrus (e.g. via its nightly update site
> http://download.eclipse.org/modeling/mdt/papyrus/updates/nightly/photon)

I will try to follow, but could you provide steps how to import what we need into the plain SDK workspace from git?

> Reproduced with Kubuntu 16.04, gtk version 3.18.9, SWT from master (updated
> today) + the 2nd gerrit patch applied.

We learned (Eric can explain it better) that GTK3 versions < 3.20 are not supported or not stable or not recommended or all of that. So can you please to test with a GTK3 >= 3.20?

I've tested today the latest patch set (14) on GTK 3.22, SDK 4.8 and https://marketplace.eclipse.org/content/wireframesketcher-wireframing-tool.

After installation into the current platform this plugin provides a nice WYSIWYG tool based on draw2d and this seem to work flawlessly now. After installation, New -> Project -> Wireframing -> Wireframe Project -> Examples -> Tutorial Project. This generates few "screen" files which one can open in an editor and change / zoom / DND whatever.


(In reply to Pierre-Charles David from comment #41)
> (In reply to Pierre-Charles David from comment #40)
> > I strongly suspect that this is low-level enough that it affects
> > GEF itself (and everything above), but I've not yet been able to confirm
> > that.
> 
> Actually, I was not able to reproduce the problem with the pure GEF
> examples, using the following steps:
> 1. Start from a fresh Photon M6 Commiters package.
> 2. Clone https://github.com/eclipse/gef-legacy.git and import the projects
> into the workspace. You can ignore/close the org.eclipse.zest* projects.
> There are some "compilation" errors but they are actually PDE checks that
> fail; the code itself runs fine.
> 3. Launch a runtime
> 4. Create a plain project, and inside of if a file named test.shapes (the
> extension is important). Close the automatically opened text editor and
> re-open the file with the "GEF Shapes Examples" editor.
> 5. Create some shapes, connections between them, scroll the editor =>
> everything seems to draw correctly.

I've also tried the GMF examples by importing all EMF, GEF, GMF projects from git into my SDK workspace but I couldn't reproduce the issue *I saw* with 4.8 M6 committers package + GMF installed neither with not without the patch. So probably some steps are missing or incolmpete, or the master of one of those projects "fixed" the problem internally since M6.

So Pierre-Charles, could you please provide steps to reproduce which are NOT based on 4.8 M6 Committers package? Or could you verify the latest patch set 14 with your "special" Sirius setup?
Comment 65 Pierre-Charles David CLA 2018-04-04 04:31:37 EDT
(In reply to Andrey Loskutov from comment #64)
> So Pierre-Charles, could you please provide steps to reproduce which are NOT
> based on 4.8 M6 Committers package?

I'll try.

> Or could you verify the latest patch set
> 14 with your "special" Sirius setup?

In the meantime I tested patch set 16 (1ebafb5): with both Sirius and the GMF "logic" example, I don't have the previous issue with edges (as seen on https://bugs.eclipse.org/bugs/attachment.cgi?id=273236), but I still have the artifacts visible on https://bugs.eclipse.org/bugs/attachment.cgi?id=273319.
Comment 66 Ansgar Radermacher CLA 2018-04-04 04:53:56 EDT
I can also still see visible artifacts with Papyrus after upgrading to GTK 3.22. I don't know how/don't have the time to create a different photon setup in which things might have been fixed (sorry).

> In the meantime I tested patch set 16 (1ebafb5): with both Sirius and the
> GMF "logic" example, I don't have the previous issue with edges (as seen on
> https://bugs.eclipse.org/bugs/attachment.cgi?id=273236), but I still have
> the artifacts visible on
> https://bugs.eclipse.org/bugs/attachment.cgi?id=273319.
Comment 67 Simeon Andreev CLA 2018-04-05 02:07:29 EDT
Please check with patch set 20 just to be sure. Neither me nor Andrey observe paint artefacts with it (GTK 3.14 resp. GTK 3.22).
Comment 68 Simeon Andreev CLA 2018-04-05 13:46:54 EDT
OK, I installed Wireframe Sketcher and validated with patch set 24 (...). On RHEL 7.2 (GTK 3.14) and RHEL 7.4 (GTK 3.22) I checked:

* bug snippets from patch
* Wireframe Sketcher tutorial project
* attached example with GEF logic editor 
* attached plug-in example with ControlDecoration showcase

Only the snippet for ControlDecoration / plug-in showcase doesn't work on GTK 3.14, see Bug 533241.

Hopefully we are done here.

Though at this point, I think it would have been more cost efficient to simply re-implement Control.print(GC). Who knows, the new GTK version may throw the patches here in the garbage (as I'm sure it has done many times for much more code). We really need to file a complaint for this; I don't know who can keep up with such massive disregard of clients...
Comment 69 Pierre-Charles David CLA 2018-04-06 02:57:37 EDT
It seems Patch Set 24 has fixed the remaining artifacts in GEF and Sirius, see https://imgur.com/a/Bs6RK for comparison with PatchSet 16. This is with GTK 3.22.11.
Comment 71 Andrey Loskutov CLA 2018-04-09 04:23:30 EDT
Patch is in master now, one can grab latest SDK nightly build for testing of integration with other tools etc. 

Thanks everyone for reporting/testing. Please keep your eyes open for possible regressions.
Comment 72 Andrey Loskutov CLA 2018-04-10 11:17:37 EDT
I hate to reopen it, but the patch seem to introduce another regression (at least on my RHEL 7.4 / GTK 3.22 config): compare editor freezes Eclipse (kind of) and it can be "unfrozen" by maximizing the window.

With jstack I see that it is not a deadlock, but a permanent re-draws/re-paints sent so that the UI is busy with itself in this stack:

"main" #1 prio=6 os_prio=0 tid=0x00007ffff000a800 nid=0x628a runnable [0x00007ffff7fc8000]
   java.lang.Thread.State: RUNNABLE
        at org.eclipse.swt.internal.gtk.GDK._gdk_cairo_region_create_from_surface(Native Method)
        at org.eclipse.swt.internal.gtk.GDK.gdk_cairo_region_create_from_surface(GDK.java:1794)
        at org.eclipse.swt.graphics.Region.gdk_region_polygon(Region.java:126)
        at org.eclipse.swt.graphics.GC.convertRgn(GC.java:420)
        at org.eclipse.swt.graphics.GC.getClippingInPixels(GC.java:2062)
        at org.eclipse.swt.graphics.GC.getClipping(GC.java:2007)
        at org.eclipse.swt.graphics.GC.init(GC.java:2617)
        at org.eclipse.swt.graphics.GC.gtk_new(GC.java:253)
        at org.eclipse.swt.widgets.Control.gtk_draw(Control.java:3578)
        at org.eclipse.swt.widgets.Composite.gtk_draw(Composite.java:349)
        at org.eclipse.swt.widgets.Canvas.gtk_draw(Canvas.java:178)
        at org.eclipse.swt.widgets.Shell.gtk_draw(Shell.java:1368)
        at org.eclipse.swt.widgets.Widget.windowProc(Widget.java:1948)
        at org.eclipse.swt.widgets.Control.windowProc(Control.java:6356)
        at org.eclipse.swt.widgets.Display.windowProc(Display.java:5887)
        at org.eclipse.swt.internal.gtk.GTK._gtk_main_do_event(Native Method)
        at org.eclipse.swt.internal.gtk.GTK.gtk_main_do_event(GTK.java:4058)
        at org.eclipse.swt.widgets.Display.eventProc(Display.java:1383)
        at org.eclipse.swt.internal.gtk.OS._g_main_context_iteration(Native Method)
        at org.eclipse.swt.internal.gtk.OS.g_main_context_iteration(OS.java:1565)
        at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:4499)
        at org.eclipse.jface.operation.ModalContext$ModalContextThread.block(ModalContext.java:165)
        at org.eclipse.jface.operation.ModalContext.run(ModalContext.java:369)
        at org.eclipse.jface.dialogs.ProgressMonitorDialog.run(ProgressMonitorDialog.java:483)
        at org.eclipse.ui.internal.progress.ProgressMonitorJobsDialog.run(ProgressMonitorJobsDialog.java:237)
        at org.eclipse.ui.internal.progress.ProgressManager.lambda$25(ProgressManager.java:820)
        at org.eclipse.ui.internal.progress.ProgressManager$$Lambda$569/339179190.run(Unknown Source)
        at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:71)
        at org.eclipse.ui.internal.progress.ProgressManager.busyCursorWhile(ProgressManager.java:853)
        at org.eclipse.ui.internal.progress.ProgressManager.busyCursorWhile(ProgressManager.java:829)
        at org.eclipse.ui.internal.progress.ProgressManager.run(ProgressManager.java:989)
        at org.eclipse.compare.internal.CompareContainer.run(CompareContainer.java:87)
        at org.eclipse.compare.CompareEditorInput.run(CompareEditorInput.java:1306)
        at org.eclipse.compare.internal.merge.DocumentMerger.doDiff(DocumentMerger.java:398)
        at org.eclipse.compare.contentmergeviewer.TextMergeViewer.doDiff(TextMergeViewer.java:3439)
        at org.eclipse.compare.contentmergeviewer.TextMergeViewer.update(TextMergeViewer.java:5307)
        at org.eclipse.compare.contentmergeviewer.TextMergeViewer.updateContent(TextMergeViewer.java:2952)
        at org.eclipse.compare.contentmergeviewer.ContentMergeViewer.internalRefresh(ContentMergeViewer.java:781)
        at org.eclipse.compare.contentmergeviewer.ContentMergeViewer.inputChanged(ContentMergeViewer.java:689)
        at org.eclipse.jface.viewers.ContentViewer.setInput(ContentViewer.java:286)
        at org.eclipse.jdt.internal.ui.compare.JavaMergeViewer.setInput(JavaMergeViewer.java:152)
        at org.eclipse.compare.CompareViewerSwitchingPane.setInput(CompareViewerSwitchingPane.java:265)
        at org.eclipse.compare.internal.CompareContentViewerSwitchingPane.setInput(CompareContentViewerSwitchingPane.java:180)
        at org.eclipse.compare.CompareEditorInput.internalSetContentPaneInput(CompareEditorInput.java:795)
        at org.eclipse.compare.CompareEditorInput.feedInput(CompareEditorInput.java:696)
        at org.eclipse.compare.CompareEditorInput.createContents(CompareEditorInput.java:533)
        at org.eclipse.compare.internal.CompareEditor.createCompareControl(CompareEditor.java:441)
        at org.eclipse.compare.internal.CompareEditor.access$6(CompareEditor.java:401)
        at org.eclipse.compare.internal.CompareEditor$1.lambda$0(CompareEditor.java:359)
        at org.eclipse.compare.internal.CompareEditor$1$$Lambda$582/568891346.run(Unknown Source)
        at org.eclipse.ui.internal.PendingSyncExec.run(PendingSyncExec.java:58)
        at org.eclipse.ui.internal.UILockListener.doPendingWork(UILockListener.java:168)
        at org.eclipse.ui.internal.UISynchronizer.lambda$0(UISynchronizer.java:150)
        at org.eclipse.ui.internal.UISynchronizer$$Lambda$399/1038809873.run(Unknown Source)
        at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:37)
        at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:182)
        - locked <0x00000007197601b8> (a org.eclipse.swt.widgets.RunnableLock)
        at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4898)
        at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:4504)

Sometimes the stack is different and shows this:

"main" #1 prio=6 os_prio=0 tid=0x00007ffff000a800 nid=0x5527 runnable [0x00007ffff7fc7000]
   java.lang.Thread.State: RUNNABLE
	at org.eclipse.swt.internal.gtk.GTK._gtk_css_provider_to_string(Native Method)
	at org.eclipse.swt.internal.gtk.GTK.gtk_css_provider_to_string(GTK.java:3873)
	at org.eclipse.swt.widgets.Display.gtk_css_parse_foreground(Display.java:2272)
	at org.eclipse.swt.widgets.Control.getContextColorGdkRGBA(Control.java:2885)
	at org.eclipse.swt.widgets.Control.getForegroundGdkRGBA(Control.java:3067)
	at org.eclipse.swt.widgets.Control.internal_new_GC(Control.java:3912)
	at org.eclipse.swt.graphics.GC.gtk_new(GC.java:251)
	at org.eclipse.swt.widgets.Control.gtk_draw(Control.java:3578)
	at org.eclipse.swt.widgets.Composite.gtk_draw(Composite.java:349)
	at org.eclipse.swt.widgets.Canvas.gtk_draw(Canvas.java:178)
	at org.eclipse.swt.widgets.Widget.windowProc(Widget.java:1948)
	at org.eclipse.swt.widgets.Control.windowProc(Control.java:6356)
	at org.eclipse.swt.widgets.Display.windowProc(Display.java:5887)
	at org.eclipse.swt.internal.gtk.GTK._gtk_main_do_event(Native Method)
	at org.eclipse.swt.internal.gtk.GTK.gtk_main_do_event(GTK.java:4058)
	at org.eclipse.swt.widgets.Display.eventProc(Display.java:1383)
	at org.eclipse.swt.internal.gtk.OS._g_main_context_iteration(Native Method)
	at org.eclipse.swt.internal.gtk.OS.g_main_context_iteration(OS.java:1565)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:4499)
	at org.eclipse.jface.operation.ModalContext$ModalContextThread.block(ModalContext.java:165)
	at org.eclipse.jface.operation.ModalContext.run(ModalContext.java:369)
	at org.eclipse.jface.dialogs.ProgressMonitorDialog.run(ProgressMonitorDialog.java:483)
	at org.eclipse.ui.internal.progress.ProgressMonitorJobsDialog.run(ProgressMonitorJobsDialog.java:237)
	at org.eclipse.ui.internal.progress.ProgressManager.lambda$25(ProgressManager.java:820)
	at org.eclipse.ui.internal.progress.ProgressManager$$Lambda$629/749013882.run(Unknown Source)
	at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:71)
	at org.eclipse.ui.internal.progress.ProgressManager.busyCursorWhile(ProgressManager.java:853)
	at org.eclipse.ui.internal.progress.ProgressManager.busyCursorWhile(ProgressManager.java:829)
	at org.eclipse.ui.internal.progress.ProgressManager.run(ProgressManager.java:989)
	at org.eclipse.compare.internal.CompareContainer.run(CompareContainer.java:87)
	at org.eclipse.compare.CompareEditorInput.run(CompareEditorInput.java:1306)
	at org.eclipse.compare.internal.merge.DocumentMerger.doDiff(DocumentMerger.java:398)
	at org.eclipse.compare.contentmergeviewer.TextMergeViewer.doDiff(TextMergeViewer.java:3439)
	at org.eclipse.compare.contentmergeviewer.TextMergeViewer.update(TextMergeViewer.java:5307)
	at org.eclipse.compare.contentmergeviewer.TextMergeViewer.updateContent(TextMergeViewer.java:2952)
	at org.eclipse.compare.contentmergeviewer.ContentMergeViewer.internalRefresh(ContentMergeViewer.java:781)
	at org.eclipse.compare.contentmergeviewer.ContentMergeViewer.inputChanged(ContentMergeViewer.java:689)
	at org.eclipse.jface.viewers.ContentViewer.setInput(ContentViewer.java:286)
	at org.eclipse.jdt.internal.ui.compare.JavaMergeViewer.setInput(JavaMergeViewer.java:152)
	at org.eclipse.compare.CompareViewerSwitchingPane.setInput(CompareViewerSwitchingPane.java:265)
	at org.eclipse.compare.internal.CompareContentViewerSwitchingPane.setInput(CompareContentViewerSwitchingPane.java:180)
	at org.eclipse.compare.CompareEditorInput.internalSetContentPaneInput(CompareEditorInput.java:795)
	at org.eclipse.compare.CompareEditorInput.feedInput(CompareEditorInput.java:696)
	at org.eclipse.compare.CompareEditorInput.createContents(CompareEditorInput.java:533)
	at org.eclipse.compare.internal.CompareEditor.createCompareControl(CompareEditor.java:441)
	at org.eclipse.compare.internal.CompareEditor.access$6(CompareEditor.java:401)
	at org.eclipse.compare.internal.CompareEditor$1.lambda$0(CompareEditor.java:359)
	at org.eclipse.compare.internal.CompareEditor$1$$Lambda$640/1693263854.run(Unknown Source)
	at org.eclipse.ui.internal.PendingSyncExec.run(PendingSyncExec.java:58)
	at org.eclipse.ui.internal.UILockListener.doPendingWork(UILockListener.java:168)
	at org.eclipse.ui.internal.UISynchronizer.lambda$0(UISynchronizer.java:150)
	at org.eclipse.ui.internal.UISynchronizer$$Lambda$403/907246458.run(Unknown Source)
	at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:37)
	at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:182)
	- locked <0x0000000773bad4a0> (a org.eclipse.swt.widgets.RunnableLock)
	at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4898)

I will attach screenshot, I guess the problem could be with the Compare editor itself doing something wrong.

To reproduce, I open Git Staging view, double click on the changed file and watch the frozen UI. To unfreeze, just double-click on the title bar.
Comment 73 Andrey Loskutov CLA 2018-04-10 11:19:03 EDT
Created attachment 273513 [details]
my workbench frozen on double click in staging view
Comment 74 Andrey Loskutov CLA 2018-04-10 11:31:26 EDT
(In reply to Andrey Loskutov from comment #73)
> Created attachment 273513 [details]
> my workbench frozen on double click in staging view

Note, I don't know if this patch is the problem or not (I have not bisected this), but the stack shows the changed code so my first assumption was that this is the root cause.

@all: can you reproduce it too?
Comment 75 Ansgar Radermacher CLA 2018-04-10 11:45:50 EDT
Created attachment 273514 [details]
ghost button above "default value"

Not sure, if the following issue is related to this bug. I noticed a "ghost" button (button with 3 dots as label) behind the "default value" entry. The original button is drawn in the line above, but further to the right. The ghost button does not react on clicks.

I have a Linux Ubuntu 16.04 with GTK 3.22. This behavior started after I upgraded GTK from 3.18 to 3.22 and is reproducible with an Eclipse photon (having the current SWT patch applied) and also with Eclipse oxygen from last year.

In order to reproduce, click on an attribute (within a class) in a Papyrus editor and open the "Properties" view.
Comment 76 Andrey Loskutov CLA 2018-04-10 12:08:20 EDT
(In reply to Ansgar Radermacher from comment #75)
> Created attachment 273514 [details]
> ghost button above "default value"
> 
> This behavior started after I
> upgraded GTK from 3.18 to 3.22 and is reproducible with an Eclipse photon
> (having the current SWT patch applied) and also with Eclipse oxygen from
> last year.

If this is reproducible with Oxygen, it is not related to this bug. Please open a separated issue for that with steps to reproduce.
Comment 77 Simeon Andreev CLA 2018-04-10 13:06:59 EDT
(In reply to Andrey Loskutov from comment #72)
> I hate to reopen it, but the patch seem to introduce another regression (at
> least on my RHEL 7.4 / GTK 3.22 config): compare editor freezes Eclipse
> (kind of) and it can be "unfrozen" by maximizing the window.
> 
> With jstack I see that it is not a deadlock, but a permanent
> re-draws/re-paints sent so that the UI is busy with itself in this stack:
> 
> To reproduce, I open Git Staging view, double click on the changed file and
> watch the frozen UI. To unfreeze, just double-click on the title bar.

I'll profile this tomorrow if I can reproduce it.

Please in future just attach YourKit. Compared to jstack snapshots, its much more reliable at figuring out where the problem is.
Comment 78 Simeon Andreev CLA 2018-04-11 01:13:59 EDT
Cannot reproduce neither on my machine (RHEL 7.2, GTK 3.14), nor on yours (RHEL 7.4, GTK 3.22).

I took:

Eclipse SDK
Version: Photon (4.8)
Build id: I20180409-2000

I'm showing diff of changed file in a commit, diffed to its previous version. I also tried comparing two source files with each other.

Can you provide the specific file you diffed / try to reproduce again?
Comment 79 Andrey Loskutov CLA 2018-04-11 01:18:07 EDT
This was independent from the file (see screenshot what it was). After reboot I still saw the problem. I will try later today to reproduce in debugger and to bisect it.
Comment 80 Andrey Loskutov CLA 2018-04-11 08:16:39 EDT
(In reply to Andrey Loskutov from comment #79)
> This was independent from the file (see screenshot what it was). After
> reboot I still saw the problem. I will try later today to reproduce in
> debugger and to bisect it.

The problem is really coming from our latest patch, but As Simeon found, the root cause is the *first* patch (parts of it, which seem to be obsoleted now with the last patch fixes for clipping).

I still have trouble to understand where the bug is coming from (I guess some rounding issues during matrix convert and clipping cause GTK to trigger re-draws again and again), but so far I found that:

1) I observe the problem on my KDE desktop on *second* monitor only (1920x1200), for windows running *full screen* (maximized), and only if the animated progress thingy is shown in the lower right corner of the status line. On the fist monitor with 1600x1200 I can't observe the problem at all.

2) As soon as the animated progress indicator is shown, one from 8 CPU cores shows 100% usage, and the UI is either VERY slow but responsible or not responsible (but also not "frozen" without repaints).

3) The only way to get out of this state is to change the window size to be *smaller*.

4) Without animated progress thingy everything works fine, on any monitor, with any window size.

5) I see this on both RHEL 7.4 locally or remote via ssh on RHEL 7.2, in the same KDE session, means => both GTK 3.14 / 3.22 are affected.

6) The patch which I will push in a moment fixes the issue.

7) Simeon can't reproduce the problem on his desktop config (single monitor), on same GTK 3.14 / 3.22 configs.


I will push the SWT patch and I would like to integrate it today. I will check with Yourkit what the code actually doing while UI is busy, but as I've said, the problem is most likely due some obscure GTK repaints due our extra clipping calculations obsoleted now with the second patch.
Comment 81 Eclipse Genie CLA 2018-04-11 10:00:56 EDT
New Gerrit change created: https://git.eclipse.org/r/121007
Comment 83 Simeon Andreev CLA 2018-04-12 05:15:15 EDT
Please check Comment 16, Comment 18 and Comment 22 for the latest patch.

The original patch here was "timid", in that I introduced the change which seemed to "go" with the rest of the code.

https://git.eclipse.org/r/121007 is the alternative suggestion, which doesn't introduce extra effort and retains functionality.
Comment 84 Andrey Loskutov CLA 2018-04-13 03:56:18 EDT
I do not see the issue from comment 72 anymore on I20180411-2000, so let assume this one is resolved. I've created bug 533533 as a follow up in case we will find new regressions *after* the release.
Comment 85 Eclipse Genie CLA 2018-04-13 04:48:54 EDT
New Gerrit change created: https://git.eclipse.org/r/121125
Comment 87 Peter Severin CLA 2018-05-03 15:49:02 EDT
I see some clipping issues on HiDPI display while testing WireframeSketcher plugin with Eclipse 4.8 I-builds. I suspect that these issues might be linked to the fix in this bug, however I am still not sure what exactly the issue and how to create a test case for that. I've narrowed it down to Path drawing, as other drawing methods seem to be unaffected. Any ideas? I'm attaching a screenshot of what I see.
Comment 88 Peter Severin CLA 2018-05-03 15:50:07 EDT
Created attachment 273917 [details]
Clipping issues on HiDPI.

Rectangles should be drawn completely, but are only seen partially. The scaling factor is x2. With normal scaling factor I don't see any issue.
Comment 89 Simeon Andreev CLA 2018-05-03 16:17:23 EDT
(In reply to Peter Severin from comment #88)
> Created attachment 273917 [details]
> Clipping issues on HiDPI.
> 
> Rectangles should be drawn completely, but are only seen partially. The
> scaling factor is x2. With normal scaling factor I don't see any issue.

Could you provide some sort of reproduction steps? I'll take a look.

We re-open this I guess?
Comment 90 Peter Severin CLA 2018-05-03 16:22:07 EDT
What I did was to install WireframeSketcher plugin into the latest Eclipse 4.8 build on a HiDPI system (Debian, with x2 scaling). The effects are visible with Tutorial project too, as some shapes are not drawn at all. Then, to make the issue more evident I've placed a Shape widget on the screen and then moved it around until somewhere around the middle of the canvas parts of the Shape started to get cut off.
Comment 91 Andrey Loskutov CLA 2018-05-03 16:30:55 EDT
(In reply to Peter Severin from comment #87)
> I see some clipping issues on HiDPI display while testing WireframeSketcher
> plugin with Eclipse 4.8 I-builds. I suspect that these issues might be
> linked to the fix in this bug, however I am still not sure what exactly the
> issue and how to create a test case for that. I've narrowed it down to Path
> drawing, as other drawing methods seem to be unaffected. Any ideas? I'm
> attaching a screenshot of what I see.

Please start Eclipse with this extra VM option: 
-Dorg.eclipse.swt.internal.gtk.cairoContextReuse=false

This should switch to the "old" drawing path,see bug 533533.
If the problem remains, the problem is somewhere else.
Comment 92 Eric Williams CLA 2018-05-03 16:38:03 EDT
It's worth noting that native scaling is disabled in SWT right now, which could be the cause of this issue (since only HiDPI users experience it). There are plans to have native scaling enabled for Photon, see bug 530932 for more info.
Comment 93 Peter Severin CLA 2018-05-03 16:48:34 EDT
I tried -Dorg.eclipse.swt.internal.gtk.cairoContextReuse=false argument and it solved the issue.

In my experience GEF editors work fairly well with the current implementation of HiDPI, so I am not sure if native scaling will improve anything here.
Comment 94 Eric Williams CLA 2018-05-03 16:55:08 EDT
(In reply to Peter Severin from comment #93)
> I tried -Dorg.eclipse.swt.internal.gtk.cairoContextReuse=false argument and
> it solved the issue.
> 
> In my experience GEF editors work fairly well with the current
> implementation of HiDPI, so I am not sure if native scaling will improve
> anything here.

Well native scaling also affects Cairo operations, so maybe this fix has exposed a bug elsewhere. But reading your comment I'm thinking that's no longer the case.
Comment 95 Andrey Loskutov CLA 2018-05-03 16:57:48 EDT
Eric, is there a way to enforce hidpi drawing while using "usual" monitor on Linux? We simply have no hidpi hardware :(
Comment 96 Eric Williams CLA 2018-05-03 17:01:31 EDT
(In reply to Andrey Loskutov from comment #95)
> Eric, is there a way to enforce hidpi drawing while using "usual" monitor on
> Linux? We simply have no hidpi hardware :(

Not sure about RHEL, but in Fedora 27 you can change the scaling in the settings, even if the monitor is not HiDPI.

Settings -> Devices -> Displays -> Scale [100% or 200%].

I imagine in RHEL 7.4 or 7.5 it should be something similar.
Comment 97 Andrey Loskutov CLA 2018-05-07 03:03:49 EDT
(In reply to Simeon Andreev from comment #89)
> We re-open this I guess?

No, I can't see this bug anymore :-)
I've opened new one with appropriate title: bug 534417.
Comment 98 Eric Williams CLA 2018-05-08 10:41:54 EDT
Andrey, can you verify this one for M7? Candidate build ID is I20180507-2205.
Comment 99 Andrey Loskutov CLA 2018-05-08 11:21:59 EDT
Control.print(GC) works in I20180507-2205, also the WireframeSketcher.
Comment 100 Peter Severin CLA 2018-11-08 04:16:43 EST
I've encountered some significant performance issues caused by the fix to this bug. Please see comment 23 in Bug 493714 for more details:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=493714#c23
Comment 101 Peter Severin CLA 2018-11-08 04:52:06 EST
I've created Bug 540908 for the performance issue.