Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8029339/webrev.13
- The javadoc for MultiResolutionImage interface is updated.
On 8/26/2015 1:45 AM, Phil Race wrote:
On 08/25/2015 02:26 PM, Jim Graham wrote:
Is MRI intended to be implemented only by classes that extend
java.awt.Image? I think that's the only place we look for it. If
so, then we should state that. Perhaps:
---------
This interface is designed to be an optional additional API supported
by some implementations of {java.awt.Image} to allow them to provide
alternate images for various rendering resolutions. The various
{Graphics.drawImage} variant methods will consult the methods of this
interface if it is implemented on the argument {Image} object in
order to choose the best representation to use for each rendering
operation.
<p>
The {MRI} interface should be implemented by any subclass of
{j.a.Image} that ...(existing text)... given image width and height.
For convenience, toolkit images obtained from {...} will implement
this interface on platforms that support naming conventions for
resolution variants of stored image media and the {Abstract} and
{Base} classes are provided to facilitate easy construction of custom
multi-resolution images from a list of related images.
---------
Should we list the naming conventions that we support (mainly just
"@2x"?), or should that be listed in the documentation of the various
getImage() and createImage() methods?
Here I think so as to keep it all together.
I have filed and issue on it: JDK-8134670 Add documentation for
@2x images loading on Mac OS X
https://bugs.openjdk.java.net/browse/JDK-8134670
The code itself seems fine so we are just discussing docs although I
also have a comment on the tests.
Jim observed elswhere that the tests use internaI APi.
I wish the tests were not doing so at all but I notice sun.awt.OSInfo
used
in one file and then another file uses jdk.testlibrary.OSInfo which seems
to be a jtreg copy to avoid that internal dependency.
Really I don't like either of these. You can work out if its OSX with one
line of code rather than introducing a dependency.
I have updated the tests to use only jdk.testlibrary.OSInfo.
Thanks,
Alexandr.
-phil.
On 8/19/15 4:51 AM, Alexander Scherbatiy wrote:
Hello,
Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8029339/webrev.12
- Comment about nonempty return list is added for
MultiResolutionImage.getResolutionVariants() method
That comment has a blank line. If it is supposed to be 2 paragraphs
then I think you need a <p> tag, otherwise the blank line should
probably be removed (I don't need to see a webrev for that).
The rest looks fine...
...jim
