eclipse.platform.swt icon indicating copy to clipboard operation
eclipse.platform.swt copied to clipboard

Published 20 hours ago • verified publisher icon eclipse-platform

[macOS Sonoma] Control.setBackgroundImage - image is upside down and flipped

Open Phillipus opened this issue 2 years ago • 35 comments
trafficstars

Describe the bug Background images on SWT controls are inverted.

This was reported here but I've opened a dedicated issue as it's not to do with the splash screen but rather Control#setBackgroundImage.

To Reproduce

import org.eclipse.swt.graphics.Image;
import org.eclipse.swt.layout.FillLayout;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Shell;

public class ImageTest {

    public static void main(String[] args) {
        Display display = new Display();
        Shell shell = new Shell(display);
        shell.setText("Image Test");
        shell.setLayout(new FillLayout());
        
        Image image = new Image(display, ImageTest.class.getResourceAsStream("splash.png"));
        shell.setBackgroundImage(image);
        
        shell.setSize(image.getBounds().width, image.getBounds().height);
        shell.open();
        
        while(!shell.isDisposed()) {
            if(!display.readAndDispatch()) display.sleep();
        }

        display.dispose();
    }
}

Use this image for testing (rename to splash.png), or your own image if you prefer:

splash

Expected behavior The image should display the right way.

Screenshots Run the snippet and see this:

inverted

Environment:

  1. Select the platform(s) on which the behavior is seen:
    • [ ] All OS
    • [ ] Windows
    • [ ] Linux
    • [x] macOS

  1. Additional OS info (e.g. OS version, Linux Desktop, etc) macOS Sonoma 14 Mac Mini M1

  2. JRE/JDK version Temurin 17

Phillipus avatar Aug 19 '23 13:08 Phillipus

In the case of displaying the splash screen when launching Eclipse or an RCP app, the image initially appears correctly but is inverted after a second or two. AFAICS, this is because the initial display of the splash is done natively in the org.eclipse.equinox.launcher.Main class which calls the org.eclipse.equinox.launcher.JNIBridge method showSplash which then calls the native _show_splash method. Presumably whatever is happening in _show_splash works OK.

But then the trail goes to org.eclipse.ui.internal.Workbench and the method createSplashWrapper. Here is where the Shell's background is set at line 786:

splashShell.setBackgroundImage(background);

Then spinEventQueueToUpdateSplash(display) is called by Workbench#createAndRunWorkbench to dispatch messages which updates the splash Shell. This is where the underlying ObjC message from calling setBackgroundImage is dispatched and the image is painted inverted.

Control#setBackgroundImage only sets the Image object and requests an update. Painting the background image is done in Control#fillBackground. The line that actually draws the background image is at line 1323:

NSColor.colorWithPatternImage(image.handle).setFill();

The code for this call is in org.eclipse.swt.internal.cocoa.NSColor:

public static NSColor colorWithPatternImage(NSImage image) {
    long result = OS.objc_msgSend(OS.class_NSColor, OS.sel_colorWithPatternImage_, image != null ? image.id : 0);
    return result != 0 ? new NSColor(result) : null;
}

And another at line 1345:

NSBezierPath.fillRect(rect);

So, there's either a bug in Apple's code or something has changed and a workaround is needed in SWT code.

I can't do any more than offer this analysis, so please feel free to help. :-)

Phillipus avatar Aug 20 '23 08:08 Phillipus

The same problem with using a Pattern with image:

import org.eclipse.swt.graphics.Image;
import org.eclipse.swt.graphics.Pattern;
import org.eclipse.swt.layout.GridLayout;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Shell;

public class ImageTestWithPattern {

    public static void main(String[] args) {
        Display display = new Display();
        Shell shell = new Shell(display);
        shell.setText("Image Test with Pattern");
        shell.setLayout(new GridLayout());
        
        Image image = new Image(display, ImageTestWithPattern.class.getResourceAsStream("splash.png"));
        shell.setSize(image.getBounds().width, image.getBounds().height);
        
        Pattern pattern = new Pattern(display, image);
        
        shell.addPaintListener(e -> {
            e.gc.setBackgroundPattern(pattern);
            e.gc.fillRectangle(0, 0, shell.getSize().x, shell.getSize().y);
        });
        
        shell.open();
        
        while(!shell.isDisposed()) {
            if(!display.readAndDispatch()) display.sleep();
        }

        display.dispose();
    }
}

Phillipus avatar Aug 20 '23 08:08 Phillipus

This will be in macOS Sonoma final as the behaviour exists in the Release Candidate.

Phillipus avatar Sep 12 '23 20:09 Phillipus

A self-contained snippet:

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

public class ImageTest {

    public static void main(String[] args) {
        Display display = new Display();
        Shell shell = new Shell(display);
        shell.setText("Image Test");
        shell.setLayout(new GridLayout());
        
        shell.setSize(200, 200);
        
        Image image = new Image(Display.getDefault(), 200, 200);
        GC gc = new GC(image);
        gc.setForeground(new Color(null, 0, 0, 0));
        gc.drawText("Hello World", 50, 100);
        gc.dispose();
        
        shell.setBackgroundImage(image);

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

        display.dispose();
    }
}

hello

Phillipus avatar Sep 15 '23 13:09 Phillipus

I'm hoping this can be addressed asap; it will hold up our adoption of this latest release

TIBCOeddie avatar Oct 02 '23 18:10 TIBCOeddie

I'm hoping this can be addressed asap; it will hold up our adoption of this latest release

I've documented my analysis of the problem in the previous comments, but I don't know if this is an Apple bug (I have reported it) or something that needs to change in SWT. It needs someone else to answer that.

Phillipus avatar Oct 02 '23 18:10 Phillipus

I am experiencing the same problem on Sonoma/MacMini M2 Pro. It exists in Eclipse 2023-03 as well as 2023-09.

In addition, the name of the application at the left side of the Menu Bar (just to the right of the Apple icon) says "New Application" instead of "Eclipse".

DonaldWills avatar Oct 05 '23 11:10 DonaldWills

I am experiencing the same problem on Sonoma/MacMini M2 Pro. It exists in Eclipse 2023-03 as well as 2023-09.

Yes, that's because the problem lies in macOS 14.

In addition, the name of the application at the left side of the Menu Bar (just to the right of the Apple icon) says "New Application" instead of "Eclipse".

Yes, and that has been reported (and fixed) here - https://github.com/eclipse-platform/eclipse.platform.swt/issues/779

Phillipus avatar Oct 05 '23 11:10 Phillipus

One of referenced commits from @enplotz above has a nice workaround for the splash screen if anyone is interested:

https://github.com/knime/knime-product/commit/095c88e22ba6d84e22a480f50734141836098169

The code flips the image which you can set in the splash handler's background image. I've re-purposed it to use in our RCP app's splash handler:

private Image flipImage(Display display, Image srcImage) {
    Rectangle bounds = srcImage.getBounds();
    Image newImage = new Image(display, bounds.width, bounds.height);
    
    GC gc = new GC(newImage);
    gc.setAdvanced(true);
    gc.setAntialias(SWT.ON);
    gc.setInterpolation(SWT.HIGH);
    
    Transform transform = new Transform(display);
    transform.setElements(1, 0, 0, -1, 0, 0);
    transform.translate(0, -bounds.height);
    gc.setTransform(transform);
    
    gc.drawImage(srcImage, 0, 0, bounds.width, bounds.height,
                           0, 0, bounds.width, bounds.height);
    
    gc.dispose();
    transform.dispose();
    
    return newImage;
}

Note that the method there isFlipBugPresent() doesn't work so you'll have to test for whether the bug is present in another way.

I test for it like this:

if(PlatformUtils.isMac() && PlatformUtils.compareOSVersion("14.0") >= 0) {
    // Flip the image
    shell.setBackgroundImage(flipImage(shell.getDisplay(), shell.getBackgroundImage()));
    
    // We are now responsible for disposing of the new image
    shell.addDisposeListener(e -> {
        shell.getBackgroundImage().dispose();
    });
}

Phillipus avatar Oct 15 '23 14:10 Phillipus

@Phillipus unfortunately the workaround doesn't work. I was testing on pre-Sonoma and seems flip-detection won't do the trick, unfortunately - see discussion ->

  • https://github.com/knime/knime-product/commit/517efcf9a9f540cb9bd28b96b70f0b4b3af88c51#commitcomment-129894276

swtdev avatar Oct 15 '23 22:10 swtdev

@swtdev Yes, that's why I said "Note that the method there isFlipBugPresent() doesn't work so you'll have to test for whether the bug is present in another way."

Phillipus avatar Oct 15 '23 22:10 Phillipus

Lol would help if I'd read the whole comment 🙃

swtdev avatar Oct 15 '23 22:10 swtdev

But, it would be better if a Mac expert could say why the image is flipped when calling the native ObjectiveC methods. Perhaps the co-ordinate system changed for colorWithPatternImage?

Phillipus avatar Oct 15 '23 22:10 Phillipus

I suspect that it's related to flipping in Image, see for example https://github.com/eclipse-platform/eclipse.platform.swt/blob/f37b9eb623e732999016b808915e9e39e581431b/bundles/org.eclipse.swt/Eclipse%20SWT/cocoa/org/eclipse/swt/graphics/Image.java#L1535-L1538

SyntevoAlex avatar Oct 15 '23 22:10 SyntevoAlex

This code isn’t invoked during splash rendering ;) I even tried migration from deprecated graphicPort to CGContext. It didn’t help

I suspect that it's related to flipping in Image, see for example

https://github.com/eclipse-platform/eclipse.platform.swt/blob/f37b9eb623e732999016b808915e9e39e581431b/bundles/org.eclipse.swt/Eclipse%20SWT/cocoa/org/eclipse/swt/graphics/Image.java#L1535-L1538

zulus avatar Oct 16 '23 00:10 zulus

Splash rendering (at least on Linux) is handled by the launcher thus the MacOS probably lives in https://github.com/eclipse-equinox/equinox/tree/master/features/org.eclipse.equinox.executable.feature/library/cocoa

akurtakov avatar Oct 16 '23 05:10 akurtakov

The splash image is just one case. The real issue is Control#setBackgroundImage

Phillipus avatar Oct 16 '23 08:10 Phillipus

And also the same problem with GC#setBackgroundPattern, see https://github.com/eclipse-platform/eclipse.platform.swt/issues/772#issuecomment-1685228348

Here the constructor Pattern(Device device, Image image) calls NSColor.colorWithPatternImage.

As with Control#setBackgroundImage things point to NSColor.colorWithPatternImage.

Phillipus avatar Oct 16 '23 09:10 Phillipus

Here's a Snippet that narrows the problem down to native calls to NSColor.colorWithPatternImage:

import org.eclipse.swt.graphics.Color;
import org.eclipse.swt.graphics.GC;
import org.eclipse.swt.graphics.Image;
import org.eclipse.swt.internal.cocoa.NSBezierPath;
import org.eclipse.swt.internal.cocoa.NSColor;
import org.eclipse.swt.internal.cocoa.NSGraphicsContext;
import org.eclipse.swt.internal.cocoa.NSRect;
import org.eclipse.swt.layout.GridLayout;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Shell;

@SuppressWarnings("restriction")
public class ColorWithPatternImage {

    public static void main(String[] args) {
        Display display = new Display();
        Shell shell = new Shell(display);
        shell.setText("Image Test");
        shell.setLayout(new GridLayout());
        
        shell.setSize(200, 200);
        
        Image image = new Image(Display.getDefault(), 200, 200);
        GC gc = new GC(image);
        gc.setForeground(new Color(null, 0, 0, 0));
        gc.drawText("Hello World", 50, 100);
        gc.dispose();
        
        shell.addPaintListener(e -> {
            NSGraphicsContext context = NSGraphicsContext.currentContext();
            context.saveGraphicsState();
            
            NSColor.colorWithPatternImage(image.handle).setFill();
            
            NSRect rect = new NSRect();
            rect.width = 200;
            rect.height = 200;
            
            NSBezierPath.fillRect(rect);
            
            context.restoreGraphicsState();
        });

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

        display.dispose();
    }
}

This gives:

hello

Phillipus avatar Oct 16 '23 09:10 Phillipus

I suspect that it's related to flipping in Image, see for example

https://github.com/eclipse-platform/eclipse.platform.swt/blob/f37b9eb623e732999016b808915e9e39e581431b/bundles/org.eclipse.swt/Eclipse%20SWT/cocoa/org/eclipse/swt/graphics/Image.java#L1535-L1538

@SyntevoAlex I tried setting the value to false:

NSGraphicsContext flippedContext = NSGraphicsContext.graphicsContextWithGraphicsPort(context.graphicsPort(), false);

This fixed the problem only in one case - https://github.com/eclipse-platform/eclipse.platform.swt/issues/772#issuecomment-1721254721 because that is called from GC(Drawable drawable, int style)

Phillipus avatar Oct 16 '23 09:10 Phillipus

Updated to Sonoma 14.1 this morning and the issue is not fixed (but the "NewApplication" menu bar is).

kysmith-csg avatar Oct 30 '23 14:10 kysmith-csg

I guess we need to find out whether Apple changed the co-ordinate system for colorWithPatternImage in Sonoma. See colorWithPatternImage

Phillipus avatar Oct 30 '23 14:10 Phillipus

Reason might be even simpler. When application starts, NSView created by native code have isFlipped marked as false, but later, all views created via SWTCanvasView.alloc() have isFlipped marked as true (which is strange, I cannot find reason why). So looks like there is "isFlipped" mix between views and display context

zulus avatar Nov 01 '23 23:11 zulus

Reason might be even simpler. When application starts, NSView created by native code have isFlipped marked as false, but later, all views created via SWTCanvasView.alloc() have isFlipped marked as true (which is strange, I cannot find reason why). So looks like there is "isFlipped" mix between views and display context

Are you able to determine where this isFlipped is set?

Phillipus avatar Nov 02 '23 09:11 Phillipus

There is another flipped creation of the context in Control: https://github.com/eclipse-platform/eclipse.platform.swt/blob/f37b9eb623e732999016b808915e9e39e581431b/bundles/org.eclipse.swt/Eclipse%20SWT/cocoa/org/eclipse/swt/widgets/Control.java#L2202-L2203

I have provided a pull-request that fixes both of the snippets in https://github.com/eclipse-platform/eclipse.platform.swt/issues/772#issuecomment-1764080659 and https://github.com/eclipse-platform/eclipse.platform.swt/issues/772#issuecomment-1721254721

sratz avatar Nov 06 '23 12:11 sratz

Ok, well, turns out this is way more complicated and doesn't solve all the issues... In fact, it causes line numbers in the editor to be flipped now.

I found this: https://developer.apple.com/forums/thread/736618

Looks like we are also in that case 3, as we configure isFlipped: https://github.com/eclipse-platform/eclipse.platform.swt/blob/030807afad9c7cd7a3faad765ef4061c8f7a3e08/bundles/org.eclipse.swt/Eclipse%20SWT/cocoa/org/eclipse/swt/widgets/Display.java#L2704 for some of the widgets:

  • SWTCanvasView: https://github.com/eclipse-platform/eclipse.platform.swt/blob/030807afad9c7cd7a3faad765ef4061c8f7a3e08/bundles/org.eclipse.swt/Eclipse%20SWT/cocoa/org/eclipse/swt/widgets/Display.java#L2781-L2799
  • SWTImageView: https://github.com/eclipse-platform/eclipse.platform.swt/blob/030807afad9c7cd7a3faad765ef4061c8f7a3e08/bundles/org.eclipse.swt/Eclipse%20SWT/cocoa/org/eclipse/swt/widgets/Display.java#L2854-L2857
  • SWTController: https://github.com/eclipse-platform/eclipse.platform.swt/blob/030807afad9c7cd7a3faad765ef4061c8f7a3e08/bundles/org.eclipse.swt/Eclipse%20SWT/cocoa/org/eclipse/swt/widgets/Display.java#L2972-L2982
  • SWTView: https://github.com/eclipse-platform/eclipse.platform.swt/blob/030807afad9c7cd7a3faad765ef4061c8f7a3e08/bundles/org.eclipse.swt/Eclipse%20SWT/cocoa/org/eclipse/swt/widgets/Display.java#L3183-L3187

The corresponding native code is https://github.com/eclipse-platform/eclipse.platform.swt/blob/030807afad9c7cd7a3faad765ef4061c8f7a3e08/bundles/org.eclipse.swt/Eclipse%20SWT%20PI/cocoa/library/os_custom.c#L89-L99

sratz avatar Nov 06 '23 13:11 sratz

That clearly affects things. I commented out this line and the image displays correctly in the Snippets:

https://github.com/eclipse-platform/eclipse.platform.swt/blob/030807afad9c7cd7a3faad765ef4061c8f7a3e08/bundles/org.eclipse.swt/Eclipse%20SWT/cocoa/org/eclipse/swt/widgets/Display.java#L2799

Phillipus avatar Nov 06 '23 14:11 Phillipus

In general looks like flipped property isn't correctly overriden in SWTCanvasView and others (overriding getter is not enough in sonoma probably due SWIFT bindings), class_addProperty should be used

zulus avatar Nov 09 '23 20:11 zulus

We just tested the latest version of the Apple macOS developer preview, version 15.0 dev beta 5 (24A5309E) , and the problem appears to be fixed there.

If that turns out to make it to the stable version, we can add an upper bound to workaround in https://github.com/eclipse-platform/eclipse.platform.ui/blob/a8e0a729330a68ad70e027c664bd1ef43725df0c/bundles/org.eclipse.ui.workbench/Eclipse%20UI/org/eclipse/ui/internal/Workbench.java#L864

sratz avatar Aug 07 '24 08:08 sratz

We just tested the latest version of the Apple macOS developer preview, version 15.0 dev beta 5 (24A5309E) , and the problem appears to be fixed there.

Interesting. Did you test with the snippet? Does this mean that launching Eclipse 4.32 on macOS 15 the splash screen is upside down again (because it's been flipped with the workaround)?

Phillipus avatar Aug 07 '24 08:08 Phillipus