-
Notifications
You must be signed in to change notification settings - Fork 147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
"Graphic is disposed" on Windows with pull/1496 #1639
Comments
Thank you for reporting the issue, Tobias! |
@tobiasmelcher thx for reporting that issue. I had a look into it, but I cannot reproduce a new issue with that. I took a simpler variant of your snippet:
With that I get the similar exception even in the 2022-12 release (just picked any old release). Perhaps my simplified snippet behaves different as in your use case, so do you have a complete reproducer for the that worked before in the same setup and setting? Please give me your full monitor setup then. My current assumption is, that your setup could cause that and the default cached variant of the ImageData is different than before, so the second variant shall be created. Nevertheless, I didn't have a too deep look into the jface implementation, but it looks to me that disposing the tempImage will lead to issues when two different variants of that icon are requested based on the implementation in CachedImageImageDataProvider -> e.g. with two monitors (100 & 200) and the experimental rescaling at runtime set to true. |
@akoch-yatta, here the snippet to reproduce the problem public class Snippet {
public static void main(String[] args) {
Display display = new Display();
Shell shell = new Shell(display);
shell.setText("Test");
shell.setSize(300, 200);
Image tempImage = new Image(display, 20, 20);
ImageDescriptor[] overlaysArray = new ImageDescriptor[6];
Image image = new DecorationOverlayIcon(tempImage, overlaysArray).createImage();
tempImage.dispose();
Button button = new Button(shell, SWT.PUSH);
button.setImage(image);
shell.open();
while (!shell.isDisposed()) {
if (!display.readAndDispatch()) {
display.sleep();
}
}
image.dispose();
display.dispose();
}
} The zoom value on my machine is 200.
Thread [main] (Suspended (breakpoint at line 111 in CompositeImageDescriptor$CachedImageImageDataProvider))
CompositeImageDescriptor$CachedImageImageDataProvider.getImageData(int) line: 111
DecorationOverlayIcon(CompositeImageDescriptor).getZoomedImageData(ImageDataProvider) line: 457
DecorationOverlayIcon(CompositeImageDescriptor).drawImage(ImageDataProvider, int, int) line: 267
DecorationOverlayIcon.drawCompositeImage(int, int) line: 217
DecorationOverlayIcon(CompositeImageDescriptor).getImageData(int) line: 375
0x00000222a802bc10.getImageData(int) line: not available
DPIUtil.lambda$2(ImageDataProvider, Integer) line: 543
0x00000222a8029c00.apply(Object) line: not available
DPIUtil.getElementAtZoom(Function<Integer,T>, int) line: 572
DPIUtil.validateAndGetImageDataAtZoom(ImageDataProvider, int) line: 543
Image.getImageMetadata(int) line: 768
Image.getBounds(int) line: 1306
Image.getBounds() line: 1301
ImageList.add(Image) line: 61
Button._setImage(Image) line: 144
Button.setImage(Image) line: 875
Snippet.main(String[]) line: 25 |
This commit adapts the bounds calculation in Image to not create any handle, but use and scale the bounds of an existing handle if no handle for the desired zoom is available Fixes eclipse-platform#1639
@tobiasmelcher Thx for the Snippet and the further analysis. The issue was that getBounds created new handles on demand instead of scaling existing ones. I created a PR for that. I want to emphasize, that the underlyind issue remains. The implementation of CachedImageImageDataProvider relies on the baseImage (=tempImage) not being disposed. The exception will reappear if a second variant of the image is requested (see my Snippet). Could you open an issue for jface with your usecase? Perhaps it is intended, but if not the implementation could be changed to no rely on the passed image not being disposed. |
Hi colleagues, the problem raised again. I am again getting the "Graphic is disposed" exception. Could you please fix again? Thanks a lot,
|
Thank you, @tobiasmelcher! I can reproduce the issue at 200% with your snippet (#1639 (comment)). We will have a look. |
This commit unifies the getBounds calculation for ImageDataProvider and ImageFileNameProvider by always scaling the bounds of an existing handle of an image, if the bounds for a different zoom are requested. fixes eclipse-platform#1639
This effect was re-introduced with c493ae8. I will adapt the causing place to not call getImageData anymore. But I just want to raise the awareness, that the underlying issue remains. The implementation of CachedImageImageDataProvider relies on the baseImage (=tempImage) not being disposed. The exception will reappear if a second variant of the image is requested (see my Snippet). |
#1496 introduces an incompatibility which breaks existing implementations.
Following pattern is used where the base image is disposed directly after creating an instance of DecorationOverlayIcon.
This produces now a "Graphic is disposed" exception on Windows with following stack:
Could you please improve the enhancements of #1496 so that this exception does no longer occur?
Thanks and best regards,
Tobias
The text was updated successfully, but these errors were encountered: