dolphinsmalltalk / Dolphin

Dolphin Smalltalk Core Image
MIT License
301 stars 58 forks source link

Display icons using smart scaling in StaticIcon (or somehow) #1150

Closed daniels220 closed 2 years ago

daniels220 commented 2 years ago

I occasionally want to use Icon warning at 16x16 size, e.g. next to a text field to indicate a validation error, or next to a description of a problem. The Win32 SS_STATIC control style rather wants to display the icon at 32x32 size, and though Dolphin forces it to the size determined by the LayoutManager, the 32x32 version of the icon is still used, just scaled down. Unfortunately this looks terrible, while the actual 16x16 Icon warning is quite good.

I'd like StaticIcon (and ImageView when supplied with an Icon, I suppose) to auto-choose which icon resource to use based on displayed size, similar to how other controls (presumably? mostly?) already do when they make use of icons internally. If getting this working with SS_STATIC isn't possible, maybe it would be better to only do it in ImageView, or even make a specialized ImageView subclass—one way or another, to keep SS_STATIC for the things it does work for.

One additional consideration: ideally, though this is not a hard requirement, displaying system icons should avoid allocating additional USER objects for repeated display of the same icon. I see Icon>>load:fromInstance:extent: and CommCtrl.h's LoadIconWithScaleDown, which looks like the way to handle this "manually", but a naive implementation using it ends up making a bunch of extra handles, since I can't see a way to involve IconImageManager with StaticIcon.

I'm treating this as a feature request rather than a bug because the problem seems to be with the options offered by Win32, and getting something that works right might be a lot of reimplementation.

blairmcg commented 2 years ago

The static control in Icon mode is pretty limited. Windows is able to use some special magic to reload icons at the required size if they were either loaded from a file or a DLL resource. I'm not sure how it does this, and the behaviour is not really documented that I can find. An example is setting the icon for a top-level Window. We generally only set the large icon by sending a 32x32 HICON, but Windows is able to use the correct image for the small icon (when available) without scaling down. The static icon control doesn't do this, however. Other more recent controls tend not to expect to be given HICONs, but rather make use of image lists, e.g. list and tree views. The purpose of IconImageManager is not to provide HICONs. Rather it is to manage a set of image lists of different sizes that are populated from the best available images for those sizes.

However, you could:

  1. Use an ImageView. This is in the ControlView hierarchy, but isn't really a control at all. It is just a specialised Dolphin window that paints images. This will draw the best icon image for it's client extent (depending on mode). It won't leak handles. This can be seen by running the example below and changing the size of the window. I've chosen an icon that has quite different images so you can see this happening. Another example would be the icon of ClassBrowserShell, as this has a deliberately simplified image at 16x16.

    s := ImagePresenter show: 'Basic image'.
    s view isWholeBackgroundErased: true. "Otherwise you'll get a mess when resizing icons with alpha"
    s model value: FontDialog icon.

    Or demonstrating what you asked for:

    s := ImagePresenter show: 'Basic image'.
    s topShell view layoutManager: FlowLayout new; extent: 50@75; backcolor: Color buttonFace.
    s view 
    preferredExtent: 16@16; 
    usePreferredExtent: true;
    isWholeBackgroundErased: true.
    s model value: Icon warning.
  2. Modify John Aspinall's IconWithExtent so that it works correctly for system icons. These don't load correctly through the LoadImage() API, but can be loaded at specific sizes with alternate APIs. It will work correctly for non-system icons already, e.g.:

    s := ImagePresenter show: 'Static icon'.
    s topShell view layoutManager: FlowLayout new; extent: 50@75; backcolor: Color buttonFace.
    s view preferredExtent: 16@16; usePreferredExtent: true.
    s model value: ClassBrowserShell icon asSmallIcon
  3. Coincidentally I'll soon be submitting an extensive change to the Dolphin 8 branch that revises the Image hierarchy to load using initializers. This includes the ability to specify the loading Icon loading extent making IconWithExtent redundant. This could be backported to Dolphin 7, although will be quite a lot of work. Here's how the StaticIcon looks with that (border on), exactly as it would with a fixed IconWithExtent: image

I would have thought that the first option of using the existing ImageView, would work perfectly well for your requirement though.

daniels220 commented 2 years ago

Makes sense about the limitations of the static control—that's why I said "or somehow" in the title, since I figured it might not be possible to make the static control do this. I thought I had tried using ImageView, but...well, this came up in the process of updating an application from Dolphin 7.0 to 7.1, and it looks like I only tried ImageView in 7.0, where it doesn't handle the scaling correctly. I just tried it in 7.1 and it does work as you say, so that may be a good solution going forward. Sorry about the confusion.