From b9457167f75513d621585159e19edd271051296b Mon Sep 17 00:00:00 2001 From: Heiko Klare Date: Wed, 12 Aug 2026 17:22:40 +0200 Subject: [PATCH 1/3] [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/3] [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/3] [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); } }