GC.setClipping(path) doesn't paint on Linux when transformed #3120#3121
GC.setClipping(path) doesn't paint on Linux when transformed #3120#3121ptziegler wants to merge 1 commit intoeclipse-platform:masterfrom
Conversation
|
I've tested this with all "Bug531667_" snippets and couldn't see any regression. So I think this change is compatible with the fix for bug 531667. |
|
@Phillipus I've also tested this change with your |
Test Results (linux) 58 files - 30 58 suites - 30 10m 39s ⏱️ - 4m 38s Results for commit 1aefce1. ± Comparison against base commit cc2762f. This pull request removes 40 tests.♻️ This comment has been updated with latest results. |
And I've tested it with our RCP app, Archi, and the problem disappeared there as well. Incredible detective work, thank-you so much! |
HeikoKlare
left a comment
HeikoKlare
left a comment
There was a problem hiding this comment.
Thank you for directly providing a proposal for a fix of the reported issue.
What I do not understand at a first glance is how the changed implementation is supposed to properly deal with multiple calls to setClipping(). In my understanding, calls to that method are supposed to replace the previously set clipping (thus doing a resetClipping() everytime the method is called). After this change, basically the union of the passed paths is calculated.
I adapted the snippet to not set a transform to the GC and instead set a different (much smaller) clipping before the actual one (adding the following before gc.setClipping(p):
Path p2 = new Path(display);
p2.addRectangle(100, 400, 10, 10);
gc.setClipping(p2);The result used to be this:
And with this change the result is this:
Maybe the correct solution would be to still call resetClipping() but in that method temporarily revert the transformation applied to Cairo for setting the original clipping bounds or otherwise adapt the original clipping bounds applied in resetClipping() to be transformed by an inverse of the GC's transform?
Previously the clipping was reset with |
I tried that and that seems to work for all cases. |
Actually, in real world usage it doesn't. I tested it on our RCP app and as you scroll up and down some of the figures' parts are being drawn outside the GEF editor: 
.
And another case: 
|
From what I understand, this is because the clipping is shared between all widgets. So callling In the end, I've applied @HeikoKlare suggestion of still calling |
Many thanks for continuing to look at this. I tested the latest commit and it works in Draw2d if scaling is 100% but not when scaling is any other value. Here's the snippet from the original report using a import org.eclipse.draw2d.Figure;
import org.eclipse.draw2d.FigureCanvas;
import org.eclipse.draw2d.Graphics;
import org.eclipse.draw2d.ScalableFreeformLayeredPane;
import org.eclipse.draw2d.geometry.Rectangle;
import org.eclipse.swt.graphics.Color;
import org.eclipse.swt.graphics.Path;
import org.eclipse.swt.layout.FillLayout;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Shell;
public class LinuxClipGraphics {
public static void main(String[] args) {
Display display = new Display();
Shell shell = new Shell(display);
shell.setLayout(new FillLayout());
shell.setSize(600, 300);
shell.setText("Clip Graphics");
ScalableFreeformLayeredPane layeredPane = new ScalableFreeformLayeredPane();
layeredPane.setOpaque(true);
layeredPane.setBackgroundColor(new Color(255, 255, 255));
layeredPane.setPreferredSize(580, 800);
layeredPane.setScale(2);
FigureCanvas canvas = new FigureCanvas(shell);
canvas.setContents(layeredPane);
Figure figure1 = new TestFigure();
figure1.setBounds(new Rectangle(10, 120, 120, 120));
layeredPane.add(figure1);
Figure figure2 = new TestFigure();
figure2.setBounds(new Rectangle(10, 300, 120, 120));
layeredPane.add(figure2);
shell.open();
while(!shell.isDisposed()) {
if(!display.readAndDispatch()) {
display.sleep();
}
}
}
private static class TestFigure extends Figure {
Color background = new Color(181, 255, 255);
@Override
protected void paintFigure(Graphics graphics) {
Rectangle rect = getBounds().getCopy();
graphics.setBackgroundColor(background);
graphics.fillRectangle(rect);
graphics.setLineWidth(6);
Path path = new Path(null);
path.addRectangle(rect.x, rect.y, rect.width, rect.height);
graphics.setClip(path);
graphics.drawPath(path);
path.dispose();
}
}
} |
ptziegler
force-pushed
the
issue3120
branch
2 times, most recently
from
4893673 to
8934a73
Compare
…platform#3120 The fix for Bug 531667 (d7ce597) introduced a regression when calling `setClipping(Path)` after `setTransform(Transform)`. The rectangle used when resetting the clip doesn't consider the previously done transformation, producing an empty clip if this rectangle and the `Path` don't intersect. Instead of using the local `clipping` variable (which contains the initial clip when the GC was created), the `resetClipping()` method now uses the rectangle returned by `getClipping()`, which internally applies the transformation matrix. To reproduce, execute the attached snippet. The rectangle in this example should be green, as it is drawn over the red rectangle. Closes eclipse-platform#3120
Test Results162 files - 8 162 suites - 8 10s ⏱️ - 28m 0s For more details on these errors, see this check. Results for commit a0b6fb0. ± Comparison against base commit cc2762f. This pull request removes 4567 tests. |
|
I think there are some infra problems going on? All tests are crashing the moment the Surefire thread is started. Nonetheless, I've added/reactivated some tests to verify some assumptions. The short version is that clipping on Linux is weird. When using a |
|
I have noticed the test failure too but so far I have no clue what caused it. |
|
So the problem is #3129 |
4 participants
The fix for Bug 531667 (d7ce597) introduced a regression when calling
setClipping(Path)aftersetTransform(Transform). The rectangle used when resetting the clip doesn't consider the previously done transformation, producing an empty clip if this rectangle and thePathdon't intersect.Instead of using the local
clippingvariable (which contains the initial clip when the GC was created), theresetClipping()method now uses the rectangle returned bygetClipping(), which internally applies the transformation matrix.To reproduce, execute the attached snippet. The rectangle in this example should be green, as it is drawn over the red rectangle.
Closes #3120