Hello Jim,
Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8029339/webrev.10/
- the suggested comments are updated
- the MultiResolutionRenderingHintsTest is updated to use custom
GraphicsConfiguration and SurfaceData
On 7/23/2015 8:25 PM, Jim Graham wrote:
Hi Alexandr,
On 7/21/15 7:41 AM, Alexander Scherbatiy wrote:
Hello Jim,
Thank your for the review.
Could you review the updated fix there the suggested comments are
updated:
http://cr.openjdk.java.net/~alexsch/8029339/webrev.09/
Minor comments embedded below.
On 7/14/2015 2:36 AM, Jim Graham wrote:
AbstractMRI - getGraphics() should include "getGraphics() not
supported on Multi-Resolution Images" as the exception description
text.
AbstractMRI - the getGraphics() description string contains an
embedded '\n' and an embedded '\"', are those typos?
Updated.
BaseMRI - class comment - "The implementation will return the first
... satisfy the rendering request." Add another sentence right
there
"The last image in the array will be returned if no suitable
image is
found that is larger than the rendering request."
BaseMRI - whoops, my bad. "that is larger than" should probably be
"that is as large as" to match the <= comparison
Updated.
SG2D.getResolutionVariant - using destRegionWH to calculate
destImageWH can involve a lot of "do some math and then later have
additional code undo it". Would it be faster to just compute
destImageWH directly, as in:
float destImageWidth, destImageHeight;
if (BASE) {
destImageWH = srcWH;
} else if (DPI) {
destImageWH = (float) abs(srcWH * devScale);
} else {
double destRegionWidth, destRegionHeight;
if (type) {
...
}
destImageWH = (float) abs(srcWH * destRegionWH / swh);
}
Image rv = img.getRV(destImageWH);
For the BASE and DPI_FIT cases shouldn't you use srcWidth,srcHeight
instead of sw,sh? The sw,sh are affected by sub-image parameters, but
srcWidth,srcHeight are literally the size of the original image, which
is what you want to search the MRI for. The sw,sh should only be used
to scale the srcWidth in the "else" clause - as in "srcWidth *
destRegionWidth / sw".
You are right. sw and sh are sizes of a region in the original
image. I have updated the fix to use srcWidth/Height.
Is there intent to have the default case be mapped to DPI_FIT for
Windows?
I think yes.
I didn't see where this was being done - is that a TBD follow-on fix
or did I miss a default somewhere?
I think that it could be fixed with the updating WToolkit to load
high-resolution toolkit images in the same way as it is done in
LWCToolkit.
At least it can be filled as a separate issue.
MRRHTest - why not render to a BufferedImage and avoid Robot?
Could it
tap into internal APIs to create a BImg/compatibleImage with a given
devScale?
The DPI_FIT test includes case there a graphics transform is
identity but the device configuration transform has scale 2.
There should be a way to set scale for the device configuration
transform;
Ah yes, DPI_FIT needs a default transform. Also, without a way to
manually create a device with a transform, that means that DPI_FIT is
only successfully tested if the tests are run on a HiDPI machine,
right?
MRRHTest - why allow up to delta=50 on the comparison?
It is just for monitors that are calibrated on Mac OS X and they
colors are different from that which was set. The test uses colors
which
have at least one color component differ by 255.
Another issue that might go away if we could use BImg instead of
robot.
MRRHTest - since the scale factor comes from a dialog, we depend on
running this on a HiDPI display to fully test the hints? Could using
"scaled compatible images" allow testing this more definitively?
Could you explain more what does it mean "scaled compatible
images"?
I seem to recall that there is a mechanism in there to create
backbuffer images for swing that record the fact that they are
scaled. I forget how this is done, but I'm wondering if it could be
used to run all of this test code in a simulated scale environment
rather than using the actual configurations that AWT creates for the
current system.
I have updated the test to use custom GraphicsConfiguration and
SurfaceData. This allows to use custom scales both on the graphics
configuration and on graphics.
Thanks,
Alexandr.
...jim
