[win32] Refactor toolbar refresh logic after DPI change

This commit refactors the callback use for ToolBar and ToolItem after a DPI change in the win32 implementation. It makes use of the scalable ImageList to reuse the existing items in the toolbar and only resets the ImageList with a correctly scaled one. Due to the limitations of the win32 implementation all items must be removed and re-added via the win32-API to make shrinking of toolbars possible. Additionally, the commit removes the now unused callback in ToolItem

Contributes to #62 and #131.

Author: akoch-yatta

bundles/org.eclipse.swt/Eclipse SWT PI/win32/org/eclipse/swt/internal/win32/OS.java

@@ -1362,6 +1362,7 @@ public class OS extends C {
 	public static final int TBS_DOWNISLEFT = 0x0400;
 	public static final int TBS_HORZ = 0x0;
 	public static final int TBS_VERT = 0x2;
+	public static final int TB_ADDBUTTONS = 0x444;
 	public static final int TB_ADDSTRING = 0x44d;
 	public static final int TB_AUTOSIZE = 0x421;
 	public static final int TB_BUTTONCOUNT = 0x418;

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/internal/ImageList.java

@@ -25,8 +25,8 @@ public class ImageList {
 	int style, refCount;
 	Image [] images;
 	private final int zoom;
-	private int width, height;
-	private int flags;
+	private final int width, height;
+	private final int flags;
 
 	private HashMap<Integer, Long> zoomToHandle = new HashMap<>();
 
@@ -36,8 +36,9 @@ public ImageList (int style, int zoom) {
 
 public ImageList (int style, int width, int height, int zoom) {
 	this.style = style;
-	this.flags = OS.ILC_MASK | OS.ILC_COLOR32;
-	if ((style & SWT.RIGHT_TO_LEFT) != 0) this.flags |= OS.ILC_MIRROR;
+	int listFlags = OS.ILC_MASK | OS.ILC_COLOR32;
+	if ((style & SWT.RIGHT_TO_LEFT) != 0) listFlags |= OS.ILC_MIRROR;
+	this.flags = listFlags;
 	this.height = height;
 	this.width = width;
 	handle = OS.ImageList_Create (width, height, this.flags, 16, 16);

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolBar.java

@@ -1046,10 +1046,11 @@ void setDropDownItems (boolean set) {
 }
 
 void setDisabledImageList (ImageList imageList) {
-	long hImageList = OS.SendMessage (handle, OS.TB_GETDISABLEDIMAGELIST, 0, 0);
+	long hImageList = 0;
 	if ((disabledImageList = imageList) != null) {
+		hImageList = OS.SendMessage(handle, OS.TB_GETDISABLEDIMAGELIST, 0, 0);
 		long newImageList = disabledImageList.getHandle(getZoom());
-		if(hImageList == newImageList) return;
+		if (hImageList == newImageList) return;
 		hImageList = newImageList;
 	}
 	setDropDownItems (false);
@@ -1084,10 +1085,11 @@ public void setFont (Font font) {
 }
 
 void setHotImageList (ImageList imageList) {
-	long hImageList = OS.SendMessage (handle, OS.TB_GETHOTIMAGELIST, 0, 0);
+	long hImageList = 0;
 	if ((hotImageList = imageList) != null) {
+		hImageList = OS.SendMessage(handle, OS.TB_GETHOTIMAGELIST, 0, 0);
 		long newImageList = hotImageList.getHandle(getZoom());
-		if(hImageList == newImageList) return;
+		if (hImageList == newImageList) return;
 		hImageList = newImageList;
 	}
 	setDropDownItems (false);
@@ -1096,8 +1098,9 @@ void setHotImageList (ImageList imageList) {
 }
 
 void setImageList (ImageList imageList) {
-	long hImageList = OS.SendMessage (handle, OS.TB_GETIMAGELIST, 0, 0);
+	long hImageList = 0;
 	if ((this.imageList = imageList) != null) {
+		hImageList = OS.SendMessage(handle, OS.TB_GETIMAGELIST, 0, 0);
 		long newImageList = imageList.getHandle(getZoom());
 		if (hImageList == newImageList) return;
 		hImageList = newImageList;
@@ -1745,75 +1748,59 @@ private static void handleDPIChange(Widget widget, int newZoom, float scalingFac
 	if (!(widget instanceof ToolBar toolBar)) {
 		return;
 	}
-	ToolItem[] toolItems = toolBar._getItems ();
-	// Only Items with SWT.Sepreator Style have an own width assigned to them
-	var seperatorWidth  =  new int[toolItems.length];
-	var enabledState = new boolean[toolItems.length];
-	var selectedState = new boolean[toolItems.length];
-	for (int i = 0; i < toolItems.length; i++) {
+	ToolItem[] toolItems = toolBar._getItems();
+	var seperatorWidth = new int[toolItems.length];
+	int itemCount = toolItems.length;
+
+	// Remove and re-add all button the let Windows resize the tool bar
+	ToolItem[] items = new ToolItem[itemCount];
+	TBBUTTON[] buttondata = new TBBUTTON[itemCount];
+	for (int i = itemCount - 1; i >= 0; i--) {
 		ToolItem item = toolItems[i];
-		if((item.style & SWT.SEPARATOR) != 0) {
-			// Take note of widths, so we can re-apply them later
+		if ((item.style & SWT.SEPARATOR) != 0 && item.getControl() != null) {
+			// Take note of widths of separators with control, so they can be resized
+			// at the end
 			seperatorWidth[i] = item.getWidth();
 		}
-		// Remember states of ToolItem to apply them later
-		enabledState[i] = item.getEnabled();
-		selectedState[i] = item.getSelection();
-
-	}
-	for (ToolItem item : toolItems) {
-//		toolBar.destroyItem(item);
-		// Resize after, as zoom update changes references to imageLists
 		DPIZoomChangeRegistry.applyChange(item, newZoom, scalingFactor);
-	}
-
-	for (int i = 0; i < toolItems.length; i++) {
-		ToolItem toolItem = toolItems[i];
-
-//		toolBar.createItem(toolItem, i);
-		String currentText =  toolItem.getText();
-		toolItem.setText(" ");
-		toolItem.setText(currentText);
-
-		// Refresh images (upscaling already performed by toolItem)
-		Image image = toolItem.getImage();
-		toolItem.setImage(null);
-		toolItem.setImage(image);
 
-		Image hotImage = toolItem.getHotImage();
-		toolItem.setHotImage(null);
-		toolItem.setHotImage(hotImage);
-
-		Image disabledImage = toolItem.getDisabledImage();
-		toolItem.setDisabledImage(null);
-		toolItem.setDisabledImage(disabledImage);
-
-		var content = toolItem.getControl();
-		toolItem.setControl(null);
-		toolItem.setControl(content);
-
-		// In SWT, Width can only be set for Separators
-		if ((toolItem.style & SWT.SEPARATOR) != 0) {
-			var width = (int)((seperatorWidth[i]) * scalingFactor);
-			toolItem.setWidth(width);
-			toolItem.resizeControl();
+		TBBUTTON lpButton = new TBBUTTON ();
+		OS.SendMessage (toolBar.handle, OS.TB_GETBUTTON, i, lpButton);
+		buttondata[lpButton.idCommand] = lpButton;
+		items[lpButton.idCommand] = item;
+		OS.SendMessage(toolBar.handle, OS.TB_DELETEBUTTON, i, 0);
+	}
+
+	// Refresh the image lists so the image list for the correct zoom is used
+	toolBar.setImageList(toolBar.getImageList());
+	toolBar.setDisabledImageList(toolBar.getDisabledImageList());
+	toolBar.setHotImageList(toolBar.getHotImageList());
+
+	OS.SendMessage(toolBar.handle, OS.TB_BUTTONSTRUCTSIZE, TBBUTTON.sizeof, 0);
+	for (int i = 0; i < buttondata.length; i++) {
+		TBBUTTON button = buttondata[i];
+		if (button != null) {
+			OS.SendMessage(toolBar.handle, OS.TB_ADDBUTTONS, 1, button);
+			ToolItem item = items[i];
+
+			// The text is not retained correctly, so we need to reset it
+			String text = item.getText();
+			if (text != null) {
+				item.setText("");
+				item.setText(text);
+			}
 		}
-
-		toolItem.setEnabled(enabledState[i]);
-		toolItem.setSelection(selectedState[i]);
 	}
+	OS.SendMessage(toolBar.handle, OS.TB_AUTOSIZE, 0, 0);
 
-	// Force a refresh of the toolbar by resetting the Font
-	toolBar.setDropDownItems(false);
-	long hFont = OS.SendMessage(toolBar.handle, OS.WM_GETFONT, 0, 0);
-	OS.SendMessage(toolBar.handle, OS.WM_SETFONT, hFont, 0);
-	if((toolBar.style & SWT.VERTICAL) != 0) {
-		// Reset row count to prevent wrapping of buttons
-		toolBar.setRowCount((int)OS.SendMessage (toolBar.handle, OS.TB_BUTTONCOUNT, 0, 0));
+	for (int i = 0; i < itemCount; i++) {
+		ToolItem item = toolItems[i];
+		// If the separator is used with a control, we must reset the size to the cached value,
+		// cause windows will treat the separator as normal separator and shrinks it accordingly
+		if ((item.style & SWT.SEPARATOR) != 0 && item.getControl() != null) {
+			item.setWidth(seperatorWidth[i]);
+		}
 	}
-	toolBar.setDropDownItems(true);
 	toolBar.layout(true);
-	toolBar.sendResize();
-	toolBar.redraw();
 }
 }

bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolItem.java

@@ -50,10 +50,6 @@ public class ToolItem extends Item {
 	short cx;
 	int foreground = -1, background = -1;
 
-	static {
-		DPIZoomChangeRegistry.registerHandler(ToolItem::handleDPIChange, ToolItem.class);
-	}
-
 /**
  * Constructs a new instance of this class given its parent
  * (which must be a <code>ToolBar</code>) and a style value
@@ -1218,28 +1214,4 @@ LRESULT wmCommandChild (long wParam, long lParam) {
 	sendSelectionEvent (SWT.Selection);
 	return null;
 }
-
-private static void handleDPIChange(Widget widget, int newZoom, float scalingFactor) {
-	if (!(widget instanceof ToolItem item)) {
-		return;
-	}
-	Image image = item.getImage();
-	if (image != null) {
-		ToolBar parent = item.getParent();
-		Display display = item.getDisplay();
-		int listStyle = parent.style & SWT.RIGHT_TO_LEFT;
-
-		Rectangle bounds = DPIUtil.scaleBounds(image.getBounds(), newZoom, 100);
-		if (parent.getImageList() == null) {
-			parent.setImageList (display.getImageListToolBar (listStyle, bounds.width, bounds.height, item.getZoom()));
-		}
-		if (parent.getDisabledImageList() == null) {
-			parent.setDisabledImageList (display.getImageListToolBarDisabled (listStyle, bounds.width, bounds.height, item.getZoom()));
-		}
-		if (parent.getHotImageList() == null) {
-			parent.setHotImageList (display.getImageListToolBarHot (listStyle, bounds.width, bounds.height, item.getZoom()));
-		}
-	}
-	item.setWidthInPixels(0);
-}
 }