This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 32101 - Look.{icon,openedIcon} should be of type Icon not Image
Summary: Look.{icon,openedIcon} should be of type Icon not Image
Status: RESOLVED WONTFIX
Alias: None
Product: contrib
Classification: Unclassified
Component: Looks (show other bugs)
Version: 3.x
Hardware: All All
: P2 blocker (vote)
Assignee: Petr Hrebejk
URL:
Keywords: API
Depends on: 33268 52562
Blocks:
  Show dependency tree
 
Reported: 2003-03-18 16:10 UTC by Jesse Glick
Modified: 2008-11-18 11:36 UTC (History)
5 users (show)

See Also:
Issue Type: ENHANCEMENT
Exception Reporter:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2003-03-18 16:10:48 UTC
It is a historical mistake in the Nodes API that
Image was used instead of Icon for nodes; probably
because of java.beans.Introspector. In fact
NodeRenderer simply makes ImageIcon's out of them
anyway, since JTree (like most everything else in
Swing) expects Icon. We should not copy this
mistake into Looks. Consider using Icon instead.

LookNode.getIcon still needs to return an Image -
it could check the Look.icon for instanceof
ImageIcon, as a workaround, else create a buffered
image and paint to it. Of course if we don't use
Node's and render Look.NodeSubstitute's directly
then there is no problem and no Image need be
created in general. We could also deprecate
Node.{getIcon,getOpenedIcon} and create

/** please treat as abstract */
public Icon Node.getIcon(int type, boolean opened) {
    Image i = opened ? getOpenedIcon(type) :
getIcon(type);
    return image2Icon(i); // as above
}

with corresponding changes in AbstractNode,
FilterNode, etc.
Comment 1 Jesse Glick 2003-03-18 16:14:55 UTC
We should likely also have a cache for Icon's; see Utilities.loadImage.
Comment 2 Jesse Glick 2003-03-18 16:33:50 UTC
Forgot to mention that the distinction between COLOR and MONO icons is
quite useless; I do not know of any code that actually ships a
monochrome icon nor even pays attention to which was requested.

Use of the 32x32 variants is also questionable; very little existing
code ships 32x32 icons, not to mention that they are usually long
obsolete, so the likelihood of making a useful 32x32-icon explorer
view is low. You could always just scale up a 16x16 icon, I guess, if
this is needed for accessibility purposes. Anyway it seems that most
of Swing in Metal L&F at least is expecting 16x16 icons, based on menu
item height and so on, and our TreeView expects them too. Only
IconView could possibly benefit from the big icons, and no has used
this view or even proposed a serious use for it.

So I might recommend simply:

Icon Look.getIcon(Look.NodeSubstitute, boolean opened);
Comment 3 Jaroslav Tulach 2003-03-19 12:55:52 UTC
I have no problems with replacing Image with Icon. 
I have no problems with not using mono icons.

I do not buy "very little existing
code ships 32x32 icons, not to mention that they are usually long
obsolete, so the likelihood of making a useful 32x32-icon explorer" -
well, if we started to need it, then it would be much more complicated
to add it. 

Icon Look.getIcon(Look.NodeSubstitute, int type)
or
Icon Look.getIcon(Look.NodeSubstitute, int type, boolean opened)

can do everything.
Comment 4 Jesse Glick 2003-03-19 15:13:20 UTC
Re. 32x32 icons - "if we started to need it, then it would be much
more complicated to add it": yes, though you can easily scale up 16x16
icons (won't look good enough for general use but enough for
accessibility purposes). I don't feel strongly about it, but it seems
strange to explicitly support something we have never used and have no
plans to use. There is no support for 24x24 or 48x48 icons, remember,
and these are common sizes for e.g. toolbar buttons.

It would be great if Looks made it simple to add or subtract named &
typed properties like "Icon icon32" without having to go through
Lookup, but this would probably make the API slower in tight display
loops and harder to grasp for a novice user who is expecting to see a
simple list of methods to override.

Or, just support for rarely used or newly added aspects of a look node
could go through lookup - e.g. for nonstandard sized icons, use in
rendering code:

Icon icon24x24;
ScaledIconProducer sip = (SIP)Look.getLookup(obj).lookup(SIP.class);
if (sip != null) {
    icon24x24 = sip.getIcon(/*opened*/false, 24, 24);
} else {
    icon24x24 = scaleFrom16x16(Look.getIcon(obj, false), 24, 24);
}

Not as streamlined, but we are expecting such rendering code to be
less common if it ever exists at all - e.g. some desktop icon view
that will probably only show a handful of icons at once, where
rendering speed is not important. Also more work to add the scaled
icon to the node, but then again it is likely to be only a small
number of nodes which would have scaled icons.

Just an idea.
Comment 5 Jaroslav Tulach 2003-03-19 16:06:48 UTC
As far as I know Hrebejk is playing with an idea of "dynamic features"
supported by looks. The style you used lookup is probably the good way
how to get such behaviour. If it is widely use, we could restrict the
amount of methods in Look itself to the (presentation) minimum.

Comment 6 Jesse Glick 2003-03-19 17:49:32 UTC
Re. dynamic features - sensible. See for example issue #29192 for
similar suggestions.
Comment 7 Jesse Glick 2003-04-28 15:51:05 UTC
Dynamic features - issue #33268.
Comment 8 Petr Hrebejk 2003-05-06 09:32:59 UTC
I'm not sure that Icon is the right interface to return. It is 
generaly harder to work with Icons than with Images (e.g. for 
caching, badging etc.) I would rather leave the method as is with
Image.
Comment 9 Jesse Glick 2003-05-06 18:32:34 UTC
x
Comment 10 Jesse Glick 2003-05-06 18:32:50 UTC
x
Comment 11 Marian Mirilovic 2003-07-28 16:35:26 UTC
verifying.
Comment 12 _ tboudreau 2004-04-14 21:13:03 UTC
Reopening as an enhancement per discussion with Jarda & Hrebejk.  Jarda's proposal:

 - Nodes should return an Image object that implements Icon (implement this in 
Utilities.loadImage())
 - Looks should use Icon
 - Add a method Utilities.imageToIcon() that view components can use to get an icon; this 
will also implement caching for images that do not implement Icon.

Also relevant is Petr Nejedly's idea of some kind of binary data icon cache;  right now we 
could provide images that implement Icon because we copy every image we load into a 
BufferedImage for memory consumption reasons (though I have a feeling Image.flush() 
might enable us to return the images loaded by Toolkit.createImage()).  If we switched to 
using ImageIO.read (which would at least be synchronous), however, we'd probably want 
to kill off image copying in IconManager, since we'd already have perfectly good 
BufferedImages - in which case the ability to return our own Icon-implementing 
BufferedImage subclass goes away.
Comment 13 Petr Hrebejk 2004-07-20 14:58:43 UTC
Changing target milestone on all Looks issues to future
Comment 14 Petr Hrebejk 2007-01-09 15:06:30 UTC
x