From b9457167f75513d621585159e19edd271051296b Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Wed, 12 Aug 2026 17:22:40 +0200 Subject: [PATCH] [Win32] Encapsulate ToolItem's access to tool bar image lists ToolItem manipulated ToolBar's three normal/hot/disabled ImageLists directly: reading them via ToolBar's getters, creating them inline when absent, and calling add()/put() on each of them itself. This spread the bookkeeping for a single item's images across both classes and made ToolItem responsible for details that are really ToolBar's to own, such as lazily creating the image lists sized to the first image added. With this change, ToolBar exposes addImage/putImage/clearImage instead, and ToolItem goes through these instead of touching ImageList directly. ToolBar's internal representation (three separate ImageList fields, and their existing setImageList/setHotImageList/setDisabledImageList synchronization methods) is otherwise unchanged. This is a behavior-preserving refactoring: the image lists are still created, filled and synchronized with the same values as before. The disabledImageList != null guard in ToolItem.updateImages is dropped rather than moved: in that branch the item already has an image index, so all three image lists necessarily exist, as they are only ever created together in addImage and cleared together in destroyItem and releaseWidget. Related to https://github.com/eclipse-platform/eclipse.platform.swt/issues/3466 --- .../org/eclipse/swt/widgets/ToolBar.java | 46 +++++++++++++++- .../org/eclipse/swt/widgets/ToolItem.java | 55 ++++--------------- 2 files changed, 57 insertions(+), 44 deletions(-) diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolBar.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolBar.java index 68e839fcefe..8335dc82f93 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolBar.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolBar.java @@ -55,7 +55,7 @@ public class ToolBar extends Composite { ToolItem [] items; ToolItem [] tabItemList; boolean ignoreResize, ignoreMouse; - ImageList imageList, disabledImageList, hotImageList; + private ImageList imageList, disabledImageList, hotImageList; static final long ToolBarProc; static final TCHAR ToolBarClass = new TCHAR (OS.TOOLBARCLASSNAME, true); static { @@ -143,6 +143,34 @@ public ToolBar (Composite parent, int style) { } } +/* + * The given image bounds are the bounds of the tool item's image and determine which shared image + * lists are used. They are intentionally not derived from the images actually added: for a disabled + * item with CHECK or RADIO style, those are the disabled images, which may have different bounds. + * Note that the icon size of an image list is defined by the first image added to it. + */ +int addImage(Rectangle imageBounds, Image image, Image hotImage, Image disabledImage) { + int listStyle = style & SWT.RIGHT_TO_LEFT; + if (imageList == null) { + imageList = display.getImageListToolBar(listStyle, imageBounds.width, imageBounds.height, getAutoscalingZoom()); + } + if (hotImageList == null) { + hotImageList = display.getImageListToolBarHot(listStyle, imageBounds.width, imageBounds.height, + getAutoscalingZoom()); + } + if (disabledImageList == null) { + disabledImageList = display.getImageListToolBarDisabled(listStyle, imageBounds.width, imageBounds.height, + getAutoscalingZoom()); + } + int index = imageList.add(image); + hotImageList.add(hotImage); + disabledImageList.add(disabledImage); + setImageList(imageList); + setHotImageList(hotImageList); + setDisabledImageList(disabledImageList); + return index; +} + @Override long callWindowProc (long hwnd, int msg, long wParam, long lParam) { if (handle == 0) return 0; @@ -199,6 +227,10 @@ public void layout (boolean changed) { super.layout(changed); } +void clearImage(int index) { + putImage(index, null, null, null); +} + void clearSizeCache(boolean changed) { // If changed, discard the cached layout information if (changed) { @@ -869,6 +901,18 @@ boolean mnemonicMatch (char ch) { return findMnemonic (items [id [0]].text) != '\0'; } +void putImage(int index, Image image, Image hotImage, Image disabledImage) { + if (imageList != null) { + imageList.put(index, image); + } + if (hotImageList != null) { + hotImageList.put(index, hotImage); + } + if (disabledImageList != null) { + disabledImageList.put(index, disabledImage); + } +} + @Override void releaseChildren (boolean destroy) { if (items != null) { diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolItem.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolItem.java index 94b020a5a77..b75fbad4c86 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolItem.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolItem.java @@ -535,12 +535,7 @@ void releaseImages () { * an image and one is never assigned, this is not a problem. */ if ((info.fsStyle & OS.BTNS_SEP) == 0 && info.iImage != OS.I_IMAGENONE) { - ImageList imageList = parent.getImageList (); - ImageList hotImageList = parent.getHotImageList (); - ImageList disabledImageList = parent.getDisabledImageList(); - if (imageList != null) imageList.put (info.iImage, null); - if (hotImageList != null) hotImageList.put (info.iImage, null); - if (disabledImageList != null) disabledImageList.put (info.iImage, null); + parent.clearImage(info.iImage); } } @@ -1099,21 +1094,7 @@ void updateImages (boolean enabled) { info.dwMask = OS.TBIF_IMAGE; OS.SendMessage (hwnd, OS.TB_GETBUTTONINFO, id, info); if (info.iImage == OS.I_IMAGENONE && image == null) return; - ImageList imageList = parent.getImageList (); - ImageList hotImageList = parent.getHotImageList (); - ImageList disabledImageList = parent.getDisabledImageList(); if (info.iImage == OS.I_IMAGENONE) { - Rectangle boundsInPoints = image.getBounds(); - int listStyle = parent.style & SWT.RIGHT_TO_LEFT; - if (imageList == null) { - imageList = display.getImageListToolBar (listStyle, boundsInPoints.width, boundsInPoints.height, getAutoscalingZoom()); - } - if (disabledImageList == null) { - disabledImageList = display.getImageListToolBarDisabled (listStyle, boundsInPoints.width, boundsInPoints.height, getAutoscalingZoom()); - } - if (hotImageList == null) { - hotImageList = display.getImageListToolBarHot (listStyle, boundsInPoints.width, boundsInPoints.height, getAutoscalingZoom()); - } Image disabled = disabledImage; if (disabledImage == null) { if (disabledImage2 != null) disabledImage2.dispose (); @@ -1134,27 +1115,19 @@ void updateImages (boolean enabled) { if ((style & (SWT.CHECK | SWT.RADIO)) != 0) { if (!enabled) image2 = hot = disabled; } - info.iImage = imageList.add (image2); - disabledImageList.add (disabled); - hotImageList.add (hot != null ? hot : image2); - parent.setImageList (imageList); - parent.setDisabledImageList (disabledImageList); - parent.setHotImageList (hotImageList); + info.iImage = parent.addImage(image.getBounds(), image2, hot != null ? hot : image2, disabled); } else { Image disabled = null; - if (disabledImageList != null) { - if (image != null) { - if (disabledImage2 != null) disabledImage2.dispose (); - disabledImage2 = null; - disabled = disabledImage; - if (disabledImage == null) { - disabled = image; - if (!enabled) { - disabled = disabledImage2 = new Image (display, image, SWT.IMAGE_DISABLE); - } + if (image != null) { + if (disabledImage2 != null) disabledImage2.dispose (); + disabledImage2 = null; + disabled = disabledImage; + if (disabledImage == null) { + disabled = image; + if (!enabled) { + disabled = disabledImage2 = new Image (display, image, SWT.IMAGE_DISABLE); } } - disabledImageList.put (info.iImage, disabled); } /* * Bug in Windows. When a tool item with the style @@ -1167,12 +1140,8 @@ void updateImages (boolean enabled) { if ((style & (SWT.CHECK | SWT.RADIO)) != 0) { if (!enabled) image2 = hot = disabled; } - if (imageList != null) { - imageList.put (info.iImage, image2); - } - if (hotImageList != null) { - hotImageList.put (info.iImage, hot != null ? hot : image2); - } + + parent.putImage(info.iImage, image2, hot != null ? hot : image2, disabled); if (image == null) info.iImage = OS.I_IMAGENONE; }