Community
Participate
Working Groups
We have a Figure with the following (simplified) paint logic, which essentially adds a drop shadow by first filling a rectangle with the shadow and then painting over it: public class RectangleWithShadow extends RectangleFigure { @Override protected void fillShape(Graphics graphics) { drawShadow(graphics); super.fillShape(graphics); } private void drawShadow(Graphics graphics) { final int shadow = EditorConstants.SHADOW_WIDTH; final Rectangle bounds = getBounds(); graphics.pushState(); graphics.setBackgroundColor(ColorConstants.gray); graphics.setAlpha(140); graphics.setClip(new Rectangle(bounds.x, bounds.y, bounds.width+shadow, bounds.height+shadow)); final Rectangle shadowRect = bounds.getCopy().translate(shadow, shadow); graphics.fillRectangle(shadowRect); graphics.popState(); } } This works fine in the editor itself, however it causes strange effects in the outline view, where some or all of the shadow rectangle is displayed in front of the figure rather than behind it.
Created attachment 165540 [details] Screenshot showing effect in outline
I can reproduce this with the logic example, if I adjust the LEDFigure paint method as follows: /** * @see org.eclipse.draw2d.Figure#paintFigure(Graphics) */ protected void paintFigure(Graphics g) { Rectangle r = getBounds().getCopy(); drawShadow(g); g.translate(r.getLocation()); g.setBackgroundColor(LogicColorConstants.logicGreen); g.setForegroundColor(LogicColorConstants.connectorGreen); g.fillRectangle(0, 2, r.width, r.height - 4); int right = r.width - 1; g.drawLine(0, Y1, right, Y1); g.drawLine(0, Y1, 0, Y2); g.setForegroundColor(LogicColorConstants.connectorGreen); g.drawLine(0, Y2, right, Y2); g.drawLine(right, Y1, right, Y2); // Draw the gaps for the connectors g.setForegroundColor(ColorConstants.listBackground); for (int i = 0; i < 4; i++) { g.drawLine(GAP_CENTERS_X[i] - 2, Y1, GAP_CENTERS_X[i] + 3, Y1); g.drawLine(GAP_CENTERS_X[i] - 2, Y2, GAP_CENTERS_X[i] + 3, Y2); } // Draw the connectors g.setForegroundColor(LogicColorConstants.connectorGreen); g.setBackgroundColor(LogicColorConstants.connectorGreen); for (int i = 0; i < 4; i++) { connector.translate(GAP_CENTERS_X[i], 0); g.fillPolygon(connector); g.drawPolygon(connector); connector.translate(-GAP_CENTERS_X[i], 0); bottomConnector.translate(GAP_CENTERS_X[i], r.height - 1); g.fillPolygon(bottomConnector); g.drawPolygon(bottomConnector); bottomConnector.translate(-GAP_CENTERS_X[i], -r.height + 1); } // Draw the display g.setBackgroundColor(LogicColorConstants.logicHighlight); g.fillRectangle(displayHighlight); g.setBackgroundColor(DISPLAY_SHADOW); g.fillRectangle(displayShadow); g.setBackgroundColor(ColorConstants.black); g.fillRectangle(displayRectangle); // Draw the value g.setFont(DISPLAY_FONT); g.setForegroundColor(DISPLAY_TEXT); g.drawText(value, valuePoint); } private void drawShadow(Graphics graphics) { final int shadow = 10; final Rectangle bounds = getBounds(); graphics.pushState(); graphics.setClip(new Rectangle(bounds.x + shadow, bounds.y + shadow, bounds.width + shadow, bounds.height + shadow)); graphics.setBackgroundColor(ColorConstants.gray); graphics.setAlpha(140); final Rectangle shadowRect = bounds.getCopy().translate(shadow, shadow); graphics.fillRectangle(shadowRect); graphics.popState(); }
Created attachment 245855 [details] Screenshot demonstrating the problem
I tracked it down to be related to the fact that the source figure is broken down into tiles within Thumbnail$ThumbnailUpdater. If I change ThumbnailUpdater#resetTileValues() to always use hTiles = vTiles = 1, I do not observe these effects. However, I have not identified the root cause of this yet.
It seems that the drop shadow is rendered twice, i.e. not properly clipped when rendering the thumbnail. By adding a "thumbnailGraphics.drawRectangle(rect);" in addition to the "thumbnailGraphics.fillRectangle(rect);" within ThumnailUpdater#run() that can be properly observed.
The cause for this seems to be as follows: ThumbnailUpdater sets the clipping rectangle for the tile it is going to update in one of its run. This clipping rectangle is used to constraint the painting of the source figure within this run, however, as the drop shadow pushes the graphics state and applies its own clip (which extends the figure bounds), it paints into a larger area than actually covered by the current tile clipping rectangle, thus overwriting parts of the image belonging to tiles that have already been rendered in earlier runs. There are two problems related to this (one is actually a concern for Thumbnail): 1) The fact that the figure actually paints outside of its bounds is a violation of the paint contract, so Thumbnail actually would not have to compensate for this. 2) Even if the clipping area used to draw the drop shadow is not set outside the clipping area of the source figure the overpaint problem would still occur; this is a problem of Thumbnail because it does not prevent the figure from overpainting the tile clipping area. We will probably have to used the wrapped in SWTGraphics (wrapped inside the thumbnailGraphics) to apply the clipping.
A proper solution that still preserves the tile-based rendering of the thumbnail would have to ensure that whatever clipping the user specifies within the sourceFigure's paint(Graphics), this never extends the area of the current tile that is updated. This could be achieved by replacing the SWTGraphics that wraps around the thumbnailGC with a custom implementation that overwrites setClip(Rectangle) and setClip(Path) to intersect the passed in value with the current tile area. However, as SWT is not capable of intersecting with Paths (see bug #87776) this is only realizable for setClip(Rectangle) (or we would have to implement Path intersection ourselves). As such, there are IMHO two alternatives how to solve this: 1) Preserve the tile rendering by using a second image into which the sourceFigure renders and from which the current tile area is then copied to the thumbnailImage within run() 2) Drop the tile-based rendering and update the thumbnail image as a whole within run()
I performed the following changes: - Added a tile image, into which the source figure now paints (instead of painting directly into the thumbnail image). The thumnail image is now udpated by copying the contents of this tile image into the thumbnail (at the specified tile area) in each run. This way, even if the source figure sets an own clip on the passed in Graphics inside paint(Graphics), already painted areas in the thumbnail image can never be corrupted (as we can control what gets copied). - Fixed that the SWTGraphics that is constructed around the GC was never disposed. Verified that the thumbnail within logic editor's outline now paints fine, even if the LEDFigure is tweaked as described. Resolving as fixed in 3.10.0M2.
Created attachment 249779 [details] Workbench screenshot
Hello, In the case of Sirius for Eclipse Mars, we recently launched our SWTBot test suite on this platform. We have an error raised during the execution of some of these tests. We do not reproduce this error manually: java.lang.AssertionError: Error(s) raised during test : org.eclipse.sirius.tests.swtbot.ContainerCreationTest . Log Plugin : org.eclipse.core.runtime . Error from plugin:org.eclipse.ui, message: Unhandled event loop exception, exception: org.eclipse.swt.SWTException: Failed to execute runnable (java.lang.IllegalArgumentException: Argument not valid) (...) Caused by: java.lang.IllegalArgumentException: Argument not valid at org.eclipse.swt.SWT.error(SWT.java:4446) at org.eclipse.swt.SWT.error(SWT.java:4380) at org.eclipse.swt.SWT.error(SWT.java:4351) at org.eclipse.swt.graphics.GC.drawImage(GC.java:870) at org.eclipse.draw2d.parts.Thumbnail$ThumbnailUpdater.run(Thumbnail.java:185) at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35) at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:136) ... 25 more I placed a breakpoint on GC [line: 870] - drawImage(Image, int, int, int, int, int, int, int, int) and this error is raised because of a negative srcWidth passed during the call of this method, line 185 in org.eclipse.draw2d.parts.Thumbnail: thumbnailGC.drawImage(tileImage, 0, 0, sx2 - sx1, sy2 - sy1, sx1, sy1, sx2 - sx1, sy2 - sy1); The variable state: sy1 738 sy2 861 sx1 1659 sx2 1301 Do you have any idea about what could lead to this inconsistent state ? I Joined a screen-shot of the workbench at the moment where this error is raised. Regards, Florian
Not obviously. Could you try to strip-down the example so we have a chance to reproduce it?
Hi Alexander, After Florian, I analyzed our problem on Sirius test. It's difficult to reproduce, as it is random. But after several debug sessions with one of our failing test (org.eclipse.sirius.tests.swtbot.ContainerCreationTest.testContainerCreationInDiagramWithScrollAndChangeZoom()), I noticed that this problem occurs when the zoom is changed in the middle of iteration on Thumbnail.ThumbnailUpdater.run(), that calls itself in asyncExec several times. I reproduced this problem manually with org.eclipse.gef.examples.logic. This needs a change in ThumbnailUpdater to have time to change the zoom during the iteration. The patch with this update is linked to this issue: "309912-AddAsyncExecSleep.patch". Steps to reproduce: * Apply the patch to Thumbnail class * Ensure you have the project org.eclipse.gef.examples.logic in your workspace * Launch a runtime * Create a simple project * Create a fourBitAdder1.logic diagram (Examples/GEF (Graphical Editing Framework)/Logic Diagram --> Four-bit Adder Model) * Display Error Log * In the Outline click on "Show Overview" button * Change the zoom quickly several times --> An error appears in error log "Unhandled event loop exception" Corresponding stack in the error log: org.eclipse.swt.SWTException: Failed to execute runnable (java.lang.IllegalArgumentException: Argument not valid) at org.eclipse.swt.SWT.error(SWT.java:4481) at org.eclipse.swt.SWT.error(SWT.java:4396) at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:139) at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:3790) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3428) ... Caused by: java.lang.IllegalArgumentException: Argument not valid at org.eclipse.swt.SWT.error(SWT.java:4462) at org.eclipse.swt.SWT.error(SWT.java:4396) at org.eclipse.swt.SWT.error(SWT.java:4367) at org.eclipse.swt.graphics.GC.drawImage(GC.java:870) at org.eclipse.draw2d.parts.Thumbnail$ThumbnailUpdater.run(Thumbnail.java:185) at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35) at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:136) ... 24 more I have not reopened this issue but I think it should be.
Created attachment 252071 [details] 309912-AddAsyncExecSleep.patch for comment 12
Hi Alexander, Is there a chance that this regression (comment 12) be fixed for M7? Best regards, Laurent
(In reply to Laurent Redor from comment #14) > Hi Alexander, > Is there a chance that this regression (comment 12) be fixed for M7? > Best regards, > Laurent Laurent, your proposed patch does not look as a reasonable fix to me. The problem seems to be merely circumvented by introducing some idle time before running the update. Who guarantees that 500ms are sufficient in all cases? The only chance I see to resolve this in the Mars release timeframe is that we isolate the root cause for it soon and fix it properly before M7. Can you confirm whether the steps to reproduce you named in comment #12 cannot be used to reproduce it with earlier versions (i.e. the test regression is actually the same problem as the one you manually reproduce)?
I've not proposed a patch to solve the problem. The patch is only here to reproduce the problem more easily. I will try to reproduce the problem with earlier versions (ie before commit d212bdd [1]) with an equivalent modification (ie add sleep(500)). [1] http://git.eclipse.org/c/gef/org.eclipse.gef.git/commit/org.eclipse.draw2d/src/org/eclipse/draw2d/parts/Thumbnail.java?id=d212bdd4794f5866669e8bbd0425c31ee7632ae7
(In reply to Laurent Redor from comment #16) > I've not proposed a patch to solve the problem. The patch is only here to > reproduce the problem more easily. > > I will try to reproduce the problem with earlier versions (ie before commit > d212bdd [1]) with an equivalent modification (ie add sleep(500)). > > [1] > http://git.eclipse.org/c/gef/org.eclipse.gef.git/commit/org.eclipse.draw2d/ > src/org/eclipse/draw2d/parts/Thumbnail. > java?id=d212bdd4794f5866669e8bbd0425c31ee7632ae7 Ok, thanks. That is, without the application of your patch, the issue is only reproducible within the test-setting?
(In reply to Alexander Nyßen from comment #17) > Ok, thanks. That is, without the application of your patch, the issue is > only reproducible within the test-setting? Yes, this is the only way I found to reproduce easily the problem that we detected in our SWTBot tests since the commit d212bdd.
I confirm that before the commit d212bdd I cannot reproduce the problem (even with adding the same "sleep(500)" in the middle of iteration that reveals the problem).
That is really hard to reproduce, it only happens in very few occasions. From looking at the code, I would assume there is some race-condition when the thumbnail-image gets requested repeatedly. Could you please re-try after having made Thumbnail#getThumbnailImage() synchronized?
Same problem after having made Thumbnail#getThumbnailImage() synchronized --> protected synchronized Image getThumbnailImage()
This also seems to have occurred outside your test scenario, as we have 4 occurrences that were reported via the Eclipse error reporting initiative. As the issue targeted in this bugzilla has been resolved, I am resolving it as fixed and have created #467528 to keep track of the ThumbnailUpdater problem. Please let's discuss anything further there.
(In reply to Alexander Nyßen from comment #22) > This also seems to have occurred outside your test scenario, as we have 4 > occurrences that were reported via the Eclipse error reporting initiative. > As the issue targeted in this bugzilla has been resolved, I am resolving it > as fixed and have created #467528 to keep track of the ThumbnailUpdater > problem. Please let's discuss anything further there. Bug #467528.
Laurent, I added a patch to #467528. Could you please try it out and give me some feedback?
(In reply to Alexander Nyßen from comment #24) > Laurent, I added a patch to #467528. Could you please try it out and give me > some feedback? The test org.eclipse.sirius.tests.swtbot.ContainerCreationTest.testContainerCreationInDiagramWithScrollAndChangeZoom() (see comment 12) is OK now with this patch. So I think your patch fix the problem.
The thumbnail is not rendering on MacOs Mojave. See https://github.com/archimatetool/archi/issues/401 I've been trying for a long time to devise a workaround. If I set MAX_NUMBER_OF_TILES to 1 the thumbnail is rendered correctly. However I feel there must be a better workaround. Can anyone help?
