Re: RFR: 8297191 : [macos] printing page range "page 2 to 2" or "page 2 to 4" on macOS leads to not print
On Wed, 26 Jun 2024 20:30:55 GMT, Alisen Chung wrote: > > Yes, there is some disconnect. For all OS (MAC with above changes) print > > range is working properly only with following values firstPage =0 and > > lastPage =-1 at **RasterPrinterJob.java** and highly depend on > > **pageRangesAttr** . I don't think there is any issue with native code, > > with this change I have brought MAC same as other OS. > > Shouldn't the solution then be to fix the issue with RasterPrinterJob.java > not working properly with correct firstPage and lastPage values? The first question to answer here is whether the page range is set and preserved in the print job attributes. I haven't looked into it, but I presume this is the case. The page range could be handled on the Java level, it may not be passed to the native level at all. What I mean is that JDK uses the page range to print the specified pages. The implementation on macOS could be adjusted in a similar way, as I said in [my previous comment](https://github.com/openjdk/jdk/pull/19740#issuecomment-2183359828). It should be possible to print a range of pages (which can consist of several intervals) without displaying the Print dialog, thus the `PageRanges` should be handled. As far as I understand, the `SunPageSelection.RANGE` attribute is somewhat informational, and specifies that `PageRanges` attribute is set. > It also looks like the indices are different between macOS and java. Perhaps > there's code somewhere that should have but hasn't converted the indices over? It's true. Different classes use different indices. The intervals in `PageRanges` count pages from 1 whereas `PrinterJob` uses 0-based indexing. According to Apple documentation, [`knowsPageRange`](https://developer.apple.com/documentation/appkit/nsview/1483774-knowspagerange?language=objc) returns `YES` if the range is known, in which case the [NSRange](https://developer.apple.com/documentation/foundation/nsrange?language=objc#4292523) structure contains the range of pages, and the pages start from 1. - PR Comment: https://git.openjdk.org/jdk/pull/19740#issuecomment-2194389772
Re: RFR: 8297191 : [macos] printing page range "page 2 to 2" or "page 2 to 4" on macOS leads to not print
On Fri, 21 Jun 2024 14:46:33 GMT, Renjith Kannath Pariyangad wrote: > Yes, there is some disconnect. For all OS (MAC with above changes) print > range is working properly only with following values firstPage =0 and > lastPage =-1 at **RasterPrinterJob.java** and highly depend on > **pageRangesAttr** . I don't think there is any issue with native code, with > this change I have brought MAC same as other OS. Shouldn't the solution then be to fix the issue with RasterPrinterJob.java not working properly with correct firstPage and lastPage values? It also looks like the indices are different between macOS and java. Perhaps there's code somewhere that should have but hasn't converted the indices over? - PR Comment: https://git.openjdk.org/jdk/pull/19740#issuecomment-2192573235
Re: RFR: 8297191 : [macos] printing page range "page 2 to 2" or "page 2 to 4" on macOS leads to not print
On Fri, 21 Jun 2024 14:46:33 GMT, Renjith Kannath Pariyangad wrote: > I don't think there is any issue with native code, with this change I have > brought MAC same as other OS. Hm… It works correctly now. But if a page range is used, `SunPageSelection.RANGE` is added to attributes. Now it's not added as far as I can see. In addition to that, when a page range is set, the native code doesn't work correctly even though the arguments with the range are passed correctly. I believe there's a problem with the native code or with passing the parameters. Perhaps, the fix could be to always return `NO` from `knowsPageRange`. - PR Comment: https://git.openjdk.org/jdk/pull/19740#issuecomment-2183359828
Re: RFR: 8297191 : [macos] printing page range "page 2 to 2" or "page 2 to 4" on macOS leads to not print
On Mon, 17 Jun 2024 05:54:37 GMT, Renjith Kannath Pariyangad
wrote:
> Hi Reviewers,
>
> This fix will resolve page range not printing proper pages if the rage begin
> from 2 or above on Mac machines.
> I have verified the manual range related tests like PageRanges.java,
> ClippedImages.java and test.java and confirmed its fixing the issue.
>
> Please review and let me know your suggestions if any.
> I added some traces and ran the test `PageRanges.java` with and without your
> proposed changes.
>
> The diff:
>
> ```diff
> diff --git
> a/src/java.desktop/macosx/classes/sun/lwawt/macosx/CPrinterJob.java
> b/src/java.desktop/macosx/classes/sun/lwawt/macosx/CPrinterJob.java
> index 416a3ee002b..6ddf46896b2 100644
> --- a/src/java.desktop/macosx/classes/sun/lwawt/macosx/CPrinterJob.java
> +++ b/src/java.desktop/macosx/classes/sun/lwawt/macosx/CPrinterJob.java
> @@ -33,6 +33,7 @@
> import java.net.URI;
> import java.security.AccessController;
> import java.security.PrivilegedAction;
> +import java.util.Arrays;
> import java.util.concurrent.atomic.AtomicReference;
>
> import javax.print.*;
> @@ -325,6 +325,8 @@ public void print(PrintRequestAttributeSet attributes)
> throws PrinterException {
> lastPage = mDocument.getNumberOfPages() - 1;
> }
> }
> +System.out.println("print: firstPage = " + firstPage + "; "
> + + "lastPage = " + lastPage);
>
> try {
> synchronized (this) {
> @@ -338,7 +340,11 @@ public void print(PrintRequestAttributeSet attributes)
> throws PrinterException {
> :
> (PageRanges)attributes.get(PageRanges.class);
> int[][] prMembers = (pr == null) ? new int[0][0] :
> pr.getMembers();
> int loopi = 0;
> +System.out.println("pr = " + pr);
> +System.out.println("prMembers = " +
> Arrays.deepToString(prMembers));
> do {
> +System.out.println("printLoop: firstPage = " + firstPage +
> "; "
> + + "lastPage = " + lastPage);
> if (EventQueue.isDispatchThread()) {
> // This is an AWT EventQueue, and this print rendering
> loop needs to block it.
>
> ```
>
> The output:
>
> ```
> 2-3 + 3-5 without Renjith changes
> print: firstPage = 1; lastPage = 2
> pr = 2-3
> prMembers = [[2, 3]]
> printLoop: firstPage = 1; lastPage = 2
> print: firstPage = 2; lastPage = 4
> pr = 3-5
> prMembers = [[3, 5]]
> printLoop: firstPage = 2; lastPage = 4
>
> 2-3 + 3-5 with Renjith changes
> print: firstPage = 0; lastPage = -1
> pr = 2-3
> prMembers = [[2, 3]]
> printLoop: firstPage = 0; lastPage = -1
> print: firstPage = 0; lastPage = -1
> pr = 3-5
> prMembers = [[3, 5]]
> printLoop: firstPage = 0; lastPage = -1
> ```
>
> In the first case, I got page 3 and page 5 printed; with Renjith's changes, I
> got the correct range printed 2-3 and 3-5.
>
> So it works… kind of. I believe the bug is somewhere in native code.
>
> What would be printed if `PageRanges` contains several ranges? This case
> should be supported as well.
>
> Since the last page to be printed is -1, `PrinterView->knowsPageRange` sets
> `aRange->length` to `NSIntegerMax`. This does not look right, even though it
> works.
>
> https://github.com/openjdk/jdk/blob/08ace27da1d9cd215c77471eabf41417ff6282d2/src/java.desktop/macosx/native/libawt_lwawt/awt/PrinterView.m#L130-L152
Yes, there is some disconnect. For all OS (MAC with above changes) print range
is working properly only with following values firstPage =0 and lastPage =-1 at
**RasterPrinterJob.java** and highly depend on **pageRangesAttr** . I don't
think there is any issue with native code, with this change I have brought MAC
same as other OS.
-
PR Comment: https://git.openjdk.org/jdk/pull/19740#issuecomment-2182895930
Re: RFR: 8297191 : [macos] printing page range "page 2 to 2" or "page 2 to 4" on macOS leads to not print
On Thu, 20 Jun 2024 18:40:29 GMT, Alexey Ivanov wrote: > Do you refer to > [`test/jdk/java/awt/print/PrinterJob/PageRanges.java`](https://github.com/openjdk/jdk/blob/master/test/jdk/java/awt/print/PrinterJob/PageRanges.java) > and > [`test/jdk/java/awt/print/PrinterJob/ImagePrinting/ClippedImages.java`](https://github.com/openjdk/jdk/blob/master/test/jdk/java/awt/print/PrinterJob/ImagePrinting/ClippedImages.java)? > Is `Test.java` the test case attached to > [JDK-8297191](https://bugs.openjdk.org/browse/JDK-8297191)? > Yes that is correct, Picked these test because one with native UI and another test with Swing UI. > I couldn't test with `ClippedImages.java`, I didn't find a way to print to > PDF. > I have installed Print to PDF app on MAC and made this as default printer. This may help - PR Comment: https://git.openjdk.org/jdk/pull/19740#issuecomment-2182571163
Re: RFR: 8297191 : [macos] printing page range "page 2 to 2" or "page 2 to 4" on macOS leads to not print
On Mon, 17 Jun 2024 05:54:37 GMT, Renjith Kannath Pariyangad
wrote:
> Hi Reviewers,
>
> This fix will resolve page range not printing proper pages if the rage begin
> from 2 or above on Mac machines.
> I have verified the manual range related tests like PageRanges.java,
> ClippedImages.java and test.java and confirmed its fixing the issue.
>
> Please review and let me know your suggestions if any.
I added some traces and ran the test `PageRanges.java` with and without your
proposed changes.
The diff:
diff --git a/src/java.desktop/macosx/classes/sun/lwawt/macosx/CPrinterJob.java
b/src/java.desktop/macosx/classes/sun/lwawt/macosx/CPrinterJob.java
index 416a3ee002b..6ddf46896b2 100644
--- a/src/java.desktop/macosx/classes/sun/lwawt/macosx/CPrinterJob.java
+++ b/src/java.desktop/macosx/classes/sun/lwawt/macosx/CPrinterJob.java
@@ -33,6 +33,7 @@
import java.net.URI;
import java.security.AccessController;
import java.security.PrivilegedAction;
+import java.util.Arrays;
import java.util.concurrent.atomic.AtomicReference;
import javax.print.*;
@@ -325,6 +325,8 @@ public void print(PrintRequestAttributeSet attributes)
throws PrinterException {
lastPage = mDocument.getNumberOfPages() - 1;
}
}
+System.out.println("print: firstPage = " + firstPage + "; "
+ + "lastPage = " + lastPage);
try {
synchronized (this) {
@@ -338,7 +340,11 @@ public void print(PrintRequestAttributeSet attributes)
throws PrinterException {
:
(PageRanges)attributes.get(PageRanges.class);
int[][] prMembers = (pr == null) ? new int[0][0] : pr.getMembers();
int loopi = 0;
+System.out.println("pr = " + pr);
+System.out.println("prMembers = " +
Arrays.deepToString(prMembers));
do {
+System.out.println("printLoop: firstPage = " + firstPage + "; "
+ + "lastPage = " + lastPage);
if (EventQueue.isDispatchThread()) {
// This is an AWT EventQueue, and this print rendering
loop needs to block it.
The output:
2-3 + 3-5 without Renjith changes
print: firstPage = 1; lastPage = 2
pr = 2-3
prMembers = [[2, 3]]
printLoop: firstPage = 1; lastPage = 2
print: firstPage = 2; lastPage = 4
pr = 3-5
prMembers = [[3, 5]]
printLoop: firstPage = 2; lastPage = 4
2-3 + 3-5 with Renjith changes
print: firstPage = 0; lastPage = -1
pr = 2-3
prMembers = [[2, 3]]
printLoop: firstPage = 0; lastPage = -1
print: firstPage = 0; lastPage = -1
pr = 3-5
prMembers = [[3, 5]]
printLoop: firstPage = 0; lastPage = -1
In the first case, I got page 3 and page 5 printed; with Renjith's changes, I
got the correct range printed 2-3 and 3-5.
So it works… kind of. I believe the bug is somewhere in native code.
What would be printed if `PageRanges` contains several ranges? This case should
be supported as well.
Since the last page to be printed is -1, `PrinterView->knowsPageRange` sets
`aRange->length` to `NSIntegerMax`. This does not look right, even though it
works.
https://github.com/openjdk/jdk/blob/08ace27da1d9cd215c77471eabf41417ff6282d2/src/java.desktop/macosx/native/libawt_lwawt/awt/PrinterView.m#L130-L152
Then, the Print dialog itself: with Renjith's changes the range of pages is
preserved. Without the changes, the range of pages isn't preserved. It's a
different bug: the dialog should be initialised using the attributes.
If `PageRanges` attribute is set and is supported, the dialog should select
**Range** radio button and set the correct page range.
A new test could be written (if it doesn't exist already) to confirm whether
attributes are taken into account when initialising the Print dialog and then a
new bug should be submitted.
-
PR Comment: https://git.openjdk.org/jdk/pull/19740#issuecomment-2182540891
PR Comment: https://git.openjdk.org/jdk/pull/19740#issuecomment-2182543047
Re: RFR: 8297191 : [macos] printing page range "page 2 to 2" or "page 2 to 4" on macOS leads to not print
On Thu, 20 Jun 2024 11:10:59 GMT, Renjith Kannath Pariyangad
wrote:
>> src/java.desktop/macosx/classes/sun/lwawt/macosx/CPrinterJob.java line 225:
>>
>>> 223: if (isRangeSet) {
>>> 224: attributes.add(new PageRanges(from+1, to+1));
>>> 225: attributes.add(SunPageSelection.RANGE);
>>
>> why was this attribute added, and why is it being removed now? is the bug in
>> SunPageSelection?
>
>> why was this attribute added, and why is it being removed now? is the bug in
>> SunPageSelection?
>
> I am not sure why this attribute added, but noticed for other OS's this
> workflow will be taken care by _RasterPrinterJob_ . With this attribute
> application pass through range and ended up in invalid page printing range.
This attribute is usually set, it should be set, I think.
The proposed changeset does resolve the problem where some pages are missing
from printouts, but I cannot find a reasonable explanation as to why the
problem goes away.
On the unmodified version, I tried adding traces into the printing code on
macOS, the range of pages that's passed to the native code is correct, but the
result is wrong number of pages pages printed if the first page is equal or
greater than 2.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19740#discussion_r1647963800
Re: RFR: 8297191 : [macos] printing page range "page 2 to 2" or "page 2 to 4" on macOS leads to not print
On Mon, 17 Jun 2024 05:54:37 GMT, Renjith Kannath Pariyangad wrote: > Hi Reviewers, > > This fix will resolve page range not printing proper pages if the rage begin > from 2 or above on Mac machines. > I have verified the manual range related tests like PageRanges.java, > ClippedImages.java and test.java and confirmed its fixing the issue. > > Please review and let me know your suggestions if any. Is it possible to write an automated test which prints to a file? It would reduce the effort for testing if the expected range of pages is printed. Do you refer to [`test/jdk/java/awt/print/PrinterJob/PageRanges.java`](https://github.com/openjdk/jdk/blob/master/test/jdk/java/awt/print/PrinterJob/PageRanges.java) and [`test/jdk/java/awt/print/PrinterJob/ImagePrinting/ClippedImages.java`](https://github.com/openjdk/jdk/blob/master/test/jdk/java/awt/print/PrinterJob/ImagePrinting/ClippedImages.java)? Is `Test.java` the test case attached to [JDK-8297191](https://bugs.openjdk.org/browse/JDK-8297191)? I couldn't test with `ClippedImages.java`, I didn't find a way to print to PDF. I used `PageRanges.java`; this test cannot be run as a jtreg test, but it works as a stand-alone app that can be run directly. It is relatively more convenient this way, the test needs to be updated, it is part of [JDK-8320677](https://bugs.openjdk.org/browse/JDK-8320677). You should add 8297191 to the list of bugids to the tests that you're using to verify this fix. I also noticed that after I apply your patch, the Print dialog preserves the previously selected range. I mean if I select pages 2 to 3 when the Print dialog is shown for the first time, the dialog comes up with Range from 2 to 3 selected when it's shown for the second time. Without the patch, the dialog has no selection (neither All Pages nor Range is selected) when it's shown for the first time. As far as I can see, there were two bugs which added support for page ranges on macOS: [JDK-7183520](https://bugs.openjdk.org/browse/JDK-7183520) and [JDK-8042713](https://bugs.openjdk.org/browse/JDK-8042713). - PR Review: https://git.openjdk.org/jdk/pull/19740#pullrequestreview-2131074084
Re: RFR: 8297191 : [macos] printing page range "page 2 to 2" or "page 2 to 4" on macOS leads to not print
On Tue, 18 Jun 2024 21:48:54 GMT, Alisen Chung wrote: > why was this attribute added, and why is it being removed now? is the bug in > SunPageSelection? I am not sure why this attribute added, but noticed for other OS's this workflow will be taken care by _RasterPrinterJob_ . With this attribute application pass through range and ended up in invalid page printing range. - PR Review Comment: https://git.openjdk.org/jdk/pull/19740#discussion_r1647405586
Re: RFR: 8297191 : [macos] printing page range "page 2 to 2" or "page 2 to 4" on macOS leads to not print
On Mon, 17 Jun 2024 05:54:37 GMT, Renjith Kannath Pariyangad
wrote:
> Hi Reviewers,
>
> This fix will resolve page range not printing proper pages if the rage begin
> from 2 or above on Mac machines.
> I have verified the manual range related tests like PageRanges.java,
> ClippedImages.java and test.java and confirmed its fixing the issue.
>
> Please review and let me know your suggestions if any.
src/java.desktop/macosx/classes/sun/lwawt/macosx/CPrinterJob.java line 225:
> 223: if (isRangeSet) {
> 224: attributes.add(new PageRanges(from+1, to+1));
> 225: attributes.add(SunPageSelection.RANGE);
why was this attribute added, and why is it being removed now? is the bug in
SunPageSelection?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19740#discussion_r1645134514
RFR: 8297191 : [macos] printing page range "page 2 to 2" or "page 2 to 4" on macOS leads to not print
Hi Reviewers, This fix will resolve page range not printing proper pages if the rage begin from 2 or above on Mac machines. I have verified the manual range related tests like PageRanges.java, ClippedImages.java and test.java and confirmed its fixing the issue. Please review and let me know your suggestions if any. - Commit messages: - JDK-8297191:[macos] printing page range "page 2 to 2" or "page 2 to 4" on macOS leads to not print Changes: https://git.openjdk.org/jdk/pull/19740/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19740&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8297191 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19740.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19740/head:pull/19740 PR: https://git.openjdk.org/jdk/pull/19740
Re: RFR: 8297191: [macos] printing page range "page 2 to 2" or "page 2 to 4" on macOS leads to not print
On Thu, 29 Dec 2022 09:36:32 GMT, Tibor Malanik wrote: > The problem is noticeable when setPageable on java.awt.print.PrinterJob is > used So why is the Test.java in the bugreport not using it ? And yet claiming it shows the problem > And can we have a test with this fix, even if it is manual, so I can run it BEFORE and AFTER the fix to verify it. Perhaps you can identify an existing failing test that you can reference ? - PR: https://git.openjdk.org/jdk/pull/11266
Re: RFR: 8297191: [macos] printing page range "page 2 to 2" or "page 2 to 4" on macOS leads to not print
On Mon, 21 Nov 2022 14:32:50 GMT, Christian Heilmann wrote: > 8297191: [macos] printing page range "page 2 to 2" or "page 2 to 4" on macOS > leads to not print The problem is noticeable when setPageable on java.awt.print.PrinterJob is used. In that case the pageIndex 0 will be used in "PageFormat getPageFormat(int pageIndex)" even though user select page 2 to 2 (or 3, 4, ..). JavaFX printing is based on the given page index, which is in that case wrong. - PR: https://git.openjdk.org/jdk/pull/11266
Re: RFR: 8297191: [macos] printing page range "page 2 to 2" or "page 2 to 4" on macOS leads to not print
On Mon, 21 Nov 2022 14:32:50 GMT, Christian Heilmann wrote: > 8297191: [macos] printing page range "page 2 to 2" or "page 2 to 4" on macOS > leads to not print This PR (and bug) could use a better title. I suggest : "[macos] pages are missing when printing using page ranges" Of course it doesn't matter until we understand why it isn't reproducible. - PR: https://git.openjdk.org/jdk/pull/11266
Re: RFR: 8297191: [macos] printing page range "page 2 to 2" or "page 2 to 4" on macOS leads to not print
On Mon, 21 Nov 2022 14:32:50 GMT, Christian Heilmann wrote: > 8297191: [macos] printing page range "page 2 to 2" or "page 2 to 4" on macOS > leads to not print So .. this looks and sounds reasonable except that I can't reproduce it. I see the "0" there at least as far back as JDK 8 (so I guess forever), but I can't reproduce with any of JDK 8, 17 or 19. I just ran you Test.java and selected page ranges 2-4 and I got the expected lines 20-39, 40-59, 60-79 on 3 pages. - PR: https://git.openjdk.org/jdk/pull/11266
