eclipse-platform / eclipse.platform.swt

Eclipse SWT
https://www.eclipse.org/swt/
Eclipse Public License 2.0
118 stars 137 forks source link

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

Open Phillipus opened 1 year ago

Phillipus commented 1 year ago

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
  2. Additional OS info (e.g. OS version, Linux Desktop, etc) macOS Sonoma 14 Mac Mini M1

  3. JRE/JDK version Temurin 17

Phillipus commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

Phillipus commented 1 year ago

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

TIBCOeddie commented 1 year ago

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

Phillipus commented 1 year ago

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.

DonaldWills commented 1 year ago

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".

Phillipus commented 1 year ago

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 commented 1 year ago

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();
    });
}
swtdev commented 1 year ago

@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 ->

Phillipus commented 1 year ago

@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."

swtdev commented 1 year ago

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

Phillipus commented 1 year ago

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?

SyntevoAlex commented 1 year ago

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 commented 1 year ago

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

akurtakov commented 1 year ago

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

Phillipus commented 1 year ago

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

Phillipus commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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)

kysmith-csg commented 1 year ago

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

Phillipus commented 1 year ago

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

zulus commented 1 year ago

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

Phillipus commented 1 year ago

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?

sratz commented 12 months ago

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 commented 12 months ago

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:

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

Phillipus commented 12 months ago

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

zulus commented 11 months ago

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

sratz commented 2 months ago

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

Phillipus commented 2 months ago

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)?

sratz commented 2 months ago

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.

Did you test with the snippet?

Not yet, we will set up some dev env there and do that next.

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)?

Yes.

Phillipus commented 2 months ago

OK, perhaps aim for 4.33 for the version check change. Is there a release note from Apple about this change?

Phillipus commented 2 months ago

I opened PR https://github.com/eclipse-platform/eclipse.platform.ui/pull/2170.

soethoff commented 2 months ago

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.

Did you test with the snippet?

Not yet, we will set up some dev env there and do that next.

We set up a dev environment and did a quick test using the self-contained snippet (https://github.com/eclipse-platform/eclipse.platform.swt/issues/772#issuecomment-1721254721).

selfContainedSnippet_devBeta5

Looks like Apple made some changes. There are release notes for the Developer Beta (https://developer.apple.com/documentation/macos-release-notes/macos-15-release-notes), but I'm not sure if one of the mentioned changes is related.

Phillipus commented 2 months ago

@soethoff Thanks for the test. It seems likely we can revert the current workaround for the splash image. See https://github.com/eclipse-platform/eclipse.platform.ui/pull/2170