Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v10]
On Wed, 17 Feb 2021 15:54:22 GMT, Prasanta Sadhukhan wrote: >> The API doc for Graphics2D.clip(shape s) claims that passing a null argument >> would actually clear the existing clipping area, which is incorrect. >> This statement is applicable only to G2D.setClip() and not for the clip() >> method. G2D.clip() would throw a NullPointerException when it encounters a >> null argument. >> Updated spec to rectify this. > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > javadoc change Marked as reviewed by azvegint (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v10]
On Wed, 17 Feb 2021 15:54:22 GMT, Prasanta Sadhukhan wrote: >> The API doc for Graphics2D.clip(shape s) claims that passing a null argument >> would actually clear the existing clipping area, which is incorrect. >> This statement is applicable only to G2D.setClip() and not for the clip() >> method. G2D.clip() would throw a NullPointerException when it encounters a >> null argument. >> Updated spec to rectify this. > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > javadoc change Marked as reviewed by kizune (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v10]
On Wed, 17 Feb 2021 15:55:04 GMT, Alexey Ivanov wrote: >> Prasanta Sadhukhan has updated the pull request incrementally with one >> additional commit since the last revision: >> >> javadoc change > > Marked as reviewed by aivanov (Reviewer). Thanks @aivanov-jdk . Would you mind adding yourself as a reviewer to the CSR? - PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v10]
On Wed, 17 Feb 2021 15:54:22 GMT, Prasanta Sadhukhan wrote: >> The API doc for Graphics2D.clip(shape s) claims that passing a null argument >> would actually clear the existing clipping area, which is incorrect. >> This statement is applicable only to G2D.setClip() and not for the clip() >> method. G2D.clip() would throw a NullPointerException when it encounters a >> null argument. >> Updated spec to rectify this. > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > javadoc change Marked as reviewed by aivanov (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v9]
On Wed, 17 Feb 2021 15:48:12 GMT, Prasanta Sadhukhan
wrote:
>> The API doc for Graphics2D.clip(shape s) claims that passing a null argument
>> would actually clear the existing clipping area, which is incorrect.
>> This statement is applicable only to G2D.setClip() and not for the clip()
>> method. G2D.clip() would throw a NullPointerException when it encounters a
>> null argument.
>> Updated spec to rectify this.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> javadoc change
src/java.desktop/share/classes/java/awt/Graphics2D.java line 1207:
> 1205: * with the current clip, it will throw {@code NullPointerException}
> 1206: * for a null shape unless the user clip is also {@code null}.
> 1207: * So calling this method with a {@code null} argument is not
> recommended.
I wonder why not “{@code null} shape” at line 1206?
-
PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v10]
> The API doc for Graphics2D.clip(shape s) claims that passing a null argument > would actually clear the existing clipping area, which is incorrect. > This statement is applicable only to G2D.setClip() and not for the clip() > method. G2D.clip() would throw a NullPointerException when it encounters a > null argument. > Updated spec to rectify this. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: javadoc change - Changes: - all: https://git.openjdk.java.net/jdk/pull/2476/files - new: https://git.openjdk.java.net/jdk/pull/2476/files/a9bc437b..e447b562 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2476&range=09 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2476&range=08-09 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2476.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2476/head:pull/2476 PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v8]
On Wed, 17 Feb 2021 15:38:10 GMT, Prasanta Sadhukhan
wrote:
>> The API doc for Graphics2D.clip(shape s) claims that passing a null argument
>> would actually clear the existing clipping area, which is incorrect.
>> This statement is applicable only to G2D.setClip() and not for the clip()
>> method. G2D.clip() would throw a NullPointerException when it encounters a
>> null argument.
>> Updated spec to rectify this.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> javadoc change
src/java.desktop/share/classes/java/awt/Graphics2D.java line 1207:
> 1205: * with the current clip, it will throw {@code NullPointerException}
> 1206: * for a null shape unless the user clip is also {@code null}.
> 1207: * So calling this method with a null argument is not recommended.
This suggestion still applies too.
Suggestion:
* for a {@code null} shape unless the user clip is also {@code null}.
* So calling this method with a {@code null} argument is not recommended.
-
PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v9]
> The API doc for Graphics2D.clip(shape s) claims that passing a null argument > would actually clear the existing clipping area, which is incorrect. > This statement is applicable only to G2D.setClip() and not for the clip() > method. G2D.clip() would throw a NullPointerException when it encounters a > null argument. > Updated spec to rectify this. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: javadoc change - Changes: - all: https://git.openjdk.java.net/jdk/pull/2476/files - new: https://git.openjdk.java.net/jdk/pull/2476/files/3a1faf33..a9bc437b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2476&range=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2476&range=07-08 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2476.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2476/head:pull/2476 PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v7]
On Wed, 17 Feb 2021 15:25:35 GMT, Alexey Ivanov wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> javadoc change
>
> src/java.desktop/share/classes/java/awt/Graphics2D.java line 1206:
>
>> 1204: * with the current clip, it will throw {@code
>> NullPointerException}
>> 1205: * for a null shape unless the user clip is also {@code null}.
>> 1206: * So calling this method with a null argument is not recommended.
>
> Since you're editing the javadoc for this method, wouldn't adding a couple
> `` break up the description to make it clearer?
>
> 1198 * The user clip modified by this method is independent of
> the
> 1203 * user clip.
> * Since this method intersects the specified shape
> The rendered javadoc will have clear separation between different paragraphs
> and will facilitate scanning the method description.
>
> Does it make any sense?
ok. sure...
-
PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v8]
> The API doc for Graphics2D.clip(shape s) claims that passing a null argument > would actually clear the existing clipping area, which is incorrect. > This statement is applicable only to G2D.setClip() and not for the clip() > method. G2D.clip() would throw a NullPointerException when it encounters a > null argument. > Updated spec to rectify this. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: javadoc change - Changes: - all: https://git.openjdk.java.net/jdk/pull/2476/files - new: https://git.openjdk.java.net/jdk/pull/2476/files/530cad4d..3a1faf33 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2476&range=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2476&range=06-07 Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2476.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2476/head:pull/2476 PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v7]
On Wed, 17 Feb 2021 15:08:58 GMT, Prasanta Sadhukhan
wrote:
>> The API doc for Graphics2D.clip(shape s) claims that passing a null argument
>> would actually clear the existing clipping area, which is incorrect.
>> This statement is applicable only to G2D.setClip() and not for the clip()
>> method. G2D.clip() would throw a NullPointerException when it encounters a
>> null argument.
>> Updated spec to rectify this.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> javadoc change
src/java.desktop/share/classes/java/awt/Graphics2D.java line 1206:
> 1204: * with the current clip, it will throw {@code NullPointerException}
> 1205: * for a null shape unless the user clip is also {@code null}.
> 1206: * So calling this method with a null argument is not recommended.
Suggestion:
* for a {@code null} shape unless the user clip is also {@code null}.
* So calling this method with a {@code null} argument is not recommended.
src/java.desktop/share/classes/java/awt/Graphics2D.java line 1206:
> 1204: * with the current clip, it will throw {@code NullPointerException}
> 1205: * for a null shape unless the user clip is also {@code null}.
> 1206: * So calling this method with a null argument is not recommended.
Since you're editing the javadoc for this method, wouldn't adding a couple
`` break up the description to make it clearer?
1198 * The user clip modified by this method is independent of the
1203 * user clip.
* Since this method intersects the specified shape
The rendered javadoc will have clear separation between different paragraphs
and will facilitate scanning the method description.
Does it make any sense?
-
PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v6]
On Wed, 17 Feb 2021 14:20:47 GMT, Alexey Ivanov wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> javdoc tag for NPE
>
> src/java.desktop/share/classes/java/awt/Graphics2D.java line 1205:
>
>> 1203: * user clip. Since this method intersects the specified shape
>> 1204: * with the current clip, it will throw {@code
>> NullPointerException} for a null shape
>> 1205: * unless the user clip is also null.
>
> Shall the text be re-wrapped?
>
> I'm not sure if null should always be {@code null}, it's not used
> consistently. At the same time, two lines above it's “…with a {@code null}
> argument…”; it makes sense to make Javadoc consistent in this regard at least
> for one method.
ok .fair enough...done..
-
PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v7]
> The API doc for Graphics2D.clip(shape s) claims that passing a null argument > would actually clear the existing clipping area, which is incorrect. > This statement is applicable only to G2D.setClip() and not for the clip() > method. G2D.clip() would throw a NullPointerException when it encounters a > null argument. > Updated spec to rectify this. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: javadoc change - Changes: - all: https://git.openjdk.java.net/jdk/pull/2476/files - new: https://git.openjdk.java.net/jdk/pull/2476/files/0951a478..530cad4d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2476&range=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2476&range=05-06 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2476.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2476/head:pull/2476 PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v6]
On Mon, 15 Feb 2021 12:48:01 GMT, Prasanta Sadhukhan
wrote:
>> The API doc for Graphics2D.clip(shape s) claims that passing a null argument
>> would actually clear the existing clipping area, which is incorrect.
>> This statement is applicable only to G2D.setClip() and not for the clip()
>> method. G2D.clip() would throw a NullPointerException when it encounters a
>> null argument.
>> Updated spec to rectify this.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> javdoc tag for NPE
src/java.desktop/share/classes/java/awt/Graphics2D.java line 1205:
> 1203: * user clip. Since this method intersects the specified shape
> 1204: * with the current clip, it will throw {@code
> NullPointerException} for a null shape
> 1205: * unless the user clip is also null.
Shall the text be re-wrapped?
I'm not sure if null should always be {@code null}, it's not used consistently.
At the same time, two lines above it's “…with a {@code null} argument…”; it
makes sense to make Javadoc consistent in this regard at least for one method.
-
PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method
On Mon, 15 Feb 2021 12:44:28 GMT, Prasanta Sadhukhan wrote: >> The API doc for Graphics2D.clip(shape s) claims that passing a null argument >> would actually clear the existing clipping area, which is incorrect. >> This statement is applicable only to G2D.setClip() and not for the clip() >> method. G2D.clip() would throw a NullPointerException when it encounters a >> null argument. >> Updated spec to rectify this. > > @prrace Can you please have a look at the CSR JDK-8261426? @mrserb @prrace @jayathirthrao @aivanov-jdk @azvegint Any more comments on this? If not, can this please be approved? - PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v5]
On Fri, 12 Feb 2021 14:42:30 GMT, Alexey Ivanov wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Review comment fix. Test updated
>
> src/java.desktop/share/classes/java/awt/Graphics2D.java line 1204:
>
>> 1202: * argument, the specified {@code Shape} becomes the new
>> 1203: * user clip. Since this method intersects the specified shape
>> 1204: * with the current clip, it will throw NPE for a null shape
>
> Shall NPE be spelled out as `{@code NullPointerException}`?
>
> Also should it rather be, “…it will throws NPE…”?
Done..As for the statement change, I think "will throw" is more grammatically
correct" so kept it unchanged.
-
PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method
On Tue, 9 Feb 2021 11:57:44 GMT, Prasanta Sadhukhan wrote: > The API doc for Graphics2D.clip(shape s) claims that passing a null argument > would actually clear the existing clipping area, which is incorrect. > This statement is applicable only to G2D.setClip() and not for the clip() > method. G2D.clip() would throw a NullPointerException when it encounters a > null argument. > Updated spec to rectify this. @prrace Can you please have a look at the CSR JDK-8261426? - PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v6]
> The API doc for Graphics2D.clip(shape s) claims that passing a null argument > would actually clear the existing clipping area, which is incorrect. > This statement is applicable only to G2D.setClip() and not for the clip() > method. G2D.clip() would throw a NullPointerException when it encounters a > null argument. > Updated spec to rectify this. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: javdoc tag for NPE - Changes: - all: https://git.openjdk.java.net/jdk/pull/2476/files - new: https://git.openjdk.java.net/jdk/pull/2476/files/51fbe247..0951a478 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2476&range=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2476&range=04-05 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2476.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2476/head:pull/2476 PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v5]
On Thu, 11 Feb 2021 15:44:57 GMT, Prasanta Sadhukhan
wrote:
>> The API doc for Graphics2D.clip(shape s) claims that passing a null argument
>> would actually clear the existing clipping area, which is incorrect.
>> This statement is applicable only to G2D.setClip() and not for the clip()
>> method. G2D.clip() would throw a NullPointerException when it encounters a
>> null argument.
>> Updated spec to rectify this.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Review comment fix. Test updated
src/java.desktop/share/classes/java/awt/Graphics2D.java line 1204:
> 1202: * argument, the specified {@code Shape} becomes the new
> 1203: * user clip. Since this method intersects the specified shape
> 1204: * with the current clip, it will throw NPE for a null shape
Shall NPE be spelled out as `{@code NullPointerException}`?
Also should it rather be, “…it will throws NPE…”?
-
PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v5]
> The API doc for Graphics2D.clip(shape s) claims that passing a null argument > would actually clear the existing clipping area, which is incorrect. > This statement is applicable only to G2D.setClip() and not for the clip() > method. G2D.clip() would throw a NullPointerException when it encounters a > null argument. > Updated spec to rectify this. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Review comment fix. Test updated - Changes: - all: https://git.openjdk.java.net/jdk/pull/2476/files - new: https://git.openjdk.java.net/jdk/pull/2476/files/e4053de8..51fbe247 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2476&range=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2476&range=03-04 Stats: 5 lines in 3 files changed: 2 ins; 1 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2476.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2476/head:pull/2476 PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v4]
On Thu, 11 Feb 2021 04:14:54 GMT, Prasanta Sadhukhan
wrote:
>> The API doc for Graphics2D.clip(shape s) claims that passing a null argument
>> would actually clear the existing clipping area, which is incorrect.
>> This statement is applicable only to G2D.setClip() and not for the clip()
>> method. G2D.clip() would throw a NullPointerException when it encounters a
>> null argument.
>> Updated spec to rectify this.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Fix as per Phil's comment
test/jdk/java/awt/Graphics2D/TestNullClip.java line 41:
> 39:
> 40: g2d.setClip(0, 0, 100, 100);
> 41: Shape clip = g2d.getClip();
Looks like the `clip` variable is not used.
Also I've just noticed that the years are not updated in copyright headers of
Graphics and Graphics2d.
test/jdk/java/awt/Graphics2D/TestNullClip.java line 46:
> 44: if (clip1 != null) {
> 45: throw new RuntimeException("Clip is not cleared");
> 46: }
There is no check that `g2d.clip(null);` call does not throw NPE when no user
clip is set.
-
PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v4]
> The API doc for Graphics2D.clip(shape s) claims that passing a null argument > would actually clear the existing clipping area, which is incorrect. > This statement is applicable only to G2D.setClip() and not for the clip() > method. G2D.clip() would throw a NullPointerException when it encounters a > null argument. > Updated spec to rectify this. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Fix as per Phil's comment - Changes: - all: https://git.openjdk.java.net/jdk/pull/2476/files - new: https://git.openjdk.java.net/jdk/pull/2476/files/0f1ab211..e4053de8 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2476&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2476&range=02-03 Stats: 21 lines in 2 files changed: 11 ins; 8 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2476.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2476/head:pull/2476 PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v3]
On Wed, 10 Feb 2021 18:46:33 GMT, Phil Race wrote:
>> From what I can tell, if clip is null, then calling clip(null) has no effect.
>> And it won't throw an NPE in that case. So the proposed documentation is
>> less accurate than no documentation at all.
>> And I would shy away from changing the implementation on this without LOTS
>> of VERY careful testing.
>> Oh the javadoc clause you propose is split by many lines of comments. I see
>> you have the rider "and clip is already set via {@code setClip}".
>> Well I am not sure why it has to be set by setClip for this to be true but
>> just having to say this sounds weird.
>
>> usrClip is set to null if "sh" is null so clip is cleared...I will update
>> the setClip doc too..
>
> Huh ? Not understanding you. If the userClip is non-null then we enter this
> if (usrClip != null) {
> s = intersectShapes(usrClip, s, true, true);
> }
> which i where you get the NPE from you are proposing to document.
It seems to me to be a little accidental from the implementation that
clip(null) will just silently return
if the user clip is currently null.
But I think it unduly risky to "clean that up" after so many years.
So I think we are stuck with some variant on the weird wording.
The bit about using setClip is bogus. I suggest adding the clause worded like
this
@throws NullPointerException if {@code s} is {@ code null) and a user clip is
currently set.
and explanatory text like this
Since this method intersects the specified shape with the current clip, it will
throw NPE for a null shape
unless the user clip is also null. So calling this method with a null argument
is not recommended.
-
PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v3]
On Wed, 10 Feb 2021 18:30:18 GMT, Phil Race wrote:
>> As per code
>> public void setClip(Shape sh) {
>> usrClip = transformShape(sh);
>>
>> usrClip is set to null if "sh" is null so clip is cleared...I will update
>> the setClip doc too..
>
> From what I can tell, if clip is null, then calling clip(null) has no effect.
> And it won't throw an NPE in that case. So the proposed documentation is less
> accurate than no documentation at all.
> And I would shy away from changing the implementation on this without LOTS of
> VERY careful testing.
> Oh the javadoc clause you propose is split by many lines of comments. I see
> you have the rider "and clip is already set via {@code setClip}".
> Well I am not sure why it has to be set by setClip for this to be true but
> just having to say this sounds weird.
> usrClip is set to null if "sh" is null so clip is cleared...I will update the
> setClip doc too..
Huh ? Not understanding you. If the userClip is non-null then we enter this
if (usrClip != null) {
s = intersectShapes(usrClip, s, true, true);
}
which i where you get the NPE from you are proposing to document.
-
PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v3]
On Wed, 10 Feb 2021 06:01:59 GMT, Prasanta Sadhukhan
wrote:
>> The API doc for Graphics2D.clip(shape s) claims that passing a null argument
>> would actually clear the existing clipping area, which is incorrect.
>> This statement is applicable only to G2D.setClip() and not for the clip()
>> method. G2D.clip() would throw a NullPointerException when it encounters a
>> null argument.
>> Updated spec to rectify this.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Fix
test/jdk/java/awt/Graphics2D/TestNullClip.java line 52:
> 50: throw new RuntimeException("NPE is expected");
> 51: } catch (NullPointerException e) {
> 52: //expected
Why is this wrapping everything ? There's only one line here where you expect
an NPE and so you should only catch an exception from that one line.
-
PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v3]
On Wed, 10 Feb 2021 04:51:33 GMT, Prasanta Sadhukhan
wrote:
>> If "*setClip*(null)" has to clear the clip then it should be specified,
>> currently, that method said nothing about the null parameter.
>
> As per code
> public void setClip(Shape sh) {
> usrClip = transformShape(sh);
>
> usrClip is set to null if "sh" is null so clip is cleared...I will update the
> setClip doc too..
>From what I can tell, if clip is null, then calling clip(null) has no effect.
And it won't throw an NPE in that case. So the proposed documentation is less
accurate than no documentation at all.
And I would shy away from changing the implementation on this without LOTS of
VERY careful testing.
Oh the javadoc clause you propose is split by many lines of comments. I see you
have the rider "and clip is already set via {@code setClip}".
Well I am not sure why it has to be set by setClip for this to be true but just
having to say this sounds weird.
-
PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v3]
> The API doc for Graphics2D.clip(shape s) claims that passing a null argument > would actually clear the existing clipping area, which is incorrect. > This statement is applicable only to G2D.setClip() and not for the clip() > method. G2D.clip() would throw a NullPointerException when it encounters a > null argument. > Updated spec to rectify this. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Fix - Changes: - all: https://git.openjdk.java.net/jdk/pull/2476/files - new: https://git.openjdk.java.net/jdk/pull/2476/files/94a04228..0f1ab211 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2476&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2476&range=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2476.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2476/head:pull/2476 PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method [v2]
> The API doc for Graphics2D.clip(shape s) claims that passing a null argument > would actually clear the existing clipping area, which is incorrect. > This statement is applicable only to G2D.setClip() and not for the clip() > method. G2D.clip() would throw a NullPointerException when it encounters a > null argument. > Updated spec to rectify this. Prasanta Sadhukhan has updated the pull request incrementally with three additional commits since the last revision: - Fix - Document setClip(null) behaviour too - Document setClip(null) behaviour too - Changes: - all: https://git.openjdk.java.net/jdk/pull/2476/files - new: https://git.openjdk.java.net/jdk/pull/2476/files/a7798fb5..94a04228 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2476&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2476&range=00-01 Stats: 12 lines in 3 files changed: 10 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2476.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2476/head:pull/2476 PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method
On Wed, 10 Feb 2021 04:47:05 GMT, Sergey Bylokhov wrote:
>> I am not sure if it's implementation bug. As Jim Graham has mentioned in
>> thebug,
>>
>>> clip(null) is not legal.
>>>
>>> It is *setClip*(null) that clears the clip.
>> I could rephrase the doc to specify NPE willbe thrrown for null Shape if a
>> clip is already set.
>
> If "*setClip*(null)" has to clear the clip then it should be specified,
> currently, that method said nothing about the null parameter.
As per code
public void setClip(Shape sh) {
usrClip = transformShape(sh);
usrClip is set to null if "sh" is null so clip is cleared...I will update the
setClip doc too..
-
PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method
On Wed, 10 Feb 2021 04:26:17 GMT, Prasanta Sadhukhan wrote: >> Looks like this is just a bug in the implementation, the null should reset >> the clip. > > I am not sure if it's implementation bug. As Jim Graham has mentioned in > thebug, > >> clip(null) is not legal. >> >> It is *setClip*(null) that clears the clip. > I could rephrase the doc to specify NPE willbe thrrown for null Shape if a > clip is already set. If "*setClip*(null)" has to clear the clip then it should be specified, currently, that method said nothing about the null parameter. - PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method
On Tue, 9 Feb 2021 18:10:44 GMT, Sergey Bylokhov wrote: >>> Also, if there is no clip set, then the spec statement " If s is null, this >>> method clears the current Clip" does not carry any meaning, so in both >>> regard, setClip() should be there, I presume. >> >> The old javadoc is definitely does not conform the current behavior. But as >> of now it clearly says that it will throw NPE if null argument passed. > > Looks like this is just a bug in the implementation, the null should reset > the clip. I am not sure if it's implementation bug. As Jim Graham has mentioned in thebug, > clip(null) is not legal. > > It is *setClip*(null) that clears the clip. I could rephrase the doc to specify NPE willbe thrrown for null Shape if a clip is already set. - PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method
On Tue, 9 Feb 2021 13:59:32 GMT, Alexander Zvegintsev wrote: >> @prsadhuk At first glance i also thought clip() should be called after >> calling setClip() but that is not the case. >> >> If we see SunGraphics2D implementation of clip() we dont exit if there is no >> clip(usrClip object) is set using setClip(). So clip() doesnt depend on >> whether setClip() is used or not. > >> Also, if there is no clip set, then the spec statement " If s is null, this >> method clears the current Clip" does not carry any meaning, so in both >> regard, setClip() should be there, I presume. > > The old javadoc is definitely does not conform the current behavior. But as > of now it clearly says that it will throw NPE if null argument passed. Looks like this is just a bug in the implementation, the null should reset the clip. - PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method
On Tue, 9 Feb 2021 13:13:51 GMT, Jayathirth D V wrote: >> Also, if there is no clip set, then the spec statement " If s is null, this >> method clears the current Clip" does not carry any meaning, so in both >> regard, setClip() should be there, I presume. > > @prsadhuk At first glance i also thought clip() should be called after > calling setClip() but that is not the case. > > If we see SunGraphics2D implementation of clip() we dont exit if there is no > clip(usrClip object) is set using setClip(). So clip() doesnt depend on > whether setClip() is used or not. > Also, if there is no clip set, then the spec statement " If s is null, this > method clears the current Clip" does not carry any meaning, so in both > regard, setClip() should be there, I presume. The old javadoc is definitely does not conform the current behavior. But as of now it clearly says that it will throw NPE if null argument passed. - PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method
On Tue, 9 Feb 2021 12:54:26 GMT, Prasanta Sadhukhan wrote: >> The spec says "s - the Shape to be intersected with the current Clip" so I >> assume it means there should be a current clip set, so that is why I have >> used setClip to "set" a clip. So, setClip() should be there as far I see. > > Also, if there is no clip set, then the spec statement " If s is null, this > method clears the current Clip" does not carry any meaning, so in both > regard, setClip() should be there, I presume. @prsadhuk At first glance i also thought clip() should be called after calling setClip() but that is not the case. If we see SunGraphics2D implementation of clip() we dont exit if there is no clip(usrClip object) is set using setClip(). So clip() doesnt depend on whether setClip() is used or not. - PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method
On Tue, 9 Feb 2021 12:52:55 GMT, Prasanta Sadhukhan
wrote:
>> src/java.desktop/share/classes/java/awt/Graphics2D.java line 1206:
>>
>>> 1204: * @param s the {@code Shape} to be intersected with the current
>>> 1205: * {@code Clip}.
>>> 1206: * @throws NullPointerException if {@code s} is {@code null}
>>
>> Actually it is not always true, you can check it by commenting `setClip()`
>> call in the test.
>
> The spec says "s - the Shape to be intersected with the current Clip" so I
> assume it means there should be a current clip set, so that is why I have
> used setClip to "set" a clip. So, setClip() should be there as far I see.
Also, if there is no clip set, then the spec statement " If s is null, this
method clears the current Clip" does not carry any meaning, so in both regard,
setClip() should be there, I presume.
-
PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method
On Tue, 9 Feb 2021 12:48:44 GMT, Alexander Zvegintsev
wrote:
>> The API doc for Graphics2D.clip(shape s) claims that passing a null argument
>> would actually clear the existing clipping area, which is incorrect.
>> This statement is applicable only to G2D.setClip() and not for the clip()
>> method. G2D.clip() would throw a NullPointerException when it encounters a
>> null argument.
>> Updated spec to rectify this.
>
> src/java.desktop/share/classes/java/awt/Graphics2D.java line 1206:
>
>> 1204: * @param s the {@code Shape} to be intersected with the current
>> 1205: * {@code Clip}.
>> 1206: * @throws NullPointerException if {@code s} is {@code null}
>
> Actually it is not always true, you can check it by commenting `setClip()`
> call in the test.
The spec says "s - the Shape to be intersected with the current Clip" so I
assume it means there should be a current clip set, so that is why I have used
setClip to "set" a clip. So, setClip() should be there as far I see.
-
PR: https://git.openjdk.java.net/jdk/pull/2476
Re: [OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method
On Tue, 9 Feb 2021 11:57:44 GMT, Prasanta Sadhukhan
wrote:
> The API doc for Graphics2D.clip(shape s) claims that passing a null argument
> would actually clear the existing clipping area, which is incorrect.
> This statement is applicable only to G2D.setClip() and not for the clip()
> method. G2D.clip() would throw a NullPointerException when it encounters a
> null argument.
> Updated spec to rectify this.
src/java.desktop/share/classes/java/awt/Graphics2D.java line 1206:
> 1204: * @param s the {@code Shape} to be intersected with the current
> 1205: * {@code Clip}.
> 1206: * @throws NullPointerException if {@code s} is {@code null}
Actually it is not always true, you can check it by commenting `setClip()` call
in the test.
-
PR: https://git.openjdk.java.net/jdk/pull/2476
