Skip to content
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

[Windows 11] MenuItem with style SWT.CHECK and image doesn't show state #501

Open
Phillipus opened this issue Dec 14, 2022 · 75 comments
Open
Labels
bug Something isn't working help wanted Extra attention is needed Windows Happens on Windows OS

Comments

@Phillipus
Copy link
Contributor

Windows 11
Eclipse 4.26

Run this Snippet on Windows 10 and Window 11 and compare the drop down menu after selecting it:

import org.eclipse.swt.SWT;
import org.eclipse.swt.graphics.Image;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Menu;
import org.eclipse.swt.widgets.MenuItem;
import org.eclipse.swt.widgets.Shell;

public class MenuImageTest {
    
	public static void main(String[] args) {
		final Display display = new Display();
		Shell shell = new Shell(display);
		shell.setText("Menu Image Test");
		
		Menu appMenuBar = display.getMenuBar();
		if(appMenuBar == null) {
			appMenuBar = new Menu(shell, SWT.BAR);
			shell.setMenuBar(appMenuBar);
		}
		
		MenuItem file = new MenuItem(appMenuBar, SWT.CASCADE);
		file.setText("File");
		
		Menu dropdown = new Menu(appMenuBar);
		file.setMenu(dropdown);
		
		MenuItem testMenu = new MenuItem(dropdown, SWT.CHECK);
		testMenu.setImage(new Image(display, MenuImageTest.class.getResourceAsStream("image.png")));
		testMenu.setText("Test Menu");
		
		shell.open();
		
		while (!shell.isDisposed()) {
			if (!display.readAndDispatch())
				display.sleep();
		}
		display.dispose();
	}
}

Use an image for image.png such as:

image

On Windows 10 selected state looks like this:

menu

In Windows 11 selected state looks like this:

menu 2

@SyntevoAlex
Copy link
Member

image

@SyntevoAlex
Copy link
Member

The screenshot above shows a difference between

  • SWT.CHECK menu with .setImage()
  • regular SWT.CHECK menu

It seems that on Win11, theme decided to no longer paint blue square around the icon, hence the difference. I would say that the other problem here is that the image is always painted, whether checked or not.

@Phillipus
Copy link
Contributor Author

I would say that the other problem here is that the image is always painted, whether checked or not.

Isn't that the expected behaviour?

@SyntevoAlex
Copy link
Member

Looking at the default check mark (in regular SWT.CHECK menu without .setImage()) I would say that check image shall only appear when item is checked.

@SyntevoAlex
Copy link
Member

SyntevoAlex commented Dec 24, 2022

Studied the code more. It turns out that check and image are independent things. SWT even tries to manipulate MNS_CHECKORBMP style in attempt to save space when menu only has images and no checks.

However, since XP theming, Windows seems to ignore that; it always paints check and image at the same spot.

Here's a screenshot of a native snippet on Win7, using disable visual themes compatibility flag. On the left, flag is enabled (that is, XP theming disabled). On the right, flag is disabled (that is, XP theming enabled). The "up arrow" image is just a random image I grabbed.
image

It can be seen that same code, on same Windows, behaves differently based on XP theming.

@SyntevoAlex
Copy link
Member

SyntevoAlex commented Dec 24, 2022

For comparison, same native snippet on Win11. It can be seen that checked image item looks the same as non-checked image item. Just like in SWT.

image

@SyntevoAlex
Copy link
Member

Conclusions: Windows theming issue, not an SWT bug.

@Phillipus
Copy link
Contributor Author

Conclusions: Windows theming issue, not an SWT bug.

If so, should we close this?

@SyntevoAlex
Copy link
Member

Probably.

@Phillipus
Copy link
Contributor Author

Closing...

@BeckerWdf
Copy link
Contributor

Conclusions: Windows theming issue, not an SWT bug.

What does that mean? You mean it's a bug of Windows?

@SyntevoAlex
Copy link
Member

It's hard to say if it's a "bug" or "design decision". SWT relies on how Windows styled things, but Windows decided to style it differently in Win11. Will that decision be reverted in Win11? Who knows

@BeckerWdf
Copy link
Contributor

It's hard to say if it's a "bug" or "design decision". SWT relies on how Windows styled things, but Windows decided to style it differently in Win11. Will that decision be reverted in Win11? Who knows

Could we "work around" this in SWT? Or can we open a bug report at MS?

@tmssngr
Copy link
Contributor

tmssngr commented Jun 22, 2023

It should be easy to work around (not perfectly, but good enough) using this (partly) pseudo-code

final int margin = getDpiScalingFactor(display);
final Rectangle size = image.getBounds();
final Image selectedImage = new Image(image.getDevice(), size.width + 2 * margin, size.height + 2 * margin);
final GC gc = new GC(selectedImage);
try {
	final Color background = createBlendedColor(image.getDevice().getSystemColor(SWT.COLOR_LIST_BACKGROUND),
	                                            image.getDevice().getSystemColor(SWT.COLOR_LIST_SELECTION), 0.25);
	gc.setBackground(background);
	gc.fillRectangle(0, 0, size.width + 2 * margin, size.height + 2 * margin);
	gc.drawImage(image, margin, margin);
}
finally {
	gc.dispose();
}

@Gudi-24
Copy link

Gudi-24 commented Jun 22, 2023

It is very easy to say that this is a bug! Even if you like to call it "design decision" to hide important information, it is still a bug. For example in the junit view it is not possible to see any more which layout (horizontally, vertically or automatically) is currently applied.
Please reopen this issue and implement a solution!

@Phillipus
Copy link
Contributor Author

Please reopen this issue and implement a solution!

Are you able to provide a solution and pull request?

@tmssngr
Copy link
Contributor

tmssngr commented Jun 22, 2023

Please reopen this issue and implement a solution!

Maybe you misunderstood Alex. He told that the problem occurs with pure native code, too, so this is no bug in SWT, but rather in Windows (if Microsoft not intentionally changed this). You need to send your request for a solution to Microsoft!

Alternatively, SWT is open source - please feel free to send a pull request with the solution.

@Gudi-24
Copy link

Gudi-24 commented Jun 22, 2023

Please reopen this issue and implement a solution!

Maybe you misunderstood Alex. He told that the problem occurs with pure native code, too, so this is no bug in SWT, but rather in Windows (if Microsoft not intentionally changed this). You need to send your request for a solution to Microsoft!

One thing is for sure: There is a bug in Eclipse! And this is independent from the fact if I send a pull request or not!

@Phillipus
Copy link
Contributor Author

One thing is for sure: There is a bug in Eclipse! And this is independent from the fact if I send a pull request or not!

What part of "this is no bug in SWT, but rather in Windows" do you not understand?

@Gudi-24
Copy link

Gudi-24 commented Jun 22, 2023

One thing is for sure: There is a bug in Eclipse! And this is independent from the fact if I send a pull request or not!

What part of "this is no bug in SWT, but rather in Windows" do you not understand?

Well, I understand that, but it doesn't help. Because most probably it won't be fixed by Windows and then the bug will stay in many eclipse views. And this is a major problem. Or do you have a different opinion?

@Phillipus
Copy link
Contributor Author

Phillipus commented Jun 22, 2023

Or do you have a different opinion?

If this is how it is because of a change in Windows 11, as @SyntevoAlex explains above, then you're out of luck. However, if you want to implement a workaround then I suggest to follow the suggested code hint from @tmssngr in the comment above.

@BeckerWdf
Copy link
Contributor

It should be easy to work around (not perfectly, but good enough) using this (partly) pseudo-code

final int margin = getDpiScalingFactor(display);
final Rectangle size = image.getBounds();
final Image selectedImage = new Image(image.getDevice(), size.width + 2 * margin, size.height + 2 * margin);
final GC gc = new GC(selectedImage);
try {
	final Color background = createBlendedColor(image.getDevice().getSystemColor(SWT.COLOR_LIST_BACKGROUND),
	                                            image.getDevice().getSystemColor(SWT.COLOR_LIST_SELECTION), 0.25);
	gc.setBackground(background);
	gc.fillRectangle(0, 0, size.width + 2 * margin, size.height + 2 * margin);
	gc.drawImage(image, margin, margin);
}
finally {
	gc.dispose();
}

Can we put this into SWT? I understand that this is a bug or design decision of MS but I also see that Eclipse (and other SWT-based apps really have an issue with it. So I think it's work to fix this in SWT.
What do you think?

@Phillipus
Copy link
Contributor Author

As there is an active interest in this issue, let's re-open it so it's visible.

@Phillipus Phillipus reopened this Jun 22, 2023
@merks
Copy link
Contributor

merks commented Jun 22, 2023

I don’t think anybody will care whose fault the problem is. If I can’t tell whether a menu item in my Eclipse application is checked or not then my perception will be that it’s a bug and a really bad one at that. We could tell 100,000 users it’s someone else’s fault but do we want to tell them we can’t do anything about it? Or worse that we don’t want to do anything about it. Of course it’s free so no one is obligated to do anything.

@Phillipus
Copy link
Contributor Author

Phillipus commented Jul 4, 2023

It turns out that some of my images are not centred in themselves, even though they're the same size.

Try this snippet:

import org.eclipse.swt.SWT;
import org.eclipse.swt.graphics.Image;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Menu;
import org.eclipse.swt.widgets.MenuItem;
import org.eclipse.swt.widgets.Shell;

public class MenuImageTest {
    
	public static void main(String[] args) {
		final Display display = new Display();
		Shell shell = new Shell(display);
		shell.setText("Menu Image Test");
		shell.setBounds(200, 200, 200, 200);
		
		Menu appMenuBar = display.getMenuBar();
		if(appMenuBar == null) {
			appMenuBar = new Menu(shell, SWT.BAR);
			shell.setMenuBar(appMenuBar);
		}
		
		MenuItem file = new MenuItem(appMenuBar, SWT.CASCADE);
		file.setText("File");
		
		Menu dropdown = new Menu(appMenuBar);
		file.setMenu(dropdown);
		
		Image image = new Image(display, MenuImageTest.class.getResourceAsStream("image.png"));
		
		for(int i = 0; i < 6; i++) {
		    MenuItem m = new MenuItem(dropdown, SWT.CHECK);
	        m.setImage(image);
	        m.setText("Test Menu");
	        m.setSelection(true);
        }
		
		shell.open();
		
		while (!shell.isDisposed()) {
			if (!display.readAndDispatch())
				display.sleep();
		}
		display.dispose();
	}
}

With this image.png

image

Now gives this result, unchecked and checked:

1
2

@laeubi
Copy link
Contributor

laeubi commented Jul 4, 2023

@Phillipus any chance to post a screenshot good / bad to see the difference?

@SyntevoAlex
Copy link
Member

So the approach works fine and has now apparent problems?

@Phillipus
Copy link
Contributor Author

So the approach works fine and has now apparent problems?

Problems I see:

  1. Space is taken even if not checked
  2. Needs more margin between image and text

@SyntevoAlex
Copy link
Member

SyntevoAlex commented Jul 4, 2023

Space is taken even if not checked

This doesn't sound like a problem to me.
Afterall checkbox needs to live somewhere.
Saving some space in a menu where ALL items are not checked sounds more like a problem to me: menus would look different.

Needs more margin between image and text

Just add a few pixels in MenuItem.wmMeasureChild

if (image != null) {
Rectangle rect = image.getBoundsInPixels ();
width = rect.width;
height = rect.height;

@iloveeclipse
Copy link
Member

Problems I see:

  1. Space is taken even if not checked
  2. Needs more margin between image and text

Compared with current state (no indication of checked state at all) we probably should accept the problems.
WDYT? PR for M1?

@Phillipus
Copy link
Contributor Author

Saving some space in a menu where ALL items are not checked sounds more like a problem to me

That's how it works on Mac.

@Phillipus
Copy link
Contributor Author

Phillipus commented Jul 4, 2023

Compared with current state (no indication of checked state at all) we probably should accept the problems.
WDYT? PR for M1?

As it's a Windows 11 issue, Windows 10 users might not like the change.

@SyntevoAlex
Copy link
Member

The change in code can be guarded with OS.WIN32_BUILD >= OS.WIN32_BUILD_WIN11_21H2

@SyntevoAlex
Copy link
Member

Also, this code should only be enabled if a menu has items with both SWT.CHECK and an image.

@iloveeclipse
Copy link
Member

@SyntevoAlex : would you please push a patch we can check?

@SyntevoAlex
Copy link
Member

To my knowledge, our product doesn't use this scenario (menu items that have both image and SWT.CHECK). For this reason, I won't be able to contribute a workaround.

I think my boss will complain if I do :)

@tmssngr
Copy link
Contributor

tmssngr commented Jul 5, 2023

Now gives this result, unchecked and checked:

1 2

If this look is considered bad, how it should look like in good state? Our products use no images in menus and these look similar (Windows 10, 4k monitor):
2023-07-05 07_10_25- 2023-07-05 07_10_11-

@Phillipus
Copy link
Contributor Author

If this look is considered bad, how it should look like in good state?

It looked bad when my images were not aligned. Using the same image it's better. It now needs a couple more pixels margin between the image and the text.

@Phillipus
Copy link
Contributor Author

Maybe it would be better to figure out how to draw the (blue) highlight around the checked image, as it's done now in Windows 10. If it had worked like that from the get go in 11, this would not be an issue.

@Phillipus
Copy link
Contributor Author

Another problem I just noticed is that menu item heights are smaller if using the tick and image. I think W11 theme uses bigger menu item heights.

1 2

@Phillipus
Copy link
Contributor Author

Phillipus commented Jul 5, 2023

And with accelerator text (mnemonic) things are a bit squashed:

2 1

And the menu background is different.

Basically, if theming is not used it looks bad and doesn't look like a native W11 menu.

@ShahzaibIbrahim
Copy link
Contributor

Have we found any acceptable resolution for this? I would like to hear it as this relates to my issue: https://github.com/vi-eclipse/Eclipse-JDT/issues/15

@merks
Copy link
Contributor

merks commented Nov 27, 2023

Sorry, no. I have not had time to work on this, despite good intentions to do so. I'm still on Windows 10 so I still avoid the issue. 😱

@tmssngr
Copy link
Contributor

tmssngr commented Nov 27, 2023

@ShahzaibIbrahim: maybe a workaround would help you for the moment?

It should be easy to work around (not perfectly, but good enough) using this (partly) pseudo-code

final int margin = getDpiScalingFactor(display);
final Rectangle size = image.getBounds();
final Image selectedImage = new Image(image.getDevice(), size.width + 2 * margin, size.height + 2 * margin);
final GC gc = new GC(selectedImage);
try {
	final Color background = createBlendedColor(image.getDevice().getSystemColor(SWT.COLOR_LIST_BACKGROUND),
	                                            image.getDevice().getSystemColor(SWT.COLOR_LIST_SELECTION), 0.25);
	gc.setBackground(background);
	gc.fillRectangle(0, 0, size.width + 2 * margin, size.height + 2 * margin);
	gc.drawImage(image, margin, margin);
}
finally {
	gc.dispose();
}

@wuendi
Copy link

wuendi commented Mar 14, 2024

Is there any plan to fix the issue in June 2024? When more customers are starting to use Windows 11, this issue causes a lot of confusion as no indicator shows menu is selected or not

@iloveeclipse
Copy link
Member

Is there any plan to fix the issue in June 2024? When more customers are starting to use Windows 11, this issue causes a lot of confusion as no indicator shows menu is selected or not

This is an open source project and every change can be moved forward by contributing a fix.
Feel free to contribute one, may be based on workaround proposed here.

@iloveeclipse iloveeclipse added help wanted Extra attention is needed bug Something isn't working Windows Happens on Windows OS labels Apr 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed Windows Happens on Windows OS
Projects
None yet
Development

No branches or pull requests