jdk
jdk copied to clipboard
openjdk
83221410: javax/swing/JTable/JTableScrollPrintTest.java does not print the rows and columns of the table in NimbusLookAndFeel
Fix suggested in bug 8210807 had a regression in Nimbus L&F yet it resolved the issue in other L&F. The better approach would be to handle MultiResolutionImages in PathGraphics class getBufferedImage method and is suggested here. The fix doesn't cause any regression and is verified in CI system. The test javax/swing/JTable/JTableScrollPrintTest.java is verified for all platforms and all L&F (Except in Windows it doesn't work due an issue introduced in 22, yet to investigate on it). And also fix 8210807 has been reverted, retaining the test.
Progress
- [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
- [x] Change must not contain extraneous whitespace
- [ ] Commit message must refer to an issue
Integration blocker
⚠️ Failed to retrieve information on issue 83221410. Please make sure it exists and is accessible.
Issue
- ⚠️ Failed to retrieve information on issue
83221410.
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/18187/head:pull/18187
$ git checkout pull/18187
Update a local copy of the PR:
$ git checkout pull/18187
$ git pull https://git.openjdk.org/jdk.git pull/18187/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 18187
View PR using the GUI difftool:
$ git pr show -t 18187
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/18187.diff
:wave: Welcome back tr! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.
![bridgekeeper[bot] avatar](https://avatars.githubusercontent.com/u/41768318?v=4 "Published by a pub.dev verified publisher"){kind=small}
@TejeshR13 The following label will be automatically applied to this pull request:
client
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.
@TejeshR13 This change now passes all automated pre-integration checks.
ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.
After integration, the commit message for the final commit will be:
8322140: javax/swing/JTable/JTableScrollPrintTest.java does not print the rows and columns of the table in Nimbus and Aqua LookAndFeel
Reviewed-by: psadhukhan, abhiscxk
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.
At the time when this comment was updated there had been 115 new commits pushed to the master branch:
- ece7d4349a13f75c654e2ca0f4d1b66d3af5cf10: 8329416: Split relocation pointer map into read-write and read-only maps
- 0db42906e390a98b3a6be078f1b8c3f2a03a838f: 8330049: Remove unused AbstractLinker::linkerByteOrder
- 5808f30b89382af22027c43ebf14e36b0c16f041: 8330026: Serial: Move some includes to vmStructs_serial.hpp
- 31ee5108e059afae0a3809947adb7b91e19baec6: 8241503: C2: Share MacroAssembler between mach nodes during code emission
- 0656f0809208160f83a7dd1ae91d9f09b582ce35: 8329469: Generational ZGC: Move the methods forwarding_[index|find|insert] from zRelocate.cpp to ZForwarding
- 16061874ffdd1b018fe1cad7e6d8ba8bdbdbbee1: 8326947: Minimize MakeBase.gmk
- 2e3682a7f2983cd58b9564253dc698760faba4b8: 8319678: Several tests from corelibs areas ignore VM flags
- 63684cd18300d862f3128bd13995e5c82307b50c: 8327250: assert(!method->is_old()) failed: Should not be installing old methods
- ecc603ca9b441cbb7ad27fbc2529fcb0b1da1992: 8201183: sjavac build failures: "Connection attempt failed: Connection refused"
- ff5c9a4ddecbe3ee453a30fcfd49fd677c174f06: 8329528: G1 does not update TAMS correctly when dropping retained regions during Concurrent Start pause
- ... and 105 more: https://git.openjdk.org/jdk/compare/b9da14012da5f1f72d4f6e690c18a43e87523173...master
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.
➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.
Webrevs
- 03: Full - Incremental (ebd42ae0)
- 02: Full - Incremental (3dfb3120)
- 01: Full - Incremental (41fcb586)
- 00: Full (38bbbe75)
The test javax/swing/JTable/JTableScrollPrintTest.java is verified for all platforms and all L&F (Except in Windows it doesn't work due an issue introduced in 22, yet to investigate on it).
Better if you specify the JBS issue link in the description.
Can you please explain why you need to handle MultiResolutionImage for this printing issue for NimbusL&F and why was it not needed for other L&F Also, you need to add this bugid to the test
Can you please explain why you need to handle MultiResolutionImage for this printing issue for NimbusL&F and why was it not needed for other L&F Also, you need to add this bugid to the test
The fix is for L&F other than Nimbus. It is working in Nimbus since Image drawing is handled by SunGraphics2D class in drawHIDPIImage() method. Whereas other L&F, pathGraphics handles ImageDrawing where MultiResolutionImage is not yet handled. The fix which I proposed for 8210807 bug fixed for Non-Nimbus L&F but caused regression for Nimbus. Hence after further analysis and study the root cause was found out to be Non-handling of MultiResolutionImage in getBufferedImage().
Can you please explain why you need to handle MultiResolutionImage for this printing issue for NimbusL&F and why was it not needed for other L&F Also, you need to add this bugid to the test
The fix is for L&F other than Nimbus & Aqua. It is working in Nimbus since Image drawing is handled by SunGraphics2D class in drawHIDPIImage() method. Whereas other L&F, pathGraphics handles ImageDrawing where MultiResolutionImage is not yet handled. The fix which I proposed for 8210807 bug fixed for Non-Nimbus L&F but caused regression for Nimbus (Since in Nimbus &Aqua, SunGraphics2D is handling Image drawing where I proposed a fix to retain PathGraphics, hence casued regression for Nimbus). Hence after further analysis and study the root cause was found out to be Non-handling of MultiResolutionImage in getBufferedImage().
Can you please explain why you need to handle MultiResolutionImage for this printing issue for NimbusL&F and why was it not needed for other L&F Also, you need to add this bugid to the test
The fix is for L&F other than Nimbus. It is working in Nimbus since Image drawing is handled by SunGraphics2D class in drawHIDPIImage() method. Whereas other L&F, pathGraphics handles ImageDrawing where MultiResolutionImage is not yet handled. The fix which I proposed for 8210807 bug fixed for Non-Nimbus L&F but caused regression for Nimbus. Hence after further analysis and study the root cause was found out to be Non-handling of MultiResolutionImage in getBufferedImage().
I guess the PathGraphics path should be used for printing images and should be common for all so why Nimbus is going via SunGraphics2D? [and it seems you updated the summary to include Aqua so Aqua is also going via SunGraphics2D?]
Can you please explain why you need to handle MultiResolutionImage for this printing issue for NimbusL&F and why was it not needed for other L&F Also, you need to add this bugid to the test
The fix is for L&F other than Nimbus. It is working in Nimbus since Image drawing is handled by SunGraphics2D class in drawHIDPIImage() method. Whereas other L&F, pathGraphics handles ImageDrawing where MultiResolutionImage is not yet handled. The fix which I proposed for 8210807 bug fixed for Non-Nimbus L&F but caused regression for Nimbus. Hence after further analysis and study the root cause was found out to be Non-handling of MultiResolutionImage in getBufferedImage().
I guess the PathGraphics path should be used for printing images and should be common for all so why Nimbus is going via SunGraphics2D? [and it seems you updated the summary to include Aqua so Aqua is also going via SunGraphics2D?]
Yes, the issue is found in Aqua also and that too uses SunGraphics2D. And regarding why SunGraphics2D is used for Nimbus and Aqua, its because in Nimbus PeekGraphics is used which in-turn useses SunGraphics2D and in other its PW/WPathGraphics. I'm not sure why this is different.
Can you please explain why you need to handle MultiResolutionImage for this printing issue for NimbusL&F and why was it not needed for other L&F Also, you need to add this bugid to the test
The fix is for L&F other than Nimbus. It is working in Nimbus since Image drawing is handled by SunGraphics2D class in drawHIDPIImage() method. Whereas other L&F, pathGraphics handles ImageDrawing where MultiResolutionImage is not yet handled. The fix which I proposed for 8210807 bug fixed for Non-Nimbus L&F but caused regression for Nimbus. Hence after further analysis and study the root cause was found out to be Non-handling of MultiResolutionImage in getBufferedImage().
I guess the PathGraphics path should be used for printing images and should be common for all so why Nimbus is going via SunGraphics2D? [and it seems you updated the summary to include Aqua so Aqua is also going via SunGraphics2D?]
Yes, the issue is found in Aqua also and that too uses SunGraphics2D. And regarding why SunGraphics2D is used for Nimbus and Aqua, its because in Nimbus PeekGraphics is used which in-turn useses SunGraphics2D and in other its PW/WPathGraphics. I'm not sure why this is different.
Can you point to the code where it uses PeekGraphics in Nimbus/Aqua and the subsequent code from where is starts to bifurcate in other L&F meaning where it starts to uses PeekGraphics for Nimbus/Aqua and PathGraphics for others?
Can you please explain why you need to handle MultiResolutionImage for this printing issue for NimbusL&F and why was it not needed for other L&F Also, you need to add this bugid to the test
The fix is for L&F other than Nimbus. It is working in Nimbus since Image drawing is handled by SunGraphics2D class in drawHIDPIImage() method. Whereas other L&F, pathGraphics handles ImageDrawing where MultiResolutionImage is not yet handled. The fix which I proposed for 8210807 bug fixed for Non-Nimbus L&F but caused regression for Nimbus. Hence after further analysis and study the root cause was found out to be Non-handling of MultiResolutionImage in getBufferedImage().
I guess the PathGraphics path should be used for printing images and should be common for all so why Nimbus is going via SunGraphics2D? [and it seems you updated the summary to include Aqua so Aqua is also going via SunGraphics2D?]
Yes, the issue is found in Aqua also and that too uses SunGraphics2D. And regarding why SunGraphics2D is used for Nimbus and Aqua, its because in Nimbus PeekGraphics is used which in-turn useses SunGraphics2D and in other its PW/WPathGraphics. I'm not sure why this is different.
Can you point to the code where it uses PeekGraphics in Nimbus/Aqua and the subsequent code from where is starts to bifurcate in other L&F meaning where it starts to uses PeekGraphics for Nimbus/Aqua and PathGraphics for others?
Yeah, it is from RasterPrinterJob class here. And that is because of nonSolidColors present in Nimbus and Aqua L&F.
Can you please explain why you need to handle MultiResolutionImage for this printing issue for NimbusL&F and why was it not needed for other L&F Also, you need to add this bugid to the test
The fix is for L&F other than Nimbus. It is working in Nimbus since Image drawing is handled by SunGraphics2D class in drawHIDPIImage() method. Whereas other L&F, pathGraphics handles ImageDrawing where MultiResolutionImage is not yet handled. The fix which I proposed for 8210807 bug fixed for Non-Nimbus L&F but caused regression for Nimbus. Hence after further analysis and study the root cause was found out to be Non-handling of MultiResolutionImage in getBufferedImage().
I guess the PathGraphics path should be used for printing images and should be common for all so why Nimbus is going via SunGraphics2D? [and it seems you updated the summary to include Aqua so Aqua is also going via SunGraphics2D?]
Yes, the issue is found in Aqua also and that too uses SunGraphics2D. And regarding why SunGraphics2D is used for Nimbus and Aqua, its because in Nimbus PeekGraphics is used which in-turn useses SunGraphics2D and in other its PW/WPathGraphics. I'm not sure why this is different.
Can you point to the code where it uses PeekGraphics in Nimbus/Aqua and the subsequent code from where is starts to bifurcate in other L&F meaning where it starts to uses PeekGraphics for Nimbus/Aqua and PathGraphics for others?
Yeah, it is from RasterPrinterJob class here. And that is because of nonSolidColors present in Nimbus and Aqua L&F.
I didn't get how it creates PeekGraphics for Nimbus/Aqua and PathGraphics for others? I guess RasterPrinterJob will delegate to WPrinterJob in windows and PSPrinterJob in unix and it is not dependant on L&F...
Can you please explain why you need to handle MultiResolutionImage for this printing issue for NimbusL&F and why was it not needed for other L&F Also, you need to add this bugid to the test
The fix is for L&F other than Nimbus. It is working in Nimbus since Image drawing is handled by SunGraphics2D class in drawHIDPIImage() method. Whereas other L&F, pathGraphics handles ImageDrawing where MultiResolutionImage is not yet handled. The fix which I proposed for 8210807 bug fixed for Non-Nimbus L&F but caused regression for Nimbus. Hence after further analysis and study the root cause was found out to be Non-handling of MultiResolutionImage in getBufferedImage().
I guess the PathGraphics path should be used for printing images and should be common for all so why Nimbus is going via SunGraphics2D? [and it seems you updated the summary to include Aqua so Aqua is also going via SunGraphics2D?]
Yes, the issue is found in Aqua also and that too uses SunGraphics2D. And regarding why SunGraphics2D is used for Nimbus and Aqua, its because in Nimbus PeekGraphics is used which in-turn useses SunGraphics2D and in other its PW/WPathGraphics. I'm not sure why this is different.
Can you point to the code where it uses PeekGraphics in Nimbus/Aqua and the subsequent code from where is starts to bifurcate in other L&F meaning where it starts to uses PeekGraphics for Nimbus/Aqua and PathGraphics for others?
Yeah, it is from RasterPrinterJob class here. And that is because of nonSolidColors present in Nimbus and Aqua L&F.
I didn't get how it creates PeekGraphics for Nimbus/Aqua and PathGraphics for others? I guess RasterPrinterJob will delegate to WPrinterJob in windows and PSPrinterJob in unix and it is not dependant on L&F...
It is because of this condition check metrics.hasNonSolidColors() which is true for Nimbus & Aqua. Here.
Going to push as commit 717a07b932e3dcabbad130d299b15cb963d50a67.
Since your change was applied there have been 122 commits pushed to the master branch:
- aebfd53e9d19f5939c81fa1a2fc75716c3355900: 8329660: G1: Improve TestGCLogMessages to be more precise
- 006a516aa0e10d74ffafca2e2da2ae89faf47457: 8329962: Remove CardTable::invalidate
- c7fcd62302a4b70214e4aea7052e661a2aa9b03b: 8330006: Serial: Extract out ContiguousSpace::block_start_const
- 2c8b432b8911bc1a52b02def89e4820c76ea67ba: 8330003: Serial: Move the logic of FastEvacuateFollowersClosure to SerialHeap
- 2c45eca15943826cb6bfbdf6e6fd88abc196e8f7: 8328879: G1: Some gtests modify global state crashing the JVM during GC after JDK-8289822
- bde3fc0c03c87d1f2605ae6bb84c33fadb7aa865: 8330106: C2: VectorInsertNode::make() shouldn't call ConINode::make() directly
- e45fea5a801ac09c3d572ac07d6179e80c422942: 8329757: Crash with fatal error: DEBUG MESSAGE: Fast Unlock lock on stack
- ece7d4349a13f75c654e2ca0f4d1b66d3af5cf10: 8329416: Split relocation pointer map into read-write and read-only maps
- 0db42906e390a98b3a6be078f1b8c3f2a03a838f: 8330049: Remove unused AbstractLinker::linkerByteOrder
- 5808f30b89382af22027c43ebf14e36b0c16f041: 8330026: Serial: Move some includes to vmStructs_serial.hpp
- ... and 112 more: https://git.openjdk.org/jdk/compare/b9da14012da5f1f72d4f6e690c18a43e87523173...master
Your commit was automatically rebased without conflicts.
@TejeshR13 Pushed as commit 717a07b932e3dcabbad130d299b15cb963d50a67.
:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.