Hi Phil,
Yes I agree with that. This test case is only for those read() and write()
functions which tend to use createImageXXXStream() and not for those which get
stream directly as part of argument and follow the spec. Thanks for your inputs.
-Jay
From: Phil Race
Sent: Tuesday,
> One comes from the SampleModel, the other comes from the DataBuffer.
In the other methods that is true but not in this one.
This method creates the SampleModel using the type of the DataBuffer.
The webrev diff is insufficient to make this apparent.
-phil.
On 3/29/16, 7:17 PM, Jim Graham
One comes from the SampleModel, the other comes from the DataBuffer.
The reason to check them both is to make sure they agree. There is no
"it must be DataBuffer.TYPE_BYTE" here, there is only "it *should* be
DataBuffer.TYPE_BYTE" and that warrants a test and a fallback to the
generic
Raster.java, line 742: This branch of the switch statement instantiates
a SWR which doesn't have a specific buffer type. It could be replaced
with just "break;" and let the code after the switch create the raster
(it can't be deleted from the switch statement because the default would
throw
Ah, yes, now I see it. I was combing through the various classes in
java/awt/image that *are* public to do some research while I was reading
the code review and somehow thought that the classes being modified were
in the same directory/package.
I now see that all of the modified constructors
Hi Prahalad,
This latest version looks like it should produce correct answers.
I'd like to see benchmark results on more platforms, particularly
Windows since the different compilers may optimize these things differently.
Also, you didn't mention what data set you used for benchmarking. I'd
On 29.03.16 23:19, Jim Graham wrote:
This sounded like such a great idea that I don't think we realized the
one obvious reason we can't do this...
We should take into account that this classes in sun/awt/image and is
not accessible on old jdk when the securitymanager is enabled, and will
be
This sounded like such a great idea that I don't think we realized the
one obvious reason we can't do this...
You are talking about changing the method signatures on existing public
constructors. While it might be nice to have had them typed from the
get-go, I don't think we can change them
Like the related fix passing down page ranges to the dialog this looks
reasonable
but I have not had time to apply the patch and test it.
You should test the combination of this fix + that for 8061258 before
pushing ..
-phil.
On 03/22/2016 04:25 AM, prasanta sadhukhan wrote:
Hi Phil,
I
Hi,
You are calling these unconditionally :
387 jint minPage = JNFCallIntMethod(env, srcPrinterJob, jm_getMinPage);
388 jint maxPage = JNFCallIntMethod(env, srcPrinterJob, jm_getMaxPage);
But it seems they are only used/needed in the else {..}
Other than that it looks reasonable but
Please see the updated webrev:
http://cr.openjdk.java.net/~avstepan/8149028/webrev.01/
(one more test was added).
Thanks,
Alexander
On 3/25/2016 6:46 PM, Alexander Stepanov wrote:
Hello,
Could you please review the following fix
http://cr.openjdk.java.net/~avstepan/8149028/webrev.00/
for
This is OK but you ought to remember that read() and write() only HAPPEN
to use createImageXXXStream(). They could get the stream in some other way
and still be a spec. compliant implementation.
-phil.
On 03/29/2016 02:27 AM, Jayathirth D V wrote:
Hi Prasanta,
Actually returns null in test
Vote: yes
On 3/18/2016 01:49, Philip Race wrote:
I hereby nominate Alexander Scherbatiy to membership in the 2D group.
Alexander Scherbatiy is a current member of the Swing group
and has contributed almost 200 changesets to OpenJDK :-
I am not sure what is the reason of this code?
1546 try {
1547 output.delete();
1548 stream = createImageOutputStream(output);
1549 } catch (IOException e) {
1550 throw e;
1551 }
On 29.03.16 1:12, Philip Race wrote:
This all looks fine
Looks ok.
Regards
Prasanta
On 3/29/2016 2:57 PM, Jayathirth D V wrote:
Hi Prasanta,
Actually returns null in test description is for
createImageXXXStream() and not for read() and write() functions. But
for more clarity I have added why we catch IOException also in test
description.
Hi Prasanta,
Actually returns null in test description is for createImageXXXStream() and not
for read() and write() functions. But for more clarity I have added why we
catch IOException also in test description.
Please review the updated webrev:
+1.
Looks ok to me. Only thing is that whether the test summary is appropriate "
Test verifies that when createImageInputStream() or
28 * createImageOutputStream() returns null while read or write,"
Should it not be "verifies whether ...throws appropriate exception"?
Regards
17 matches
Mail list logo
