Re: [OpenJDK 2D-Dev] RFR: 8272878: JEP 381 cleanup: Remove unused Solaris code in sun.font.TrueTypeGlyphMapper
On Mon, 23 Aug 2021 22:05:13 GMT, Daniel Gredler wrote: > During the recent JEP 381 removal of Solaris code, a few Solaris-specific > constants and private methods were left behind in > sun.font.TrueTypeGlyphMapper. This PR removes these unused odds and ends. I do not have any objection to this looking into comments in future and not in this PR. What about the code that has solaris in it? https://github.com/openjdk/jdk/blob/master/src/java.desktop/unix/classes/sun/awt/X11/XScrollbarPeer.java#L37 https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/native/liblcms/lcms2.h#L534 [it might need change in upstream though] - PR: https://git.openjdk.java.net/jdk/pull/5232
Re: [OpenJDK 2D-Dev] RFR: 8272878: JEP 381 cleanup: Remove unused Solaris code in sun.font.TrueTypeGlyphMapper
On Mon, 23 Aug 2021 22:05:13 GMT, Daniel Gredler wrote: > During the recent JEP 381 removal of Solaris code, a few Solaris-specific > constants and private methods were left behind in > sun.font.TrueTypeGlyphMapper. This PR removes these unused odds and ends. As per JEP 381, comments pointing to Solaris also need to be looked into or purged. There seem to be around 96 mention of Solaris still in java_desktop module...Can it be looked into in this PR? - PR: https://git.openjdk.java.net/jdk/pull/5232
Re: [OpenJDK 2D-Dev] RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked [v6]
On Tue, 17 Aug 2021 06:09:51 GMT, Alexander Zuev wrote: >> Initial implementation and a test case. >> >> The problem is that Aqua LaF shows the focused component with the glow on >> the border, hence when the border is not painted the foxus is not displayed. >> The idea is to paint the glowing border on the focused component anyways. > > Alexander Zuev has updated the pull request incrementally with one additional > commit since the last revision: > > Restore Graphics color before leaving paintFocus() Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/5082
Re: [OpenJDK 2D-Dev] RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked [v4]
On Tue, 17 Aug 2021 06:04:39 GMT, Alexander Zuev wrote: >> You can trace the usage of graphics.getColor() for example in >> AquaMenuItemUI/WindowsMenuUI/BevelBorder/etc to check that the old color >> property is usually saved and then restored. > >> You can trace the usage of graphics.getColor() for example in >> AquaMenuItemUI/WindowsMenuUI/BevelBorder/etc to check that the old color >> property is usually saved and then restored. > > A few lines above paintFocus() is called there is a section that sets color > to something different in case of opaque button and that does not affect the > next step - text painting - since text painter will grab text color and set > the graphics draw color to it. However in the interest of moving forward i > will ad saving the color - after al it is not a performance critical task. Have you checked if button does not have any text? It might affect the drawRoundRect values which are hardcoded now...In MetalButtonUI#paintFocus, it seems they cater to Button focus ring with and without text by taking care of setBounds(). Do we need to do something similar here? - PR: https://git.openjdk.java.net/jdk/pull/5082
Re: [OpenJDK 2D-Dev] RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked [v3]
On Fri, 13 Aug 2021 06:18:49 GMT, Alexander Zuev wrote:
>> Initial implementation and a test case.
>>
>> The problem is that Aqua LaF shows the focused component with the glow on
>> the border, hence when the border is not painted the foxus is not displayed.
>> The idea is to paint the glowing border on the focused component anyways.
>
> Alexander Zuev has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Update copyright year
src/java.desktop/macosx/classes/com/apple/laf/AquaButtonUI.java line 338:
> 336:
> 337: }
> 338: Color ringColor =
> UIManager.getLookAndFeelDefaults().getColor("Focus.color");
I guess we normally call like this in Basic L which is extended by different
L so that it will pick up the defaults from the particular L in question,
otherwise UIManager.getColor(). should suffice as Focus.color is defined in
AquaLookAndFeel.
But I am not sure with this hardcoded values..Can't we leverage viewRect or
textRect to get the required coordinates?
-
PR: https://git.openjdk.java.net/jdk/pull/5082
Re: [OpenJDK 2D-Dev] RFR: 8205138: Remove Applet references from Font2DTest [v2]
On Thu, 12 Aug 2021 19:16:55 GMT, Phil Race wrote: >> Applet support was removed already but the .html file was left as well as >> docs on applet issues >> and a parameter only relevant to applets. > > Phil Race has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains three additional commits since > the last revision: > > - 8205138: Remove Applet references from Font2DTest > - Merge remote-tracking branch 'origin/master' into f2d_applet >Merge > - 8205138: Remove Applet references from Font2DTest Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4831
Re: [OpenJDK 2D-Dev] RFR: 8269951: [macos] Focus not painted in JButton when setBorderPainted(false) is invoked
On Wed, 11 Aug 2021 11:22:50 GMT, Alexander Zuev wrote:
> Initial implementation and a test case.
>
> The problem is that Aqua LaF shows the focused component with the glow on the
> border, hence when the border is not painted the foxus is not displayed. The
> idea is to paint the glowing border on the focused component anyways.
src/java.desktop/macosx/classes/com/apple/laf/AquaButtonUI.java line 333:
> 331: final Border border = b.getBorder();
> 332: if (border instanceof AquaButtonBorder) {
> 333: ((AquaButtonBorder)border).paintButton(b, g, 0, 0,
> b.getWidth(), b.getHeight());
Does having 0,0 not create a problem in multiscreen environment?
-
PR: https://git.openjdk.java.net/jdk/pull/5082
Re: [OpenJDK 2D-Dev] RFR: 8205138: Remove Applet references from Font2DTest
On Mon, 19 Jul 2021 20:50:14 GMT, Phil Race wrote: > Applet support was removed already but the .html file was left as well as > docs on applet issues > and a parameter only relevant to applets. src/demo/share/jfc/Font2DTest/Font2DTest.java line 272: > 270: fileMenu.addSeparator(); > 271: if ( !isApplet ) > 272: fileMenu.add( new MenuItemV2( "Exit", this )); If it is not Applet, should we not retain this line? src/demo/share/jfc/Font2DTest/README.txt line 142: > 140: - There are still some bugs around the error handling. > 141: Most of these problems will usually get fixed when some parameters > 142: are changed, or the screen is refreshed. Are the 1st 2 known problems are not there now? - PR: https://git.openjdk.java.net/jdk/pull/4831
[OpenJDK 2D-Dev] Integrated: 8267940: [macos] java/awt/print/Dialog/DialogOwnerTest.java fails
On Mon, 19 Jul 2021 04:46:59 GMT, Prasanta Sadhukhan wrote: > This test fails to follow the test instruction which says ""On Top" print > dialogs should stay behind the "Owner Window" > but it seems to be the behaviour right from jdk11b18 where DialogOwner class > and this test was inducted via JDK-8203796 not only in mac but in windows too > ie, > "On Top" print dialogs should stay behind the "Owner Window" as per > instruction but is not. > On Top print dialogs actually stay above the "owner window" > > I guess there is some ambiguity in test instruction or clarity in > understanding test instructions is not there > because Test instructions says "For On Top tests all windows should stay > *behind* the owner window." > but "Owner Window" instruction says "For tests that are 'Owned' or 'On Top' > the dialog must always stay *above* this window. " > > Rectified the test instruction to be same as owner window instruction This pull request has now been integrated. Changeset: 9131a8f5 Author:Prasanta Sadhukhan URL: https://git.openjdk.java.net/jdk/commit/9131a8f5f241b04c28a875fddb7a060cc9a3c252 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8267940: [macos] java/awt/print/Dialog/DialogOwnerTest.java fails Reviewed-by: azvegint, prr - PR: https://git.openjdk.java.net/jdk/pull/4823
[OpenJDK 2D-Dev] RFR: 8267940: [macos] java/awt/print/Dialog/DialogOwnerTest.java fails
This test fails to follow the test instruction which says ""On Top" print dialogs should stay behind the "Owner Window" but it seems to be the behaviour right from jdk11b18 where DialogOwner class and this test was inducted via JDK-8203796 not only in mac but in windows too ie, "On Top" print dialogs should stay behind the "Owner Window" as per instruction but is not. On Top print dialogs actually stay above the "owner window" I guess there is some ambiguity in test instruction or clarity in understanding test instructions is not there because Test instructions says "For On Top tests all windows should stay *behind* the owner window." but "Owner Window" instruction says "For tests that are 'Owned' or 'On Top' the dialog must always stay *above* this window. " Rectified the test instruction to be same as owner window instruction - Commit messages: - [macos] java/awt/print/Dialog/DialogOwnerTest.java fails Changes: https://git.openjdk.java.net/jdk/pull/4823/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4823=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8267940 Stats: 2 lines in 1 file changed: 1 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4823.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4823/head:pull/4823 PR: https://git.openjdk.java.net/jdk/pull/4823
Re: [OpenJDK 2D-Dev] RFR: 8266159: macOS ARM + Metal pipeline shows artifacts on Swing Menu with Java L [v3]
On Tue, 8 Jun 2021 07:22:48 GMT, Ajit Ghaisas wrote: >> This PR fixes an issue exclusively seen on Apple M1 systems when SwingSet2 >> demo is run with uiScale=1.0. >> >> **Issue :** >> SwingSet2 Demo - As reported in JBS description >> J2DDemo - As reported in a comment on JBS >> >> **Root Cause :** >> DrawPixel path is used only with uiScale=1.0. >> MTLPrimitiveTypePoint is used to draw a pixel while encoding a render >> command. >> As mentioned in the documentation - >> https://developer.apple.com/documentation/metal/mtlprimitivetype/mtlprimitivetypepoint?language=objc >> >> "The vertex shader must provide [[point_size]], or the point size is >> undefined." >> >> In our shader functions, we do not define this point size. It is harmless on >> x86_64 based mac systems, but causes visual artifacts on M1 mac systems. >> >> **Solution :** >> Explicitly define point size in shader functions that draw >> MTLPrimitiveTypePoint. > > Ajit Ghaisas has updated the pull request incrementally with one additional > commit since the last revision: > > fix review comment on test test/jdk/java/awt/Graphics/DrawOvalTest.java line 26: > 24: /** > 25: * @test > 26: * @key headful I feel there's nop need to have headful tag for this test as there is no frame being made visible, all rendering are into volatileimage - PR: https://git.openjdk.java.net/jdk/pull/4356
Re: [OpenJDK 2D-Dev] RFR: 8266159: macOS ARM + Metal pipeline shows artifacts on Swing Menu with Java L [v2]
On Mon, 7 Jun 2021 12:03:35 GMT, Ajit Ghaisas wrote:
>> This PR fixes an issue exclusively seen on Apple M1 systems when SwingSet2
>> demo is run with uiScale=1.0.
>>
>> **Issue :**
>> SwingSet2 Demo - As reported in JBS description
>> J2DDemo - As reported in a comment on JBS
>>
>> **Root Cause :**
>> DrawPixel path is used only with uiScale=1.0.
>> MTLPrimitiveTypePoint is used to draw a pixel while encoding a render
>> command.
>> As mentioned in the documentation -
>> https://developer.apple.com/documentation/metal/mtlprimitivetype/mtlprimitivetypepoint?language=objc
>>
>> "The vertex shader must provide [[point_size]], or the point size is
>> undefined."
>>
>> In our shader functions, we do not define this point size. It is harmless on
>> x86_64 based mac systems, but causes visual artifacts on M1 mac systems.
>>
>> **Solution :**
>> Explicitly define point size in shader functions that draw
>> MTLPrimitiveTypePoint.
>
> Ajit Ghaisas has updated the pull request incrementally with one additional
> commit since the last revision:
>
> add automated test
test/jdk/java/awt/Graphics/DrawOvalTest.java line 54:
> 52: render(vi.createGraphics());
> 53:
> 54: BufferedImage snapshot = vi.getSnapshot();
I guess we need to obtain this snapshot in a do-while loop to check for
volatile image contents is not lost similar to
do {
vi.validate(gc);
render();
vi.getShapshot();
} while(vi.getContentsLost())
-
PR: https://git.openjdk.java.net/jdk/pull/4356
Re: [OpenJDK 2D-Dev] RFR: JDK-8263362: Avoid division by 0 in java/awt/font/TextJustifier.java justify [v2]
On Wed, 5 May 2021 14:11:16 GMT, Matthias Baesken wrote: >> In java/awt/font/TextJustifier.java justify-method there is a potential code >> path where divison by zero might happen , see also the Sonar finding >> https://sonarcloud.io/project/issues?id=shipilev_jdk=AXcqMwpm8sPJZZzONu1k=false=CRITICAL=BUG >> >> >> boolean hitLimit = (weight == 0) || (!lastPass && ((delta < 0) >> == (delta < gslimit))); >> boolean absorbing = hitLimit && absorbweight > 0; >> // predivide delta by weight >> float weightedDelta = delta / weight; // not used if weight == 0 >> >> In case of (weight == 0) the division should not be done because the value >> of weightedDelta is unused in this case anyway. > > Matthias Baesken has updated the pull request incrementally with one > additional commit since the last revision: > > adjust absorbweight check Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2912
Re: [OpenJDK 2D-Dev] RFR: JDK-8263362: Avoid division by 0 in java/awt/font/TextJustifier.java justify
On Tue, 4 May 2021 15:00:29 GMT, Matthias Baesken wrote: >> I meant that since we are dividing by weight and absorbweight >> ` weightedDelta = delta / weight;` >> and >> weightedAbsorb = (delta - gslimit) / absorbweight; >> where both weight and absorbweight uses += gi.weight values so if one is >> checking for >0 >> then to be consistent, in my opinion, >> other one should be same or absorbweight also probably should check !=0. > > should we then better check for > if (weight > 0) > instead of > if (weight != 0) > like it is done for absorbweight ? > (If you think consistency is important here) I guess since it was mentioned before that nothing prevents the value from being -ve, I guess better will be to use != 0 check for absorbweight too? - PR: https://git.openjdk.java.net/jdk/pull/2912
[OpenJDK 2D-Dev] Integrated: 8266284: ProblemList java/awt/Graphics2D/DrawString/DrawRotatedStringUsingRotatedFont.java
On Thu, 29 Apr 2021 07:57:17 GMT, Prasanta Sadhukhan wrote: > It seems the newly deproblemlisted test > DrawRotatedStringUsingRotatedFont.java which is paasing in windows,linux-x64 > and macos-x64 is failing in newly added linux-aarch64 systems. > Problemlisting the test for now as it is font layout issue which might need > some upgrade in harfbuzz library in those CI systems or maybe in product. This pull request has now been integrated. Changeset: 5574922f Author:Prasanta Sadhukhan URL: https://git.openjdk.java.net/jdk/commit/5574922ff69e976bf29f1d766a4c1a67d341ef8c Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod 8266284: ProblemList java/awt/Graphics2D/DrawString/DrawRotatedStringUsingRotatedFont.java Reviewed-by: jdv - PR: https://git.openjdk.java.net/jdk/pull/3790
[OpenJDK 2D-Dev] RFR: 8266284: ProblemList java/awt/Graphics2D/DrawString/DrawRotatedStringUsingRotatedFont.java
It seems the newly deproblemlisted test DrawRotatedStringUsingRotatedFont.java which is paasing in windows,linux-x64 and macos-x64 is failing in newly added linux-aarch64 systems. Problemlisting the test for now as it is font layout issue which might need some upgrade in harfbuzz library in those CI systems or maybe in product. - Commit messages: - 8266284: ProblemList java/awt/Graphics2D/DrawString/DrawRotatedStringUsingRotatedFont.java Changes: https://git.openjdk.java.net/jdk/pull/3790/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3790=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8266284 Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3790.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3790/head:pull/3790 PR: https://git.openjdk.java.net/jdk/pull/3790
Re: [OpenJDK 2D-Dev] RFR: 8263984: Invalidate printServices when there are no printers [v3]
I have not looked at entire file but since this function was modified, I made that suggestion. I am not that particular if you wish to ignore that. Regards Prasanta Get Outlook for Android<https://aka.ms/AAb9ysg> From: 2d-dev <2d-dev-r...@openjdk.java.net> on behalf of Alexey Ivanov Sent: Wednesday, April 7, 2021 4:22:29 PM To: 2d-dev@openjdk.java.net <2d-dev@openjdk.java.net> Subject: Re: [OpenJDK 2D-Dev] RFR: 8263984: Invalidate printServices when there are no printers [v3] On Wed, 7 Apr 2021 04:12:55 GMT, Prasanta Sadhukhan wrote: >> [Code Conventions for >> Java](https://www.oracle.com/java/technologies/javase/codeconventions-indentation.html#248) >> say, “Line wrapping for `if` statements should generally use the 8-space >> rule, since conventional (4 space) indentation makes seeing the body >> difficult.” (It's the second to last block on the page.) > > If we are adding a new line, then I think we should need to add at l129, l138 > otherwise it will look odd doing it at one place only. I haven't touched that code at all. Not that odd because it's isolated to the new function now. What is your suggestion? Refactor all if statements in these two functions? Submit a separate bug for refactor all if statements the entire file? Revert back to no new line, leaving this particular if untouched as well? - PR: https://git.openjdk.java.net/jdk/pull/3151
Re: [OpenJDK 2D-Dev] RFR: 8263984: Invalidate printServices when there are no printers [v3]
On Tue, 6 Apr 2021 20:39:04 GMT, Alexey Ivanov wrote: >> I added a blank line between the condition and the body. >> Does it look good? @mrserb, @jayathirthrao > > [Code Conventions for > Java](https://www.oracle.com/java/technologies/javase/codeconventions-indentation.html#248) > say, “Line wrapping for `if` statements should generally use the 8-space > rule, since conventional (4 space) indentation makes seeing the body > difficult.” (It's the second to last block on the page.) If we are adding a new line, then I think we should need to add at l129, l138 otherwise it will look odd doing it at one place only. - PR: https://git.openjdk.java.net/jdk/pull/3151
Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline [v4]
On Thu, 1 Apr 2021 14:15:37 GMT, Jayathirth D V wrote: >> In MTLBlitLoops.copyArea() we use standalone encoder which has no clip state >> information because of which we ignore clip parameters set in rect clip and >> shape clip. We need to query and use encoders from EncoderManager to honour >> clip states in copyArea. > > Jayathirth D V has updated the pull request incrementally with one additional > commit since the last revision: > > Refactor code to use drawTex2Tex Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3283
Re: [OpenJDK 2D-Dev] RFR: 8264475: CopyArea ignores clip state in metal rendering pipeline [v3]
On Thu, 1 Apr 2021 05:49:51 GMT, Jayathirth D V wrote: >> In MTLBlitLoops.copyArea() we use standalone encoder which has no clip state >> information because of which we ignore clip parameters set in rect clip and >> shape clip. We need to query and use encoders from EncoderManager to honour >> clip states in copyArea. > > Jayathirth D V has updated the pull request incrementally with one additional > commit since the last revision: > > Comment update src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLBlitLoops.m line 826: > 824: id interEncoder = > 825: [mtlc.encoderManager getTextureEncoder:interTexture > 826:isSrcOpaque:dstOps->isOpaque Should it not be srcOps->isOpaque? src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLBlitLoops.m line 854: > 852: atIndex:MeshVertexBuffer]; > 853: [finalEncoder setFragmentTexture:interTexture atIndex: 0]; > 854: [finalEncoder drawPrimitives:MTLPrimitiveTypeTriangle > vertexStart:0 Can't we reuse drawTex2Tex() for this snippet? - PR: https://git.openjdk.java.net/jdk/pull/3283
Re: [OpenJDK 2D-Dev] RFR: 8263984: Invalidate printServices when there are no printers
On Wed, 24 Mar 2021 09:25:56 GMT, Alexey Ivanov wrote: >> src/java.desktop/windows/classes/sun/print/PrintServiceLookupProvider.java >> line 120: >> >>> 118: // don't get the default. >>> 119: invalidateServices(); >>> 120: printServices = new PrintService[0]; >> >> I am not sure this call to invalidateServices() is needed as we are creating >> a new `printServices` object of 0 length so making the old printServices >> invalid is redundant as it is same global variable. > > That's exactly the problem I am addressing. > > In the regular case, the existing services left in `printServices` are > invalidated before `newServices` is assigned to it. Yet no existing services > are invalidated if all the printers were removed from the system. > > Why is it needed to invalidate deleted services if the list of printers is > updated and if at least one printer is left in the system? Why is it not > needed to invalidate deleted services if all the printers are removed from > the system? Since this is windows specific code, I am not sure if system will not have any printers. I guess by default, Microsoft XPS Document Writer, Microsoft Print-to-PDF, Fax are present in printers list. That is the reason we do not have getPrintService() return 0 printer although there may not be any real printer present in windows system...reason for some failure in jtreg test which caused us to use `@key printer `tag in those tests to make real printers are configured. - PR: https://git.openjdk.java.net/jdk/pull/3151
Re: [OpenJDK 2D-Dev] RFR: 8263984: Invalidate printServices when there are no printers
On Tue, 23 Mar 2021 13:45:33 GMT, Alexey Ivanov wrote: > When `getAllPrinterNames()` returns null, the list of `printServices` is > assigned a new empty array without invalidating old services which were in > the array before. > > The old print services should be invalidated. src/java.desktop/windows/classes/sun/print/PrintServiceLookupProvider.java line 120: > 118: // don't get the default. > 119: invalidateServices(); > 120: printServices = new PrintService[0]; I am not sure this call to invalidateServices() is needed as we are creating a new `printServices` object of 0 length so making the old printServices invalid is redundant as it is same global variable. - PR: https://git.openjdk.java.net/jdk/pull/3151
Re: [OpenJDK 2D-Dev] RFR: 8262470: Printed GlyphVector outline with low DPI has bad quality on Windows [v2]
On Thu, 18 Mar 2021 12:29:01 GMT, Alexander Scherbatiy wrote: >> Printing text using GlyphVector outline has bad quality on printers with low >> DPI on Windows. >> The GDI system used for text printing on Windows accepts only integer path >> coordinates. >> Rounding GlyphVector outline coordinates leads to distorted printed text. >> >> The issue had been reported as JDK-8256264 but was reverted because of the >> regression JDK-8259007 "This test printed a blank page". >> >> The fix JDK-8256264 scaled coordinates in wPrinterJob.moveTo()/lineTo() >> methods up and scaled transforms in wPrinterJob.beginPath()/endPath() down. >> >> The regression was in the WPathGraphics.deviceDrawLine() method which uses >> wPrinterJob.moveTo()/lineTo() methods without surrounding them with >> wPrinterJob.beginPath()/endPath() so the line coordinates were only scaled >> up. >> >> I tried to put wPrinterJob.beginPath()/endPath() methods around >> wPrinterJob.moveTo()/lineTo() in the method WPathGraphics.deviceDrawLine() >> but the line was not drawn at all even without scaling coordinates up and >> transform down (without JDK-8256264 fix). It looks like GDI treats this case >> as an empty shape. >> >> The proposed fix applies path coordinates and transform scaling only in >> WPathGraphics.convertToWPath() method. >> The one more PathPrecisionScaleFactorShapeTest.java manual test is added >> which checks that all methods that draw paths in WPathGraphics are used: >> line in WPathGraphics.deviceDrawLine() and SEG_LINETO/SEG_QUADTO/SEG_CUBICTO >> in WPathGraphics.convertToWPath() . >> >> The `java/awt/print` and `java/awt/PrintJob` automatic and manual tests were >> run on Windows 10 Pro with the fix. >> >> There are two failed automated tests which fail without the fix as well: >> java/awt/print/PrinterJob/GlyphPositions.java >> java/awt/print/PrinterJob/PSQuestionMark.java >> >> The following manual tests have issues on my system: >> - `java/awt/print/Dialog/PrintDlgPageable.java` >> java.lang.IllegalAccessException: class >> com.sun.javatest.regtest.agent.MainWrapper$MainThread cannot access a member >> of class PrintDlgPageable with modifiers "public static" >> >> - `java/awt/print/PrinterJob/PrintAttributeUpdateTest.java` I select pages >> radio button, press the print button but the test does not finish and I do >> not see any other dialogs with pass/fail buttons. >> >> - `java/awt/PrintJob/PrintCheckboxTest/PrintCheckboxManualTest.java` Tests >> that there is no ClassCastException thrown in printing checkbox and >> scrollbar with XAWT. Error. Can't find HTML file: >> test\jdk\java\awt\PrintJob\PrintCheckboxTest\PrintCheckboxManualTest.html >> >> >> - `java/awt/print/PrinterJob/SecurityDialogTest.java` A windows with >> instructions is shown but it does not contain print/pass/fail buttons and >> it is not possible to close the window. >> >> - The tests below fail with "Error. Parse Exception: Arguments to `manual' >> option not supported: yesno" message: >> java/awt/print/Dialog/DialogOrient.java >> java/awt/print/Dialog/DialogType.java >> java/awt/print/PrinterJob/ImagePrinting/ClippedImages.java >> java/awt/print/PrinterJob/ImagePrinting/ImageTypes.java >> java/awt/print/PrinterJob/ImagePrinting/PrintARGBImage.java >> java/awt/print/PrinterJob/PageDialogTest.java >> java/awt/print/PrinterJob/PageRanges.java >> java/awt/print/PrinterJob/PageRangesDlgTest.java >> java/awt/print/PrinterJob/PrintGlyphVectorTest.java >> java/awt/print/PrinterJob/PrintLatinCJKTest.java >> java/awt/print/PrinterJob/PrintTextTest.java >> java/awt/print/PrinterJob/SwingUIText.java >> java/awt/PrintJob/ConstrainedPrintingTest/ConstrainedPrintingTest.java >> java/awt/PrintJob/PageSetupDlgBlockingTest/PageSetupDlgBlockingTest.java >> java/awt/PrintJob/SaveDialogTitleTest.java > > Alexander Scherbatiy has updated the pull request incrementally with two > additional commits since the last revision: > > - Use DASSERT to check SetGraphicsMode and WorldTransform results > - Change setGraphicsMode() type to void Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2756
Re: [OpenJDK 2D-Dev] RFR: 8247370: Clean up unused printing code in awt_PrintJob.cpp
On Thu, 18 Mar 2021 22:59:57 GMT, Phil Race wrote: > This removes a long time un-used code path. Maybe we should put noreg-cleanup in JBS. - Marked as reviewed by psadhukhan (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3083
Re: [OpenJDK 2D-Dev] RFR: 8263311: Watch registry changes for remote printers update instead of polling
On Wed, 10 Mar 2021 15:38:27 GMT, Alexey Ivanov wrote: > [JDK-8153732](https://bugs.openjdk.java.net/browse/JDK-8153732) implemented > polling for remote printers. > That bug description also mentions watching the registry for changes and > links to the article which describes the method yet it does so in terms of > WMI. Using WMI is not necessary to watch for the registry updates. > > It is possible to replace polling mechanism with registry change > notifications. If the registry at `HKCU\Printers\Connections` is updated, > refresh the list of print services. > > It works perfectly well in my own testing with sharing a Generic / Text Only > printer from another laptop. The notification comes as soon as the printer is > installed, it results in a new key created under `Connections`. If a remote > printer is removed, the notification is also triggered as the key > corresponding to that printer is removed from the registry. > > I updated the steps in the manual test: `RemotePrinterStatusRefresh.java`. Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2915
Re: [OpenJDK 2D-Dev] RFR: 8263311: Watch registry changes for remote printers update instead of polling
On Fri, 12 Mar 2021 12:30:13 GMT, Alexey Ivanov wrote: >> I can understand that as a user, you cannot /shouldn't change the name of a >> remote network printer but a network admin can change the name, so shouldn't >> we get notification on that name change when this method gets called after >> the name change? or would it be notified as a new printer in that case? Then >> what will happen to the old stale printer in the registry? > > You can't. Try it yourself. The printer connection is per-user, after all > it's stored under HKCU. > > Windows displays the name of remote printers as “Generic / Text Only on > 192.168.1.18”: _the name of the printer_ and _the remote host_. > > If you open the *Printer Properties* dialog of a remote printer and edit its > name, you change the name of the printer on the remote host which shares it. > It changes *absolutely nothing* on the local system. > >> shouldn't we get notification on that name change when this method gets >> called after the name change? > > Yes, we should. But we cannot. > > For Windows, nothing has changed on the local system, it's the remote system > that has been changed. > >> or would it be notified as a new printer in that case? > > No, it wouldn't _because nothing has changed on the local system_. > >> Then what will happen to the old stale printer in the registry? > > Nothing, again. It just stays there but Windows reports an error if you try > to open its Printer Properties, Windows reports an error if you try to print > to it. I tried printing with Java and Notepad: the behaviour is the same. The > difference is that Notepad displays the error message whereas Java throws an > exception (it depends on the Java app, I guess). > > The behaviour aligns to the one seen when the printer is unreachable because > of, let's say, network issues. OK. - PR: https://git.openjdk.java.net/jdk/pull/2915
Re: [OpenJDK 2D-Dev] RFR: 8263311: Watch registry changes for remote printers update instead of polling
On Fri, 12 Mar 2021 11:29:17 GMT, Alexey Ivanov wrote: >> Is this only about addition/removal? What about printer name change? >> Shouldn't we get notified in that case as trying to print on printer with >> old name will not find the printer!! >> If yes, in that regard I guess REG_NOTIFY_CHANGE_LAST_SET is the one to go >> for as it states >> `"Notify the caller of changes to a value of the key. This can include >> adding or deleting a value, or changing an existing value. "` > >> Is this only about addition/removal? What about printer name change? > > You cannot change the name of a remote printer. > > Opening *Printer Properties* dialog from the printer context menu on the > local host and editing its name changes the name of the printer on the remote > host which shares it. Windows warns, “This is a shared printer. If you rename > a shared printer, existing connections to this printer from other computers > will break and will have to be created again.” > > Nothing has been updated on the local system after renaming the printer. As > the warning said, the printer does not work any more because it refers to a > printer that does not exist. > > To fix this, one has to add a new remote printer which refers to the new name > of the printer on the remote host. > >> Shouldn't we get notified in that case as trying to print on printer with >> old name will not find the printer!! > > I'm afraid there's nothing Java can do to mitigate renaming the printer. > >> If yes, in that regard I guess `REG_NOTIFY_CHANGE_LAST_SET` is the one to go >> for as it states >> _"Notify the caller of changes to a value of the key. This can include >> adding or deleting a value, or changing an existing value. "_ > > Since no values are created, removed, or changed when a remote printer is > added or removed under `Connections` key, adding `REG_NOTIFY_CHANGE_LAST_SET` > to `dwNotifyFilter` is not required. There are some values stored in the > printer key but Java does not use them directly; if any value is changed > there, getting notifications about such an event only creates noise. > > So, as far as Java is concerned, getting notifications about new keys being > created or removed under `Connections` is enough. This is what > `REG_NOTIFY_CHANGE_NAME` filter does. I can understand that as a user, you cannot /shouldn't change the name of a remote network printer but a network admin can change the name, so shouldn't we get notification on that name change when this method gets called after the name change? or would it be notified as a new printer in that case? Then what will happen to the old stale printer in the registry? - PR: https://git.openjdk.java.net/jdk/pull/2915
Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Thu, 11 Mar 2021 18:00:15 GMT, Ajit Ghaisas wrote: >> **Description :** >> This is the implementation of [JEP 382 : New macOS Rendering >> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8238361) >> It implements a Java 2D internal rendering pipeline for macOS using the >> Apple Metal API. >> The entire work on this was done under [OpenJDK Project - >> Lanai](http://openjdk.java.net/projects/lanai/) >> >> We iterated through several Early Access (EA) builds and have reached a >> stage where it is ready to be integrated to openjdk/jdk. The latest EA build >> is available at - https://jdk.java.net/lanai/ >> >> A new option -Dsun.java2d.metal=true | True needs to be used to use this >> pipeline. >> >> **Testing :** >> This implementation has been tested with the tests present at - [Test Plan >> for JEP 382: New macOS Rendering >> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8251396) >> >> **Note to reviewers :** >> 1) Default rendering pipeline on macOS has not been changed by this PR. >> OpenGL still stays as the default rendering pipeline and Metal rendering >> pipeline is optional to choose. >> >> 2) To apply and test this PR - >> To enable the metal pipeline you must specify on command line >> -Dsun.java2d.metal=true (No message will be printed in this case) or >> -Dsun.java2d.metal=True (A message indicating Metal rendering pipeline is >> enabled gets printed) >> >> 3) Review comments (including some preliminary informal review comments) are >> tracked with JBS issues - https://bugs.openjdk.java.net/issues/?filter=40598 > > Ajit Ghaisas has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains 47 additional > commits since the last revision: > > - Lanai PR#214 - 8263324 - avu > - Merge branch 'master' into 8260931_lanai_JEP_branch > - Lanai PR#213 - 8263325 - avu > - Lanai PR#212 - 8259825 - aghaisas > - Lanai PR#211 - 8262882 - aghaisas > - Merge branch 'master' into 8260931_lanai_JEP_branch > - Lanai PR#210 - 8263159 - jdv > - Lanai PR#209 - 8262936 - jdv > - Lanai PR#208 - 8262928 - jdv > - Lanai PR#207 - 8262750 - jdv > - ... and 37 more: > https://git.openjdk.java.net/jdk/compare/d4addb43...369c3d21 Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Thu, 11 Mar 2021 18:00:15 GMT, Ajit Ghaisas wrote: >> **Description :** >> This is the implementation of [JEP 382 : New macOS Rendering >> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8238361) >> It implements a Java 2D internal rendering pipeline for macOS using the >> Apple Metal API. >> The entire work on this was done under [OpenJDK Project - >> Lanai](http://openjdk.java.net/projects/lanai/) >> >> We iterated through several Early Access (EA) builds and have reached a >> stage where it is ready to be integrated to openjdk/jdk. The latest EA build >> is available at - https://jdk.java.net/lanai/ >> >> A new option -Dsun.java2d.metal=true | True needs to be used to use this >> pipeline. >> >> **Testing :** >> This implementation has been tested with the tests present at - [Test Plan >> for JEP 382: New macOS Rendering >> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8251396) >> >> **Note to reviewers :** >> 1) Default rendering pipeline on macOS has not been changed by this PR. >> OpenGL still stays as the default rendering pipeline and Metal rendering >> pipeline is optional to choose. >> >> 2) To apply and test this PR - >> To enable the metal pipeline you must specify on command line >> -Dsun.java2d.metal=true (No message will be printed in this case) or >> -Dsun.java2d.metal=True (A message indicating Metal rendering pipeline is >> enabled gets printed) >> >> 3) Review comments (including some preliminary informal review comments) are >> tracked with JBS issues - https://bugs.openjdk.java.net/issues/?filter=40598 > > Ajit Ghaisas has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains 47 additional > commits since the last revision: > > - Lanai PR#214 - 8263324 - avu > - Merge branch 'master' into 8260931_lanai_JEP_branch > - Lanai PR#213 - 8263325 - avu > - Lanai PR#212 - 8259825 - aghaisas > - Lanai PR#211 - 8262882 - aghaisas > - Merge branch 'master' into 8260931_lanai_JEP_branch > - Lanai PR#210 - 8263159 - jdv > - Lanai PR#209 - 8262936 - jdv > - Lanai PR#208 - 8262928 - jdv > - Lanai PR#207 - 8262750 - jdv > - ... and 37 more: > https://git.openjdk.java.net/jdk/compare/b163fb99...369c3d21 src/java.desktop/macosx/classes/sun/awt/CGraphicsConfig.java line 82: > 80: * Creates a new SurfaceData that will be associated with the given > 81: * CGLLayer. > 82: */ I guess we need to specify CGLLayer/MTLLayer as now we are implementing this method in both pipeline. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: [OpenJDK 2D-Dev] RFR: 8263311: Watch registry changes for remote printers update instead of polling
On Thu, 11 Mar 2021 14:59:55 GMT, Alexey Ivanov wrote: >> [JDK-8153732](https://bugs.openjdk.java.net/browse/JDK-8153732) implemented >> polling for remote printers. >> That bug description also mentions watching the registry for changes and >> links to the article which describes the method yet it does so in terms of >> WMI. Using WMI is not necessary to watch for the registry updates. >> >> It is possible to replace polling mechanism with registry change >> notifications. If the registry at `HKCU\Printers\Connections` is updated, >> refresh the list of print services. >> >> It works perfectly well in my own testing with sharing a Generic / Text Only >> printer from another laptop. The notification comes as soon as the printer >> is installed, it results in a new key created under `Connections`. If a >> remote printer is removed, the notification is also triggered as the key >> corresponding to that printer is removed from the registry. >> >> I updated the steps in the manual test: `RemotePrinterStatusRefresh.java`. > > src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp line 258: > >> 256: >> REG_NOTIFY_CHANGE_NAME, >> 257: NULL, >> 258: FALSE); > > [`RegNotifyChangeKeyValue`](https://docs.microsoft.com/en-us/windows/win32/api/winreg/nf-winreg-regnotifychangekeyvalue) > notifies the caller about changes to the attributes or contents of a > specified registry key. > > • `hKey`: A handle to `HKEY_CURRENT_USER\Printers\Connections` key which is > opened above. > • `bWatchSubtree = TRUE`: The function reports changes in the specified key > and its subkeys. > • `dwNotifyFilter = REG_NOTIFY_CHANGE_NAME`: Notify the caller if a subkey is > added or deleted. > • `hEvent = NULL`: If `fAsynchronous` is FALSE, `hEvent` is ignored. > • `fAsynchronous = FALSE`: The function does not return until a change has > occurred. > > When a new remote printer is added, a new key is created under > `HKCU\Printers\Connections`; when an existing remote printer is removed, the > key below `Connections` is removed; no values are added or removed in > `Connections` key, thus `REG_NOTIFY_CHANGE_LAST_SET` filter is not needed. Is this only about addition/removal? What about printer name change? Shouldn't we get notified in that case as trying to print on printer with old name will not find the printer!! If yes, in that regard I guess REG_NOTIFY_CHANGE_LAST_SET is the one to go for as it states `"Notify the caller of changes to a value of the key. This can include adding or deleting a value, or changing an existing value. "` - PR: https://git.openjdk.java.net/jdk/pull/2915
Re: [OpenJDK 2D-Dev] RFR: 8263311: Watch registry changes for remote printers update instead of polling
On Thu, 11 Mar 2021 14:49:59 GMT, Alexey Ivanov wrote: >> I also am not sure on this. But I think since this is for remote printer, >> sometimes network availability issue might be there so it may fail more >> compared to local printer so I guess we should give this method a fair >> chance, more than that of local printer change, and not bail out on one >> failure.maybe try out after some duration...or 5 times spaced out...as >> you did for the other EnumPrinter fix.. > > No, network connectivity cannot affect this. The function watches for > registry changes, specifically keys created or removed under > `HKCU\Printers\Connections`. If network is down, you won't be able to add a > new printer. Yet you can still remove an existing printer. > > The case with `EnumPrinters` is very different. A printer may be renamed or a > new printer may be added therefore the allocated buffer becomes to small to > fit the updated data. Thus retrying with larger buffer makes perfect sense. > > In this case, `RegNotifyChangeKeyValue` could fail only because of invalid > parameters. If it does, it will fail on the retry because the parameters > haven't changed. > > In that sense, it's the same as with local printers. If the wait/notification > function fails the first time, it will likely fail the second time and so on… Ok - PR: https://git.openjdk.java.net/jdk/pull/2915
Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Fri, 12 Mar 2021 00:48:58 GMT, Sergey Bylokhov wrote:
>> Ajit Ghaisas has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains 47 additional
>> commits since the last revision:
>>
>> - Lanai PR#214 - 8263324 - avu
>> - Merge branch 'master' into 8260931_lanai_JEP_branch
>> - Lanai PR#213 - 8263325 - avu
>> - Lanai PR#212 - 8259825 - aghaisas
>> - Lanai PR#211 - 8262882 - aghaisas
>> - Merge branch 'master' into 8260931_lanai_JEP_branch
>> - Lanai PR#210 - 8263159 - jdv
>> - Lanai PR#209 - 8262936 - jdv
>> - Lanai PR#208 - 8262928 - jdv
>> - Lanai PR#207 - 8262750 - jdv
>> - ... and 37 more:
>> https://git.openjdk.java.net/jdk/compare/aca5b193...369c3d21
>
> src/java.desktop/macosx/native/libawt_lwawt/awt/AWTSurfaceLayers.m line 73:
>
>> 71: // Updates back buffer size of the layer if it's an OpenGL/Metal layer
>> 72: // including all OpenGL/Metal sublayers
>> 73: + (void) repaintLayersRecursively:(CALayer*)aLayer {
>
> The operation of this code is still being investigated. @prsadhuk please add
> a bugid here.
JDK-8263485
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: [OpenJDK 2D-Dev] RFR: 8263311: Watch registry changes for remote printers update instead of polling
On Thu, 11 Mar 2021 10:54:12 GMT, Alexey Ivanov wrote:
>> src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp line 259:
>>
>>> 257: NULL,
>>> 258: FALSE);
>>> 259: if (keepMonitoring) {
>>
>> I guess having "FALSE" as fAsynchronous value mean the function does not
>> return until a change has occurred so do we still need this do-while
>> monitoring loop? And if the function fails once, should we stop monitoring?
>
>> I guess having "FALSE" as fAsynchronous value mean the function does not
>> return until a change has occurred so do we still need this do-while
>> monitoring loop?
>
> You're right, `FALSE` for `fAsynchronous` means the function doesn't return
> until a change occurred.
>
> If a change occurs, we refresh the list of print services and then start to
> wait again. If we exit the loop, we'll not catch other changes that may occur.
>
>> And if the function fails once, should we stop monitoring?
> I followed Sergey's approach in `notifyLocalPrinterChange`, namely if
> `FindNextPrinterChangeNotification` returns an error, we quit the loop.
>
> I can't see how we can fix the error if it occurs. Will it succeed the next
> time? Probably not. Thus I decided to quit the loop in case of an error.
I also am not sure on this. But I think since this is for remote printer,
sometimes network availability issue might be there so it may fail more
compared to local printer so I guess we should give this method a fair chance,
more than that of local printer change, and not bail out on one
failure.maybe try out after some duration...or 5 times spaced out...as you
did for the other EnumPrinter fix..
-
PR: https://git.openjdk.java.net/jdk/pull/2915
Re: [OpenJDK 2D-Dev] RFR: 8263311: Watch registry changes for remote printers update instead of polling
On Wed, 10 Mar 2021 15:38:27 GMT, Alexey Ivanov wrote:
> [JDK-8153732](https://bugs.openjdk.java.net/browse/JDK-8153732) implemented
> polling for remote printers.
> That bug description also mentions watching the registry for changes and
> links to the article which describes the method yet it does so in terms of
> WMI. Using WMI is not necessary to watch for the registry updates.
>
> It is possible to replace polling mechanism with registry change
> notifications. If the registry at `HKCU\Printers\Connections` is updated,
> refresh the list of print services.
>
> It works perfectly well in my own testing with sharing a Generic / Text Only
> printer from another laptop. The notification comes as soon as the printer is
> installed, it results in a new key created under `Connections`. If a remote
> printer is removed, the notification is also triggered as the key
> corresponding to that printer is removed from the registry.
>
> I updated the steps in the manual test: `RemotePrinterStatusRefresh.java`.
src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp line 259:
> 257: NULL,
> 258: FALSE);
> 259: if (keepMonitoring) {
I guess having "FALSE" as fAsynchronous value mean the function does not
return until a change has occurred so do we still need this do-while monitoring
loop? And if the function fails once, should we stop monitoring?
-
PR: https://git.openjdk.java.net/jdk/pull/2915
Re: [OpenJDK 2D-Dev] RFR: JDK-8263362: Avoid division by 0 in java/awt/font/TextJustifier.java justify
On Thu, 11 Mar 2021 06:45:05 GMT, Matthias Baesken wrote: >> By that same logic, then shouldn't absorbweight also be checked as != 0 >> instead of > 0 as that also uses += gi.weight > > Hi, I am not sure about the absorbweight check; but currently the > calculated value weightedAbsorb is only used when absorbing is true. And > there the > 0 check is present too > > boolean absorbing = hitLimit && absorbweight > 0; I meant that since we are dividing by weight and absorbweight ` weightedDelta = delta / weight;` and weightedAbsorb = (delta - gslimit) / absorbweight; where both weight and absorbweight uses += gi.weight values so if one is checking for >0 then to be consistent, in my opinion, other one should be same or absorbweight also probably should check !=0. - PR: https://git.openjdk.java.net/jdk/pull/2912
Re: [OpenJDK 2D-Dev] RFR: JDK-8263362: Avoid division by 0 in java/awt/font/TextJustifier.java justify
On Wed, 10 Mar 2021 18:48:50 GMT, Phil Race wrote: >> Hi, I am not sure about the weight > 0 check ; weight is initialized with >> 0: weight = 0; and later some values are potentially added up to weight: >> weight += gi.weight; >> I am not sure about those gi.weight values, maybe they can be negative too ? > > Nothing throws an exception or otherwise prevent this being negative but that > would be a weird usage. Typically the weight is either zero or based on the > font size .. which ought not to be negative but I don't think anything > prevents it and I think we would treat it essentially as a transform. So If > you really want to be careful here, then yes assume weight could be negative. By that same logic, then shouldn't absorbweight also be checked as != 0 instead of > 0 as that also uses += gi.weight - PR: https://git.openjdk.java.net/jdk/pull/2912
Re: [OpenJDK 2D-Dev] RFR: JDK-8263362: Avoid division by 0 in java/awt/font/TextJustifier.java justify
On Wed, 10 Mar 2021 12:31:31 GMT, Matthias Baesken wrote:
> In java/awt/font/TextJustifier.java justify-method there is a potential code
> path where divison by zero might happen , see also the Sonar finding
> https://sonarcloud.io/project/issues?id=shipilev_jdk=AXcqMwpm8sPJZZzONu1k=false=CRITICAL=BUG
>
>
> boolean hitLimit = (weight == 0) || (!lastPass && ((delta < 0) ==
> (delta < gslimit)));
> boolean absorbing = hitLimit && absorbweight > 0;
> // predivide delta by weight
> float weightedDelta = delta / weight; // not used if weight == 0
>
> In case of (weight == 0) the division should not be done because the value of
> weightedDelta is unused in this case anyway.
src/java.desktop/share/classes/java/awt/font/TextJustifier.java line 159:
> 157: // predivide delta by weight
> 158: float weightedDelta = 0;
> 159: if (weight != 0) { // not used if weight == 0
Can it ever be -ve? Maybe we can do weight > 0 check just as we do for
absorbweight?
-
PR: https://git.openjdk.java.net/jdk/pull/2912
Re: [OpenJDK 2D-Dev] RFR: 8262470: Printed GlyphVector outline with low DPI has bad quality on Windows
On Fri, 26 Feb 2021 19:40:22 GMT, Alexander Scherbatiy wrote: > Printing text using GlyphVector outline has bad quality on printers with low > DPI on Windows. > The GDI system used for text printing on Windows accepts only integer path > coordinates. > Rounding GlyphVector outline coordinates leads to distorted printed text. > > The issue had been reported as JDK-8256264 but was reverted because of the > regression JDK-8259007 "This test printed a blank page". > > The fix JDK-8256264 scaled coordinates in wPrinterJob.moveTo()/lineTo() > methods up and scaled transforms in wPrinterJob.beginPath()/endPath() down. > > The regression was in the WPathGraphics.deviceDrawLine() method which uses > wPrinterJob.moveTo()/lineTo() methods without surrounding them with > wPrinterJob.beginPath()/endPath() so the line coordinates were only scaled up. > > I tried to put wPrinterJob.beginPath()/endPath() methods around > wPrinterJob.moveTo()/lineTo() in the method WPathGraphics.deviceDrawLine() > but the line was not drawn at all even without scaling coordinates up and > transform down (without JDK-8256264 fix). It looks like GDI treats this case > as an empty shape. > > The proposed fix applies path coordinates and transform scaling only in > WPathGraphics.convertToWPath() method. > The one more PathPrecisionScaleFactorShapeTest.java manual test is added > which checks that all methods that draw paths in WPathGraphics are used: line > in WPathGraphics.deviceDrawLine() and SEG_LINETO/SEG_QUADTO/SEG_CUBICTO in > WPathGraphics.convertToWPath() . > > The `java/awt/print` and `java/awt/PrintJob` automatic and manual tests were > run on Windows 10 Pro with the fix. > > There are two failed automated tests which fail without the fix as well: > java/awt/print/PrinterJob/GlyphPositions.java > java/awt/print/PrinterJob/PSQuestionMark.java > > The following manual tests have issues on my system: > - `java/awt/print/Dialog/PrintDlgPageable.java` > java.lang.IllegalAccessException: class > com.sun.javatest.regtest.agent.MainWrapper$MainThread cannot access a member > of class PrintDlgPageable with modifiers "public static" > > - `java/awt/print/PrinterJob/PrintAttributeUpdateTest.java` I select pages > radio button, press the print button but the test does not finish and I do > not see any other dialogs with pass/fail buttons. > > - `java/awt/PrintJob/PrintCheckboxTest/PrintCheckboxManualTest.java` Tests > that there is no ClassCastException thrown in printing checkbox and scrollbar > with XAWT. Error. Can't find HTML file: > test\jdk\java\awt\PrintJob\PrintCheckboxTest\PrintCheckboxManualTest.html > > > - `java/awt/print/PrinterJob/SecurityDialogTest.java` A windows with > instructions is shown but it does not contain print/pass/fail buttons and it > is not possible to close the window. > > - The tests below fail with "Error. Parse Exception: Arguments to `manual' > option not supported: yesno" message: > java/awt/print/Dialog/DialogOrient.java > java/awt/print/Dialog/DialogType.java > java/awt/print/PrinterJob/ImagePrinting/ClippedImages.java > java/awt/print/PrinterJob/ImagePrinting/ImageTypes.java > java/awt/print/PrinterJob/ImagePrinting/PrintARGBImage.java > java/awt/print/PrinterJob/PageDialogTest.java > java/awt/print/PrinterJob/PageRanges.java > java/awt/print/PrinterJob/PageRangesDlgTest.java > java/awt/print/PrinterJob/PrintGlyphVectorTest.java > java/awt/print/PrinterJob/PrintLatinCJKTest.java > java/awt/print/PrinterJob/PrintTextTest.java > java/awt/print/PrinterJob/SwingUIText.java > java/awt/PrintJob/ConstrainedPrintingTest/ConstrainedPrintingTest.java > java/awt/PrintJob/PageSetupDlgBlockingTest/PageSetupDlgBlockingTest.java > java/awt/PrintJob/SaveDialogTitleTest.java src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp line 2033: > 2031: xForm.eDy = (FLOAT) elems[5]; > 2032: > 2033: ::SetWorldTransform((HDC)printDC, ); Probably we should check for the return value of all this system APIs SetGraphicsMode, GetWorldTransform, SetWorldTransform, ModifyWorldTransform to see if it succeeded? - PR: https://git.openjdk.java.net/jdk/pull/2756
Re: [OpenJDK 2D-Dev] RFR: 8262470: Printed GlyphVector outline with low DPI has bad quality on Windows
On Fri, 26 Feb 2021 19:40:22 GMT, Alexander Scherbatiy
wrote:
> Printing text using GlyphVector outline has bad quality on printers with low
> DPI on Windows.
> The GDI system used for text printing on Windows accepts only integer path
> coordinates.
> Rounding GlyphVector outline coordinates leads to distorted printed text.
>
> The issue had been reported as JDK-8256264 but was reverted because of the
> regression JDK-8259007 "This test printed a blank page".
>
> The fix JDK-8256264 scaled coordinates in wPrinterJob.moveTo()/lineTo()
> methods up and scaled transforms in wPrinterJob.beginPath()/endPath() down.
>
> The regression was in the WPathGraphics.deviceDrawLine() method which uses
> wPrinterJob.moveTo()/lineTo() methods without surrounding them with
> wPrinterJob.beginPath()/endPath() so the line coordinates were only scaled up.
>
> I tried to put wPrinterJob.beginPath()/endPath() methods around
> wPrinterJob.moveTo()/lineTo() in the method WPathGraphics.deviceDrawLine()
> but the line was not drawn at all even without scaling coordinates up and
> transform down (without JDK-8256264 fix). It looks like GDI treats this case
> as an empty shape.
>
> The proposed fix applies path coordinates and transform scaling only in
> WPathGraphics.convertToWPath() method.
> The one more PathPrecisionScaleFactorShapeTest.java manual test is added
> which checks that all methods that draw paths in WPathGraphics are used: line
> in WPathGraphics.deviceDrawLine() and SEG_LINETO/SEG_QUADTO/SEG_CUBICTO in
> WPathGraphics.convertToWPath() .
>
> The `java/awt/print` and `java/awt/PrintJob` automatic and manual tests were
> run on Windows 10 Pro with the fix.
>
> There are two failed automated tests which fail without the fix as well:
> java/awt/print/PrinterJob/GlyphPositions.java
> java/awt/print/PrinterJob/PSQuestionMark.java
>
> The following manual tests have issues on my system:
> - `java/awt/print/Dialog/PrintDlgPageable.java`
> java.lang.IllegalAccessException: class
> com.sun.javatest.regtest.agent.MainWrapper$MainThread cannot access a member
> of class PrintDlgPageable with modifiers "public static"
>
> - `java/awt/print/PrinterJob/PrintAttributeUpdateTest.java` I select pages
> radio button, press the print button but the test does not finish and I do
> not see any other dialogs with pass/fail buttons.
>
> - `java/awt/PrintJob/PrintCheckboxTest/PrintCheckboxManualTest.java` Tests
> that there is no ClassCastException thrown in printing checkbox and scrollbar
> with XAWT. Error. Can't find HTML file:
> test\jdk\java\awt\PrintJob\PrintCheckboxTest\PrintCheckboxManualTest.html
>
>
> - `java/awt/print/PrinterJob/SecurityDialogTest.java` A windows with
> instructions is shown but it does not contain print/pass/fail buttons and it
> is not possible to close the window.
>
> - The tests below fail with "Error. Parse Exception: Arguments to `manual'
> option not supported: yesno" message:
> java/awt/print/Dialog/DialogOrient.java
> java/awt/print/Dialog/DialogType.java
> java/awt/print/PrinterJob/ImagePrinting/ClippedImages.java
> java/awt/print/PrinterJob/ImagePrinting/ImageTypes.java
> java/awt/print/PrinterJob/ImagePrinting/PrintARGBImage.java
> java/awt/print/PrinterJob/PageDialogTest.java
> java/awt/print/PrinterJob/PageRanges.java
> java/awt/print/PrinterJob/PageRangesDlgTest.java
> java/awt/print/PrinterJob/PrintGlyphVectorTest.java
> java/awt/print/PrinterJob/PrintLatinCJKTest.java
> java/awt/print/PrinterJob/PrintTextTest.java
> java/awt/print/PrinterJob/SwingUIText.java
> java/awt/PrintJob/ConstrainedPrintingTest/ConstrainedPrintingTest.java
> java/awt/PrintJob/PageSetupDlgBlockingTest/PageSetupDlgBlockingTest.java
> java/awt/PrintJob/SaveDialogTitleTest.java
src/java.desktop/windows/classes/sun/awt/windows/WPrinterJob.java line 1025:
> 1023: * {@code GM_COMPATIBLE} or {@code GM_ADVANCED}.
> 1024: */
> 1025: private int setGraphicsMode(int mode) {
Is there any need of "int" return value? I dont see it is used in
restoreTransform()
-
PR: https://git.openjdk.java.net/jdk/pull/2756
Re: [OpenJDK 2D-Dev] RFR: 8262829: Native crash in Win32PrintServiceLookup.getAllPrinterNames()
On Thu, 4 Mar 2021 21:37:33 GMT, Alexey Ivanov wrote: > **Root cause:** > `getPrinterNames()` has two calls to > [`::EnumPrinters`](https://docs.microsoft.com/en-us/windows/win32/printdocs/enumprinters). > The first call is to get the required buffer size to contain the structures > and any strings. The second call is to get the list of printers. > > Yet the list of printers or the names of printers can change between the two > calls. If it happens, the second call to `EnumPrinters` fails but it is not > checked. > > I couldn't reproduce the crash myself. However, I reproduced the failure in > the second call to `::EnumPrinters` by adding `::Sleep(500)` and by renaming > the printers so that the data doesn't fit into the allocated buffer: > > 1: bResult: 0, cbNeeded: 504, cReturned: 0 > 2: bResult: 0, cbNeeded: 512, cReturned: 0 > !!! error > > During my testing, `cReturned` has always been zero whenever `EnumPrinters` > fails. > > The crash dumps show that `cReturned` is non-zero but the `pPrinterEnum` > buffer doesn't contain valid data. Reading `info4->pPrinterName` at the line > utf_str = JNU_NewStringPlatform(env, info4->pPrinterName); > raises Access Violation exception. > > **The fix:** > Check the return value of `EnumPrinters` and allow for 5 retries. If the > function still returns failure, make sure `cReturned` is set to zero. > > I'm going to close > [JDK-6996051](https://bugs.openjdk.java.net/browse/JDK-6996051) and > [JDK-8182683](https://bugs.openjdk.java.net/browse/JDK-8182683) reported > previously about the same crash as duplicate of the current > [JDK-8262829](https://bugs.openjdk.java.net/browse/JDK-8262829). > > **Testing:** > I used > [`RemotePrinterStatusRefresh.java`](https://github.com/openjdk/jdk/blob/master/test/jdk/java/awt/print/RemotePrinterStatusRefresh/RemotePrinterStatusRefresh.java) > for testing, it shows the list of currently available printers. In the > background I ran `PrinterRename.ps1` PowerShell script which remains a > printer, the script is attached to the JBS issue. > > Without the fix, the list of available printers was empty occasionally > because `EnumPrinters` returned failure and set `cReturned` to zero. With the > fix, the list of printers is always filled. Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2836
Re: [OpenJDK 2D-Dev] RFR: 8262829: Native crash in Win32PrintServiceLookup.getAllPrinterNames()
On Fri, 5 Mar 2021 03:42:29 GMT, Prasanta Sadhukhan
wrote:
>> src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp line 136:
>>
>>> 134:
>>> 135: try {
>>> 136: ::EnumPrinters(flags, NULL, 4, NULL,
>>
>> Don't we need to check that this method call succeeds? Probably it is a
>> crash reason when the EnumPrinters fails but we use cbNeeded anyway for
>> array allocation?
>
> I guess since we are changing this method anyway, can we use
> PRINTER_ENUM_CONNECTIONS flag instead of hardcoded 4 so that it is more
> informative!!
ok, it is for flags and not for level. please ignore
-
PR: https://git.openjdk.java.net/jdk/pull/2836
Re: [OpenJDK 2D-Dev] RFR: 8262829: Native crash in Win32PrintServiceLookup.getAllPrinterNames()
On Fri, 5 Mar 2021 01:27:47 GMT, Sergey Bylokhov wrote:
>> **Root cause:**
>> `getPrinterNames()` has two calls to
>> [`::EnumPrinters`](https://docs.microsoft.com/en-us/windows/win32/printdocs/enumprinters).
>> The first call is to get the required buffer size to contain the structures
>> and any strings. The second call is to get the list of printers.
>>
>> Yet the list of printers or the names of printers can change between the two
>> calls. If it happens, the second call to `EnumPrinters` fails but it is not
>> checked.
>>
>> I couldn't reproduce the crash myself. However, I reproduced the failure in
>> the second call to `::EnumPrinters` by adding `::Sleep(500)` and by renaming
>> the printers so that the data doesn't fit into the allocated buffer:
>>
>> 1: bResult: 0, cbNeeded: 504, cReturned: 0
>> 2: bResult: 0, cbNeeded: 512, cReturned: 0
>> !!! error
>>
>> During my testing, `cReturned` has always been zero whenever `EnumPrinters`
>> fails.
>>
>> The crash dumps show that `cReturned` is non-zero but the `pPrinterEnum`
>> buffer doesn't contain valid data. Reading `info4->pPrinterName` at the line
>> utf_str = JNU_NewStringPlatform(env, info4->pPrinterName);
>> raises Access Violation exception.
>>
>> **The fix:**
>> Check the return value of `EnumPrinters` and allow for 5 retries. If the
>> function still returns failure, make sure `cReturned` is set to zero.
>>
>> I'm going to close
>> [JDK-6996051](https://bugs.openjdk.java.net/browse/JDK-6996051) and
>> [JDK-8182683](https://bugs.openjdk.java.net/browse/JDK-8182683) reported
>> previously about the same crash as duplicate of the current
>> [JDK-8262829](https://bugs.openjdk.java.net/browse/JDK-8262829).
>>
>> **Testing:**
>> I used
>> [`RemotePrinterStatusRefresh.java`](https://github.com/openjdk/jdk/blob/master/test/jdk/java/awt/print/RemotePrinterStatusRefresh/RemotePrinterStatusRefresh.java)
>> for testing, it shows the list of currently available printers. In the
>> background I ran `PrinterRename.ps1` PowerShell script which remains a
>> printer, the script is attached to the JBS issue.
>>
>> Without the fix, the list of available printers was empty occasionally
>> because `EnumPrinters` returned failure and set `cReturned` to zero. With
>> the fix, the list of printers is always filled.
>
> src/java.desktop/windows/native/libawt/windows/WPrinterJob.cpp line 136:
>
>> 134:
>> 135: try {
>> 136: ::EnumPrinters(flags, NULL, 4, NULL,
>
> Don't we need to check that this method call succeeds? Probably it is a crash
> reason when the EnumPrinters fails but we use cbNeeded anyway for array
> allocation?
I guess since we are changing this method anyway, can we use
PRINTER_ENUM_CONNECTIONS flag instead of hardcoded 4 so that it is more
informative!!
-
PR: https://git.openjdk.java.net/jdk/pull/2836
Re: [OpenJDK 2D-Dev] RFR: 8261170: Upgrade to freetype 2.10.4
On Wed, 24 Feb 2021 18:34:31 GMT, Phil Race wrote: > This upgrades JDK (Java2D) from using freetype 2.10.2 to 2.10.4. > A minor upgrade but there's been some refactoring of include files so most > files are touched. > Testing looks good. Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2710
Re: [OpenJDK 2D-Dev] RFR: 8262161 Refactor manual I/O stream copying to new convinient methods in java.desktop
On Mon, 21 Dec 2020 07:54:17 GMT, Andrey Turbanov
wrote:
> Cleanup code to use new handy methods in
> `java.io.InputStream`/`java.nio.file.Files` instead of manual stream copy:
> 1. java.io.InputStream#readAllBytes
> 2. java.io.InputStream#transferTo
> 3. java.nio.file.Files#copy
>
> Similar issue - https://bugs.openjdk.java.net/browse/JDK-8080272
src/java.desktop/windows/classes/sun/print/Win32PrintJob.java line 436:
> 434: if (mDestination != null) { // if destination attribute is
> set
> 435: try {
> 436: Files.copy(instream, Path.of(mDestination),
> StandardCopyOption.REPLACE_EXISTING);
Don't we need to close the instream in finally block?
-
PR: https://git.openjdk.java.net/jdk/pull/1856
[OpenJDK 2D-Dev] Integrated: 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. This pull request has now been integrated. Changeset: 0c21dd05 Author:Prasanta Sadhukhan URL: https://git.openjdk.java.net/jdk/commit/0c21dd05 Stats: 69 lines in 3 files changed: 63 ins; 0 del; 6 mod 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method Reviewed-by: aivanov, kizune, azvegint - PR: https://git.openjdk.java.net/jdk/pull/2476
[OpenJDK 2D-Dev] Integrated: 8196301: java/awt/print/PrinterJob/Margins.java times out
On Wed, 17 Feb 2021 03:21:03 GMT, Prasanta Sadhukhan wrote: > This test was timing out in windows in mach5 nightly testing. Investigation > reveals that 70% of the time, it is failing due to printer being chosen was > Microsoft Print to PDF which opens up a File Save Dialog when "OK" was > clicked in printer pagedialog. Since no user intervention is done to dismiss > the modal filedialog, so subsequent pagedialog was not dismissed resulting in > timeout. > Other times, it was found that "One Note" software printer was used as > default printerservice which again, opens up a OneNote app which again gets > focus and obscure pagedialog so PageDialog did not get dismissed. > Updated test to ignore "Print To PDF", "OneNote" and "XPS Document > Writer"(which again opens filedialog like PDF printer) and run it for several > iteration in mach5 platforms which is ok. Link in JBS. This pull request has now been integrated. Changeset: ed93bc9a Author:Prasanta Sadhukhan URL: https://git.openjdk.java.net/jdk/commit/ed93bc9a Stats: 52 lines in 3 files changed: 32 ins; 18 del; 2 mod 8196301: java/awt/print/PrinterJob/Margins.java times out Reviewed-by: prr - PR: https://git.openjdk.java.net/jdk/pull/2598
Re: [OpenJDK 2D-Dev] RFR: 8196301: java/awt/print/PrinterJob/Margins.java times out [v2]
> This test was timing out in windows in mach5 nightly testing. Investigation > reveals that 70% of the time, it is failing due to printer being chosen was > Microsoft Print to PDF which opens up a File Save Dialog when "OK" was > clicked in printer pagedialog. Since no user intervention is done to dismiss > the modal filedialog, so subsequent pagedialog was not dismissed resulting in > timeout. > Other times, it was found that "One Note" software printer was used as > default printerservice which again, opens up a OneNote app which again gets > focus and obscure pagedialog so PageDialog did not get dismissed. > Updated test to ignore "Print To PDF", "OneNote" and "XPS Document > Writer"(which again opens filedialog like PDF printer) and run it for several > iteration in mach5 platforms which is ok. Link in JBS. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Remove pdf printer check - Changes: - all: https://git.openjdk.java.net/jdk/pull/2598/files - new: https://git.openjdk.java.net/jdk/pull/2598/files/d8d6b702..783374c0 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2598=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2598=00-01 Stats: 7 lines in 1 file changed: 0 ins; 7 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2598.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2598/head:pull/2598 PR: https://git.openjdk.java.net/jdk/pull/2598
Re: [OpenJDK 2D-Dev] RFR: 8196301: java/awt/print/PrinterJob/Margins.java times out
On Thu, 18 Feb 2021 03:40:24 GMT, Prasanta Sadhukhan wrote: >>> Maybe @key printer is not being taken seriously during system setup which >>> is why no real printer is being used. >> >> That should be irrelevant. jtreg should only be invoking tests which do NOT >> have the printer keyword since the test job has this : keywords=headful & >> !printer which are handed directly to jtreg. In other words it is not >> filtering on whether there is actually a printer, but on whether the keyword >> is present. >> >> So if this test is being invoked on tests which specify that then there's >> something badly wrong with jtreg or the harness is mangling it. >> >> But I think this bug was filed back when this test was MISSING that keyword >> - it was added here >> http://hg.openjdk.java.net/jdk/client/rev/f13dba72a5ea >> >> So likely you should be focused on the timing issue and not the file printers > >> That should be irrelevant. jtreg should only be invoking tests which do NOT >> have the printer keyword since the test job has this : keywords=headful & >> !printer which are handed directly to jtreg. In other words it is not >> filtering on whether there is actually a printer, but on whether the keyword >> is present. >> >> So if this test is being invoked on tests which specify that then there's >> something badly wrong with jtreg or the harness is mangling it. > > I have to change script to have keywords=headful & printer as it was not > running this test in mach5 without it. Since this is one of the few printer test which is automated, it will fail locally too if someone has "Print to PDF", "XPS" or "OneNote" printer as default since it will open up filedialog or app which will prevent the focus to be on "OK" button of pagedialog, so the fix is applicable locally too, so I would like this fix to be considered. - PR: https://git.openjdk.java.net/jdk/pull/2598
Re: [OpenJDK 2D-Dev] RFR: 8196301: java/awt/print/PrinterJob/Margins.java times out
On Wed, 17 Feb 2021 18:50:11 GMT, Phil Race wrote: > That should be irrelevant. jtreg should only be invoking tests which do NOT > have the printer keyword since the test job has this : keywords=headful & > !printer which are handed directly to jtreg. In other words it is not > filtering on whether there is actually a printer, but on whether the keyword > is present. > > So if this test is being invoked on tests which specify that then there's > something badly wrong with jtreg or the harness is mangling it. I have to change script to have keywords=headful & printer as it was not running this test in mach5 without it. - PR: https://git.openjdk.java.net/jdk/pull/2598
Re: [OpenJDK 2D-Dev] RFR: 8196301: java/awt/print/PrinterJob/Margins.java times out
On Wed, 17 Feb 2021 18:50:11 GMT, Phil Race wrote: >> Maybe @key printer is not being taken seriously during system setup which is >> why no real printer is being used. > >> Maybe @key printer is not being taken seriously during system setup which is >> why no real printer is being used. > > That should be irrelevant. jtreg should only be invoking tests which do NOT > have the printer keyword since the test job has this : keywords=headful & > !printer which are handed directly to jtreg. In other words it is not > filtering on whether there is actually a printer, but on whether the keyword > is present. > > So if this test is being invoked on tests which specify that then there's > something badly wrong with jtreg or the harness is mangling it. > > But I think this bug was filed back when this test was MISSING that keyword - > it was added here > http://hg.openjdk.java.net/jdk/client/rev/f13dba72a5ea > > So likely you should be focused on the timing issue and not the file printers As I told, I already fixed the timing issue too by moving thread start to pass ENTER keypress closer to actual pagedialog invocation and the mach5 job where all the tests passed is there in JBS. - PR: https://git.openjdk.java.net/jdk/pull/2598
Re: [OpenJDK 2D-Dev] RFR: 8196301: java/awt/print/PrinterJob/Margins.java times out
On Wed, 17 Feb 2021 18:02:49 GMT, Phil Race wrote: >> This test was timing out in windows in mach5 nightly testing. Investigation >> reveals that 70% of the time, it is failing due to printer being chosen was >> Microsoft Print to PDF which opens up a File Save Dialog when "OK" was >> clicked in printer pagedialog. Since no user intervention is done to dismiss >> the modal filedialog, so subsequent pagedialog was not dismissed resulting >> in timeout. >> Other times, it was found that "One Note" software printer was used as >> default printerservice which again, opens up a OneNote app which again gets >> focus and obscure pagedialog so PageDialog did not get dismissed. >> Updated test to ignore "Print To PDF", "OneNote" and "XPS Document >> Writer"(which again opens filedialog like PDF printer) and run it for >> several iteration in mach5 platforms which is ok. Link in JBS. > > The problem I have with this is that it does not scale. > It is a maintenance nightmare every possible "file printer" on Windows to > every test. > and back porting it too ? > removing or disabling one note and the rest s supposed to be part of the set > up of the test systems. > > And when you say "70%" what is the other 30 % ? other 30% is mixture of OneNote, XPS being used as printer service and timing issue which is why I moved the Thread start closer to actual pagedialog invocation. - PR: https://git.openjdk.java.net/jdk/pull/2598
Re: [OpenJDK 2D-Dev] RFR: 8196301: java/awt/print/PrinterJob/Margins.java times out
On Wed, 17 Feb 2021 18:05:41 GMT, Prasanta Sadhukhan wrote: >> The problem I have with this is that it does not scale. >> It is a maintenance nightmare every possible "file printer" on Windows to >> every test. >> and back porting it too ? >> removing or disabling one note and the rest s supposed to be part of the set >> up of the test systems. >> >> And when you say "70%" what is the other 30 % ? > > other 30% is mixture of OneNote, XPS being used as printer service and timing > issue which is why I moved the Thread start closer to actual pagedialog > invocation. Maybe @key printer is not being taken seriously during system setup which is why no real printer is being used. - PR: https://git.openjdk.java.net/jdk/pull/2598
Re: [OpenJDK 2D-Dev] RFR: 8196301: java/awt/print/PrinterJob/Margins.java times out
On Wed, 17 Feb 2021 17:34:22 GMT, Alexander Zuev wrote: >> This test was timing out in windows in mach5 nightly testing. Investigation >> reveals that 70% of the time, it is failing due to printer being chosen was >> Microsoft Print to PDF which opens up a File Save Dialog when "OK" was >> clicked in printer pagedialog. Since no user intervention is done to dismiss >> the modal filedialog, so subsequent pagedialog was not dismissed resulting >> in timeout. >> Other times, it was found that "One Note" software printer was used as >> default printerservice which again, opens up a OneNote app which again gets >> focus and obscure pagedialog so PageDialog did not get dismissed. >> Updated test to ignore "Print To PDF", "OneNote" and "XPS Document >> Writer"(which again opens filedialog like PDF printer) and run it for >> several iteration in mach5 platforms which is ok. Link in JBS. > > Shouldn't you update copyright years in the header of the test? No, it's not mandatory to update the copyright year as there is script that should update it at end of release. - PR: https://git.openjdk.java.net/jdk/pull/2598
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]
> 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=2476=09 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2476=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 [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=2476=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2476=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=2476=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2476=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 [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=2476=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2476=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
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
[OpenJDK 2D-Dev] RFR: 8196301: java/awt/print/PrinterJob/Margins.java times out
This test was timing out in windows in mach5 nightly testing. Investigation reveals that 70% of the time, it is failing due to printer being chosen was Microsoft Print to PDF which opens up a File Save Dialog when "OK" was clicked in printer pagedialog. Since no user intervention is done to dismiss the modal filedialog, so subsequent pagedialog was not dismissed resulting in timeout. Other times, it was found that "One Note" software printer was used as default printerservice which again, opens up a OneNote app which again gets focus and obscure pagedialog so PageDialog did not get dismissed. Updated test to ignore "Print To PDF", "OneNote" and "XPS Document Writer"(which again opens filedialog like PDF printer) and run it for several iteration in mach5 platforms which is ok. Link in JBS. - Commit messages: - 8196301: java/awt/print/PrinterJob/Margins.java times out Changes: https://git.openjdk.java.net/jdk/pull/2598/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2598=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8196301 Stats: 59 lines in 3 files changed: 39 ins; 18 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2598.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2598/head:pull/2598 PR: https://git.openjdk.java.net/jdk/pull/2598
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=2476=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2476=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: 8261533: Java_sun_font_CFont_getCascadeList leaks memory according to Xcode
On Thu, 11 Feb 2021 18:42:30 GMT, Phil Race wrote:
> The various CF*Copy* functions called here need a matching CFRelease
src/java.desktop/macosx/native/libawt_lwawt/font/AWTFont.m line 578:
> 576: (*env)->CallBooleanMethod(env, arrayListOfString, addMID,
> jFontName);
> 577: if ((*env)->ExceptionOccurred(env)) {
> 578: CFRelease(fds);
but DeleteLocalRef(jFontName) shouldn't be called here too?
-
PR: https://git.openjdk.java.net/jdk/pull/2532
Re: [OpenJDK 2D-Dev] RFR: 8261533: Java_sun_font_CFont_getCascadeList leaks memory according to Xcode
On Thu, 11 Feb 2021 18:42:30 GMT, Phil Race wrote: > The various CF*Copy* functions called here need a matching CFRelease src/java.desktop/macosx/native/libawt_lwawt/font/AWTFont.m line 575: > 573: #endif > 574: jstring jFontName = (jstring)NSStringToJavaString(env, fontname); > 575: CFRelease(fontname); Should we call CFRelease or should we call DeleteLocalRef()? - PR: https://git.openjdk.java.net/jdk/pull/2532
Re: [OpenJDK 2D-Dev] RFR: 8261533: Java_sun_font_CFont_getCascadeList leaks memory according to Xcode
On Fri, 12 Feb 2021 04:42:34 GMT, Prasanta Sadhukhan wrote: >> The various CF*Copy* functions called here need a matching CFRelease > > src/java.desktop/macosx/native/libawt_lwawt/font/AWTFont.m line 575: > >> 573: #endif >> 574: jstring jFontName = (jstring)NSStringToJavaString(env, >> fontname); >> 575: CFRelease(fontname); > > Should we call CFRelease or should we call DeleteLocalRef()? OK. I see DeleteLocalRef is called for jFontName.. - PR: https://git.openjdk.java.net/jdk/pull/2532
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=2476=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2476=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]
> 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=2476=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2476=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
[OpenJDK 2D-Dev] Integrated: 4841153: java.awt.geom.Rectangle2D.add(double, double) documented incorrectly
On Wed, 10 Feb 2021 13:10:15 GMT, Prasanta Sadhukhan wrote: > Fix a typo in Rectangle2D.add(double, double) and add(Point2D pt). It has > already been fixed in Rectangle.add(int, int) > https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/java/awt/Rectangle.java#L921 This pull request has now been integrated. Changeset: 40754f12 Author: Prasanta Sadhukhan URL: https://git.openjdk.java.net/jdk/commit/40754f12 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod 4841153: java.awt.geom.Rectangle2D.add(double,double) documented incorrectly Reviewed-by: prr - PR: https://git.openjdk.java.net/jdk/pull/2508
[OpenJDK 2D-Dev] RFR: 4841153: java.awt.geom.Rectangle2D.add(double, double) documented incorrectly
Fix a typo in Rectangle2D.add(double, double) and add(Point2D pt). It has already been fixed in Rectangle.add(int, int) https://github.com/openjdk/jdk/blob/master/src/java.desktop/share/classes/java/awt/Rectangle.java#L921 - Commit messages: - 4841153: java.awt.geom.Rectangle2D.add(double,double) documented incorrectly Changes: https://git.openjdk.java.net/jdk/pull/2508/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2508=00 Issue: https://bugs.openjdk.java.net/browse/JDK-4841153 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/2508.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2508/head:pull/2508 PR: https://git.openjdk.java.net/jdk/pull/2508
Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v4]
On Tue, 9 Feb 2021 12:29:52 GMT, Ajit Ghaisas wrote: >> **Description :** >> This is the implementation of [JEP 382 : New macOS Rendering >> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8238361) >> It implements a Java 2D internal rendering pipeline for macOS using the >> Apple Metal API. >> The entire work on this was done under [OpenJDK Project - >> Lanai](http://openjdk.java.net/projects/lanai/) >> >> We iterated through several Early Access (EA) builds and have reached a >> stage where it is ready to be integrated to openjdk/jdk. The latest EA build >> is available at - https://jdk.java.net/lanai/ >> >> A new option -Dsun.java2d.metal=true | True needs to be used to use this >> pipeline. >> >> **Testing :** >> This implementation has been tested with the tests present at - [Test Plan >> for JEP 382: New macOS Rendering >> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8251396) >> >> **Note to reviewers :** >> 1) Default rendering pipeline on macOS has not been changed by this PR. >> OpenGL still stays as the default rendering pipeline and Metal rendering >> pipeline is optional to choose. >> >> 2) To apply and test this PR - >> To enable the metal pipeline you must specify on command line >> -Dsun.java2d.metal=true (No message will be printed in this case) or >> -Dsun.java2d.metal=True (A message indicating Metal rendering pipeline is >> enabled gets printed) >> >> 3) Review comments (including some preliminary informal review comments) are >> tracked with JBS issues - https://bugs.openjdk.java.net/issues/?filter=40598 > > Ajit Ghaisas has updated the pull request incrementally with two additional > commits since the last revision: > > - Lanai PR#177 - 8261430 - aghaisas > - Lanai PR#176 - 8261399 - jdv src/java.desktop/macosx/classes/sun/java2d/metal/MTLBlitLoops.java line 110: > 108: // MTLSurfaceData.PF_BYTE_GRAY), > 109: // new MTLSwToSurfaceBlit(SurfaceType.UshortGray, > 110: // MTLSurfaceData.PF_USHORT_GRAY), Probably we could remove this commented lines - PR: https://git.openjdk.java.net/jdk/pull/2403
[OpenJDK 2D-Dev] Integrated: 6211242: AreaAveragingScaleFilter(int, int): IAE is not specified
On Mon, 8 Feb 2021 09:30:02 GMT, Prasanta Sadhukhan wrote: > Constructor AreaAveragingScaleFilter(int, int) for > java.awt.image.AreaAveragingScaleFilter class throws IllegalArgumentException > when 0 is passed for width,height but it's not specified in the spec. > Updated spec to illustrate this. This pull request has now been integrated. Changeset: 752f92bc Author: Prasanta Sadhukhan URL: https://git.openjdk.java.net/jdk/commit/752f92bc Stats: 51 lines in 2 files changed: 51 ins; 0 del; 0 mod 6211242: AreaAveragingScaleFilter(int, int): IAE is not specified Reviewed-by: azvegint, trebari, serb - PR: https://git.openjdk.java.net/jdk/pull/2456
[OpenJDK 2D-Dev] Integrated: 6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified
On Wed, 3 Feb 2021 12:11:40 GMT, Prasanta Sadhukhan wrote: > Method createStrokedShape(Shape) for java.awt.BasicStroke class throws > NullPointerException when passed a null object reference for a input > parameter but it's not specified in the spec. > Updated spec to illustrate this. This pull request has now been integrated. Changeset: 3af334a5 Author: Prasanta Sadhukhan URL: https://git.openjdk.java.net/jdk/commit/3af334a5 Stats: 45 lines in 2 files changed: 45 ins; 0 del; 0 mod 6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified Reviewed-by: serb, prr, aivanov - PR: https://git.openjdk.java.net/jdk/pull/2377
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=2476=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2476=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=2476=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2476=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 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 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
[OpenJDK 2D-Dev] RFR: 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method
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. - Commit messages: - 6206189: Graphics2D.clip specifies incorrectly that a 'null' is a valid value for this method Changes: https://git.openjdk.java.net/jdk/pull/2476/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2476=00 Issue: https://bugs.openjdk.java.net/browse/JDK-6206189 Stats: 50 lines in 2 files changed: 48 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: 6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified [v4]
> Method createStrokedShape(Shape) for java.awt.BasicStroke class throws > NullPointerException when passed a null object reference for a input > parameter but it's not specified in the spec. > Updated spec to illustrate this. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Remove redundant return statement - Changes: - all: https://git.openjdk.java.net/jdk/pull/2377/files - new: https://git.openjdk.java.net/jdk/pull/2377/files/d65b8e0a..bdc97f6a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2377=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2377=02-03 Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2377.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2377/head:pull/2377 PR: https://git.openjdk.java.net/jdk/pull/2377
[OpenJDK 2D-Dev] RFR: 6211242: AreaAveragingScaleFilter(int, int): IAE is not specified
Constructor AreaAveragingScaleFilter(int, int) for java.awt.image.AreaAveragingScaleFilter class throws IllegalArgumentException when 0 is passed for width,height but it's not specified in the spec. Updated spec to illustrate this. - Commit messages: - Fix - Fix - 6211242: AreaAveragingScaleFilter(int, int): IAE is not specified Changes: https://git.openjdk.java.net/jdk/pull/2456/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2456=00 Issue: https://bugs.openjdk.java.net/browse/JDK-6211242 Stats: 51 lines in 2 files changed: 51 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2456.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2456/head:pull/2456 PR: https://git.openjdk.java.net/jdk/pull/2456
Re: [OpenJDK 2D-Dev] RFR: 6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified [v3]
> Method createStrokedShape(Shape) for java.awt.BasicStroke class throws > NullPointerException when passed a null object reference for a input > parameter but it's not specified in the spec. > Updated spec to illustrate this. Prasanta Sadhukhan has updated the pull request incrementally with three additional commits since the last revision: - Address review comments - Revert "Address review comments" This reverts commit 3fff74d7563a6141d67cb18fd7c3dda731a4c752. - Address review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/2377/files - new: https://git.openjdk.java.net/jdk/pull/2377/files/f7d6e8fe..d65b8e0a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2377=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2377=01-02 Stats: 4 lines in 2 files changed: 0 ins; 1 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/2377.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2377/head:pull/2377 PR: https://git.openjdk.java.net/jdk/pull/2377
Re: [OpenJDK 2D-Dev] RFR: 6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified [v2]
On Wed, 3 Feb 2021 23:34:01 GMT, Sergey Bylokhov wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Doc change to @throws
>
> src/java.desktop/share/classes/java/awt/BasicStroke.java line 297:
>
>> 295: * @param s the {@code Shape} boundary be stroked
>> 296: * @return the {@code Shape} of the stroked outline.
>> 297: * @exception NullPointerException if {@code Shape} is {@code null}
>
> We use "@throws" in the new code.
any feedback on this?
-
PR: https://git.openjdk.java.net/jdk/pull/2377
Re: [OpenJDK 2D-Dev] RFR: 8261107: ArrayIndexOutOfBoundsException in the ICC_Profile.getInstance(InputStream)
On Thu, 4 Feb 2021 01:56:57 GMT, Sergey Bylokhov wrote: > Some broken data in the InputStream may cause the wrong exception to be > thrown - ArrayIndexOutOfBoundsException instead of IllegalArgumentException, > if the data is too small and we try to validate it. Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2394
[OpenJDK 2D-Dev] Integrated: 6436374: Graphics.setColor(null) is not documented
On Wed, 3 Feb 2021 07:04:22 GMT, Prasanta Sadhukhan wrote: > Document setting null setColor similar to way setFont(null) is documented. This pull request has now been integrated. Changeset: 60f440de Author: Prasanta Sadhukhan URL: https://git.openjdk.java.net/jdk/commit/60f440de Stats: 48 lines in 2 files changed: 48 ins; 0 del; 0 mod 6436374: Graphics.setColor(null) is not documented Reviewed-by: serb, pbansal - PR: https://git.openjdk.java.net/jdk/pull/2371
Re: [OpenJDK 2D-Dev] RFR: 6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified [v2]
> Method createStrokedShape(Shape) for java.awt.BasicStroke class throws > NullPointerException when passed a null object reference for a input > parameter but it's not specified in the spec. > Updated spec to illustrate this. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Doc change to @throws - Changes: - all: https://git.openjdk.java.net/jdk/pull/2377/files - new: https://git.openjdk.java.net/jdk/pull/2377/files/899868c0..f7d6e8fe Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2377=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2377=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2377.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2377/head:pull/2377 PR: https://git.openjdk.java.net/jdk/pull/2377
[OpenJDK 2D-Dev] RFR: 6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified
Method createStrokedShape(Shape) for java.awt.BasicStroke class throws NullPointerException when passed a null object reference for a input parameter but it's not specified in the spec. Updated spec to illustrate this. - Commit messages: - 6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified - 6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified Changes: https://git.openjdk.java.net/jdk/pull/2377/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2377=00 Issue: https://bugs.openjdk.java.net/browse/JDK-6211257 Stats: 47 lines in 2 files changed: 47 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2377.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2377/head:pull/2377 PR: https://git.openjdk.java.net/jdk/pull/2377
Re: [OpenJDK 2D-Dev] RFR: 6436374: Graphics.setColor(null) is not documented [v2]
> Document setting null setColor similar to way setFont(null) is documented. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Add test - Changes: - all: https://git.openjdk.java.net/jdk/pull/2371/files - new: https://git.openjdk.java.net/jdk/pull/2371/files/dc4fe3c5..a2c6c93e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2371=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2371=00-01 Stats: 47 lines in 1 file changed: 47 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2371.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2371/head:pull/2371 PR: https://git.openjdk.java.net/jdk/pull/2371
Re: [OpenJDK 2D-Dev] RFR: 6436374: Graphics.setColor(null) is not documented
On Wed, 3 Feb 2021 07:57:17 GMT, Sergey Bylokhov wrote: >>> >>> >>> @mrserb has indicated that a [compatibility and >>> specification](https://wiki.openjdk.java.net/display/csr/Main) (CSR) >>> request is needed for this pull request. >>> @prsadhuk please create a CSR request and add link to it in >>> [JDK-6436374](https://bugs.openjdk.java.net/browse/JDK-6436374). This pull >>> request cannot be integrated until the CSR request is approved. >> >> Done..JDK-8261018...Please review... > > While CSR is under review, maybe we can add a small test? What will be the objective of the test...It's just documentation change..no code is changed...setting null will be ignored before this and after this change... - PR: https://git.openjdk.java.net/jdk/pull/2371
Re: [OpenJDK 2D-Dev] RFR: 6436374: Graphics.setColor(null) is not documented
On Wed, 3 Feb 2021 07:38:02 GMT, Pankaj Bansal wrote: >> Document setting null setColor similar to way setFont(null) is documented. > > Marked as reviewed by pbansal (Reviewer). > > > @mrserb has indicated that a [compatibility and > specification](https://wiki.openjdk.java.net/display/csr/Main) (CSR) request > is needed for this pull request. > @prsadhuk please create a CSR request and add link to it in > [JDK-6436374](https://bugs.openjdk.java.net/browse/JDK-6436374). This pull > request cannot be integrated until the CSR request is approved. Done..JDK-8261018...Please review... - PR: https://git.openjdk.java.net/jdk/pull/2371
[OpenJDK 2D-Dev] RFR: 6436374: Graphics.setColor(null) is not documented
Document setting null setColor similar to way setFont(null) is documented. - Commit messages: - 6436374: Graphics.setColor(null) is not documented Changes: https://git.openjdk.java.net/jdk/pull/2371/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2371=00 Issue: https://bugs.openjdk.java.net/browse/JDK-6436374 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2371.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2371/head:pull/2371 PR: https://git.openjdk.java.net/jdk/pull/2371
[OpenJDK 2D-Dev] Integrated: 8198343: Test java/awt/print/PrinterJob/TestPgfmtSetMPA.java may fail w/o printer
On Wed, 3 Feb 2021 05:41:23 GMT, Prasanta Sadhukhan wrote: > This test require a printer to test its objective, so printer existence check > is added. This pull request has now been integrated. Changeset: cb127a4b Author:Prasanta Sadhukhan URL: https://git.openjdk.java.net/jdk/commit/cb127a4b Stats: 6 lines in 2 files changed: 5 ins; 1 del; 0 mod 8198343: Test java/awt/print/PrinterJob/TestPgfmtSetMPA.java may fail w/o printer Reviewed-by: jdv, trebari - PR: https://git.openjdk.java.net/jdk/pull/2370
[OpenJDK 2D-Dev] RFR: 8198343: Test java/awt/print/PrinterJob/TestPgfmtSetMPA.java may fail w/o printer
This test require a printer to test its objective, so printer existence check is added. - Commit messages: - 8198343: Test java/awt/print/PrinterJob/TestPgfmtSetMPA.java may fail w/o printer - 8198343: Test java/awt/print/PrinterJob/TestPgfmtSetMPA.java may fail w/o printer Changes: https://git.openjdk.java.net/jdk/pull/2370/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2370=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8198343 Stats: 6 lines in 2 files changed: 5 ins; 1 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2370.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2370/head:pull/2370 PR: https://git.openjdk.java.net/jdk/pull/2370
Re: [OpenJDK 2D-Dev] RFR: 8261010: Delete the Netbeans "default" license header
On Wed, 3 Feb 2021 04:01:51 GMT, Sergey Bylokhov wrote: > Trivial cleanup, the "default" license header is removed in a few components. Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2368
Re: [OpenJDK 2D-Dev] RFR: 8197825: [Test] Intermittent timeout with javax/swing JColorChooser Test [v4]
On Fri, 29 Jan 2021 23:46:14 GMT, Sergey Bylokhov wrote: >> src/java.desktop/windows/native/libawt/windows/awt_Toolkit.h line 478: >> >>> 476: BOOL m_breakOnError; >>> 477: >>> 478: volatile BOOL m_breakMessageLoop; >> >> Does this volatile modifier resolve the now-removed infinite loop in `while >> (!tk.IsDisposed())` in `WToolkit_shutdown`? > >> Does this volatile modifier resolve the now-removed infinite loop in `while >> (!tk.IsDisposed())` in `WToolkit_shutdown`? > > The loop should not be removed. Unfortunately, volatile modifier has no effect if infinite loop is reinstated.. - PR: https://git.openjdk.java.net/jdk/pull/2220
Re: [OpenJDK 2D-Dev] RFR: 8197825: [Test] Intermittent timeout with javax/swing JColorChooser Test [v4]
> This test was failing in our nightly mach5 testing. Appropriate stability > code in form of waitForIdle and delay is added. > mach5 job running for several iterations on all platforms is ok. Link in JBS. Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Declare m_breakMessageLoop volatile - Changes: - all: https://git.openjdk.java.net/jdk/pull/2220/files - new: https://git.openjdk.java.net/jdk/pull/2220/files/6fdcc0d7..d7dc2a52 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2220=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2220=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2220.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2220/head:pull/2220 PR: https://git.openjdk.java.net/jdk/pull/2220
Re: [OpenJDK 2D-Dev] RFR: 8197825: [Test] Intermiitent timeout with javax/swing JColorChooser Test [v3]
On Thu, 28 Jan 2021 19:50:56 GMT, Sergey Bylokhov wrote: >>> >>> My point is that this is not a test bug, so the test should not be changed. >> >> The test never dispose of the frame. Why is it expected to shut down the >> toolkit? Shall the frame be disposed of when the main thread in the test >> finishes? > >> The test never dispose of the frame. Why is it expected to shut down the >> toolkit? Shall the frame be disposed of when the main thread in the test >> finishes? > > The shutdown is caused by the System.exit call while the toolkit active, so > we should shut down it before the end. It seems "m_breakMessageLoop" is never true for unsuccessful run even though AwtToolkit::QuitMessageLoop finish execution (where m_breakMessageLoop is set to true), so AwtToolkit::MessageLoop never ends, seems to be some timing issue. - PR: https://git.openjdk.java.net/jdk/pull/2220
