Regression drawing image arising from fix to #185 and other regressions in Image class

[Phillipus]

Windows only.

There have been a few changes to the way transparency is handled in the Image class on Windows from @jonathan-meier recently. However, we were left with a bug that caused problems that was apparently fixed here with #185

Unfortunately this fix has caused yet another bug which now makes our RCP application broken when targetted with the latest SWT version.

The regression is caused by the fix in 8def011 - however the underlying cause is somewhere deeper than that arising from previous changes to Image class on Windows

I have put together a snippet that shows the problem (see later post)

If you run the snippet with a version of SWT prior to 8def011 a red box should be drawn:

But running the snippet with a version of SWT after 8def011 gives a wrong colored image:

[Phillipus]

Here is the test snippet:

import org.eclipse.swt.graphics.Color;
import org.eclipse.swt.graphics.GC;
import org.eclipse.swt.graphics.Image;
import org.eclipse.swt.graphics.ImageData;
import org.eclipse.swt.layout.GridLayout;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Shell;

public class ColorBug {

    public static void main(String[] args) {
        Display display = new Display();
        Shell shell = new Shell(display);
        shell.setText("Image Color Bug");
        shell.setSize(300, 200);
        shell.setLayout(new GridLayout());
        
        Image image = new Image(display, getImageData());
        
        shell.addPaintListener(e -> {
            e.gc.drawImage(image, 0, 0);
        });
        
        shell.open();
        
        while(!shell.isDisposed()) {
            if(!display.readAndDispatch()) display.sleep();
        }
        
        display.dispose();
        image.dispose();
    }
    
    static ImageData getImageData() {
        Image tmp = new Image(Display.getDefault(), 100, 100);
        
        GC gc = new GC(tmp);
        gc.setBackground(new Color(255, 0, 0));
        gc.fillRectangle(0, 0, 100, 100);
        gc.dispose();
        
        ImageData id = tmp.getImageData(100);
        tmp.dispose();
        
        return id;
    }
}

[Phillipus]

The problem with these changes from @jonathan-meier is that we are still uncovering regressions that affect how Images are handled on Windows. And for a production RCP app such as ours these regressions have a negative impact and mean that we cannot build on the latest version of Eclipse in order to resolve other regressions.

Would it be over-stepping the mark to ask for the following changes to be reverted?

def5c63
22797d3
8def011

[SyntevoAlex]

I would rather give @jonathan-meier some time to fix it. I understand that the entire work is not trivial, and connected to older bugs that were lurking in SWT without being noticed. It's not too good to just revert things whenever something happens. This way, a large work has chances to be abandoned completely.

[Phillipus]

I would rather give @jonathan-meier some time to fix it. I understand that the entire work is not trivial, and connected to older bugs that were lurking in SWT without being noticed. It's not too good to just revert things whenever something happens. This way, a large work has chances to be abandoned completely.

Sure, but I am afraid that @jonathan-meier is no longer involved with supporting these changes.

Here is my problem:

1. We have a free RCP app, Archi (which is generally downloaded about 7000 times very month)
2. We currently build on Eclipse 4.23 but this has a regression that involves RTL text not rendering correctly so we would like to upgrade...
3. So we try to use Eclipse 4.24....but this has an issue with drawing *,ico images and no menus on upcoming macOS Ventura,,,
4. So we try to use Eclipse 4.25...but this has this regression with drawing images....
5. So what now? Wait for Eclipse 4,26...4.27....4.28....?

Sorry for my complaint but this is getting so stressful that I am seriously considering abandoning Archi.

[SyntevoAlex]

One possible approach would be to either

• Cherry-pick desired fixes to older version
• Revert what causes problems in newer version

and build SWT yourself. That's how we do it in our product.

[Phillipus]

and build SWT yourself. That's how we do it in our product.

I know. But I don't have the ability to do that. We use the whole Tycho/Maven build of Eclipse. Also, this can affect plug-ins that use Eclipse as a host, so no control over the version of SWT used in the host.

But now I am getting off-topic and don't want to derail the original issue.

[Phillipus]

Here is another test snippet. With the older, working, version of SWT it should draw a black box but with the new version it is transparent.

Interestingly, if the Color is changed to Color color = new Color(1, 0, 0); it is not transparent.

import org.eclipse.swt.SWT;
import org.eclipse.swt.graphics.Color;
import org.eclipse.swt.graphics.GC;
import org.eclipse.swt.graphics.Image;
import org.eclipse.swt.graphics.ImageData;
import org.eclipse.swt.layout.GridLayout;
import org.eclipse.swt.widgets.Button;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Shell;

public class ColorBug2 {

    public static void main(String[] args) {
        Display display = new Display();
        Shell shell = new Shell(display);
        shell.setText("Image Color Bug 2");
        shell.setSize(300, 200);
        shell.setLayout(new GridLayout());
        
        Button button = new Button(shell, SWT.FLAT);
        Image image = new Image(display, getImageData());
        button.setImage(image);
        
        shell.open();
        
        while(!shell.isDisposed()) {
            if(!display.readAndDispatch()) display.sleep();
        }
        
        display.dispose();
        image.dispose();
    }
    
    private static ImageData getImageData() {
        Image tmp = new Image(Display.getDefault(), 100, 70);
        
        // Change this and it's not transparent:
        // Color color = new Color(1, 0, 0);
        Color color = new Color(0, 0, 0);
        
        GC gc = new GC(tmp);
        gc.setBackground(color);
        gc.fillRectangle(0, 0, 100, 70);
        gc.dispose();
        
        ImageData id = tmp.getImageData(100);
        tmp.dispose();
        
        return id;
    }
}

[jonathan-meier]

This is indeed a regression of PR #244 (Issue #185). I provided a fix in PR #303. Please test my patch and also make sure that issue #185 is not reintroduced by it.

Instead of allowing alpha channel data in ImageData.data only for icons when initializing an Image, PR #244 also allowed this for bitmaps, which does not work. Alpha channel data in ImageData.data always has to be ignored and only read from ImageData.alphaData for bitmaps.

Unfortunately, Image.getImageData as used in the reproducer snippets above (thanks for providing them @Phillipus!) returns alpha channel data in ImageData.data that is problematic for the initialization of the Image later on (it is set to fully transparent instead of fully opaque). I improved this situation as well in my PR to make the result of Image.getImageData more compatible even though alpha channel data in ImageData.data should always be ignored for bitmaps.

As a workaround for the current SWT version one can fix the ImageData by overwriting the alpha channel data manually before constructing an Image from it:

static void makeOpaque(ImageData imageData) {
    if (imageData.depth != 32) {
    	return;
    }
    byte[] data = imageData.data;
    for (int dp = 3; dp < data.length; dp += 4) {
    	data[dp] = (byte) 0xFF;
    }
}