Alexey,
We already nixed this approach
See https://mail.openjdk.java.net/pipermail/awt-dev/2019-January/014972.html
Shashi is re-thinking it to use MultiResolutionImage.
-phil.
On 3/1/19 12:25 PM, Alexey Ivanov wrote:
Hi Shashi,
Sorry for my belated review.
On 22/01/2019 18:03, Shashidhara Veerabhadraiah wrote:
Hi All, Please review the below new fix compared to earlier webrevs.
Now the new api to get the icons of different sizes is being made
part of a new class SystemIcon. Please consider this as a different
solution than the one that was under review for a long time till now.
http://cr.openjdk.java.net/~sveerabhadra/8182043/webrev.05/
This version looks cleaner.
The new SystemIcon class contains only one method, getSystemIcon. Does
it justify creating a new class for this? Can't we add this extended
version into FileSystemView class?
FileSystemView has method:
public Icon getSystemIcon(File f)
We're extending it with
public Icon getSystemIcon(File f, int size)
Are there any objections to this approach?
The class name, if we're going this route, could be more specific:
FileSystemIcon seems a better alternative.
Javadoc for SystemIcon says, “SystemIcon is JFileChooser's gateway to
the file system icons.” Yet JFileChooser uses
FileSystemView.getSystemIcon(File). So Javadoc is not accurate at the
very least.
There should also be @since tag for the class.
As this is a new API, I think it makes sense to throw
IllegalArgumentException for invalid parameters: file == null or size
is out of supported range.
You don't usually want to print exception stack trace or messages from
a public API:
74 System.err.println("FileSystemView.getShellFolder: f="+f);
75 e.printStackTrace();
As Phil mentioned, you should clearly state when a null value can be
returned.
static public Icon getSystemIcon(File f, int size)
should be
public static Icon getSystemIcon(File f, int size)
At this time, SystemIcon is an utility class which provides only
static methods. Yet it can be instantiated which does not look good.
The code indents by three spaces instead of four.
Win32ShellFolder2.java
1111 public Image getIcon(int size) {
The method should have @Override annotation.
It can also be added to
1040 public Image getIcon(final boolean getLargeIcon) {
Win32ShellFolderManager2.java
383 key.startsWith("shell32LargeIcon
")?LARGE_ICON_SIZE : SMALL_ICON_SIZE);
There should be spaces around ‘?’.
Part of the fix is being borrowed from Semyon’s fix and would like to
thank Semyon for that.
You can give Semyon attribution by including "Contributed-by" tag in
your commit message and referencing Semyon and yourself there.
Regards,
Alexey
Thanks and regards,
Shashi
<SNIP>
