From b9457167f75513d621585159e19edd271051296b Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Wed, 12 Aug 2026 17:22:40 +0200 Subject: [PATCH 1/5] [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; } From d0fd66f759234d843a19d179d2a714253669ec9f Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Thu, 13 Aug 2026 10:59:43 +0200 Subject: [PATCH 2/5] [Win32] Consolidate tool bar image list native synchronization ToolBar's setImageList/setHotImageList/setDisabledImageList each carried a near-identical copy of the logic to compare the current TB_GET*IMAGELIST handle against the ImageList's handle for the current zoom, and, if different, apply it via TB_SET*IMAGELIST while toggling setDropDownItems around it to avoid a Windows layout glitch. handleDPIChange and addImage each called all three setters in turn whenever any one image list might have changed. This change consolidates that duplicated logic into a single refreshImageLists method operating on the current field values, replacing the three getters and three setters. ToolBar's representation is still three separate ImageList fields. It prepares the ground for introducing a class that owns the three image lists as one unit. This is not a purely behavior-preserving refactoring: refreshImageLists toggles setDropDownItems at most once per refresh batch instead of once per list, and only when the tool bar's buttons are actually being added or recreated (addImage, handleDPIChange) rather than for updateOrientation. Both the toggling and the native updates are skipped entirely when the image lists already set on the tool bar are the current ones, which in particular avoids any native calls for tool bars without images. Related to https://github.com/eclipse-platform/eclipse.platform.swt/issues/3466 --- .../org/eclipse/swt/widgets/ToolBar.java | 101 +++++++----------- 1 file changed, 41 insertions(+), 60 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 8335dc82f93..04b02f53de4 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 @@ -165,9 +165,7 @@ int addImage(Rectangle imageBounds, Image image, Image hotImage, Image disabledI int index = imageList.add(image); hotImageList.add(hotImage); disabledImageList.add(disabledImage); - setImageList(imageList); - setHotImageList(hotImageList); - setDisabledImageList(disabledImageList); + refreshImageLists(true); return index; } @@ -527,18 +525,6 @@ void enableWidget (boolean enabled) { } } -ImageList getDisabledImageList () { - return disabledImageList; -} - -ImageList getHotImageList () { - return hotImageList; -} - -ImageList getImageList () { - return imageList; -} - /** * Returns the item at the given, zero-relative index in the * receiver. Throws an exception if the index is out of range. @@ -913,6 +899,44 @@ void putImage(int index, Image image, Image hotImage, Image disabledImage) { } } +private void refreshImageLists(boolean itemsChanged) { + int zoom = getAutoscalingZoom(); + long imageListHandle = getImageListHandle(imageList, zoom); + long hotImageListHandle = getImageListHandle(hotImageList, zoom); + long disabledImageListHandle = getImageListHandle(disabledImageList, zoom); + boolean imageListOutdated = isImageListOutdated(OS.TB_GETIMAGELIST, imageListHandle); + boolean hotImageListOutdated = isImageListOutdated(OS.TB_GETHOTIMAGELIST, hotImageListHandle); + boolean disabledImageListOutdated = isImageListOutdated(OS.TB_GETDISABLEDIMAGELIST, disabledImageListHandle); + if (!imageListOutdated && !hotImageListOutdated && !disabledImageListOutdated) { + return; + } + // clear the BTNS_DROPDOWN bits while the image lists are exchanged, see + // setDropDownItems() + if (itemsChanged) { + setDropDownItems(false); + } + if (imageListOutdated) { + OS.SendMessage(handle, OS.TB_SETIMAGELIST, 0, imageListHandle); + } + if (hotImageListOutdated) { + OS.SendMessage(handle, OS.TB_SETHOTIMAGELIST, 0, hotImageListHandle); + } + if (disabledImageListOutdated) { + OS.SendMessage(handle, OS.TB_SETDISABLEDIMAGELIST, 0, disabledImageListHandle); + } + if (itemsChanged) { + setDropDownItems(true); + } +} + +private static long getImageListHandle(ImageList imageList, int zoom) { + return imageList != null ? imageList.getHandle(zoom) : 0; +} + +private boolean isImageListOutdated(int getMessageCode, long expectedHandle) { + return OS.SendMessage(handle, getMessageCode, 0, 0) != expectedHandle; +} + @Override void releaseChildren (boolean destroy) { if (items != null) { @@ -1044,19 +1068,6 @@ void setDropDownItems (boolean set) { } } -void setDisabledImageList (ImageList imageList) { - long hImageList = 0; - if ((disabledImageList = imageList) != null) { - hImageList = OS.SendMessage(handle, OS.TB_GETDISABLEDIMAGELIST, 0, 0); - long newImageList = disabledImageList.getHandle(getAutoscalingZoom()); - if (hImageList == newImageList) return; - hImageList = newImageList; - } - setDropDownItems (false); - OS.SendMessage (handle, OS.TB_SETDISABLEDIMAGELIST, 0, hImageList); - setDropDownItems (true); -} - @Override public void setFont (Font font) { checkWidget (); @@ -1083,32 +1094,6 @@ public void setFont (Font font) { layoutItems (); } -void setHotImageList (ImageList imageList) { - long hImageList = 0; - if ((hotImageList = imageList) != null) { - hImageList = OS.SendMessage(handle, OS.TB_GETHOTIMAGELIST, 0, 0); - long newImageList = hotImageList.getHandle(getAutoscalingZoom()); - if (hImageList == newImageList) return; - hImageList = newImageList; - } - setDropDownItems (false); - OS.SendMessage (handle, OS.TB_SETHOTIMAGELIST, 0, hImageList); - setDropDownItems (true); -} - -void setImageList (ImageList imageList) { - long hImageList = 0; - if ((this.imageList = imageList) != null) { - hImageList = OS.SendMessage(handle, OS.TB_GETIMAGELIST, 0, 0); - long newImageList = imageList.getHandle(getAutoscalingZoom()); - if (hImageList == newImageList) return; - hImageList = newImageList; - } - setDropDownItems (false); - OS.SendMessage (handle, OS.TB_SETIMAGELIST, 0, hImageList); - setDropDownItems (true); -} - @Override public boolean setParent (Composite parent) { checkWidget (); @@ -1298,12 +1283,10 @@ void updateOrientation () { display.releaseToolImageList (imageList); display.releaseToolHotImageList (hotImageList); display.releaseToolDisabledImageList (disabledImageList); - OS.SendMessage (handle, OS.TB_SETIMAGELIST, 0, newImageList.getHandle(getAutoscalingZoom())); - OS.SendMessage (handle, OS.TB_SETHOTIMAGELIST, 0, newHotImageList.getHandle(getAutoscalingZoom())); - OS.SendMessage (handle, OS.TB_SETDISABLEDIMAGELIST, 0, newDisabledImageList.getHandle(getAutoscalingZoom())); imageList = newImageList; hotImageList = newHotImageList; disabledImageList = newDisabledImageList; + refreshImageLists(false); OS.InvalidateRect (handle, null, true); } } @@ -1788,9 +1771,7 @@ record ToolItemData(ToolItem toolItem, TBBUTTON button) { } } // Refresh the image lists so the image list for the correct zoom is used - setImageList(getImageList()); - setDisabledImageList(getDisabledImageList()); - setHotImageList(getHotImageList()); + refreshImageLists(true); boolean toolBarEnabled = getEnabled(); for (int i = 0; i < itemCount; i++) { ToolItem item = toolItems[i]; From 71bcb33b43e695f9b5d25af68b1636621def990c Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Fri, 14 Aug 2026 12:57:39 +0200 Subject: [PATCH 3/5] [Win32] Consolidate tool bar image list release and disposal destroyItem (when the last button is removed) and releaseWidget each released the three image lists and cleared their native references with near-identical, duplicated code; updateOrientation released and swapped them inline as well. Consolidates all three into shared clearAndReleaseImageLists/releaseImageLists helpers built on top of refreshImageLists. All of them refresh with itemsChanged=false: in destroyItem and releaseWidget no tool bar buttons remain that the drop-down padding workaround would have to protect, and in updateOrientation the buttons are only re-pointed at their migrated images rather than added or recreated. updateOrientation released the old image lists before pointing the native tool bar at the new ones, leaving a window where the control could reference an already-disposed image list. It now assigns the fields to the freshly created lists upfront, migrating each button's images from the old lists (kept in local variables) into them, and only refreshes the native references and releases the old lists afterwards, matching clearAndReleaseImageLists. Related to https://github.com/eclipse-platform/eclipse.platform.swt/issues/3466 --- .../org/eclipse/swt/widgets/ToolBar.java | 65 +++++++++---------- 1 file changed, 30 insertions(+), 35 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 04b02f53de4..9f647b3fbe4 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 @@ -482,19 +482,7 @@ void destroyItem (ToolItem item) { item.id = -1; int count = (int)OS.SendMessage (handle, OS.TB_BUTTONCOUNT, 0, 0); if (count == 0) { - if (imageList != null) { - OS.SendMessage (handle, OS.TB_SETIMAGELIST, 0, 0); - display.releaseToolImageList (imageList); - } - if (hotImageList != null) { - OS.SendMessage (handle, OS.TB_SETHOTIMAGELIST, 0, 0); - display.releaseToolHotImageList (hotImageList); - } - if (disabledImageList != null) { - OS.SendMessage (handle, OS.TB_SETDISABLEDIMAGELIST, 0, 0); - display.releaseToolDisabledImageList (disabledImageList); - } - imageList = hotImageList = disabledImageList = null; + clearAndReleaseImageLists(); items = new ToolItem [4]; } if ((style & SWT.VERTICAL) != 0) setRowCount (count - 1); @@ -953,19 +941,28 @@ void releaseChildren (boolean destroy) { @Override void releaseWidget () { super.releaseWidget (); + clearAndReleaseImageLists(); +} + +private void clearAndReleaseImageLists() { + ImageList releasedImageList = imageList; + ImageList releasedHotImageList = hotImageList; + ImageList releasedDisabledImageList = disabledImageList; + imageList = hotImageList = disabledImageList = null; + refreshImageLists(false); + releaseImageLists(releasedImageList, releasedHotImageList, releasedDisabledImageList); +} + +private void releaseImageLists(ImageList imageList, ImageList hotImageList, ImageList disabledImageList) { if (imageList != null) { - OS.SendMessage (handle, OS.TB_SETIMAGELIST, 0, 0); display.releaseToolImageList (imageList); } if (hotImageList != null) { - OS.SendMessage (handle, OS.TB_SETHOTIMAGELIST, 0, 0); display.releaseToolHotImageList (hotImageList); } if (disabledImageList != null) { - OS.SendMessage (handle, OS.TB_SETDISABLEDIMAGELIST, 0, 0); display.releaseToolDisabledImageList (disabledImageList); } - imageList = hotImageList = disabledImageList = null; } @Override @@ -1255,9 +1252,12 @@ void updateOrientation () { super.updateOrientation (); if (imageList != null) { Point sizeInPoints = imageList.getImageSize(); - ImageList newImageList = display.getImageListToolBar (style & SWT.RIGHT_TO_LEFT, sizeInPoints.x, sizeInPoints.y, getAutoscalingZoom()); - ImageList newHotImageList = display.getImageListToolBarHot (style & SWT.RIGHT_TO_LEFT, sizeInPoints.x, sizeInPoints.y, getAutoscalingZoom()); - ImageList newDisabledImageList = display.getImageListToolBarDisabled (style & SWT.RIGHT_TO_LEFT, sizeInPoints.x, sizeInPoints.y, getAutoscalingZoom()); + ImageList oldImageList = imageList; + ImageList oldHotImageList = hotImageList; + ImageList oldDisabledImageList = disabledImageList; + imageList = display.getImageListToolBar (style & SWT.RIGHT_TO_LEFT, sizeInPoints.x, sizeInPoints.y, getAutoscalingZoom()); + hotImageList = display.getImageListToolBarHot (style & SWT.RIGHT_TO_LEFT, sizeInPoints.x, sizeInPoints.y, getAutoscalingZoom()); + disabledImageList = display.getImageListToolBarDisabled (style & SWT.RIGHT_TO_LEFT, sizeInPoints.x, sizeInPoints.y, getAutoscalingZoom()); TBBUTTONINFO info = new TBBUTTONINFO (); info.cbSize = TBBUTTONINFO.sizeof; info.dwMask = OS.TBIF_IMAGE; @@ -1268,25 +1268,20 @@ void updateOrientation () { if (item.image == null) continue; OS.SendMessage (handle, OS.TB_GETBUTTONINFO, item.id, info); if (info.iImage != OS.I_IMAGENONE) { - Image image = imageList.get(info.iImage); - Image hot = hotImageList.get(info.iImage); - Image disabled = disabledImageList.get(info.iImage); - imageList.put(info.iImage, null); - hotImageList.put(info.iImage, null); - disabledImageList.put(info.iImage, null); - info.iImage = newImageList.add(image); - newHotImageList.add(hot); - newDisabledImageList.add(disabled); + Image image = oldImageList.get(info.iImage); + Image hot = oldHotImageList.get(info.iImage); + Image disabled = oldDisabledImageList.get(info.iImage); + oldImageList.put(info.iImage, null); + oldHotImageList.put(info.iImage, null); + oldDisabledImageList.put(info.iImage, null); + info.iImage = imageList.add(image); + hotImageList.add(hot); + disabledImageList.add(disabled); OS.SendMessage (handle, OS.TB_SETBUTTONINFO, item.id, info); } } - display.releaseToolImageList (imageList); - display.releaseToolHotImageList (hotImageList); - display.releaseToolDisabledImageList (disabledImageList); - imageList = newImageList; - hotImageList = newHotImageList; - disabledImageList = newDisabledImageList; refreshImageLists(false); + releaseImageLists(oldImageList, oldHotImageList, oldDisabledImageList); OS.InvalidateRect (handle, null, true); } } From 693bd517684c5d8ebdebd43921a768777aa10219 Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Fri, 14 Aug 2026 14:24:17 +0200 Subject: [PATCH 4/5] [Win32] Consolidate tool bar image lists in ToolBarImageLists ToolBar's three image lists were still three separate fields that were created, filled, cleared, moved and released individually in addImage, putImage, destroyItem, releaseWidget and updateOrientation, even though they only ever change together and have to stay index-aligned. This change introduces ToolBarImageLists, which owns the three ImageLists as one unit and offers the operations on them as single calls. Synchronizing the image lists with the native control remains in ToolBar, which retrieves the handles to set from ToolBarImageLists. ToolBar's addImage/putImage/clearImage API (and therefore ToolItem, which only calls that API) is unaffected; this change is entirely internal to ToolBar. This is a behavior-preserving internal refactor; it does not itself change what is rendered. It lays the groundwork for fixing the image-list index-alignment problems tracked in https://github.com/eclipse-platform/eclipse.platform.swt/issues/3466 --- .../org/eclipse/swt/widgets/ToolBar.java | 115 +++++++----------- .../swt/widgets/ToolBarImageLists.java | 95 +++++++++++++++ 2 files changed, 136 insertions(+), 74 deletions(-) create mode 100644 bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolBarImageLists.java 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 9f647b3fbe4..703dd50c7db 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,8 @@ public class ToolBar extends Composite { ToolItem [] items; ToolItem [] tabItemList; boolean ignoreResize, ignoreMouse; - private ImageList imageList, disabledImageList, hotImageList; + private ToolBarImageLists imageLists; + static final long ToolBarProc; static final TCHAR ToolBarClass = new TCHAR (OS.TOOLBARCLASSNAME, true); static { @@ -150,21 +151,10 @@ public ToolBar (Composite parent, int style) { * 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 (imageLists == null) { + imageLists = createImageLists(imageBounds.width, imageBounds.height); } - if (disabledImageList == null) { - disabledImageList = display.getImageListToolBarDisabled(listStyle, imageBounds.width, imageBounds.height, - getAutoscalingZoom()); - } - int index = imageList.add(image); - hotImageList.add(hotImage); - disabledImageList.add(disabledImage); + int index = imageLists.add(image, hotImage, disabledImage); refreshImageLists(true); return index; } @@ -225,8 +215,21 @@ public void layout (boolean changed) { super.layout(changed); } +private void clearAndReleaseImageLists() { + if (imageLists != null) { + // the image lists must be unset before refreshing, so that they are detached from the tool + // bar, and they must only be released once the tool bar does not reference them anymore + ToolBarImageLists releasedImageLists = imageLists; + imageLists = null; + refreshImageLists(false); + releasedImageLists.release(); + } +} + void clearImage(int index) { - putImage(index, null, null, null); + if (imageLists != null) { + imageLists.clear(index); + } } void clearSizeCache(boolean changed) { @@ -405,6 +408,10 @@ void createHandle () { OS.SendMessage (handle, OS.TB_SETEXTENDEDSTYLE, 0, bits); } +private ToolBarImageLists createImageLists(int width, int height) { + return ToolBarImageLists.create(display, style & SWT.RIGHT_TO_LEFT, width, height, getAutoscalingZoom()); +} + void createItem (ToolItem item, int index) { int count = (int)OS.SendMessage (handle, OS.TB_BUTTONCOUNT, 0, 0); if (!(0 <= index && index <= count)) error (SWT.ERROR_INVALID_RANGE); @@ -470,9 +477,9 @@ void destroyItem (ToolItem item) { * an image and one is never assigned, this is not a problem. */ if ((info.fsStyle & OS.BTNS_SEP) == 0 && info.iImage != OS.I_IMAGENONE) { - if (imageList != null) imageList.put (info.iImage, null); - if (hotImageList != null) hotImageList.put (info.iImage, null); - if (disabledImageList != null) disabledImageList.put (info.iImage, null); + if (imageLists != null) { + imageLists.clear(info.iImage); + } } OS.SendMessage (handle, OS.TB_DELETEBUTTON, index, 0); if (item.id == lastFocusId) lastFocusId = -1; @@ -876,22 +883,19 @@ boolean mnemonicMatch (char ch) { } 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); + if (imageLists != null) { + imageLists.put(index, image, hotImage, disabledImage); } } private void refreshImageLists(boolean itemsChanged) { - int zoom = getAutoscalingZoom(); - long imageListHandle = getImageListHandle(imageList, zoom); - long hotImageListHandle = getImageListHandle(hotImageList, zoom); - long disabledImageListHandle = getImageListHandle(disabledImageList, zoom); + long imageListHandle = 0, hotImageListHandle = 0, disabledImageListHandle = 0; + if (imageLists != null) { + int zoom = getAutoscalingZoom(); + imageListHandle = imageLists.getImageListHandle(zoom); + hotImageListHandle = imageLists.getHotImageListHandle(zoom); + disabledImageListHandle = imageLists.getDisabledImageListHandle(zoom); + } boolean imageListOutdated = isImageListOutdated(OS.TB_GETIMAGELIST, imageListHandle); boolean hotImageListOutdated = isImageListOutdated(OS.TB_GETHOTIMAGELIST, hotImageListHandle); boolean disabledImageListOutdated = isImageListOutdated(OS.TB_GETDISABLEDIMAGELIST, disabledImageListHandle); @@ -917,10 +921,6 @@ private void refreshImageLists(boolean itemsChanged) { } } -private static long getImageListHandle(ImageList imageList, int zoom) { - return imageList != null ? imageList.getHandle(zoom) : 0; -} - private boolean isImageListOutdated(int getMessageCode, long expectedHandle) { return OS.SendMessage(handle, getMessageCode, 0, 0) != expectedHandle; } @@ -944,27 +944,6 @@ void releaseWidget () { clearAndReleaseImageLists(); } -private void clearAndReleaseImageLists() { - ImageList releasedImageList = imageList; - ImageList releasedHotImageList = hotImageList; - ImageList releasedDisabledImageList = disabledImageList; - imageList = hotImageList = disabledImageList = null; - refreshImageLists(false); - releaseImageLists(releasedImageList, releasedHotImageList, releasedDisabledImageList); -} - -private void releaseImageLists(ImageList imageList, ImageList hotImageList, ImageList disabledImageList) { - if (imageList != null) { - display.releaseToolImageList (imageList); - } - if (hotImageList != null) { - display.releaseToolHotImageList (hotImageList); - } - if (disabledImageList != null) { - display.releaseToolDisabledImageList (disabledImageList); - } -} - @Override void removeControl (Control control) { super.removeControl (control); @@ -1250,14 +1229,10 @@ String toolTipText (NMTTDISPINFO hdr) { @Override void updateOrientation () { super.updateOrientation (); - if (imageList != null) { - Point sizeInPoints = imageList.getImageSize(); - ImageList oldImageList = imageList; - ImageList oldHotImageList = hotImageList; - ImageList oldDisabledImageList = disabledImageList; - imageList = display.getImageListToolBar (style & SWT.RIGHT_TO_LEFT, sizeInPoints.x, sizeInPoints.y, getAutoscalingZoom()); - hotImageList = display.getImageListToolBarHot (style & SWT.RIGHT_TO_LEFT, sizeInPoints.x, sizeInPoints.y, getAutoscalingZoom()); - disabledImageList = display.getImageListToolBarDisabled (style & SWT.RIGHT_TO_LEFT, sizeInPoints.x, sizeInPoints.y, getAutoscalingZoom()); + if (imageLists != null) { + Point size = imageLists.getImageSize(); + ToolBarImageLists oldImageLists = imageLists; + imageLists = createImageLists(size.x, size.y); TBBUTTONINFO info = new TBBUTTONINFO (); info.cbSize = TBBUTTONINFO.sizeof; info.dwMask = OS.TBIF_IMAGE; @@ -1268,20 +1243,12 @@ void updateOrientation () { if (item.image == null) continue; OS.SendMessage (handle, OS.TB_GETBUTTONINFO, item.id, info); if (info.iImage != OS.I_IMAGENONE) { - Image image = oldImageList.get(info.iImage); - Image hot = oldHotImageList.get(info.iImage); - Image disabled = oldDisabledImageList.get(info.iImage); - oldImageList.put(info.iImage, null); - oldHotImageList.put(info.iImage, null); - oldDisabledImageList.put(info.iImage, null); - info.iImage = imageList.add(image); - hotImageList.add(hot); - disabledImageList.add(disabled); + info.iImage = imageLists.moveFrom(oldImageLists, info.iImage); OS.SendMessage (handle, OS.TB_SETBUTTONINFO, item.id, info); } } refreshImageLists(false); - releaseImageLists(oldImageList, oldHotImageList, oldDisabledImageList); + oldImageLists.release(); OS.InvalidateRect (handle, null, true); } } diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolBarImageLists.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolBarImageLists.java new file mode 100644 index 00000000000..bf313e74a27 --- /dev/null +++ b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolBarImageLists.java @@ -0,0 +1,95 @@ +/******************************************************************************* + * Copyright (c) 2026 Vector Informatik GmbH and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.swt.widgets; + +import org.eclipse.swt.graphics.*; +import org.eclipse.swt.internal.*; + +/** + * Owns the normal, hot and disabled image lists of a tool bar as a single unit. + * The three lists are always created, filled, cleared and released together, so + * they are of equal image size and the index returned when adding an item's + * images addresses that item in all three of them. + *

+ * Synchronizing the image lists with the native tool bar is up to the owning + * tool bar, which retrieves the handles to set via + * {@link #getImageListHandle(int)} and its hot and disabled counterparts. + */ +class ToolBarImageLists { + private final Display display; + + private final ImageList imageList, disabledImageList, hotImageList; + + private ToolBarImageLists(Display display, ImageList imageList, ImageList hotImageList, + ImageList disabledImageList) { + this.display = display; + this.imageList = imageList; + this.hotImageList = hotImageList; + this.disabledImageList = disabledImageList; + } + + static ToolBarImageLists create(Display display, int style, int width, int height, int zoom) { + ImageList imageList = display.getImageListToolBar(style, width, height, zoom); + ImageList hotImageList = display.getImageListToolBarHot(style, width, height, zoom); + ImageList disabledImageList = display.getImageListToolBarDisabled(style, width, height, zoom); + return new ToolBarImageLists(display, imageList, hotImageList, disabledImageList); + } + + void clear(int index) { + imageList.put(index, null); + hotImageList.put(index, null); + disabledImageList.put(index, null); + } + + void release() { + display.releaseToolImageList(imageList); + display.releaseToolHotImageList(hotImageList); + display.releaseToolDisabledImageList(disabledImageList); + } + + int add(Image image, Image hotImage, Image disabledImage) { + int index = imageList.add(image); + hotImageList.add(hotImage); + disabledImageList.add(disabledImage); + return index; + } + + void put(int index, Image image, Image hotImage, Image disabledImage) { + imageList.put(index, image); + hotImageList.put(index, hotImage); + disabledImageList.put(index, disabledImage); + } + + int moveFrom(ToolBarImageLists source, int index) { + Image image = source.imageList.get(index); + Image hotImage = source.hotImageList.get(index); + Image disabledImage = source.disabledImageList.get(index); + source.clear(index); + return add(image, hotImage, disabledImage); + } + + long getImageListHandle(int zoom) { + return imageList.getHandle(zoom); + } + + long getHotImageListHandle(int zoom) { + return hotImageList.getHandle(zoom); + } + + long getDisabledImageListHandle(int zoom) { + return disabledImageList.getHandle(zoom); + } + + Point getImageSize() { + return imageList.getImageSize(); + } + +} From 63e18195cce27e9ff8a6e7f60475ebe1f7ef2b49 Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Wed, 12 Aug 2026 16:11:21 +0200 Subject: [PATCH 5/5] [Win32] Keep tool bar normal, hot and disabled image lists index-aligned A ToolItem could render another item's hot (hover) icon without any multiple monitors or image disposal involved: setting an image, then a hot image, then clearing the image while adding a new item to the tool bar was enough. A tool bar keeps three native image lists (normal, hot, disabled) but each button stores a single image index that addresses all three at once, so the three lists must stay index-aligned. Two things broke that in ToolItem.updateImages, now routed through ToolBarImageLists: - Clearing the normal image while a hot image was still set freed the normal and disabled slots but kept the hot slot occupied. A later item reusing the freed normal slot then rendered the first item's stale hot icon on hover. - A first image was appended to each list with three independent add() calls that could return different indices once the lists' free-slot patterns diverged. Only the normal list's index was written to the button, so its slot could point at another item's hot or disabled image. With this change, we enforce one index per item across all three lists: free the hot slot too when the normal image is cleared, and derive the index once from the normal list and store the hot and disabled image at that same index instead of appending them independently. Storing an image at a given index required ImageList.put(...) to also support the index right after the last one, which previously was one of the out-of-range indices it ignored. Its storing behavior, and that it keeps lists aligned whose free slots differ, is covered by unit tests in ImageListTests. Contributes to https://github.com/eclipse-platform/eclipse.platform.swt/issues/3466 Co-authored-by: Claude --- .../eclipse/swt/internal/ImageListTests.java | 154 ++++++++++++++++++ .../org/eclipse/swt/internal/ImageList.java | 16 +- .../swt/widgets/ToolBarImageLists.java | 8 +- .../org/eclipse/swt/widgets/ToolItem.java | 8 +- 4 files changed, 182 insertions(+), 4 deletions(-) diff --git a/bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/internal/ImageListTests.java b/bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/internal/ImageListTests.java index e6959bef316..aa470f735b7 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/internal/ImageListTests.java +++ b/bundles/org.eclipse.swt/Eclipse SWT Tests/win32/org/eclipse/swt/internal/ImageListTests.java @@ -132,4 +132,158 @@ public void testIsFittingForDistinguishesSize() { } } + @Test + public void testAddReturnsConsecutiveIndicesForConsecutiveImages() { + ImageList list = new ImageList(SWT.NONE, 16, 16, 100); + Image[] images = createImages(3); + try { + assertEquals(0, list.add(images[0])); + assertEquals(1, list.add(images[1])); + assertEquals(2, list.add(images[2])); + } finally { + disposeAll(list, images); + } + } + + @Test + public void testAddReusesSlotOfRemovedImage() { + ImageList list = new ImageList(SWT.NONE, 16, 16, 100); + Image[] images = createImages(3); + try { + list.add(images[0]); + list.add(images[1]); + list.put(0, null); + + assertEquals(0, list.add(images[2])); + assertSame(images[2], list.get(0)); + } finally { + disposeAll(list, images); + } + } + + @Test + public void testPutAppendsImageAtEndOfList() { + ImageList list = new ImageList(SWT.NONE, 16, 16, 100); + Image[] images = createImages(2); + try { + list.add(images[0]); + + list.put(1, images[1]); + + assertSame(images[1], list.get(1)); + assertEquals(2, list.size()); + } finally { + disposeAll(list, images); + } + } + + @Test + public void testPutReplacesImageInsideList() { + ImageList list = new ImageList(SWT.NONE, 16, 16, 100); + Image[] images = createImages(3); + try { + list.add(images[0]); + list.add(images[1]); + + list.put(0, images[2]); + + assertSame(images[2], list.get(0)); + assertEquals(2, list.size()); + } finally { + disposeAll(list, images); + } + } + + @Test + public void testPutWithoutImageClearsSlotInsideList() { + ImageList list = new ImageList(SWT.NONE, 16, 16, 100); + Image[] images = createImages(2); + try { + list.add(images[0]); + list.add(images[1]); + + list.put(0, null); + + assertNull(list.get(0)); + assertSame(images[1], list.get(1)); + assertEquals(1, list.size()); + } finally { + disposeAll(list, images); + } + } + + @Test + public void testPutBeyondEndOfListIsIgnored() { + ImageList list = new ImageList(SWT.NONE, 16, 16, 100); + Image[] images = createImages(2); + try { + list.add(images[0]); + + list.put(2, images[1]); + + assertEquals(1, list.size()); + } finally { + disposeAll(list, images); + } + } + + @Test + public void testPutNegativeIndexIsIgnored() { + ImageList list = new ImageList(SWT.NONE, 16, 16, 100); + Image[] images = createImages(2); + try { + list.add(images[0]); + + list.put(-1, images[1]); + + assertSame(images[0], list.get(0)); + assertEquals(1, list.size()); + } finally { + disposeAll(list, images); + } + } + + /** + * Tool bars address their normal, hot and disabled image list with a single + * index per item, so an image must be storable at a given index instead of at + * whatever slot the individual list happens to have free. + */ + @Test + public void testPutKeepsListsAlignedWhenTheirFreeSlotsDiffer() { + ImageList list = new ImageList(SWT.NONE, 16, 16, 100); + ImageList hotList = new ImageList(SWT.NONE, 16, 16, 100); + Image[] images = createImages(4); + try { + list.add(images[0]); + hotList.add(images[1]); + // only the first list has a free slot from here on + list.put(0, null); + + int index = list.add(images[2]); + hotList.put(index, images[3]); + + assertEquals(0, index); + assertSame(images[2], list.get(index)); + assertSame(images[3], hotList.get(index)); + } finally { + hotList.dispose(); + disposeAll(list, images); + } + } + + private static Image[] createImages(int count) { + Image[] images = new Image[count]; + for (int i = 0; i < count; i++) { + images[i] = new Image(Display.getDefault(), 16, 16); + } + return images; + } + + private static void disposeAll(ImageList list, Image[] images) { + list.dispose(); + for (Image image : images) { + image.dispose(); + } + } + } diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/internal/ImageList.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/internal/ImageList.java index c32726acb1b..fb7dd9ded6f 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/internal/ImageList.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/internal/ImageList.java @@ -55,6 +55,11 @@ public int add (Image image) { if (imageAtIndex == null) break; index++; } + put (index, image); + return index; +} + +private void append (int index, Image image, int count) { if (count == 0) { Rectangle bounds = image.getBounds(); width = bounds.width; @@ -68,7 +73,6 @@ public int add (Image image) { images = newImages; } images [index] = image; - return index; } private Image getOrClearIfDisposed(int index) { @@ -386,9 +390,19 @@ public int indexOf (Image image) { return -1; } +/** + * Stores the given image at the given index, replacing whatever is stored at that index. Passing + * no image clears the index. The index may also address the slot right after the last one, in + * which case a new slot is appended for the given image. Nothing happens for any other index + * outside the list's current size. + */ public void put (int index, Image image) { if ((0 <= index && index < images.length) && (images [index] == image)) return; int count = OS.ImageList_GetImageCount (handle); + if (index == count && image != null) { + append (index, image, count); + return; + } if (!(0 <= index && index < count)) return; if (image != null) setForAllHandles(index, image, count); images [index] = image; diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolBarImageLists.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolBarImageLists.java index bf313e74a27..ed77c748c5a 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolBarImageLists.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolBarImageLists.java @@ -57,8 +57,12 @@ void release() { int add(Image image, Image hotImage, Image disabledImage) { int index = imageList.add(image); - hotImageList.add(hotImage); - disabledImageList.add(disabledImage); + // Use the slot index from the normal image list as authoritative source + // for the image ordering and reuse it for the hot and disabled lists + // instead of letting each of them scan for its own free slot, so all + // three stay index-aligned. + hotImageList.put(index, hotImage); + disabledImageList.put(index, disabledImage); return index; } 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 b75fbad4c86..4d62213d857 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 @@ -1141,7 +1141,13 @@ void updateImages (boolean enabled) { if (!enabled) image2 = hot = disabled; } - parent.putImage(info.iImage, image2, hot != null ? hot : image2, disabled); + /* + * When the normal image is cleared (image2 == null) the button stops + * referencing this slot (iImage becomes I_IMAGENONE below), so the hot + * image must be freed too instead of leaving the old hot image behind + * for a later item that reuses this slot. + */ + parent.putImage(info.iImage, image2, image2 != null ? (hot != null ? hot : image2) : null, disabled); if (image == null) info.iImage = OS.I_IMAGENONE; }