Skip to content

[Win32] Consolidate tool bar image list release and disposal #3513

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.

Sign up for GitHub

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

Open
HeikoKlare wants to merge 3 commits into
base: master
Choose a base branch
from
+125 −136
Open

[Win32] Consolidate tool bar image list release and disposal #3513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b945716
[Win32] Encapsulate ToolItem's access to tool bar image lists
HeikoKlare Aug 12, 2026
d0fd66f
[Win32] Consolidate tool bar image list native synchronization
HeikoKlare Aug 13, 2026
71bcb33
[Win32] Consolidate tool bar image list release and disposal
HeikoKlare Aug 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
206 changes: 113 additions & 93 deletions bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolBar.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -143,6 +143,32 @@ 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);
refreshImageLists(true);
return index;
}

@Override
long callWindowProc (long hwnd, int msg, long wParam, long lParam) {
if (handle == 0) return 0;
Expand Down Expand Up @@ -199,6 +225,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) {
Expand Down Expand Up @@ -452,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);
Expand Down Expand Up @@ -495,18 +513,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.
Expand Down Expand Up @@ -869,6 +875,56 @@ 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);
}
}

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) {
Expand All @@ -885,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
Expand Down Expand Up @@ -1000,19 +1065,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 ();
Expand All @@ -1039,32 +1091,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 ();
Expand Down Expand Up @@ -1226,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;
Expand All @@ -1239,27 +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);
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);
releaseImageLists(oldImageList, oldHotImageList, oldDisabledImageList);
OS.InvalidateRect (handle, null, true);
}
}
Expand Down Expand Up @@ -1744,9 +1766,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];
Expand Down
55 changes: 12 additions & 43 deletions bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolItem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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 ();
Expand All @@ -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
Expand All @@ -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;
}

Expand Down
Loading