Re: RFR: 8181571: printing to CUPS fails on mac sandbox app [v3]

[Alexander Scherbatiy]

On Thu, 12 Aug 2021 19:34:44 GMT, Phil Race  wrote:

>> Alexander Scherbatiy has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Return null if printers are not found in sandboxed app on MacOS
>
> src/java.desktop/unix/classes/sun/print/CUPSPrinter.java line 96:
> 
>> 94: libFound = initIDs();
>> 95: if (libFound) {
>> 96:cupsServer = getCupsServer();
> 
> I think we should wrap all the new lines in isMac()
> Also can you explain the reasons for the logic that implies we may have a 
> server starting with "/"
> in which case your always change the cupServer to localhost even if it is not 
> sandboxed ?
> 
> I ask because it seems to contradict what you pasted
> "by the cupsServer() function and not changing the interface string to 
> "localhost""

The logic which replaces a server starting with "/" to localhost is the 
original logic which is implemented in native level in the getCupsServer() 
function:
https://github.com/openjdk/jdk/blob/f608e81ad8309a001b8a92563a93b8adee1ce2b8/src/java.desktop/unix/native/common/awt/CUPSfuncs.c#L176
The fix only moves this logic to the java level to store domainSocketPathname 
in case of sandboxed app on MacOS.

> src/java.desktop/unix/classes/sun/print/CUPSPrinter.java line 399:
> 
>> 397: return printerURIs;
>> 398: }
>> 399: }
> 
> So if getCupsDefaultPrinters() doesn't find anything you always continue to 
> the original code even though
> it would seem that you know we are in a sandboxed app and it won't find 
> anything ?

I updated the code to return null in case there are no printer names from  
j2d_cupsGetDests() function in a sandboxed on MacOS.

> src/java.desktop/unix/classes/sun/print/CUPSPrinter.java line 489:
> 
>> 487: return domainSocketPathname;
>> 488: }
>> 489: 
> 
> You will need to suppress deprecation warnings here.

Should I add `@SuppressWarnings("deprecation")` to the 
getDomainSocketPathname() method?
I see that CUPSPrinter class has `@SuppressWarnings("removal")` but its private 
method do not have any SuppressWarnings annotations.

> src/java.desktop/unix/classes/sun/print/CUPSPrinter.java line 506:
> 
>> 504: IPPPrintService.debug_println(debugPrefix+"libFound "+libFound);
>> 505: if (libFound) {
>> 506: String server = getDomainSocketPathname() != null ? 
>> getDomainSocketPathname() : getServer();
> 
> Split this long line

Updated.

> src/java.desktop/unix/native/common/awt/CUPSfuncs.c line 244:
> 
>> 242: DPRINTF("CUPSfuncs::bad alloc new array\n", "")
>> 243: (*env)->ExceptionClear(env);
>> 244: JNU_ThrowOutOfMemoryError(env, "OutOfMemoryError");
> 
> I find this weird. What is the ExceptionClear for ? The only possible 
> exception here is for
> an OOME which might be thrown by NewObjectArray. So you clear that and then 
> re-create it ?
> And who do will catch this ? What's the point ? Maybe just clear and return 
> null.

The array creation error handling is implemented in the similar way as it is 
done in the getMedia() function.
The ExceptionClear() has been added to the getMedia() by `8031001: [Parfait] 
warnings from b121 for jdk/src/solaris/native/sun/awt: JNI-related warnings`
I would prefer to have unified error handling in these methods. If 
ExceptionClear is not suitable in this place it would be better to recheck 
JDK-8031001 and update all places accordingly in a separate fix.

> src/java.desktop/unix/native/common/awt/CUPSfuncs.c line 253:
> 
>> 251: j2d_cupsFreeDests(num_dests, dests);
>> 252: DPRINTF("CUPSfuncs::bad alloc new string ->name\n", "")
>> 253: JNU_ThrowOutOfMemoryError(env, "OutOfMemoryError");
> 
> similar comments to above plus I am fairly sure you want to delete nameArray 
> since it isn't returned.
> For that matter if the NewString fails on the 4th printer you want to free 
> the 3 previous ones too before returning.

The code is updated to remove previously created strings and clear the 
nameArray in case of an error during new string creation.

-

PR: https://git.openjdk.java.net/jdk/pull/4861

Re: RFR: 8181571: printing to CUPS fails on mac sandbox app [v3]

[Alexander Scherbatiy]

> The issue is reproduced on macOS Big Sur 11.0.1 with jdk 16.0.1+9.
> 
> Create a native macOS app from the Hello.java file, sign and run it in 
> sandbox:
> 
> import javax.print.*;
> import javax.swing.*;
> 
> public class Hello {
> 
> public static void main(String[] args) throws Exception {
> SwingUtilities.invokeAndWait(() -> {
> boolean isSandboxed = System.getenv("APP_SANDBOX_CONTAINER_ID") 
> != null;
> PrintService defaultPrinter = 
> PrintServiceLookup.lookupDefaultPrintService();
> PrintService[] services = 
> PrintServiceLookup.lookupPrintServices(null, null);
> 
> StringBuilder builder = new StringBuilder();
> builder.append("is sandboxed: ").append(isSandboxed).append("\n");
> builder.append("default printer: 
> ").append(defaultPrinter).append("\n");
> int size = services.length;
> for (int i = 0; i < size; i++) {
> 
> builder.append("printer[").append(i).append("]=").append(services[i]).append("\n");
> }
> JOptionPane.showMessageDialog(null, builder.toString());
> });
> }
> }
> 
> The signed app in sandbox shows null default printer and 
> PrintServiceLookup.lookupPrintServices(null, null) returns "Unix Printer: lp".
> ![PrintSandboxedApp](https://bugs.openjdk.java.net/secure/attachment/95629/PrintSandboxedApp.png)
> 
> The problem has been discussed on  2d-dev mail list:
>   https://mail.openjdk.java.net/pipermail/2d-dev/2017-June/008375.html
>   https://mail.openjdk.java.net/pipermail/2d-dev/2017-July/008418.html
> 
> According to the discussion:
> 
>> I've submitted a DTS incident to Apple and a friend there has followed-up.
>> Their unofficial position is that java should be connecting to the cups 
>> interface returned
>> by the cupsServer() function and not changing the interface string to 
>> "localhost".
>> Security changes in 10.12.4 reject the TCP connection which they say confuses
>> network-client access with print access.  They don't seem interested in 
>> loosening that change.
> 
> 
> The proposed solution is to use the domain socket pathname in 
> httpConnect(...) cups function and cupsGetDests(...) to get list of printers 
> from cups  when the app is signed and is run in sandbox on MacOs.

Alexander Scherbatiy has updated the pull request incrementally with one 
additional commit since the last revision:

  Return null if printers are not found in sandboxed app on MacOS

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4861/files
  - new: https://git.openjdk.java.net/jdk/pull/4861/files/104e792b..067d0025

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4861=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4861=01-02

  Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4861.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4861/head:pull/4861

PR: https://git.openjdk.java.net/jdk/pull/4861

Re: RFR: 8181571: printing to CUPS fails on mac sandbox app [v2]

[Alexander Scherbatiy]

> The issue is reproduced on macOS Big Sur 11.0.1 with jdk 16.0.1+9.
> 
> Create a native macOS app from the Hello.java file, sign and run it in 
> sandbox:
> 
> import javax.print.*;
> import javax.swing.*;
> 
> public class Hello {
> 
> public static void main(String[] args) throws Exception {
> SwingUtilities.invokeAndWait(() -> {
> boolean isSandboxed = System.getenv("APP_SANDBOX_CONTAINER_ID") 
> != null;
> PrintService defaultPrinter = 
> PrintServiceLookup.lookupDefaultPrintService();
> PrintService[] services = 
> PrintServiceLookup.lookupPrintServices(null, null);
> 
> StringBuilder builder = new StringBuilder();
> builder.append("is sandboxed: ").append(isSandboxed).append("\n");
> builder.append("default printer: 
> ").append(defaultPrinter).append("\n");
> int size = services.length;
> for (int i = 0; i < size; i++) {
> 
> builder.append("printer[").append(i).append("]=").append(services[i]).append("\n");
> }
> JOptionPane.showMessageDialog(null, builder.toString());
> });
> }
> }
> 
> The signed app in sandbox shows null default printer and 
> PrintServiceLookup.lookupPrintServices(null, null) returns "Unix Printer: lp".
> ![PrintSandboxedApp](https://bugs.openjdk.java.net/secure/attachment/95629/PrintSandboxedApp.png)
> 
> The problem has been discussed on  2d-dev mail list:
>   https://mail.openjdk.java.net/pipermail/2d-dev/2017-June/008375.html
>   https://mail.openjdk.java.net/pipermail/2d-dev/2017-July/008418.html
> 
> According to the discussion:
> 
>> I've submitted a DTS incident to Apple and a friend there has followed-up.
>> Their unofficial position is that java should be connecting to the cups 
>> interface returned
>> by the cupsServer() function and not changing the interface string to 
>> "localhost".
>> Security changes in 10.12.4 reject the TCP connection which they say confuses
>> network-client access with print access.  They don't seem interested in 
>> loosening that change.
> 
> 
> The proposed solution is to use the domain socket pathname in 
> httpConnect(...) cups function and cupsGetDests(...) to get list of printers 
> from cups  when the app is signed and is run in sandbox on MacOs.

Alexander Scherbatiy has updated the pull request incrementally with two 
additional commits since the last revision:

 - Clean utf_str and nameArray references
 - Split long line in CUPSPrinter.isCupsRunning() method

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4861/files
  - new: https://git.openjdk.java.net/jdk/pull/4861/files/072cc498..104e792b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4861=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4861=00-01

  Stats: 11 lines in 2 files changed: 9 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4861.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4861/head:pull/4861

PR: https://git.openjdk.java.net/jdk/pull/4861

Re: RFR: 8181571: printing to CUPS fails on mac sandbox app

[Alexander Scherbatiy]

On Wed, 28 Jul 2021 17:18:12 GMT, NikolayTach 
 wrote:

> [JDK-8247768 ](https://bugs.openjdk.java.net/browse/JDK-8247768)
> Same here played 6 times yet moved, 8 issues missed.

What is the relation between JDK-8247768 and the current pull request?
Should something be updated in the pull request description?

-

PR: https://git.openjdk.java.net/jdk/pull/4861

Integrated: 8265761: Font with missed font family name is not properly printed on Windows

[Alexander Scherbatiy]

On Thu, 22 Apr 2021 15:13:45 GMT, Alexander Scherbatiy  
wrote:

> PDFBox 1.8 uses 
> [Graphics2D.drawGlyphVector()](https://github.com/apache/pdfbox/blob/41ae21bd4c3f304373d3b05f63af5325df248019/pdfbox/src/main/java/org/apache/pdfbox/pdmodel/font/PDSimpleFont.java#L352)
>   method with scaled glyphs to print a text and PDF 2.0 uses 
> [Graphics2D.fill()](https://github.com/apache/pdfbox/blob/4f14dee47ff821e44d9e2ff11532959d95e94d5b/pdfbox/src/main/java/org/apache/pdfbox/rendering/PageDrawer.java#L512)
>  to print glyphs path. Both methods finally calls 
> WPathGraphics.convertToWPath(...) in jdk on Windows which call GDI FillPath.
> 
> Using a custom PageDrawer to draw a text in PDFBox with 
> Graphics2D.drawString() method reveals the fact that some pdf documents which 
> are properly printed by PDFBox 1.8 and PDF 2.0 on Linux and Windows have 
> issues with printing them on Windows.
> 
> The reason is that such docs use fonts which have empty font family name.
> The awt_PrintJob.jFontToWFontA(...) method is not able to select the required 
> font when the passed font family name is empty.
> https://github.com/openjdk/jdk/blob/7c37c022a1664437bf8d2c8d76ad039521f3ffa7/src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp#L2264
>  
> 
> The proposed solution returns false from WPrinterJob.setFont(...) method when 
> the font family is empty so the text printing falls back to printing a text 
> by GDI FillPath method:
> https://github.com/openjdk/jdk/blob/6d49cc3b655433d00e967fdcec3f3759412cd925/src/java.desktop/windows/classes/sun/awt/windows/WPrinterJob.java#L1157
> 
> To reproduce the issue I created a simple 
> [SampleBowMissedFamilyName.ttf](https://bugs.openjdk.java.net/secure/attachment/94344/SampleBowMissedFamilyName.ttf)
>  font which contains only capital letters "ABCDEF" and saved it with empty 
> font family name.
> 
> Here is a simple 
> [PrintFontSample.java](https://bugs.openjdk.java.net/secure/attachment/94343/PrintFontSample.java)
>  program that helps to reproduce the issue using the 
> SampleBowMissedFamilyName.ttf font.
> 
> The PrintFontSample program draws a text using three methods:
> - Graphics2D.drawString(...)
> - Graphics2D.drawGlyphVector(...)
> - Graphics2D.drawGlyphVector(...) using transformed glyphs 
> 
> Running the program with jdk 16 on Windows (without the fix)
>>  java PrintFontSample SampleBowMissedFamilyName.ttf
> 
> shows that the first and the second lines are not properly printed: 
> [sample-doc-without-fix.pdf](https://bugs.openjdk.java.net/secure/attachment/94345/sample-doc-without-fix.pdf)
> Running the program with the fix properly prints all three lines: 
> [sample-doc-with-fix.pdf](https://bugs.openjdk.java.net/secure/attachment/94349/sample-doc-with-fix.pdf)
> 
> The provided manual test uses the created SampleBowMissedFamilyName.ttf font 
> with empty font family name.

This pull request has now been integrated.

Changeset: e9370a13
Author:Alexander Scherbatiy 
URL:   
https://git.openjdk.java.net/jdk/commit/e9370a13b6f3f99d223ef5966f9e218b94d954b4
Stats: 293 lines in 3 files changed: 293 ins; 0 del; 0 mod

8265761: Font with missed font family name is not properly printed on Windows

Reviewed-by: serb, prr

-

PR: https://git.openjdk.java.net/jdk/pull/3631

Re: RFR: 8265761: Font with missed font family name is not properly printed on Windows [v2]

[Alexander Scherbatiy]

On Wed, 28 Apr 2021 13:55:28 GMT, Alexander Scherbatiy  
wrote:

>> PDFBox 1.8 uses 
>> [Graphics2D.drawGlyphVector()](https://github.com/apache/pdfbox/blob/41ae21bd4c3f304373d3b05f63af5325df248019/pdfbox/src/main/java/org/apache/pdfbox/pdmodel/font/PDSimpleFont.java#L352)
>>   method with scaled glyphs to print a text and PDF 2.0 uses 
>> [Graphics2D.fill()](https://github.com/apache/pdfbox/blob/4f14dee47ff821e44d9e2ff11532959d95e94d5b/pdfbox/src/main/java/org/apache/pdfbox/rendering/PageDrawer.java#L512)
>>  to print glyphs path. Both methods finally calls 
>> WPathGraphics.convertToWPath(...) in jdk on Windows which call GDI FillPath.
>> 
>> Using a custom PageDrawer to draw a text in PDFBox with 
>> Graphics2D.drawString() method reveals the fact that some pdf documents 
>> which are properly printed by PDFBox 1.8 and PDF 2.0 on Linux and Windows 
>> have issues with printing them on Windows.
>> 
>> The reason is that such docs use fonts which have empty font family name.
>> The awt_PrintJob.jFontToWFontA(...) method is not able to select the 
>> required font when the passed font family name is empty.
>> https://github.com/openjdk/jdk/blob/7c37c022a1664437bf8d2c8d76ad039521f3ffa7/src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp#L2264
>>  
>> 
>> The proposed solution returns false from WPrinterJob.setFont(...) method 
>> when the font family is empty so the text printing falls back to printing a 
>> text by GDI FillPath method:
>> https://github.com/openjdk/jdk/blob/6d49cc3b655433d00e967fdcec3f3759412cd925/src/java.desktop/windows/classes/sun/awt/windows/WPrinterJob.java#L1157
>> 
>> To reproduce the issue I created a simple 
>> [SampleBowMissedFamilyName.ttf](https://bugs.openjdk.java.net/secure/attachment/94344/SampleBowMissedFamilyName.ttf)
>>  font which contains only capital letters "ABCDEF" and saved it with empty 
>> font family name.
>> 
>> Here is a simple 
>> [PrintFontSample.java](https://bugs.openjdk.java.net/secure/attachment/94343/PrintFontSample.java)
>>  program that helps to reproduce the issue using the 
>> SampleBowMissedFamilyName.ttf font.
>> 
>> The PrintFontSample program draws a text using three methods:
>> - Graphics2D.drawString(...)
>> - Graphics2D.drawGlyphVector(...)
>> - Graphics2D.drawGlyphVector(...) using transformed glyphs 
>> 
>> Running the program with jdk 16 on Windows (without the fix)
>>>  java PrintFontSample SampleBowMissedFamilyName.ttf
>> 
>> shows that the first and the second lines are not properly printed: 
>> [sample-doc-without-fix.pdf](https://bugs.openjdk.java.net/secure/attachment/94345/sample-doc-without-fix.pdf)
>> Running the program with the fix properly prints all three lines: 
>> [sample-doc-with-fix.pdf](https://bugs.openjdk.java.net/secure/attachment/94349/sample-doc-with-fix.pdf)
>> 
>> The provided manual test uses the created SampleBowMissedFamilyName.ttf font 
>> with empty font family name.
>
> Alexander Scherbatiy has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update copyright in test font

I updated the font so now it uses "Alexander Scherbatiy" as a designer, 
"SampleFont Regular created for OpenJDK" as name for humans, and copyright:

Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
Copyright (c) 2021, BELLSOFT. All rights reserved.
DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
...

-

PR: https://git.openjdk.java.net/jdk/pull/3631

Re: RFR: 8265761: Font with missed font family name is not properly printed on Windows [v2]

[Alexander Scherbatiy]

> PDFBox 1.8 uses 
> [Graphics2D.drawGlyphVector()](https://github.com/apache/pdfbox/blob/41ae21bd4c3f304373d3b05f63af5325df248019/pdfbox/src/main/java/org/apache/pdfbox/pdmodel/font/PDSimpleFont.java#L352)
>   method with scaled glyphs to print a text and PDF 2.0 uses 
> [Graphics2D.fill()](https://github.com/apache/pdfbox/blob/4f14dee47ff821e44d9e2ff11532959d95e94d5b/pdfbox/src/main/java/org/apache/pdfbox/rendering/PageDrawer.java#L512)
>  to print glyphs path. Both methods finally calls 
> WPathGraphics.convertToWPath(...) in jdk on Windows which call GDI FillPath.
> 
> Using a custom PageDrawer to draw a text in PDFBox with 
> Graphics2D.drawString() method reveals the fact that some pdf documents which 
> are properly printed by PDFBox 1.8 and PDF 2.0 on Linux and Windows have 
> issues with printing them on Windows.
> 
> The reason is that such docs use fonts which have empty font family name.
> The awt_PrintJob.jFontToWFontA(...) method is not able to select the required 
> font when the passed font family name is empty.
> https://github.com/openjdk/jdk/blob/7c37c022a1664437bf8d2c8d76ad039521f3ffa7/src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp#L2264
>  
> 
> The proposed solution returns false from WPrinterJob.setFont(...) method when 
> the font family is empty so the text printing falls back to printing a text 
> by GDI FillPath method:
> https://github.com/openjdk/jdk/blob/6d49cc3b655433d00e967fdcec3f3759412cd925/src/java.desktop/windows/classes/sun/awt/windows/WPrinterJob.java#L1157
> 
> To reproduce the issue I created a simple 
> [SampleBowMissedFamilyName.ttf](https://bugs.openjdk.java.net/secure/attachment/94344/SampleBowMissedFamilyName.ttf)
>  font which contains only capital letters "ABCDEF" and saved it with empty 
> font family name.
> 
> Here is a simple 
> [PrintFontSample.java](https://bugs.openjdk.java.net/secure/attachment/94343/PrintFontSample.java)
>  program that helps to reproduce the issue using the 
> SampleBowMissedFamilyName.ttf font.
> 
> The PrintFontSample program draws a text using three methods:
> - Graphics2D.drawString(...)
> - Graphics2D.drawGlyphVector(...)
> - Graphics2D.drawGlyphVector(...) using transformed glyphs 
> 
> Running the program with jdk 16 on Windows (without the fix)
>>  java PrintFontSample SampleBowMissedFamilyName.ttf
> 
> shows that the first and the second lines are not properly printed: 
> [sample-doc-without-fix.pdf](https://bugs.openjdk.java.net/secure/attachment/94345/sample-doc-without-fix.pdf)
> Running the program with the fix properly prints all three lines: 
> [sample-doc-with-fix.pdf](https://bugs.openjdk.java.net/secure/attachment/94349/sample-doc-with-fix.pdf)
> 
> The provided manual test uses the created SampleBowMissedFamilyName.ttf font 
> with empty font family name.

Alexander Scherbatiy has updated the pull request incrementally with one 
additional commit since the last revision:

  Update copyright in test font

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3631/files
  - new: https://git.openjdk.java.net/jdk/pull/3631/files/6b364341..1a75732b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3631=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3631=00-01

  Stats: 2 lines in 3 files changed: 1 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3631.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3631/head:pull/3631

PR: https://git.openjdk.java.net/jdk/pull/3631

Re: RFR: 8265761: Font with missed font family name is not properly printed on Windows

[Alexander Scherbatiy]

On Thu, 22 Apr 2021 15:13:45 GMT, Alexander Scherbatiy  
wrote:

> PDFBox 1.8 uses 
> [Graphics2D.drawGlyphVector()](https://github.com/apache/pdfbox/blob/41ae21bd4c3f304373d3b05f63af5325df248019/pdfbox/src/main/java/org/apache/pdfbox/pdmodel/font/PDSimpleFont.java#L352)
>   method with scaled glyphs to print a text and PDF 2.0 uses 
> [Graphics2D.fill()](https://github.com/apache/pdfbox/blob/4f14dee47ff821e44d9e2ff11532959d95e94d5b/pdfbox/src/main/java/org/apache/pdfbox/rendering/PageDrawer.java#L512)
>  to print glyphs path. Both methods finally calls 
> WPathGraphics.convertToWPath(...) in jdk on Windows which call GDI FillPath.
> 
> Using a custom PageDrawer to draw a text in PDFBox with 
> Graphics2D.drawString() method reveals the fact that some pdf documents which 
> are properly printed by PDFBox 1.8 and PDF 2.0 on Linux and Windows have 
> issues with printing them on Windows.
> 
> The reason is that such docs use fonts which have empty font family name.
> The awt_PrintJob.jFontToWFontA(...) method is not able to select the required 
> font when the passed font family name is empty.
> https://github.com/openjdk/jdk/blob/7c37c022a1664437bf8d2c8d76ad039521f3ffa7/src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp#L2264
>  
> 
> The proposed solution returns false from WPrinterJob.setFont(...) method when 
> the font family is empty so the text printing falls back to printing a text 
> by GDI FillPath method:
> https://github.com/openjdk/jdk/blob/6d49cc3b655433d00e967fdcec3f3759412cd925/src/java.desktop/windows/classes/sun/awt/windows/WPrinterJob.java#L1157
> 
> To reproduce the issue I created a simple 
> [SampleBowMissedFamilyName.ttf](https://bugs.openjdk.java.net/secure/attachment/94344/SampleBowMissedFamilyName.ttf)
>  font which contains only capital letters "ABCDEF" and saved it with empty 
> font family name.
> 
> Here is a simple 
> [PrintFontSample.java](https://bugs.openjdk.java.net/secure/attachment/94343/PrintFontSample.java)
>  program that helps to reproduce the issue using the 
> SampleBowMissedFamilyName.ttf font.
> 
> The PrintFontSample program draws a text using three methods:
> - Graphics2D.drawString(...)
> - Graphics2D.drawGlyphVector(...)
> - Graphics2D.drawGlyphVector(...) using transformed glyphs 
> 
> Running the program with jdk 16 on Windows (without the fix)
>>  java PrintFontSample SampleBowMissedFamilyName.ttf
> 
> shows that the first and the second lines are not properly printed: 
> [sample-doc-without-fix.pdf](https://bugs.openjdk.java.net/secure/attachment/94345/sample-doc-without-fix.pdf)
> Running the program with the fix properly prints all three lines: 
> [sample-doc-with-fix.pdf](https://bugs.openjdk.java.net/secure/attachment/94349/sample-doc-with-fix.pdf)
> 
> The provided manual test uses the created SampleBowMissedFamilyName.ttf font 
> with empty font family name.

The stellarpot is my account which I used to create 
[SampleBow](https://fontstruct.com/fontstructions/show/1931451/samplebow) font 
in fontconstruct.
I put a link to my github page in the fontconstruct 
[profile](https://fontstruct.com/fontstructors/1922537/stellarspot).

If it is necessary, I can recreate the font or create an another font under my 
name and the required license. What is the right license that I can use to 
create a simple font for openjdk test? What is the way to contribute the font 
and the design of all glyphs under OCA?

-

PR: https://git.openjdk.java.net/jdk/pull/3631

Re: RFR: 8265761: Font with missed font family name is not properly printed on Windows

[Alexander Scherbatiy]

On Sun, 25 Apr 2021 20:53:11 GMT, Sergey Bylokhov  wrote:

>> Would it be better to use isBlank() instead of isEmpty() to check a font 
>> family name is blank?
>> 
>> if (family.isBlank()) {
>> return false;
>> }
>
> I think so.

I changed the test SampleBow font family name to one space 
[SampleBowOneSpaceFamilyName.ttf](https://bugs.openjdk.java.net/secure/attachment/94402/SampleBowOneSpaceFamilyName.ttf)
 and to two spaces 
[SampleBowTwoSpacesFamilyName.ttf](https://bugs.openjdk.java.net/secure/attachment/94403/SampleBowTwoSpacesFamilyName.ttf)

The  PrintFontSample sample program properly prints text drawn by 
Graphics2D.drawString() and  Graphics2D.drawGlyphVector() on Windows with jdk 
16 with these two fonts.
It looks like the problem is only with selecting a font in GDI with empty font 
family name.

-

PR: https://git.openjdk.java.net/jdk/pull/3631

Re: RFR: 8265761: Font with missed font family name is not properly printed on Windows

[Alexander Scherbatiy]

On Fri, 23 Apr 2021 07:02:45 GMT, Sergey Bylokhov  wrote:

>> PDFBox 1.8 uses 
>> [Graphics2D.drawGlyphVector()](https://github.com/apache/pdfbox/blob/41ae21bd4c3f304373d3b05f63af5325df248019/pdfbox/src/main/java/org/apache/pdfbox/pdmodel/font/PDSimpleFont.java#L352)
>>   method with scaled glyphs to print a text and PDF 2.0 uses 
>> [Graphics2D.fill()](https://github.com/apache/pdfbox/blob/4f14dee47ff821e44d9e2ff11532959d95e94d5b/pdfbox/src/main/java/org/apache/pdfbox/rendering/PageDrawer.java#L512)
>>  to print glyphs path. Both methods finally calls 
>> WPathGraphics.convertToWPath(...) in jdk on Windows which call GDI FillPath.
>> 
>> Using a custom PageDrawer to draw a text in PDFBox with 
>> Graphics2D.drawString() method reveals the fact that some pdf documents 
>> which are properly printed by PDFBox 1.8 and PDF 2.0 on Linux and Windows 
>> have issues with printing them on Windows.
>> 
>> The reason is that such docs use fonts which have empty font family name.
>> The awt_PrintJob.jFontToWFontA(...) method is not able to select the 
>> required font when the passed font family name is empty.
>> https://github.com/openjdk/jdk/blob/7c37c022a1664437bf8d2c8d76ad039521f3ffa7/src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp#L2264
>>  
>> 
>> The proposed solution returns false from WPrinterJob.setFont(...) method 
>> when the font family is empty so the text printing falls back to printing a 
>> text by GDI FillPath method:
>> https://github.com/openjdk/jdk/blob/6d49cc3b655433d00e967fdcec3f3759412cd925/src/java.desktop/windows/classes/sun/awt/windows/WPrinterJob.java#L1157
>> 
>> To reproduce the issue I created a simple 
>> [SampleBowMissedFamilyName.ttf](https://bugs.openjdk.java.net/secure/attachment/94344/SampleBowMissedFamilyName.ttf)
>>  font which contains only capital letters "ABCDEF" and saved it with empty 
>> font family name.
>> 
>> Here is a simple 
>> [PrintFontSample.java](https://bugs.openjdk.java.net/secure/attachment/94343/PrintFontSample.java)
>>  program that helps to reproduce the issue using the 
>> SampleBowMissedFamilyName.ttf font.
>> 
>> The PrintFontSample program draws a text using three methods:
>> - Graphics2D.drawString(...)
>> - Graphics2D.drawGlyphVector(...)
>> - Graphics2D.drawGlyphVector(...) using transformed glyphs 
>> 
>> Running the program with jdk 16 on Windows (without the fix)
>>>  java PrintFontSample SampleBowMissedFamilyName.ttf
>> 
>> shows that the first and the second lines are not properly printed: 
>> [sample-doc-without-fix.pdf](https://bugs.openjdk.java.net/secure/attachment/94345/sample-doc-without-fix.pdf)
>> Running the program with the fix properly prints all three lines: 
>> [sample-doc-with-fix.pdf](https://bugs.openjdk.java.net/secure/attachment/94349/sample-doc-with-fix.pdf)
>> 
>> The provided manual test uses the created SampleBowMissedFamilyName.ttf font 
>> with empty font family name.
>
> src/java.desktop/windows/classes/sun/awt/windows/WPrinterJob.java line 1160:
> 
>> 1158:   int rotation, float awScale) {
>> 1159: 
>> 1160: if (family.isEmpty()) {
> 
> Not sure that the non-empty family, but spaces only will work.

Would it be better to use isBlank() instead of isEmpty() to check a font family 
name is blank?

if (family.isBlank()) {
return false;
}

-

PR: https://git.openjdk.java.net/jdk/pull/3631

Re: RFR: 8265761: Font with missed font family name is not properly printed on Windows

[Alexander Scherbatiy]

On Thu, 22 Apr 2021 15:13:45 GMT, Alexander Scherbatiy  
wrote:

> PDFBox 1.8 uses 
> [Graphics2D.drawGlyphVector()](https://github.com/apache/pdfbox/blob/41ae21bd4c3f304373d3b05f63af5325df248019/pdfbox/src/main/java/org/apache/pdfbox/pdmodel/font/PDSimpleFont.java#L352)
>   method with scaled glyphs to print a text and PDF 2.0 uses 
> [Graphics2D.fill()](https://github.com/apache/pdfbox/blob/4f14dee47ff821e44d9e2ff11532959d95e94d5b/pdfbox/src/main/java/org/apache/pdfbox/rendering/PageDrawer.java#L512)
>  to print glyphs path. Both methods finally calls 
> WPathGraphics.convertToWPath(...) in jdk on Windows which call GDI FillPath.
> 
> Using a custom PageDrawer to draw a text in PDFBox with 
> Graphics2D.drawString() method reveals the fact that some pdf documents which 
> are properly printed by PDFBox 1.8 and PDF 2.0 on Linux and Windows have 
> issues with printing them on Windows.
> 
> The reason is that such docs use fonts which have empty font family name.
> The awt_PrintJob.jFontToWFontA(...) method is not able to select the required 
> font when the passed font family name is empty.
> https://github.com/openjdk/jdk/blob/7c37c022a1664437bf8d2c8d76ad039521f3ffa7/src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp#L2264
>  
> 
> The proposed solution returns false from WPrinterJob.setFont(...) method when 
> the font family is empty so the text printing falls back to printing a text 
> by GDI FillPath method:
> https://github.com/openjdk/jdk/blob/6d49cc3b655433d00e967fdcec3f3759412cd925/src/java.desktop/windows/classes/sun/awt/windows/WPrinterJob.java#L1157
> 
> To reproduce the issue I created a simple 
> [SampleBowMissedFamilyName.ttf](https://bugs.openjdk.java.net/secure/attachment/94344/SampleBowMissedFamilyName.ttf)
>  font which contains only capital letters "ABCDEF" and saved it with empty 
> font family name.
> 
> Here is a simple 
> [PrintFontSample.java](https://bugs.openjdk.java.net/secure/attachment/94343/PrintFontSample.java)
>  program that helps to reproduce the issue using the 
> SampleBowMissedFamilyName.ttf font.
> 
> The PrintFontSample program draws a text using three methods:
> - Graphics2D.drawString(...)
> - Graphics2D.drawGlyphVector(...)
> - Graphics2D.drawGlyphVector(...) using transformed glyphs 
> 
> Running the program with jdk 16 on Windows (without the fix)
>>  java PrintFontSample SampleBowMissedFamilyName.ttf
> 
> shows that the first and the second lines are not properly printed: 
> [sample-doc-without-fix.pdf](https://bugs.openjdk.java.net/secure/attachment/94345/sample-doc-without-fix.pdf)
> Running the program with the fix properly prints all three lines: 
> [sample-doc-with-fix.pdf](https://bugs.openjdk.java.net/secure/attachment/94349/sample-doc-with-fix.pdf)
> 
> The provided manual test uses the created SampleBowMissedFamilyName.ttf font 
> with empty font family name.

The `java/awt/print` and `java/awt/PrintJob` automated and manual tests were 
run with the fix.

The automated tests pass.
The following manual tests fail with and without the fix:

java/awt/print/Dialog/DialogOrient.java Error. Parse Exception: Arguments to 
`manual' option not supported: yesno
java/awt/print/Dialog/DialogType.java Error. Parse Exception: Arguments to 
`manual' option not supported: yesno
java/awt/print/PrinterJob/ImagePrinting/ClippedImages.java Error. Parse 
Exception: Arguments to `manual' option not supported: yesno
java/awt/print/PrinterJob/ImagePrinting/ImageTypes.java Error. Parse Exception: 
Arguments to `manual' option not supported: yesno
java/awt/print/PrinterJob/ImagePrinting/PrintARGBImage.java Error. Parse 
Exception: Arguments to `manual' option not supported: yesno
java/awt/print/PrinterJob/PageDialogTest.java Error. Parse Exception: Arguments 
to `manual' option not supported: yesno
java/awt/print/PrinterJob/PageRanges.java Error. Parse Exception: Arguments to 
`manual' option not supported: yesno
java/awt/print/PrinterJob/PageRangesDlgTest.java Error. Parse Exception: 
Arguments to `manual' option not supported: yesno
java/awt/print/PrinterJob/PrintGlyphVectorTest.java Error. Parse Exception: 
Arguments to `manual' option not supported: yesno
java/awt/print/PrinterJob/PrintLatinCJKTest.java Error. Parse Exception: 
Arguments to `manual' option not supported: yesno
java/awt/print/PrinterJob/PrintTextTest.java Error. Parse Exception: Arguments 
to `manual' option not supported: yesno
java/awt/print/PrinterJob/SwingUIText.java Error. Parse Exception: Arguments to 
`manual' option not supported: yesno
java/awt/PrintJob/ConstrainedPrintingTest/ConstrainedPrintingTest.java Error. 
Parse Exception: Arguments to `manual' option not supported: yesno
java/awt/PrintJob/PageSetupDlgBlockingTest/PageSetupDlgBlockingTest.java Error. 
Parse Exception: Arguments to `manual' option not supported: yesno
java

RFR: 8265761: Font with missed font family name is not properly printed on Windows

[Alexander Scherbatiy]

PDFBox 1.8 uses 
[Graphics2D.drawGlyphVector()](https://github.com/apache/pdfbox/blob/41ae21bd4c3f304373d3b05f63af5325df248019/pdfbox/src/main/java/org/apache/pdfbox/pdmodel/font/PDSimpleFont.java#L352)
  method with scaled glyphs to print a text and PDF 2.0 uses 
[Graphics2D.fill()](https://github.com/apache/pdfbox/blob/4f14dee47ff821e44d9e2ff11532959d95e94d5b/pdfbox/src/main/java/org/apache/pdfbox/rendering/PageDrawer.java#L512)
 to print glyphs path. Both methods finally calls 
WPathGraphics.convertToWPath(...) in jdk on Windows which call GDI FillPath.

Using a custom PageDrawer to draw a text in PDFBox with Graphics2D.drawString() 
method reveals the fact that some pdf documents which are properly printed by 
PDFBox 1.8 and PDF 2.0 on Linux and Windows have issues with printing them on 
Windows.

The reason is that such docs use fonts which have empty font family name.
The awt_PrintJob.jFontToWFontA(...) method is not able to select the required 
font when the passed font family name is empty.
https://github.com/openjdk/jdk/blob/7c37c022a1664437bf8d2c8d76ad039521f3ffa7/src/java.desktop/windows/native/libawt/windows/awt_PrintJob.cpp#L2264
 

The proposed solution returns false from WPrinterJob.setFont(...) method when 
the font family is empty so the text printing falls back to printing a text by 
GDI FillPath method:
https://github.com/openjdk/jdk/blob/6d49cc3b655433d00e967fdcec3f3759412cd925/src/java.desktop/windows/classes/sun/awt/windows/WPrinterJob.java#L1157

To reproduce the issue I created a simple 
[SampleBowMissedFamilyName.ttf](https://bugs.openjdk.java.net/secure/attachment/94344/SampleBowMissedFamilyName.ttf)
 font which contains only capital letters "ABCDEF" and saved it with empty font 
family name.

Here is a simple 
[PrintFontSample.java](https://bugs.openjdk.java.net/secure/attachment/94343/PrintFontSample.java)
 program that helps to reproduce the issue using the 
SampleBowMissedFamilyName.ttf font.

The PrintFontSample program draws a text using three methods:
- Graphics2D.drawString(...)
- Graphics2D.drawGlyphVector(...)
- Graphics2D.drawGlyphVector(...) using transformed glyphs 

Running the program with jdk 16 on Windows (without the fix)
>  java PrintFontSample SampleBowMissedFamilyName.ttf

shows that the first and the second lines are not properly printed: 
[sample-doc-without-fix.pdf](https://bugs.openjdk.java.net/secure/attachment/94345/sample-doc-without-fix.pdf)
Running the program with the fix properly prints all three lines: 
[sample-doc-with-fix.pdf](https://bugs.openjdk.java.net/secure/attachment/94349/sample-doc-with-fix.pdf)

The provided manual test uses the created SampleBowMissedFamilyName.ttf font 
with empty font family name.

-

Commit messages:
 - 8265761: Font with missed font family name is not properly printed on Windows

Changes: https://git.openjdk.java.net/jdk/pull/3631/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3631=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8265761
  Stats: 292 lines in 3 files changed: 292 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3631.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3631/head:pull/3631

PR: https://git.openjdk.java.net/jdk/pull/3631

Integrated: 8262470: Printed GlyphVector outline with low DPI has bad quality on Windows

[Alexander Scherbatiy]

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

This pull request has now been integrated.

Changeset: 02287349
Author:Alexander Scherbatiy 
URL:   https://git.openjdk.java.net/jdk/commit/02287349
Stats: 731 lines in 5 files changed: 728 ins; 0 del; 3 mod

8262470: Printed GlyphVector outline with low DPI has bad quality on Windows

Reviewed-by: serb, psadhukhan

-

PR: https://git.openjdk.java.net/jdk/pull/2756

Re: RFR: 8262470: Printed GlyphVector outline with low DPI has bad quality on Windows [v2]

[Alexander Scherbatiy]

> 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

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2756/files
  - new: https://git.openjdk.java.net/jdk/pull/2756/files/e0278acd..8105885f

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=2756=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2756=00-01

  Stats: 21 lines in 2 files changed: 8 ins; 2 del; 11 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2756.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2756/head:pull/2756

PR: https://git.openjdk.java.net/jdk/pull/2756

Re: RFR: 8262470: Printed GlyphVector outline with low DPI has bad quality on Windows [v2]

[Alexander Scherbatiy]

On Wed, 10 Mar 2021 09:31:32 GMT, Prasanta Sadhukhan  
wrote:

>> 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
>
> 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()

I updated the code to return void from setGraphicsMode() method.

> 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?

I added DASSERT to check SetGraphicsMode and Get/Set/ModifyWorldTransform 
results.

-

PR: https://git.openjdk.java.net/jdk/pull/2756

RFR: 8262470: Printed GlyphVector outline with low DPI has bad quality on Windows

[Alexander Scherbatiy]

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

-

Commit messages:
 - 8262470: Printed GlyphVector outline with low DPI has bad quality on Windows

Changes: https://git.openjdk.java.net/jdk/pull/2756/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2756=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8262470
  Stats: 725 lines in 5 files changed: 722 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2756.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2756/head:pull/2756

PR: https://git.openjdk.java.net/jdk/pull/2756

Integrated: 8256264: Printed GlyphVector outline with low DPI has bad quality on Windows

[Alexander Scherbatiy]

On Thu, 12 Nov 2020 10:32:16 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.
> 
> To reproduce the issue run the 
> [PrintGlyphVectorOutlineSample](https://bugs.openjdk.java.net/secure/attachment/91398/PrintGlyphVectorOutlineSample.java)
>   file on Windows and select a low DPI
> printer in the printer dialog. The sample prints two lines, one using 
> Graphics drawString() method and another by
> filling GlyphVector outline. Chars on the second line are distorted.
> 
> It is also possible to reproduce the issue running the sample and printing 
> the text to PDF: 
> [fill-glyph-vector-outline.png](https://bugs.openjdk.java.net/secure/attachment/91397/fill-glyph-vector-outline.png)
> 
> The proposed fix introduce "sun.java2d.print.enablePathPrecisionScale" 
> property which being enabled
> scales the GDI WorldTransform down and GlyphVector outline coordinates up.
> This allows to keep some digits after a dot from being rounded.
> The value for scaling is chosen to be 1000 in the same way how it is used  by 
> `String trunc(float f)` method from PSPrinterJob class on Linux:
> https://github.com/openjdk/jdk/blob/ed615e3ca0d681e8e67cdbf1d5d964979ccd7888/src/java.desktop/share/classes/sun/print/PSPrinterJob.java#L1489
> 
> See the  
> [fill-glyph-vector-outline-enable-path-scale-factor.png](https://bugs.openjdk.java.net/secure/attachment/91399/fill-glyph-vector-outline-enable-path-scale-factor.png)
>  screenshot which shows how the GlyphVector outline is filled after the fix 
> with the enabled "sun.java2d.print.enablePathPrecisionScale" option.
> 
> [fill-glyph-vector-outline-diff.png](https://bugs.openjdk.java.net/secure/attachment/91400/fill-glyph-vector-outline-diff.png)
>  shows difference of GlyphVector outline printing before and after the fix.

This pull request has now been integrated.

Changeset: d6dd440c
Author:Alexander Scherbatiy 
URL:   https://git.openjdk.java.net/jdk/commit/d6dd440c
Stats: 470 lines in 4 files changed: 459 ins; 0 del; 11 mod

8256264: Printed GlyphVector outline with low DPI has bad quality on Windows

Reviewed-by: serb

-

PR: https://git.openjdk.java.net/jdk/pull/1183

Re: RFR: 8256264: Printed GlyphVector outline with low DPI has bad quality on Windows

[Alexander Scherbatiy]

On Sat, 28 Nov 2020 20:07:22 GMT, Sergey Bylokhov  wrote:

>> I prepared a simple [print 
>> test](http://cr.openjdk.java.net/~alexsch/8256264/performance/PrintTextPerformanceTest.java)
>>  sample which uses 4 different fonts (plain and bold) with different sizes 
>> and prints 640 lines on 10 pages.
>> 
>> I run the sample with and without the fix to PDF and measured the time which 
>> is used by the  deviceFill() method (it both converts the shape to path and 
>> fills it):
>> https://github.com/openjdk/jdk/blob/b21b96df2162c72098afcdfd2cf243a55a0f17e1/src/java.desktop/windows/classes/sun/awt/windows/WPathGraphics.java#L1489
>> 
>> The average time without the fix: 2.77s (min 2.74s, max 2,78)
>> The average time with the fix: 2.76s (min 2.74s, max 2.77)
>> 
>> I removed the `sun.java2d.print.enablePathPrecisionScale` property from the 
>> fix.
>
>> I removed the `sun.java2d.print.enablePathPrecisionScale` property from the 
>> fix.
> 
> You forgot to delete it from the test.

The `sun.java2d.print.enablePathPrecisionScale` property is removed from the 
PathPrecisionScaleFactorTest test.

-

PR: https://git.openjdk.java.net/jdk/pull/1183

Re: RFR: 8256264: Printed GlyphVector outline with low DPI has bad quality on Windows [v3]

[Alexander Scherbatiy]

> 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.
> 
> To reproduce the issue run the 
> [PrintGlyphVectorOutlineSample](https://bugs.openjdk.java.net/secure/attachment/91398/PrintGlyphVectorOutlineSample.java)
>   file on Windows and select a low DPI
> printer in the printer dialog. The sample prints two lines, one using 
> Graphics drawString() method and another by
> filling GlyphVector outline. Chars on the second line are distorted.
> 
> It is also possible to reproduce the issue running the sample and printing 
> the text to PDF: 
> [fill-glyph-vector-outline.png](https://bugs.openjdk.java.net/secure/attachment/91397/fill-glyph-vector-outline.png)
> 
> The proposed fix introduce "sun.java2d.print.enablePathPrecisionScale" 
> property which being enabled
> scales the GDI WorldTransform down and GlyphVector outline coordinates up.
> This allows to keep some digits after a dot from being rounded.
> The value for scaling is chosen to be 1000 in the same way how it is used  by 
> `String trunc(float f)` method from PSPrinterJob class on Linux:
> https://github.com/openjdk/jdk/blob/ed615e3ca0d681e8e67cdbf1d5d964979ccd7888/src/java.desktop/share/classes/sun/print/PSPrinterJob.java#L1489
> 
> See the  
> [fill-glyph-vector-outline-enable-path-scale-factor.png](https://bugs.openjdk.java.net/secure/attachment/91399/fill-glyph-vector-outline-enable-path-scale-factor.png)
>  screenshot which shows how the GlyphVector outline is filled after the fix 
> with the enabled "sun.java2d.print.enablePathPrecisionScale" option.
> 
> [fill-glyph-vector-outline-diff.png](https://bugs.openjdk.java.net/secure/attachment/91400/fill-glyph-vector-outline-diff.png)
>  shows difference of GlyphVector outline printing before and after the fix.

Alexander Scherbatiy has updated the pull request incrementally with one 
additional commit since the last revision:

  Remove Dsun.java2d.print.enablePathPrecisionScale property from the 
PathPrecisionScaleFactorTest

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1183/files
  - new: https://git.openjdk.java.net/jdk/pull/1183/files/f79b8014..b6885e5f

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=1183=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1183=01-02

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1183.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1183/head:pull/1183

PR: https://git.openjdk.java.net/jdk/pull/1183

Re: RFR: 8256264: Printed GlyphVector outline with low DPI has bad quality on Windows

[Alexander Scherbatiy]

On Fri, 20 Nov 2020 06:31:24 GMT, Sergey Bylokhov  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.
>> 
>> To reproduce the issue run the 
>> [PrintGlyphVectorOutlineSample](https://bugs.openjdk.java.net/secure/attachment/91398/PrintGlyphVectorOutlineSample.java)
>>   file on Windows and select a low DPI
>> printer in the printer dialog. The sample prints two lines, one using 
>> Graphics drawString() method and another by
>> filling GlyphVector outline. Chars on the second line are distorted.
>> 
>> It is also possible to reproduce the issue running the sample and printing 
>> the text to PDF: 
>> [fill-glyph-vector-outline.png](https://bugs.openjdk.java.net/secure/attachment/91397/fill-glyph-vector-outline.png)
>> 
>> The proposed fix introduce "sun.java2d.print.enablePathPrecisionScale" 
>> property which being enabled
>> scales the GDI WorldTransform down and GlyphVector outline coordinates up.
>> This allows to keep some digits after a dot from being rounded.
>> The value for scaling is chosen to be 1000 in the same way how it is used  
>> by `String trunc(float f)` method from PSPrinterJob class on Linux:
>> https://github.com/openjdk/jdk/blob/ed615e3ca0d681e8e67cdbf1d5d964979ccd7888/src/java.desktop/share/classes/sun/print/PSPrinterJob.java#L1489
>> 
>> See the  
>> [fill-glyph-vector-outline-enable-path-scale-factor.png](https://bugs.openjdk.java.net/secure/attachment/91399/fill-glyph-vector-outline-enable-path-scale-factor.png)
>>  screenshot which shows how the GlyphVector outline is filled after the fix 
>> with the enabled "sun.java2d.print.enablePathPrecisionScale" option.
>> 
>> [fill-glyph-vector-outline-diff.png](https://bugs.openjdk.java.net/secure/attachment/91400/fill-glyph-vector-outline-diff.png)
>>  shows difference of GlyphVector outline printing before and after the fix.
>
> I am not sure that we need the additional property for things other than 
> possibly to workaround some possible bugs. I am fine if we will use the 
> scaled version all the time, does it have any drawbacks?

I prepared a simple [print 
test](http://cr.openjdk.java.net/~alexsch/8256264/performance/PrintTextPerformanceTest.java)
 sample which uses 4 different fonts (plain and bold) with different sizes and 
prints 640 lines on 10 pages.

I run the sample with and without the fix to PDF and measured the time which is 
used by the  deviceFill() method (it both converts the shape to path and fills 
it):
https://github.com/openjdk/jdk/blob/b21b96df2162c72098afcdfd2cf243a55a0f17e1/src/java.desktop/windows/classes/sun/awt/windows/WPathGraphics.java#L1489

The average time without the fix: 2.77s (min 2.74s, max 2,78)
The average time with the fix: 2.76s (min 2.74s, max 2.77)

I removed the `sun.java2d.print.enablePathPrecisionScale` property from the fix.

-

PR: https://git.openjdk.java.net/jdk/pull/1183

Re: RFR: 8256264: Printed GlyphVector outline with low DPI has bad quality on Windows [v2]

[Alexander Scherbatiy]

> 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.
> 
> To reproduce the issue run the 
> [PrintGlyphVectorOutlineSample](https://bugs.openjdk.java.net/secure/attachment/91398/PrintGlyphVectorOutlineSample.java)
>   file on Windows and select a low DPI
> printer in the printer dialog. The sample prints two lines, one using 
> Graphics drawString() method and another by
> filling GlyphVector outline. Chars on the second line are distorted.
> 
> It is also possible to reproduce the issue running the sample and printing 
> the text to PDF: 
> [fill-glyph-vector-outline.png](https://bugs.openjdk.java.net/secure/attachment/91397/fill-glyph-vector-outline.png)
> 
> The proposed fix introduce "sun.java2d.print.enablePathPrecisionScale" 
> property which being enabled
> scales the GDI WorldTransform down and GlyphVector outline coordinates up.
> This allows to keep some digits after a dot from being rounded.
> The value for scaling is chosen to be 1000 in the same way how it is used  by 
> `String trunc(float f)` method from PSPrinterJob class on Linux:
> https://github.com/openjdk/jdk/blob/ed615e3ca0d681e8e67cdbf1d5d964979ccd7888/src/java.desktop/share/classes/sun/print/PSPrinterJob.java#L1489
> 
> See the  
> [fill-glyph-vector-outline-enable-path-scale-factor.png](https://bugs.openjdk.java.net/secure/attachment/91399/fill-glyph-vector-outline-enable-path-scale-factor.png)
>  screenshot which shows how the GlyphVector outline is filled after the fix 
> with the enabled "sun.java2d.print.enablePathPrecisionScale" option.
> 
> [fill-glyph-vector-outline-diff.png](https://bugs.openjdk.java.net/secure/attachment/91400/fill-glyph-vector-outline-diff.png)
>  shows difference of GlyphVector outline printing before and after the fix.

Alexander Scherbatiy has updated the pull request incrementally with one 
additional commit since the last revision:

  Remove sun.java2d.print.enablePathPrecisionScale property from JDK-8256264 fix

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1183/files
  - new: https://git.openjdk.java.net/jdk/pull/1183/files/120ae32b..f79b8014

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=1183=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=1183=00-01

  Stats: 18 lines in 1 file changed: 0 ins; 10 del; 8 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1183.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1183/head:pull/1183

PR: https://git.openjdk.java.net/jdk/pull/1183

RFR: 8256264: Printed GlyphVector outline with low DPI has bad quality on Windows

[Alexander Scherbatiy]

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.

To reproduce the issue run the 
[PrintGlyphVectorOutlineSample](https://bugs.openjdk.java.net/secure/attachment/91398/PrintGlyphVectorOutlineSample.java)
  file on Windows and select a low DPI
printer in the printer dialog. The sample prints two lines, one using Graphics 
drawString() method and another by
filling GlyphVector outline. Chars on the second line are distorted.

It is also possible to reproduce the issue running the sample and printing the 
text to PDF: 
[fill-glyph-vector-outline.png](https://bugs.openjdk.java.net/secure/attachment/91397/fill-glyph-vector-outline.png)

The proposed fix introduce "sun.java2d.print.enablePathPrecisionScale" property 
which being enabled
scales the GDI WorldTransform down and GlyphVector outline coordinates up.
This allows to keep some digits after a dot from being rounded.
The value for scaling is chosen to be 1000 in the same way how it is used  by 
`String trunc(float f)` method from PSPrinterJob class on Linux:
https://github.com/openjdk/jdk/blob/ed615e3ca0d681e8e67cdbf1d5d964979ccd7888/src/java.desktop/share/classes/sun/print/PSPrinterJob.java#L1489

See the  
[fill-glyph-vector-outline-enable-path-scale-factor.png](https://bugs.openjdk.java.net/secure/attachment/91399/fill-glyph-vector-outline-enable-path-scale-factor.png)
 screenshot which shows how the GlyphVector outline is filled after the fix 
with the enabled "sun.java2d.print.enablePathPrecisionScale" option.

[fill-glyph-vector-outline-diff.png](https://bugs.openjdk.java.net/secure/attachment/91400/fill-glyph-vector-outline-diff.png)
 shows difference of GlyphVector outline printing before and after the fix.

-

Commit messages:
 - 8256264: Printed GlyphVector outline with low DPI has bad quality on Windows

Changes: https://git.openjdk.java.net/jdk/pull/1183/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1183=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8256264
  Stats: 480 lines in 4 files changed: 469 ins; 0 del; 11 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1183.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1183/head:pull/1183

PR: https://git.openjdk.java.net/jdk/pull/1183

RFR: 8245938 Remove unused print_stack(void) method from XToolkit.c

[Alexander Scherbatiy]

Hello,

Could you review a simple cleanup 8245938 Remove unused 
print_stack(void) method from XToolkit.c


  Bug: https://bugs.openjdk.java.net/browse/JDK-8245938
  Fix: http://cr.openjdk.java.net/~alexsch/8245938/webrev.00

The unused print_stack function is removed from XToolkit.c.
The reason for the request is that execinfo is specific to glibc and is 
not supported by musl libc.


Thanks,
Alexander.

Re: [OpenJDK 2D-Dev] [9] Review Request: 8177461 Wrong references are used in the javadoc in the java.desktop module

[Alexander Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 23/03/17 18:24, Sergey Bylokhov wrote:

Hello,
Please review the fix for jdk9.

In java.desktop module in a various places we use incorrect links.
Most of them are in the documentation in non-public classes/methods(But there 
are two places in public API - specdiff is provided)

Common issues:
  - The method is referenced without #
  - The method which have some parameters is referenced as «#method()» instead of 
«#method» or «#method(ParameterDescription)"
  - The type which is referenced is not imported
  - The reference to non-existed method/type.
  - Copy pasted javadoc was removed.
  - The type was renamed (like AppReOpenedListener to AppReopenedListener)


Bug: https://bugs.openjdk.java.net/browse/JDK-8177461
Webrev can be found at: http://cr.openjdk.java.net/~serb/8177461/webrev.00
Specdiff: 
http://cr.openjdk.java.net/~serb/8177461/specdiff/overview-summary.html

[9] Review request for 8177625 apple.laf.JRSUIConstants.getConstantName(int) checks for THUMB_START twice

[Alexander Scherbatiy]

Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8177625
  webrev: http://cr.openjdk.java.net/~alexsch/8177625/webrev.00

  The typo with twice used THUMB_START constant is fixed.

  Thanks,
  Alexandr.

Re: [9] Review request for 8175301 Java GUI hangs on Windows when Display set to 125%

[Alexander Scherbatiy]

  Hello,

Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8175301/webrev.01

There are two places where D3DSurfaceData.swapBuffers() is called 
D3DScreenUpdateManager.run() and D3DGraphicsConfig.flip(...).
D3DScreenUpdateManager.run()  uses the surface bounds which are already 
scaled.

D3DGraphicsConfig.flip(...) uses coordinates in the user space.

The fix converts the coordinates in the D3DGraphicsConfig.flip(...) to 
the device space and converts the coordinates to the user space in the 
D3DSurfaceData.swapBuffers() method to pass them to the repaint() method.


Thanks,
Alexandr.

On 3/7/2017 4:58 PM, Alexandr Scherbatiy wrote:


Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8175301
  webrev: http://cr.openjdk.java.net/~alexsch/8175301/webrev.00

  D3DSurfaceData.swapBuffers() is updated to scale passed bounds on 
HiDPI display.


Thanks,
Alexandr.

Re: Review Request JDK:-8162959 [HiDPI] screenshot artifacts using AWT Robot

[Alexander Scherbatiy]

On 07/12/16 10:24, Prem Balakrishnan wrote:


Hi Alexander,

Please review updated patch as per review comments.

http://cr.openjdk.java.net/~pkbalakr/8162959/webrev.03/ 
<http://cr.openjdk.java.net/%7Epkbalakr/8162959/webrev.03/>



  I used a simple app [1] to create a screenshot of a frame on Mac OS X:
Rectangle rect = frame.getBounds();
rect.setLocation(frame.getLocationOnScreen());
BufferedImage img = robot.createScreenCapture(rect);

  It takes only bottom right quarter of the frame [2].

  Is it possible to use some API that makes a screenshot and returns an 
NSImage on Mac OS X?
  In this case it would be possible to get a high resolution 
representation from the NSImage and return it in the same way as it is 
updated on Windows and Linux.


  [1] 
http://cr.openjdk.java.net/~alexsch/8162959/screenshot/RobotScreenshot.java

  [2] http://cr.openjdk.java.net/~alexsch/8162959/screenshot/screenshot.png

  Thanks,
  Alexandr.


Regards,

Prem

*From:*Alexandr Scherbatiy
*Sent:* Monday, November 21, 2016 8:10 PM
*To:* Prem Balakrishnan; Sergey Bylokhov; awt-dev@openjdk.java.net; 
Phil Race
*Subject:* Re: Review Request JDK:-8162959 [HiDPI] screenshot 
artifacts using AWT Robot


On 11/16/2016 8:46 AM, Prem Balakrishnan wrote:

Hi Alexander,

Please review update patch as per review comments.

http://cr.openjdk.java.net/~pkbalakr/8162959/webrev.02/ 
<http://cr.openjdk.java.net/%7Epkbalakr/8162959/webrev.02/>


With this patch, Robot.createScreenCapture(Rectangle) method returns a 
scaled down image on the HiDPI display.



- The native part on Mac OS X and Linux should be updated as well.

- 416  * Returns BufferedImage for Non-HiDPI display and 
MultiResolutionImage

  417  * for HiDPI display with two resolution variants.

There should be added an explanation what is the content of the first 
and the second resolution variant.


Thanks,
Alexandr.


Regards,

Prem

*From:*Alexandr Scherbatiy
*Sent:* Thursday, November 03, 2016 4:05 PM
*To:* Prem Balakrishnan; Sergey Bylokhov; awt-dev@openjdk.java.net 
<mailto:awt-dev@openjdk.java.net>
*Subject:* Re: Review Request JDK:-8162959 [HiDPI] screenshot 
artifacts using AWT Robot


On 11/2/2016 1:57 PM, Prem Balakrishnan wrote:


Hi Alexander,

Please review updated patch.

http://cr.openjdk.java.net/~pkbalakr/8162959/webrev.01/ 
<http://cr.openjdk.java.net/%7Epkbalakr/8162959/webrev.01/>


Added a new public API “*Image createHiDPIScreenCapture(Rectangle 
screenRect)”.*


Returns an ordinary screenshot(BufferedImage) if the UI scale is 1.

Returns MultiResolutionImage for HiDPI display with two resolution 
variants,


1.Low Resolution/base image with user input width and height,  I have 
used interpolation algorithm for scaling , adapted from JavaFX Robot 
(http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/89a5de54b7dc),


2.High Resolution image with scaled width and height .

  - Please, check that the Robot.createScreenCapture(Rectangle) method 
returns a scaled down image on the HiDPI display

  - Probably existing Java methods can be used for an image scaling.
  For example, there is the Image.getScaledInstance(int width, int 
height, int hints) method.
  Or may be it is better just to create an image with required size 
and draw the high-resolution image into it using a specific scale and 
rendering hints.


  Thanks,
  Alexandr.



Regards,

Prem

*From:*Alexander Scherbatiy
*Sent:* Thursday, October 13, 2016 3:21 PM
*To:* Prem Balakrishnan; Sergey Bylokhov; awt-dev@openjdk.java.net 
<mailto:awt-dev@openjdk.java.net>
*Subject:* Re: Review Request JDK:-8162959 [HiDPI] screenshot 
artifacts using AWT Robot


On 06/10/16 15:28, Prem Balakrishnan wrote:

Hi*,*

**

Please review fix for JDK 9,

*Bug:*https://bugs.openjdk.java.net/browse/JDK-8162959

*Webrev:*http://cr.openjdk.java.net/~pkbalakr/8162959/webrev.00/
<http://cr.openjdk.java.net/%7Epkbalakr/8162959/webrev.00/>

I have adapted the same fix as used for JavaFX Robot

*Bug: *JDK-8162783 <https://bugs.openjdk.java.net/browse/JDK-8162783>.

*Patch: *http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/89a5de54b7dc

New Public API ” BufferedImage createScreenCapture(Rectangle
screenRect, boolean isHiDPI)”is added,

Which will have a parameter to specify if HiDPI.

  It is better to a add public method which returns MultiResolution 
image on HiDPI display and  consists of two resoltion variants

   - base image which has size requested by a user
   - high-resolution image which creates an original screen capture

  The proposed by your algorithm can be applied to the base image.
  For more details see JDK-8020618 [macosx] java.awt.Robot makes 
blurry screen captures on Retina


  Thanks,
  Alexandr.



  
Regards,

Prem
  

Re: Review Request JDK:-8162959 [HiDPI] screenshot artifacts using AWT Robot

[Alexander Scherbatiy]

On 06/10/16 15:28, Prem Balakrishnan wrote:


Hi*,*

**

Please review fix for JDK 9,

*Bug:*https://bugs.openjdk.java.net/browse/JDK-8162959

*Webrev:*http://cr.openjdk.java.net/~pkbalakr/8162959/webrev.00/ 



I have adapted the same fix as used for JavaFX Robot

*Bug: *JDK-8162783 .

*Patch: *http://hg.openjdk.java.net/openjfx/9-dev/rt/rev/89a5de54b7dc

New Public API ” BufferedImage createScreenCapture(Rectangle 
screenRect, boolean isHiDPI)”is added,

Which will have a parameter to specify if HiDPI.
  It is better to a add public method which returns MultiResolution 
image on HiDPI display and  consists of two resoltion variants

   - base image which has size requested by a user
   - high-resolution image which creates an original screen capture

  The proposed by your algorithm can be applied to the base image.
  For more details see JDK-8020618 [macosx] java.awt.Robot makes blurry 
screen captures on Retina


  Thanks,
  Alexandr.

Regards,
Prem

Re: [9] Review request for 8166591 [macos 10.12] Trackpad scrolling of text on OS X 10.12 Sierra is very fast (Trackpad, Retina only)

[Alexander Scherbatiy]

On 30/09/16 19:48, Alexander Scherbatiy wrote:

On 30/09/16 19:33, Sergey Malenkov wrote:

I'm not sure. It can be a "false" scrolling when you accidentally
touched a Magic Mouse.
I think we should use threshold on the phase end, to ignore
accumulatedDelta less than 0.1


Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8166591/webrev.07

The min threshold is added for the phase end.
  Here is the updated version where the accumulated delta is not reset 
at the phase end:

http://cr.openjdk.java.net/~alexsch/8166591/webrev.08

  Thanks,
  Alexandr.



Thanks,
Alexandr.



On Fri, Sep 30, 2016 at 6:18 PM, Sergey Bylokhov
<sergey.bylok...@oracle.com> wrote:

What will be the difference if we will scroll by one line at the "begin
phase"? probably it will be better if the the scroll will start 
immediately

on first touch?


On 30.09.16 17:59, Alexander Scherbatiy wrote:


Hello,

Could you review the updated fix:
   http://cr.openjdk.java.net/~alexsch/8166591/webrev.06

- The CPlatformResponder.handleScrollEvent(...) is updated to dispatch
a scroll event when delta or round delta is not equal to zero
- The native scrollStateWithPhase: method is updated to have 
NSEvent as

an argument

Thanks,
Alexandr.

On 30/09/16 16:58, Sergey Malenkov wrote:

In the CPlatformResponder:

phase3 0 ~ 0.0 // mayBegan
phase2 0 ~ 0.0 // began
phase3 0 ~ 0.0222015380859375
phase3 0 ~ 0.0234222412109375
phase3 0 ~ 0.023956298828125
phase3 0 ~ 0.0242919921875
phase3 0 ~ 0.02447509765625
phase3 0 ~ 0.0246124267578125
phase3 0 ~ 0.024658203125
phase3 0 ~ 0.0222015380859375
phase3 0 ~ 0.0233306884765625
phase5 1 ~ 0.0 // end

In Java:

wheelRotation=0,preciseWheelRotation=-0.0222015380859375
wheelRotation=0,preciseWheelRotation=-0.0234222412109375
wheelRotation=0,preciseWheelRotation=-0.023956298828125
wheelRotation=0,preciseWheelRotation=-0.0242919921875
wheelRotation=0,preciseWheelRotation=-0.02447509765625
wheelRotation=0,preciseWheelRotation=-0.0246124267578125
wheelRotation=0,preciseWheelRotation=-0.024658203125
wheelRotation=0,preciseWheelRotation=-0.0222015380859375
wheelRotation=0,preciseWheelRotation=-0.0233306884765625

We ignored first two events, because of 0 & 0.0
We should not ignore last event, where 1 & 0.0
Seems we need to fix DeltaAccumulator.


On Fri, Sep 30, 2016 at 2:25 PM, Alexander Scherbatiy
<alexandr.scherba...@oracle.com> wrote:

Hello,

Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8166591/webrev.05

The momentumPhase is used to detect the trackpad events.

Thanks,
Alexandr.


On 30/09/16 14:58, Sergey Malenkov wrote:

# C  [AppKit+0x3a528e] -[NSApplication _crashOnException:]+0x6d

The app is crashed as soon as I start scrolling. Investigating...
May be it is my fault during backporting.

Sorry, It was may fault.



LWCToolkit.m:
+// SCROLL EVENT MASK
+#define SCROLL_PHASE_UNSUPPORTED 1
...

replace the comment with the following one:

+// TRACKPAD SCROLL EVENT PHASE

I discovered how we should detect mouse event properly: use both
properties phase and momentumPhase.


https://developer.apple.com/library/prerelease/content/documentation/Cocoa/Conceptual/EventOverview/HandlingTouchEvents/HandlingTouchEvents.html 



"The momentumPhase property helps you detect momentum scrolling, in
which the hardware continues to issue scroll wheel events even 
though

the user is no longer physically scrolling."

if (phase==NULL) && (momentumPhase==NULL) -> this is a mouse event.

scrolling by trackpad generates the following events:

phase=mayBegan momentumPhase=null
phase=began momentumPhase=null
phase=continued momentumPhase=null
...
phase=continued momentumPhase=null
phase=ended momentumPhase=null
phase=null momentumPhase=began
phase=null momentumPhase=continued
...
phase=null momentumPhase=continued
phase=null momentumPhase=ended

So, we should generate PHASE_UNSUPPORTED only
if ((phase == NULL) && (momentumPhase == NULL))
















--
Best regards, Sergey.

Re: [9] Review request for 8166591 [macos 10.12] Trackpad scrolling of text on OS X 10.12 Sierra is very fast (Trackpad, Retina only)

[Alexander Scherbatiy]

On 30/09/16 19:43, Sergey Malenkov wrote:

+ (jint) scrollStateWithEvent: (NSEvent*) event {

scrollPhaseFromEvent sounds more clear for me

 if ([event type] != NSScrollWheel) {
 return 0;
 }

We have no corresponding SCROLL_PHASE_ constant.
This value is not processed and is processed like S_P_CONTINUED in our code.
 We do not process it because it is not the NSScrollWheel event and it 
is not handled as a scroll event on the Java level 
(CPlatformResponder.handleScrollEvent(...) is not called).


Thanks,
Alexandr.







On Fri, Sep 30, 2016 at 5:59 PM, Alexander Scherbatiy
<alexandr.scherba...@oracle.com> wrote:

Hello,

Could you review the updated fix:
   http://cr.openjdk.java.net/~alexsch/8166591/webrev.06

- The CPlatformResponder.handleScrollEvent(...) is updated to dispatch  a
scroll event when delta or round delta is not equal to zero
- The native scrollStateWithPhase: method is updated to have NSEvent as an
argument

Thanks,
Alexandr.

On 30/09/16 16:58, Sergey Malenkov wrote:

In the CPlatformResponder:

phase3 0 ~ 0.0 // mayBegan
phase2 0 ~ 0.0 // began
phase3 0 ~ 0.0222015380859375
phase3 0 ~ 0.0234222412109375
phase3 0 ~ 0.023956298828125
phase3 0 ~ 0.0242919921875
phase3 0 ~ 0.02447509765625
phase3 0 ~ 0.0246124267578125
phase3 0 ~ 0.024658203125
phase3 0 ~ 0.0222015380859375
phase3 0 ~ 0.0233306884765625
phase5 1 ~ 0.0 // end

In Java:

wheelRotation=0,preciseWheelRotation=-0.0222015380859375
wheelRotation=0,preciseWheelRotation=-0.0234222412109375
wheelRotation=0,preciseWheelRotation=-0.023956298828125
wheelRotation=0,preciseWheelRotation=-0.0242919921875
wheelRotation=0,preciseWheelRotation=-0.02447509765625
wheelRotation=0,preciseWheelRotation=-0.0246124267578125
wheelRotation=0,preciseWheelRotation=-0.024658203125
wheelRotation=0,preciseWheelRotation=-0.0222015380859375
wheelRotation=0,preciseWheelRotation=-0.0233306884765625

We ignored first two events, because of 0 & 0.0
We should not ignore last event, where 1 & 0.0
Seems we need to fix DeltaAccumulator.


On Fri, Sep 30, 2016 at 2:25 PM, Alexander Scherbatiy
<alexandr.scherba...@oracle.com> wrote:

Hello,

Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8166591/webrev.05

The momentumPhase is used to detect the trackpad events.

Thanks,
Alexandr.


On 30/09/16 14:58, Sergey Malenkov wrote:

# C  [AppKit+0x3a528e]  -[NSApplication _crashOnException:]+0x6d

The app is crashed as soon as I start scrolling. Investigating...
May be it is my fault during backporting.

Sorry, It was may fault.



LWCToolkit.m:
+// SCROLL EVENT MASK
+#define SCROLL_PHASE_UNSUPPORTED 1
...

replace the comment with the following one:

+// TRACKPAD SCROLL EVENT PHASE

I discovered how we should detect mouse event properly: use both
properties phase and momentumPhase.


https://developer.apple.com/library/prerelease/content/documentation/Cocoa/Conceptual/EventOverview/HandlingTouchEvents/HandlingTouchEvents.html
"The momentumPhase property helps you detect momentum scrolling, in
which the hardware continues to issue scroll wheel events even though
the user is no longer physically scrolling."

if (phase==NULL) && (momentumPhase==NULL) -> this is a mouse event.

scrolling by trackpad generates the following events:

phase=mayBegan momentumPhase=null
phase=began momentumPhase=null
phase=continued momentumPhase=null
...
phase=continued momentumPhase=null
phase=ended momentumPhase=null
phase=null momentumPhase=began
phase=null momentumPhase=continued
...
phase=null momentumPhase=continued
phase=null momentumPhase=ended

So, we should generate PHASE_UNSUPPORTED only
if ((phase == NULL) && (momentumPhase == NULL))

Re: [9] Review request for 8166591 [macos 10.12] Trackpad scrolling of text on OS X 10.12 Sierra is very fast (Trackpad, Retina only)

[Alexander Scherbatiy]

On 30/09/16 19:33, Sergey Malenkov wrote:

I'm not sure. It can be a "false" scrolling when you accidentally
touched a Magic Mouse.
I think we should use threshold on the phase end, to ignore
accumulatedDelta less than 0.1


Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8166591/webrev.07

The min threshold is added for the phase end.

Thanks,
Alexandr.



On Fri, Sep 30, 2016 at 6:18 PM, Sergey Bylokhov
<sergey.bylok...@oracle.com> wrote:

What will be the difference if we will scroll by one line at the "begin
phase"? probably it will be better if the the scroll will start immediately
on first touch?


On 30.09.16 17:59, Alexander Scherbatiy wrote:


Hello,

Could you review the updated fix:
   http://cr.openjdk.java.net/~alexsch/8166591/webrev.06

- The CPlatformResponder.handleScrollEvent(...) is updated to dispatch
a scroll event when delta or round delta is not equal to zero
- The native scrollStateWithPhase: method is updated to have NSEvent as
an argument

Thanks,
Alexandr.

On 30/09/16 16:58, Sergey Malenkov wrote:

In the CPlatformResponder:

phase3 0 ~ 0.0 // mayBegan
phase2 0 ~ 0.0 // began
phase3 0 ~ 0.0222015380859375
phase3 0 ~ 0.0234222412109375
phase3 0 ~ 0.023956298828125
phase3 0 ~ 0.0242919921875
phase3 0 ~ 0.02447509765625
phase3 0 ~ 0.0246124267578125
phase3 0 ~ 0.024658203125
phase3 0 ~ 0.0222015380859375
phase3 0 ~ 0.0233306884765625
phase5 1 ~ 0.0 // end

In Java:

wheelRotation=0,preciseWheelRotation=-0.0222015380859375
wheelRotation=0,preciseWheelRotation=-0.0234222412109375
wheelRotation=0,preciseWheelRotation=-0.023956298828125
wheelRotation=0,preciseWheelRotation=-0.0242919921875
wheelRotation=0,preciseWheelRotation=-0.02447509765625
wheelRotation=0,preciseWheelRotation=-0.0246124267578125
wheelRotation=0,preciseWheelRotation=-0.024658203125
wheelRotation=0,preciseWheelRotation=-0.0222015380859375
wheelRotation=0,preciseWheelRotation=-0.0233306884765625

We ignored first two events, because of 0 & 0.0
We should not ignore last event, where 1 & 0.0
Seems we need to fix DeltaAccumulator.


On Fri, Sep 30, 2016 at 2:25 PM, Alexander Scherbatiy
<alexandr.scherba...@oracle.com> wrote:

Hello,

Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8166591/webrev.05

The momentumPhase is used to detect the trackpad events.

Thanks,
Alexandr.


On 30/09/16 14:58, Sergey Malenkov wrote:

# C  [AppKit+0x3a528e]  -[NSApplication _crashOnException:]+0x6d

The app is crashed as soon as I start scrolling. Investigating...
May be it is my fault during backporting.

Sorry, It was may fault.



LWCToolkit.m:
+// SCROLL EVENT MASK
+#define SCROLL_PHASE_UNSUPPORTED 1
...

replace the comment with the following one:

+// TRACKPAD SCROLL EVENT PHASE

I discovered how we should detect mouse event properly: use both
properties phase and momentumPhase.


https://developer.apple.com/library/prerelease/content/documentation/Cocoa/Conceptual/EventOverview/HandlingTouchEvents/HandlingTouchEvents.html

"The momentumPhase property helps you detect momentum scrolling, in
which the hardware continues to issue scroll wheel events even though
the user is no longer physically scrolling."

if (phase==NULL) && (momentumPhase==NULL) -> this is a mouse event.

scrolling by trackpad generates the following events:

phase=mayBegan momentumPhase=null
phase=began momentumPhase=null
phase=continued momentumPhase=null
...
phase=continued momentumPhase=null
phase=ended momentumPhase=null
phase=null momentumPhase=began
phase=null momentumPhase=continued
...
phase=null momentumPhase=continued
phase=null momentumPhase=ended

So, we should generate PHASE_UNSUPPORTED only
if ((phase == NULL) && (momentumPhase == NULL))
















--
Best regards, Sergey.

Re: [9] Review request for 8166591 [macos 10.12] Trackpad scrolling of text on OS X 10.12 Sierra is very fast (Trackpad, Retina only)

[Alexander Scherbatiy]

Hello,

Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8166591/webrev.06

- The CPlatformResponder.handleScrollEvent(...) is updated to dispatch  
a scroll event when delta or round delta is not equal to zero
- The native scrollStateWithPhase: method is updated to have NSEvent as 
an argument


Thanks,
Alexandr.

On 30/09/16 16:58, Sergey Malenkov wrote:

In the CPlatformResponder:

phase3 0 ~ 0.0 // mayBegan
phase2 0 ~ 0.0 // began
phase3 0 ~ 0.0222015380859375
phase3 0 ~ 0.0234222412109375
phase3 0 ~ 0.023956298828125
phase3 0 ~ 0.0242919921875
phase3 0 ~ 0.02447509765625
phase3 0 ~ 0.0246124267578125
phase3 0 ~ 0.024658203125
phase3 0 ~ 0.0222015380859375
phase3 0 ~ 0.0233306884765625
phase5 1 ~ 0.0 // end

In Java:

wheelRotation=0,preciseWheelRotation=-0.0222015380859375
wheelRotation=0,preciseWheelRotation=-0.0234222412109375
wheelRotation=0,preciseWheelRotation=-0.023956298828125
wheelRotation=0,preciseWheelRotation=-0.0242919921875
wheelRotation=0,preciseWheelRotation=-0.02447509765625
wheelRotation=0,preciseWheelRotation=-0.0246124267578125
wheelRotation=0,preciseWheelRotation=-0.024658203125
wheelRotation=0,preciseWheelRotation=-0.0222015380859375
wheelRotation=0,preciseWheelRotation=-0.0233306884765625

We ignored first two events, because of 0 & 0.0
We should not ignore last event, where 1 & 0.0
Seems we need to fix DeltaAccumulator.


On Fri, Sep 30, 2016 at 2:25 PM, Alexander Scherbatiy
<alexandr.scherba...@oracle.com> wrote:

Hello,

Could you review the updated fix:
   http://cr.openjdk.java.net/~alexsch/8166591/webrev.05

The momentumPhase is used to detect the trackpad events.

Thanks,
Alexandr.


On 30/09/16 14:58, Sergey Malenkov wrote:

# C  [AppKit+0x3a528e]  -[NSApplication _crashOnException:]+0x6d

The app is crashed as soon as I start scrolling. Investigating...
May be it is my fault during backporting.

Sorry, It was may fault.



LWCToolkit.m:
+// SCROLL EVENT MASK
+#define SCROLL_PHASE_UNSUPPORTED 1
...

replace the comment with the following one:

+// TRACKPAD SCROLL EVENT PHASE

I discovered how we should detect mouse event properly: use both
properties phase and momentumPhase.

https://developer.apple.com/library/prerelease/content/documentation/Cocoa/Conceptual/EventOverview/HandlingTouchEvents/HandlingTouchEvents.html
"The momentumPhase property helps you detect momentum scrolling, in
which the hardware continues to issue scroll wheel events even though
the user is no longer physically scrolling."

if (phase==NULL) && (momentumPhase==NULL) -> this is a mouse event.

scrolling by trackpad generates the following events:

phase=mayBegan momentumPhase=null
phase=began momentumPhase=null
phase=continued momentumPhase=null
...
phase=continued momentumPhase=null
phase=ended momentumPhase=null
phase=null momentumPhase=began
phase=null momentumPhase=continued
...
phase=null momentumPhase=continued
phase=null momentumPhase=ended

So, we should generate PHASE_UNSUPPORTED only
if ((phase == NULL) && (momentumPhase == NULL))

Re: [9] Review request for 8166591 [macos 10.12] Trackpad scrolling of text on OS X 10.12 Sierra is very fast (Trackpad, Retina only)

[Alexander Scherbatiy]

Hello,

Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8166591/webrev.05

The momentumPhase is used to detect the trackpad events.

Thanks,
Alexandr.

On 30/09/16 14:58, Sergey Malenkov wrote:

# C  [AppKit+0x3a528e]  -[NSApplication _crashOnException:]+0x6d

The app is crashed as soon as I start scrolling. Investigating...
May be it is my fault during backporting.

Sorry, It was may fault.



LWCToolkit.m:
+// SCROLL EVENT MASK
+#define SCROLL_PHASE_UNSUPPORTED 1
...

replace the comment with the following one:

+// TRACKPAD SCROLL EVENT PHASE

I discovered how we should detect mouse event properly: use both
properties phase and momentumPhase.
https://developer.apple.com/library/prerelease/content/documentation/Cocoa/Conceptual/EventOverview/HandlingTouchEvents/HandlingTouchEvents.html
"The momentumPhase property helps you detect momentum scrolling, in
which the hardware continues to issue scroll wheel events even though
the user is no longer physically scrolling."

if (phase==NULL) && (momentumPhase==NULL) -> this is a mouse event.

scrolling by trackpad generates the following events:

phase=mayBegan momentumPhase=null
phase=began momentumPhase=null
phase=continued momentumPhase=null
...
phase=continued momentumPhase=null
phase=ended momentumPhase=null
phase=null momentumPhase=began
phase=null momentumPhase=continued
...
phase=null momentumPhase=continued
phase=null momentumPhase=ended

So, we should generate PHASE_UNSUPPORTED only
if ((phase == NULL) && (momentumPhase == NULL))

Re: [9] Review request for 8166591 [macos 10.12] Trackpad scrolling of text on OS X 10.12 Sierra is very fast (Trackpad, Retina only)

[Alexander Scherbatiy]

Hello,

Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8166591/webrev.04

The fix uses the proposed changes below and sets a wheel rotation to +1 
or -1 when the scroll phase is ended and the accumulates delta value is 
small than the threshold.


Thanks,
Alexandr.

On 29/09/16 22:56, Sergey Malenkov wrote:

- The SCROLL_MASK_PHASE_CANCELLED  and SCROLL_MASK_PHASE_ENDED scroll masks
are added.

Now we use the scrollMask value and the following constants:

static final int SCROLL_MASK_WHEEL = 1;
static final int SCROLL_MASK_TRACKPAD = 1 << 1;
static final int SCROLL_MASK_PHASE_BEGAN = 1 << 2;
static final int SCROLL_MASK_PHASE_CANCELLED = 1 << 3;
static final int SCROLL_MASK_PHASE_ENDED = 1 << 4;

All these masks cannot be used together.
So I suggest to replace it with the scrollPhase value:

static final int SCROLL_PHASE_UNSUPPORTED = 0; // for mouse events
static final int SCROLL_PHASE_BEGAN = 1;
static final int SCROLL_PHASE_CONTINUED = 2;
static final int SCROLL_PHASE_CANCELLED = 3;
static final int SCROLL_PHASE_ENDED = 4;

It simplifies if-statements:
- if ((scrollMask & NSEvent.SCROLL_MASK_PHASE_BEGAN) != 0) {
+ if (scrollPhase == NSEvent.SCROLL_PHASE_BEGAN) {

and the following method:

+ (jint) scrollTypeToMask: (NSEventPhase) phase {
 if (phase) return SCROLL_PHASE_UNSUPPORTED;
 switch (phase) {
 case NSEventPhaseBegan:return SCROLL_MASK_PHASE_BEGAN;
 case NSEventPhaseCancelled:return SCROLL_MASK_PHASE_CANCELLED;
 case NSEventPhaseEnded:return SCROLL_MASK_PHASE_ENDED;
 }
 return SCROLL_PHASE_CONTINUED;
}

What do you think?

Re: [9] Review request for 8166591 [macos 10.12] Trackpad scrolling of text on OS X 10.12 Sierra is very fast (Trackpad, Retina only)

[Alexander Scherbatiy]

Hello,

Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8166591/webrev.03

- NSEvent constructor call is updated in the CTrayIcon.m
- The SCROLL_MASK_PHASE_CANCELLED  and SCROLL_MASK_PHASE_ENDED scroll 
masks are added.


Thanks,
Alexandr.

On 29/09/16 17:29, Sergey Malenkov wrote:

The signature of the NSEvent constructor is changed.
It is called from AWTView.m (fixed) and CTrayIcon.m (!not fixed!)

Could you please support not only the phase start, but the phase end too?
It will be useful, when we decide to support precise scrolling in JScrollPane,
because we will be able to align precise wheel rotation to integer part.
So when an user stops scrolling we can align lines in a lists or trees.



Could you review the fix:
   bug: https://bugs.openjdk.java.net/browse/JDK-8166591
   webrev: http://cr.openjdk.java.net/~alexsch/8166591/webrev.02

   This issue has been risen and investigated by JetBrains team in the
email:
http://mail.openjdk.java.net/pipermail/awt-dev/2016-September/011991.html

   MacOS Sierra 10.12 sends many scroll events with values close to zero
when the trackpad is used. The small scroll values are rounded to +1 or -1
so one line scroll would be possible. It leads that many small scroll events
causes fast scrolling.

   The proposed fix accumulates scroll values from the trackpad until they
exceed some threshold  values. Only after that MouseWheelEvent.wheelRotation
value is set. Mouse wheel scroll events are handled as before, small values
are just rounded to + or -1.

Re: RfR JDK-8160893 [macosx] JMenuItems in JPopupMenu are not accessible

[Alexander Scherbatiy]

The fix looks good to me.

I have also added awt-dev alias because fix changes the native part.

Thanks,
Alexandr.

On 26/09/16 16:22, Anton Tarasov wrote:

Hi Pete,

The fix looks fine to me.

Thanks,
Anton.

On 9/24/2016 2:59 AM, Pete Brunet wrote:

New webrev:
http://cr.openjdk.java.net/~ptbrunet/JDK-8160893/webrev.02/

I added a null check at line 688 like the one at line 693.

Pete

On 9/23/16 2:46 PM, Pete Brunet wrote:

Please review the patch for JDK-8160893.

Bug: https://bugs.openjdk.java.net/browse/JDK-8160893
Patch: http://cr.openjdk.java.net/~ptbrunet/JDK-8160893/webrev.01/

This patch adds support to post menu related events:
kAXMenuOpenedNotification, kAXMenuClosedNotification, and
kAXMenuItemSelectedNotification.  Also note that these changes had
impacts on the current combo box behavior so you'll see combo boxes
filtered out from this fix.  The combo box behavior is currently broken
and will be fixed at a later date but at least for now I didn't want
this patch to change that (broken) behavior.

TiA,
Pete

[9] Review request for 8166591 [macos 10.12] Trackpad scrolling of text on OS X 10.12 Sierra is very fast (Trackpad, Retina only)

[Alexander Scherbatiy]

Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8166591
  webrev: http://cr.openjdk.java.net/~alexsch/8166591/webrev.02

  This issue has been risen and investigated by JetBrains team in the 
email:

http://mail.openjdk.java.net/pipermail/awt-dev/2016-September/011991.html

  MacOS Sierra 10.12 sends many scroll events with values close to zero 
when the trackpad is used. The small scroll values are rounded to +1 or 
-1 so one line scroll would be possible. It leads that many small scroll 
events causes fast scrolling.


  The proposed fix accumulates scroll values from the trackpad until 
they exceed some threshold  values. Only after that 
MouseWheelEvent.wheelRotation value is set. Mouse wheel scroll events 
are handled as before, small values are just rounded to + or -1.


  Thanks,
  Alexandr.

Re: [9] Review Request JDK-8151787 Unify the HiDPI splash screen image naming convention

[Alexander Scherbatiy]

On 10/08/16 19:24, Alexandr Scherbatiy wrote:


On 8/9/2016 11:18 AM, Rajeev Chamyal wrote:


Hello All,

Please review the following webrev.

Bug: https://bugs.openjdk.java.net/browse/JDK-8151787

Webrev: http://cr.openjdk.java.net/~rchamyal/8151787/webrev.00/ 



Issue: Currently different naming conventions are used for Hidpi 
image on different platforms.


With this change the names will be unified across all platforms.

For a unscaled image image.ext following naming convention will be 
followed.


Unscaled name: image.ext

Supported Scaled Names:

If screen scale is integer number e.g. 2: im...@2x.ext 



If screen scale is float value like 1.25: im...@125pct.ext 





The fix should be reviewed on the awt-dev alias.

+ if(*scaleFactor - (int)*scaleFactor < 0.01)

Should there be so high precision there? Could only percent values be 
compared like

 if ((*scaleFactor *100) != ((int)(*scaleFactor)) * 100)


+//map the splash co-ordinates as per system scale
+splash->x /= splash->scaleFactor;
+splash->y /= splash->scaleFactor;

It looks like the splash coordinates and sizes are rescaled in 
different places. Is it possible to do that in the same place? May be 
in java_awt_SplashScreen.c file getBounds() function?


src/java.desktop/unix/native/libsplashscreen/splashscreen_sys.c
   *scaleFactor = getNativeScaleFactor();

Could you also include the change which requires to add some default 
output screen name to the getNativeScaleFactor() function on Linux. 
There is the discussion about that: 
http://mail.openjdk.java.net/pipermail/awt-dev/2016-August/011766.html


Thanks,
Alexandr.


Thanks,
Alexandr.


Regards,

Rajeev Chamyal

Re: [9] Review request for JDK-7156316: [macosx] Ctrl+Space does generate Unknown keychar

[Alexander Scherbatiy]

On 27/06/16 21:57, Manajit Halder wrote:

Hi All,

Kindly review the fix for JDK9.

*Bug*:
https://bugs.openjdk.java.net/browse/JDK-7156316
_
_
*Webrev*:
http://cr.openjdk.java.net/~mhalder/7156316/webrev.00/ 



*Issue: *
[macosx] Ctrl+Space does generate Unknown keychar

*Cause: *
SPACK key value was received as “ “ in function handleKeyEvent and 
that was correct value, but while sending the value as a character 
to function nsToJavaChar it was getting passed as 0. The 
function nsToJavaChar was returning 0 as unichar character for SPACE 
key as there was no code to handle the situation.

*Fix: *
An extra parameter was added in handleKeyEvent function indicating 
SPACE key and was passed to nsToJavaChar method to handle it.

156 if ("".equals(chars.trim())) {
  It is better to use: chars.trim().isEmpty() or may be spaceKeyTyped =
chars.trim().isEmpty().

  Thanks,
  Alexandr.



Regards,
Manajit

Re: [9] Review Request: 8157320 The CheckboxMenuItem can not be selected

[Alexander Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 25/05/16 00:43, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk9.

The fix for JDK-6191390 [1] does not take into account that the 
action(final long when) method is overridden in the 
XCheckboxMenuItemPeer.java and after JDK-6191390 it is never called.


In the fix it is updated properly.

[1] http://hg.openjdk.java.net/jdk9/client/jdk/rev/4ca4f2c8d975

Bug: https://bugs.openjdk.java.net/browse/JDK-8157320
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/8157320/webrev.00

Re: [8] Review request for JDK-8158178: java.awt.SplashScreen.getSize() returns incorrect size for high dpi splash screens

[Alexander Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 31/05/16 00:14, Robin Stevens wrote:

Hello,

please review the patch for JDK-8158178 
(https://bugs.openjdk.java.net/browse/JDK-8158178):


http://cr.openjdk.java.net/~alexsch/robin.stevens/8158178/jdk8/webrev.00/ 



Thanks,

Robin

Re: [9] Review request for JDK-8158178 java.awt.SplashScreen.getSize() returns incorrect size for high dpi splash screens

[Alexander Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 31/05/16 00:13, Robin Stevens wrote:

Hello,

Please review the update to the MultiResolutionSplashTest

http://cr.openjdk.java.net/~alexsch/robin.stevens/8158178/webrev.00/ 



which is a test for JDK-8158178 
(https://bugs.openjdk.java.net/browse/JDK-8158178)


Thanks,

Robin

Re: [9] Review Request: 8155785 Add @Deprecated annotations to the Applet API classes

[Alexander Scherbatiy]

 The fix looks good to me.

 Thanks,
 Alexandr.

On 13/05/16 06:56, Sergey Bylokhov wrote:

cc swing-dev.

Looks fine.

On 13.05.16 4:41, Daniil Titov wrote:

Hello,



Please review the fix for JDK 9.



The fix adds @Deprecated annotations to the Applet API classes as per
JEP 289 “Deprecate the Applet API” ( http://openjdk.java.net/jeps/289 ).
@SuppressWarnings("deprecation") annotations were added where required
to suppress the compilation warnings that treated as errors.



Bug:  https://bugs.openjdk.java.net/browse/JDK-8155785

Webrev : http://cr.openjdk.java.net/~dtitov/8155785/webrev.00



Best regards,

Daniil

Re: [9] Review request for 8154539 Examine the desktop module's use of sun.misc.SoftCache

[Alexander Scherbatiy]

On 12/05/16 03:18, Philip Race wrote:

Why doesn't the test need now
@modules java.desktop/sun.awt


   The test does not create a SoftCache or cast to it. It iterates over 
some object references graph and just skip the case where a reference 
class type name equals to "sun.awt.SoftCache$ValueCell".


  Thanks,
  Alexandr.

?

-phil.

On 5/11/16, 2:59 PM, Mandy Chung wrote:
On May 11, 2016, at 2:55 PM, Alexander 
Scherbatiy<alexandr.scherba...@oracle.com>  wrote:


On 12/05/16 01:50, Phil Race wrote:
Alexander ... I am concurrently deleting GThreadHelper (see webrev 
I just sent out)
which is covered by a separate bug so please revert that part of 
the fix.
  Sorry. Here is the updated fix where only SoftCache is moved to 
the java.desktop module and the test ReferrersTest is updated.

http://cr.openjdk.java.net/~alexsch/8154539/webrev.02/


+1

Mandy

Re: [9] Review request for 8154539 Examine the desktop module's use of sun.misc.SoftCache

[Alexander Scherbatiy]

Hello,

Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8154539/webrev.01

  - The test/com/sun/jdi/ReferrersTest.java test is updated to use 
sun.awt.SoftCache
  - The sun.misc.GThreadHelper class is moved to the sun.awt package in 
the java.desktop module


  The jdk.unsupported dependency was removed from the 
src/java.desktop/share/classes/module-info.java file.
  The sun.misc.GThreadHelper class is called by JNI and may be it does 
not require the declared dependency to the jdk.unsupported module. In 
any case it seems better to move the  GThreadHelper class to the 
java.desktop module.


  Thanks,
  Alexandr.


On 12/05/16 00:15, Mandy Chung wrote:

On May 11, 2016, at 1:07 PM, Alexander Scherbatiy 
<alexandr.scherba...@oracle.com> wrote:


Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8154539
  webrev: http://cr.openjdk.java.net/~alexsch/8154539/webrev.00

  This is a request from the jigsaw team to get rid of the dependency to 
jdk.unsupported module from the java.desktop module.
  The fix moves the sun.misc.SoftCache class to the sun.awt package in the 
java.desktop module.

Looks okay.  Moving sun.misc.SoftCache to java.desktop is fine.

test/com/sun/jdi/ReferrersTest.java references sun.misc.SoftCache.  I don’t 
know whether this test finds sun.misc.SoftCache in the reference graph at all.  
I suggest to update the test to check “sun.awt.SoftCache” instead and file a 
bug to the serviceability to follow up if that check is needed or not.

Mandy

[9] Review request for 8154539 Examine the desktop module's use of sun.misc.SoftCache

[Alexander Scherbatiy]

Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8154539
  webrev: http://cr.openjdk.java.net/~alexsch/8154539/webrev.00

  This is a request from the jigsaw team to get rid of the dependency 
to jdk.unsupported module from the java.desktop module.
  The fix moves the sun.misc.SoftCache class to the sun.awt package in 
the java.desktop module.


  The way of SoftCache replacement is covered by the issue JDK-8022449 
Can we get rid of sun.misc.SoftCache?


  Thanks,
  Alexandr.

Re: [9] Review request for 8017112 JTabbedPane components have inconsistent accessibility tree

[Alexander Scherbatiy]

  Hello,

  Cold you review the updated fix: 
http://cr.openjdk.java.net/~alexsch/8017112/webrev.01


  The (this instanceof Accessible) is removed from the fix.

  Thanks,
  Alexandr.

On 29/04/16 01:37, Sergey Bylokhov wrote:

Hi, Alexander.
Is the "instance of" check is necessary at the beginning of the method?
Usually we use the "instance of" to get AccessibleContext in code like 
this:

AccessibleContext accContext = ((Accessible)this).getAccessibleContext();

But it seems we have not the cast here, or it is necessary for some 
reason?


On 28.04.16 22:39, Alexander Scherbatiy wrote:


Hello,

Could you review the fix:
   bug: https://bugs.openjdk.java.net/browse/JDK-8017112
   webrev: http://cr.openjdk.java.net/~alexsch/8017112/webrev.00


   Component.getAccessibleIndexInParent() takes parent from components
hierarchy which can be different from the accessibility tree hierarchy.
The method is updated to use the right parent.


   Thanks,
   Alexandr.

Re: [9] Review request for JDK-8151787 Unify the HiDPI splash screen image naming convention

[Alexander Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 29/04/16 13:49, Rajeev Chamyal wrote:


Hello Alexandr,

Please review the updated fix.

http://cr.openjdk.java.net/~rchamyal/8151787/webrev.02/ 



- Will the java-scale2x image be chosen if fileName2x is nil on the 
line 163?


fileName2x cannot be nil SplashGetScaledImageName is called only if a 
not null value is passed in filename and


other parameters to new method findScaledImageName are constants.

- Could you add the use case where both @2x and java-scale2x are 
provided and @2x is chosen in the test?


I have updated the test as suggested.

Regards,

Rajeev Chamyal

*From:*Alexandr Scherbatiy
*Sent:* 28 April 2016 00:20
*To:* Rajeev Chamyal; awt-dev@openjdk.java.net; Sergey Bylokhov
*Subject:* Re:  [9] Review request for JDK-8151787 Unify the 
HiDPI splash screen image naming convention


On 4/27/2016 7:40 PM, Rajeev Chamyal wrote:

Hello Alexandr,

Please review the updated fix.

http://cr.openjdk.java.net/~rchamyal/8151787/webrev.01/



 162 fileName2x = findScaledImageName(fileName, dotIndex, @"@2x");
 163 if(fileName2x != nil && ![[NSFileManager defaultManager]
 164   fileExistsAtPath: fileName2x]) {
 165 fileName2x = findScaledImageName(fileName, dotIndex, 
@".java-scale2x");

 166 }

- Will the java-scale2x image be chosen if fileName2x is nil on the 
line 163?
- Could you add the use case where both @2x and java-scale2x are 
provided and @2x is chosen in the test?


Thanks,
Alexandr.


Regards,

Rajeev Chamyal

*From:*Alexandr Scherbatiy
*Sent:* 26 April 2016 14:22
*To:* Rajeev Chamyal; awt-dev@openjdk.java.net
; Sergey Bylokhov
*Subject:* Re:  [9] Review request for JDK-8151787 Unify
the HiDPI splash screen image naming convention

On 4/26/2016 11:13 AM, Rajeev Chamyal wrote:


Hello All,

Could you please review the following fix.

Bug : https://bugs.openjdk.java.net/browse/JDK-8151787

Webrev :
http://cr.openjdk.java.net/~rchamyal/8151787/webrev.00/


This is a small enhancement to support similar HiDPI splash
screen image name convention on all platforms.

Currently we have different naming convention for scaled
images on different platforms.

Image name : image.ext

Scaled image names:

Windows : image.scale-dpiValue.ext
Linux : image.java-scale2x.ext
MAC im...@2x.ext 

After the fix naming convention on Mac and Linux would be :

Image name : image.ext

Scaled image name : image.java-scale2x./ext/

Both name conventions @2x and java-scale2x should be supported
on Mac OS X.
The more specific one @2x should be checked in the first place
and the java-scale2x in the second.

   Thanks,
   Alexandr./
//


/

Naming convention on windows :

Image name : image.ext

Scaled image name : image.java-scale./ext/

Regards,

Rajeev Chamyal

[9] Review request for 8017112 JTabbedPane components have inconsistent accessibility tree

[Alexander Scherbatiy]

Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8017112
  webrev: http://cr.openjdk.java.net/~alexsch/8017112/webrev.00


  Component.getAccessibleIndexInParent() takes parent from components 
hierarchy which can be different from the accessibility tree hierarchy. 
The method is updated to use the right parent.



  Thanks,
  Alexandr.

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

[Alexander Scherbatiy]

On 18/04/16 20:55, Sergey Bylokhov wrote:

On 18.04.16 18:36, Alexander Scherbatiy wrote:

On 4/15/2016 10:55 PM, Sergey Bylokhov wrote:

On 15.04.16 21:49, Phil Race wrote:

I think that even if no exception is thrown we ought to document what
happens.
Which of course means that we need to be sure that it does happen :-)
And the difference depending on SM may be hard to defend.
And I am referring to both methods, not just the URL one.


The spec of checkImage/prepareImage both return incorrect status in
case of null, seems both should throw NPE also. otherwise they report
null as a valid fully loaded image.


Could you give more details why do you think that
checkImage/prepareImage methods return incorrect status in case of null
file path/URL?
The checkImage just returns a status of an image construction
"Information on the flags returned by this method can be found with the
discussion of the ImageObserver interface." and the prepareImage()
redirects all results to the ImageObserver.


Because the checkImage() return ALLBITS loaded for the null image 
which means that "all image data is currently available".
prepareImage() will return true for null, which means that "the image 
has already been fully prepared"and ready for rendering, note that an 
observer is not called that there are some errors. At lest this two 
methods should be investigated.


  The question under discussion is not about null images but about a 
ToolkitImage which has File/URLImageSource with null file/url because 
the Toolkit.getImage((String)null) returns non-null ToolkitImage which 
source points to a null file.


  Thanks,
  Alexandr.



   For example the call of Toolkit.getImage(file) method with path which
does not point to a file does not lead to the FileNotFoundException. The
ToolkitImage is created and just wait for the time when it is loaded.
Only during loading the ImageObserver is called with ABORT info flag.


There is a difference between incorrect parameters and some loading 
errors. For example createImage(byte[] imagedata,...) throws an 
exception because there is no sense to load the image from null array, 
the same for URL/File. ImageIO uses the same scheme(throws an 
exception if it is obvious that load of the image data will fail). 
Passing a null value to this api is a bug in the program, and it will 
be good to report it early.




The case with null path or URL should follow the same ToolkitImage
errors handling rule.





   Thanks,
   Alexandr.






-phil.

On 04/15/2016 11:47 AM, Alexander Scherbatiy wrote:

On 15/04/16 22:11, Phil Race wrote:

And it prints the stack trace ??

This gets even messier.

Is there anything we can do to have sensible specification of
what happens with null that isn't likely to cause (much) harm ?


 I see only two options. Do not throw the NPE as it was before and
throw the NPE.
 The second one can break some old applications which do not expect
that the exception can be thrown in this case.

 If the second option is acceptable probably the Toolkit.getImage()
javadoc can be updated.

 Thanks,
 Alexandr.



-phil.


On 04/15/2016 11:10 AM, Alexander Scherbatiy wrote:

On 15/04/16 20:30, Sergey Bylokhov wrote:

How the object of URLImageSource(null) will be useful? I guess we
will get some unspecified NPE exceptions if will try to call its
methods?

  I know nothing about the URLImageSource(null) usefulness but
ImageFetcher swallows the NPE during the image loading in the
fetchloop() method:
---
try {
src.doFetch();
} catch (Exception e) {
System.err.println("Uncaught error fetching 
image:");

e.printStackTrace();
}
---

  Thanks,
  Alexandr.


On 15.04.16 18:02, Alexander Scherbatiy wrote:


Hello,

Could you review the fix:
   bug: https://bugs.openjdk.java.net/browse/JDK-8132706
   webrev: http://cr.openjdk.java.net/~alexsch/8132706/webrev.00

   The fix makes the Toolkit.getDefaultToolkit().getImage(URL)
return a
ToolkitImage based on URLImageSource with null url as it was
before the
fix JDK-8011059.

   Thanks,
   Alexandr.

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

[Alexander Scherbatiy]

On 4/15/2016 10:55 PM, Sergey Bylokhov wrote:

On 15.04.16 21:49, Phil Race wrote:

I think that even if no exception is thrown we ought to document what
happens.
Which of course means that we need to be sure that it does happen :-)
And the difference depending on SM may be hard to defend.
And I am referring to both methods, not just the URL one.


The spec of checkImage/prepareImage both return incorrect status in 
case of null, seems both should throw NPE also. otherwise they report 
null as a valid fully loaded image.


   Could you give more details why do you think that 
checkImage/prepareImage methods return incorrect status in case of null 
file path/URL?
   The checkImage just returns a status of an image construction 
"Information on the flags returned by this method can be found with the 
discussion of the ImageObserver interface." and the prepareImage() 
redirects all results to the ImageObserver.


  For example the call of Toolkit.getImage(file) method with path which 
does not point to a file does not lead to the FileNotFoundException. The 
ToolkitImage is created and just wait for the time when it is loaded. 
Only during loading the ImageObserver is called with ABORT info flag.


   The case with null path or URL should follow the same ToolkitImage 
errors handling rule.


  Thanks,
  Alexandr.






-phil.

On 04/15/2016 11:47 AM, Alexander Scherbatiy wrote:

On 15/04/16 22:11, Phil Race wrote:

And it prints the stack trace ??

This gets even messier.

Is there anything we can do to have sensible specification of
what happens with null that isn't likely to cause (much) harm ?


 I see only two options. Do not throw the NPE as it was before and
throw the NPE.
 The second one can break some old applications which do not expect
that the exception can be thrown in this case.

 If the second option is acceptable probably the Toolkit.getImage()
javadoc can be updated.

 Thanks,
 Alexandr.



-phil.


On 04/15/2016 11:10 AM, Alexander Scherbatiy wrote:

On 15/04/16 20:30, Sergey Bylokhov wrote:

How the object of URLImageSource(null) will be useful? I guess we
will get some unspecified NPE exceptions if will try to call its
methods?

  I know nothing about the URLImageSource(null) usefulness but
ImageFetcher swallows the NPE during the image loading in the
fetchloop() method:
---
try {
src.doFetch();
} catch (Exception e) {
System.err.println("Uncaught error fetching image:");
e.printStackTrace();
}
---

  Thanks,
  Alexandr.


On 15.04.16 18:02, Alexander Scherbatiy wrote:


Hello,

Could you review the fix:
   bug: https://bugs.openjdk.java.net/browse/JDK-8132706
   webrev: http://cr.openjdk.java.net/~alexsch/8132706/webrev.00

   The fix makes the Toolkit.getDefaultToolkit().getImage(URL)
return a
ToolkitImage based on URLImageSource with null url as it was
before the
fix JDK-8011059.

   Thanks,
   Alexandr.

Re: [9] Review request for 8147842: IME Composition Window is displayed at incorrect location

[Alexander Scherbatiy]

On 4/1/2016 1:29 PM, Semyon Sadetsky wrote:
Please review the updated version : 
http://cr.openjdk.java.net/~ssadetsky/8147842/webrev.01/


fix was changed to avoid possible recursion in native IME evens 
handling which the SendMessage() call might cause.


  The SendMessage call has been added by the fix  JDK-8079595 Resizing 
dialog which is JWindow parent makes JVM crash
  Could you give more details why removing this code back will not 
cause new JVM crash?


  Thanks,
  Alexandr.




--Semyon

On 3/31/2016 7:38 PM, Semyon Sadetsky wrote:



On 3/31/2016 7:11 PM, Semyon Sadetsky wrote:

The solution is to support positioning for the IME candidate window.


It should be : "the IME *composition* window" of cause...

--Semyon

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

[Alexander Scherbatiy]

On 15/04/16 22:54, Sergey Bylokhov wrote:

On 15.04.16 21:47, Alexander Scherbatiy wrote:

  If the second option is acceptable probably the Toolkit.getImage()
javadoc can be updated.


As far as I understand, we always throw NPE in case of SecManager, and 
throw NPE w/o secManager since 7u40/8u25?


  Small correction. It started to throw NPE for the method 
getImage((URL)null) but not for getImage((String)null).


  Thanks,
  Alexandr.



Also please take a look to the similar bug
https://bugs.openjdk.java.net/browse/JDK-4358053



  Thanks,
  Alexandr.



-phil.


On 04/15/2016 11:10 AM, Alexander Scherbatiy wrote:

On 15/04/16 20:30, Sergey Bylokhov wrote:

How the object of URLImageSource(null) will be useful? I guess we
will get some unspecified NPE exceptions if will try to call its
methods?

  I know nothing about the URLImageSource(null) usefulness but
ImageFetcher swallows the NPE during the image loading in the
fetchloop() method:
---
try {
src.doFetch();
} catch (Exception e) {
System.err.println("Uncaught error fetching image:");
e.printStackTrace();
}
---

  Thanks,
  Alexandr.


On 15.04.16 18:02, Alexander Scherbatiy wrote:


Hello,

Could you review the fix:
   bug: https://bugs.openjdk.java.net/browse/JDK-8132706
   webrev: http://cr.openjdk.java.net/~alexsch/8132706/webrev.00

   The fix makes the Toolkit.getDefaultToolkit().getImage(URL)
return a
ToolkitImage based on URLImageSource with null url as it was before
the
fix JDK-8011059.

   Thanks,
   Alexandr.

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

[Alexander Scherbatiy]

On 15/04/16 22:11, Phil Race wrote:

And it prints the stack trace ??

This gets even messier.

Is there anything we can do to have sensible specification of
what happens with null that isn't likely to cause (much) harm ?


 I see only two options. Do not throw the NPE as it was before and 
throw the NPE.
 The second one can break some old applications which do not expect 
that the exception can be thrown in this case.


 If the second option is acceptable probably the Toolkit.getImage() 
javadoc can be updated.


 Thanks,
 Alexandr.



-phil.


On 04/15/2016 11:10 AM, Alexander Scherbatiy wrote:

On 15/04/16 20:30, Sergey Bylokhov wrote:
How the object of URLImageSource(null) will be useful? I guess we 
will get some unspecified NPE exceptions if will try to call its 
methods?
  I know nothing about the URLImageSource(null) usefulness but 
ImageFetcher swallows the NPE during the image loading in the 
fetchloop() method:

---
try {
src.doFetch();
} catch (Exception e) {
System.err.println("Uncaught error fetching image:");
e.printStackTrace();
}
---

  Thanks,
  Alexandr.


On 15.04.16 18:02, Alexander Scherbatiy wrote:


Hello,

Could you review the fix:
   bug: https://bugs.openjdk.java.net/browse/JDK-8132706
   webrev: http://cr.openjdk.java.net/~alexsch/8132706/webrev.00

   The fix makes the Toolkit.getDefaultToolkit().getImage(URL) 
return a
ToolkitImage based on URLImageSource with null url as it was before 
the

fix JDK-8011059.

   Thanks,
   Alexandr.

Re: [9] Review Request: 8154016 [macosx] Some HiDPI code can be removed

[Alexander Scherbatiy]

  The fix looks good to me.

  Thanks,
  Alexandr.

On 15/04/16 20:22, Sergey Bylokhov wrote:

Hello,
Please review the fix for jdk9.

Noop code was removed. It was not removed before, because it was 
assumed that it can be reused for HiDPI support(but it was implemented 
differently)


Bug: https://bugs.openjdk.java.net/browse/JDK-8154016
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/8154016/webrev.00

Re: [9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

[Alexander Scherbatiy]

On 15/04/16 19:26, prasanta sadhukhan wrote:

Hi Alex,

Just one observation.
If url is null in  getImageFromHash() then wouldn't we be getting NPE 
from checkPermissions(url) as


it calls URLUtil.getConnectPermission(url); which calls 
url.toString().toLowerCase()


so it will not come to your check , right?

String key = (url == null) ? null : url.toString();


 The behavior of the Toolkit.getImage(URL) method before the fix 
JDK-8011059 was that it did not throw NPE for null URL if the security 
manager is not set and threw NPE if it is set.


 It was the behavior that some applications can rely on.

 I am not sure if it is now possible to change the 
Toolkit.getImage(URL)  method behavior that it does not throw NPE with 
null URL when the SecurityManager is set.


  Thanks,
  Alexandr.


Regards
Prasanta
On 4/15/2016 8:32 PM, Alexander Scherbatiy wrote:


Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8132706
  webrev: http://cr.openjdk.java.net/~alexsch/8132706/webrev.00

  The fix makes the Toolkit.getDefaultToolkit().getImage(URL) return 
a ToolkitImage based on URLImageSource with null url as it was before 
the fix JDK-8011059.


  Thanks,
  Alexandr.

[9] Review request for 8132706 [macosx] Toolkit.getImage() throws NPE for null URL

[Alexander Scherbatiy]

Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8132706
  webrev: http://cr.openjdk.java.net/~alexsch/8132706/webrev.00

  The fix makes the Toolkit.getDefaultToolkit().getImage(URL) return a 
ToolkitImage based on URLImageSource with null url as it was before the 
fix JDK-8011059.


  Thanks,
  Alexandr.

Re: Public RequestFocusController/AWTAccessor API

[Alexander Scherbatiy]

  Hello Reto,

  Could you provide use cases which illustrate tasks the requested API 
is intended to solve in your application?


  Please, also create a request in the bug system 
http://bugreport.java.com/bugreport


  Thanks,
  Alexandr.

On 11/04/16 12:26, Alexander Scherbatiy wrote:


 Resending the request to awt-dev alias.

 Thanks,
 Alexandr.

On 4/4/2016 4:54 PM, Reto Merz wrote:

Hello,

Jigsaw will disallow access to internal packages.
We have written a complex validation and focus management
implementation
for our closed-source RIA and maintain it since JRE 1.4.

For this we use some internal API. We have a custom implementation of
these interfaces:

sun.awt.RequestFocusController
sun.awt.AWTAccessor.ComponentAccessor

And use this getter and setter:

sun.awt.AWTAccessor#setComponentAccessor(AWTAccessor.ComponentAccessor)
sun.awt.AWTAccessor#getComponentAccessor()
sun.awt.AWTAccessor.ComponentAccessor#setRequestFocusController(RequestFocusController) 



Please make this API public.

Furthermore we need to call
java.awt.Component#revalidateSynchronously().
We do this with reflection. It would be nice to have a public API for
this.
Maybe a new static method on AWTAccessor:
AWTAccessor.revalidateSynchronously(Component)

Best Regards
Reto Merz

Re: Public RequestFocusController/AWTAccessor API

[Alexander Scherbatiy]

 Resending the request to awt-dev alias.

 Thanks,
 Alexandr.

On 4/4/2016 4:54 PM, Reto Merz wrote:

Hello,

Jigsaw will disallow access to internal packages.
We have written a complex validation and focus management
implementation
for our closed-source RIA and maintain it since JRE 1.4.

For this we use some internal API. We have a custom implementation of
these interfaces:

sun.awt.RequestFocusController
sun.awt.AWTAccessor.ComponentAccessor

And use this getter and setter:

sun.awt.AWTAccessor#setComponentAccessor(AWTAccessor.ComponentAccessor)
sun.awt.AWTAccessor#getComponentAccessor()
sun.awt.AWTAccessor.ComponentAccessor#setRequestFocusController(RequestFocusController)

Please make this API public.

Furthermore we need to call
java.awt.Component#revalidateSynchronously().
We do this with reflection. It would be nice to have a public API for
this.
Maybe a new static method on AWTAccessor:
AWTAccessor.revalidateSynchronously(Component)

Best Regards
Reto Merz

Re: [OpenJDK 2D-Dev] [9] Review request for 8073320 Windows HiDPI Graphics support

[Alexander Scherbatiy]

On 01/04/16 06:24, Jim Graham wrote:

Hi Alexandr,

Is there a bug filed to upgrade JLightweightFrame to allow non-integer 
(and X/Y) scales?  We'd need this capability to implement "Swing 
embedded in FX" correctly on Win8.1+...


  I have created JDK-8153522 Update JLightweightFrame to allow 
non-integer (and X/Y) scales

https://bugs.openjdk.java.net/browse/JDK-8153522

  Thanks,
  Alexandr.


...jim

On 9/22/2015 2:33 AM, Alexander Scherbatiy wrote:


Hello,

Could you review the fix:
   bug: https://bugs.openjdk.java.net/browse/JDK-8073320
   webrev: http://cr.openjdk.java.net/~alexsch/8073320/webrev.00

   This is an initial part of the HiDPI Graphics support on Windows for
the JEP 263: HiDPI Graphics on Windows and Linux
 http://openjdk.java.net/jeps/263

   - scale factors are added to surface dates
   - window size, events coordinates and font are scaled on native side
   - backup buffered image is scaled in SunVolatileImage
   - AwtRobot MouseMove() and GetRGBPixels() methods are updated
   - GetDpiForMonitor function is used to query the specified monitor
for the horizontal and vertical DPI values.
 If it is not available ID2D1Factory::GetDesktopDpi method is used
instead.
   - "sun.java2d.uiScale.enabled", "sun.java2d.win.uiScale",
"sun.java2d.win.uiScaleX", and "sun.java2d.win.uiScaleY" options are
added for the testing purposes.

Thanks,
Alexandr.

Re: [9] Review Request for 8080395: consider making sun.awt.CausedFocusEvent functionality public

[Alexander Scherbatiy]

  The fix looks good to me.

  Thanks,
  Alexandr.

On 4/4/2016 9:48 AM, Semyon Sadetsky wrote:

Thank you, Phil.
The updates webrev: 
http://cr.openjdk.java.net/~ssadetsky/8080395/webrev.05/


--Semyon

On 3/30/2016 7:22 PM, Phil Race wrote:

>  39 private static final long serialVersionUID = -3647309088427840738L;

Is this the value generated by JDK 8 ? It should be ..

I agree getCause() should be final.


Other than that this seems fine to me.

BTW I would have preferred that the order of files  in the webrev 
place FocusEvent
as the first file. Almost all the changes flow from that so we should 
get to read it first.


-phil.

On 03/23/2016 12:33 AM, Semyon Sadetsky wrote:

On 3/22/2016 7:54 PM, Sergey Bylokhov wrote:
I am not sure that the latest version became better than previous 
one. The code became much more complicated but do we really solved 
the problem related to serialization? Will we support both 
directions when the data serialized/deserialized in jkd9/jdk8/jdk7 
and so on. The readResolve() helped us to deserialize the data from 
the jdk8, but it does not help in case of jdk9 to jdk8 
serialisation, because our new FocusEvent from jdk9 should be 
converted to the old CausedFocusEvent but this will requires 
changes in jdk8/jdk7.
Sergey, we must not support "forward compatibility" for AWT 
serialization,  backward compatibility is enough.


I understand the idea of "CausedFocusEvent exists for 
deserialization compatibility only." but not sure how it is good. 
Probably someone knows, do we have similar cases before? When we 
have some outdated internal class just for serialization 
compatibility?


I still think that it will be better to update serial version UID, 
implement readObject() to throw an exception if "cause" is null, 
and possibly update the specification. In the same way as in most 
classes in the client.


 - Why did your remove the "final" from the getCause() method? This 
will automatically blocks us from calling this method(or at least 
use it quite carefully) inside the jdk, because it can be 
overridden by the user. I suggest to restore it.
It is not needed. There is no even one "final" method in the whole 
"java.awt.event.*" package.


--Semyon

 - Small comment: the "and" before "opposite" can be changed to ","?
182  * specified temporary state and opposite {@code Component} 
and the

183  * {@code Cause.UNKNOWN} cause.

On 02.02.16 15:24, Semyon Sadetsky wrote:

Please review the updated webrev:
http://cr.openjdk.java.net/~ssadetsky/8080395/webrev.04/

- CausedFocusEvent is restored to avoid CNFE during deserialization.

- readResolve() is added to FocusEvent to handle the null cause test.

- deserialization compatibility test scenarios added

--Semyon

On 11/6/2015 3:57 PM, Alexander Scherbatiy wrote:


There is a problem with FocusEvent deserialization that the read 
cause

value is not checked to null.
You can fix it with the current fix or just create a separate 
issue on

it.

Otherwise, the fix looks good to me.

Thanks,
Alexandr.

On 10/29/2015 12:33 AM, Sergey Bylokhov wrote:

On 27.10.15 13:01, Semyon Sadetsky wrote:



On 10/26/2015 5:31 PM, Sergey Bylokhov wrote:

The test should verify this also.
I assume it should be reverted and updated to:
252 if (cause == null)
253 throw new IllegalArgumentException("null cause");

Please review the updated webrev
http://cr.openjdk.java.net/~ssadetsky/8080395/webrev.02/


is possible to write a test fox this missed check?
What about a sentence in:

228  *  This method throws an
229  * {@code IllegalArgumentException} if {@code source}
230  * is {@code null}.

250 public FocusEvent(Component source, int id, boolean 
temporary,

251   Component opposite, Cause cause) {

Should we mention cause and IAE as well in the javadoc?

Test case is added. null case is mentioned in javadoc.
http://cr.openjdk.java.net/~ssadetsky/8080395/webrev.03/


The fix looks fine. I am not sure about this:
* {@code IllegalArgumentException} if {@code source} or {@code 
cause}

* is {@code null}.
I suppose it should be "are {@code null}?



Since you update the serialVersionUID then the comment is 
not valid

anymore: "JDK 1.1 serialVersionUID".

Also I have requested an additional clarification from the 
core libs

team to confirm that we have an ability to change
serialVersionUID in
the major release.
Please decide finally you do need a new serialVersionUID or 
you don't
need it. Generally, adding a field is a compatible change 
FocusEvent

will be able to read the previous format. It is matter of
opinion/convention only.


In this case the cause field will contain unspecified null value.
I guess we will need to file a new ccc request.






On 11.09.15 0:15, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.jav

Re: [9] Review Request: 8151773 [macosx] TrayIcon.imageAutoSize property is ignored

[Alexander Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 15/03/16 04:54, Sergey Bylokhov wrote:

Hello,
Please review the small fix for jdk9.

According to specification if imageAutoSize property was set to true 
the image should be the stretched or shrunk to fit the tray icon 
space. But on OSX this flag is ignored in the native code.


In the fix this flag is taken into account, if it was set to true. 
otherwise the previous logic is executed:

- Small images are placed as is.
- Big images are proportionally down-scaled(I suppose this is done for 
some usability issue)


Bug: https://bugs.openjdk.java.net/browse/JDK-8151773
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/8151773/webrev.00

Re: [9] Review Request for several test bugs: 8150535, 8151033, 8151037 etc.

[Alexander Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 24/03/16 18:17, Yuri Nesterenko wrote:

Hi,

please review this test-only fix, a leftover from Jigsaw M3 integration.
Some 7 tests fixed but note that not all of them always pass even fixed.
Webrev:
http://cr.openjdk.java.net/~yan/8150535/webrev.01

I need to file a bug about the fixed here manual test
java/awt/xembed/server/TestXEmbedServerJava

Thank you!

-yan

Re: [9] Review Request for 8036915: setLocationRelativeTo stopped working in Ubuntu 13.10 (Unity)

[Alexander Scherbatiy]

On 18/02/16 14:26, Semyon Sadetsky wrote:

I have found the way to avoid iterative queries for frame insets.
The updated webrev: 
http://cr.openjdk.java.net/~ssadetsky/8036915/webrev.01/

- 269 if (wm_set_insets != null && !isNull(wm_set_insets))
isNull(Insets i) function checks the given argument to null. It seems 
that the first check is unnecessary in this case.


- Could not null insets with zero values be valid insets?

-  396 if (!isNull(correction)) {
Could this construction be changed to "if (isNull(correction)) { return; 
}" ?


-if (!insets_corrected && getDecorations() != 
XWindowAttributesData.AWT_DECOR_NONE) {

+if (getDecorations() != XWindowAttributesData.AWT_DECOR_NONE) {
handleReparentNotifyEvent() calls XWM.getWM().getInsets(this, 
xe.get_window(), xe.get_parent())
in case correctWM is null but handleConfigureNotifyEvent() is updated to 
do it even insets has been corrected.

What is the reason to handle these two cases differently?

- Could you run JCK with the provided fix to check that there are no 
regressions?


Thanks,
Alexandr.



--Semyon


On 10/29/2015 12:20 AM, Sergey Bylokhov wrote:

On 06.10.15 9:15, Semyon Sadetsky wrote:

Sorry. I meant it is not guaranteed to be sent by every WM.


But fetch the data of 100 times also doesn't guarantee that we 
will get the necessary data. It will be better at least to try to 
check the specified atom for this(it seems it is supported by all our 
platfrom). For example on osx insets can be changed on the fly w/o 
notification, because of this we fetch the insets on each 
reshapeEvent and posts the necessary message up to hierarchy.





On 10/6/2015 9:03 AM, Semyon Sadetsky wrote:

In is extended event. In does not guaranteed to be sent by any WM.

On 10/5/2015 6:12 PM, Sergey Bylokhov wrote:

Why we cannot treat such a XA_NET_FRAME_EXTENTS as a ConfigureNotify
in which only insets are changed, and the window bounds/insets should
be revalidated?

On 05.10.15 14:56, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8036915
webrev: http://cr.openjdk.java.net/~ssadetsky/8036915/webrev.00/

The test scenario attached to the jira contains potential errors
because
Componet.getLocation() won't return the location at any moment of 
time.
Anyway a window location issue exists in Unity. The root cause is 
that

the real insets sent with the XA_NET_FRAME_EXTENTS event can be
overridden by the "guessed" insets which are zero on Unity. So the
returned location is increased by real insets while the real window
location is correct.
Yet another issue I found in Unity is a window size issue which 
is also
caused by the frame insets detection. The Unity WM doesn't 
provide the
frame insets immediately and XA_NET_FRAME_EXTENTS event may come 
after

the ConfigureNotify event for the frame.
The solution proposes an adaptation of the existing frame insets
request
algorithm to the Unity WM so that makes it more stable.

--Semyon

Re: Review request for 8151998 VS2010 ThemeReader.cpp(758) : error C3861: 'round': identifier not found

[Alexander Scherbatiy]

Hello,

Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8151998/webrev.01

The _MSC_VER predefined macros is used for the Visual Studio version 
detection.


 Thanks,
 Alexandr.

On 3/16/2016 4:27 PM, Alexander Scherbatiy wrote:


Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8151998
  webrev: http://cr.openjdk.java.net/~alexsch/8151998/webrev.00

  The fix uses the "ROUND_TO_INT(num) ((int) floor((num) + 0.5))" 
instead round function which is not compiled by VS2010.


Thanks,
Alexandr.

Re: [9] Review request for JDK-8145173 HiDPI splash screen support on Windows

[Alexander Scherbatiy]

On 23/03/16 13:56, Rajeev Chamyal wrote:

Hello Alexandr,

I have updated the webrev as per review comments.

http://cr.openjdk.java.net/~rchamyal/8145173/webrev.06/


  The fix looks good to me.

  Thanks,
  Alexandr.


Regards,
Rajeev Chamyal

-Original Message-
From: Alexandr Scherbatiy
Sent: 22 March 2016 20:15
To: Rajeev Chamyal; Sergey Bylokhov; awt-dev@openjdk.java.net; 
build-...@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8145173 HiDPI splash screen 
support on Windows

On 3/22/2016 12:43 AM, Rajeev Chamyal wrote:

Hello All,

Please review the updated webrev.
http://cr.openjdk.java.net/~rchamyal/8145173/webrev.05/

With VC2010  java.c ::ShowSplashScreen  fails with segmentation fault on  
calling free on scaled_splash_name .
This failure is due to different C runtime used by libjli and lib splashScreen.
Fix: Allocating a max scaled image name length buffer in java.c and
passing it to splashscreen_sys.c

 Just few small comments:
 - SplashGetScaledImageName can return a boolean value which indicates that 
high-resolution splash screen has been found
 - The getMaxScaledImageNamePostfixLen function can be added which just 
returns max length of the scaled image postfix.
   It allows to control this value on the java.desktop side rather to keep 
it in java.c file
 - systemScale.cpp copyright need to be updated to 2016

Thanks,
Alexandr.

Regards,
Rajeev Chamyal

-Original Message-
From: Rajeev Chamyal
Sent: 15 March 2016 18:26
To: Alexander Scherbatiy; Sergey Bylokhov; awt-dev@openjdk.java.net;
build-...@openjdk.java.net
Subject: RE:  [9] Review request for JDK-8145173 HiDPI splash
screen support on Windows

Hello All,

Please review the updated webrev.
http://cr.openjdk.java.net/~rchamyal/8145173/webrev.04/

Alexandr : I have build code with VS2013 and I didn't get any errors you 
mentioned.
Still I have updated the code as suggested.

Added build team for make file review. JPRT build with fix was successful.

Regards,
Rajeev Chamyal

-Original Message-
From: Alexander Scherbatiy
Sent: 14 March 2016 17:28
To: Rajeev Chamyal
Cc: Sergey Bylokhov; awt-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8145173 HiDPI splash
screen support on Windows

On 3/14/2016 8:03 AM, Rajeev Chamyal wrote:

Hello Sergey,

Could you please review the enhancement.

I have raised a new enhancement for unifying the splash screen image
names across platforms.

https://bugs.openjdk.java.net/browse/JDK-8151787


575 float *scaleFactor)
576 {
577 *scaleFactor = 1.0;
578 float dpiScaleX = -1.0f;
579 float dpiScaleY = -1.0f;
580 GetScreenDpi(getPrimaryMonitor(), , );

I have errors building the fix with VS2010:

* For target
support_native_java.desktop_libsplashscreen_splashscreen_sys.obj:
splashscreen_sys.c
jdk/src/java.desktop/windows/native/libsplashscreen/splashscreen_sys.c
(578)
: error C2143: syntax error : missing ';' before 'type'
jdk/src/java.desktop/windows/native/libsplashscreen/splashscreen_sys.c
(579)
: error C2143: syntax error : missing ';' before 'type'


You need to move variables declaration to the beginning of a statement.

Thanks,
Alexandr.


Regards,

Rajeev Chamyal

*From:*Alexandr Scherbatiy
*Sent:* 10 March 2016 18:46
*To:* Rajeev Chamyal; awt-dev@openjdk.java.net; Sergey Bylokhov
*Subject:* Re:  [9] Review request for JDK-8145173 HiDPI
splash screen support on Windows

The fix looks good to me.

Thanks,
Alexandr.

On 3/10/2016 3:05 AM, Rajeev Chamyal wrote:

  Hello Alexandr,

  Thanks for the review. Below is the updated webrev as per review
  comments.

  http://cr.openjdk.java.net/~rchamyal/8145173/webrev.03/
  <http://cr.openjdk.java.net/%7Erchamyal/8145173/webrev.03/>

  Regards,

  Rajeev Chamyal

  *From:*Alexandr Scherbatiy
  *Sent:* 10 March 2016 11:39
  *To:* Rajeev Chamyal; awt-dev@openjdk.java.net
  <mailto:awt-dev@openjdk.java.net>; Sergey Bylokhov
  *Subject:* Re:  [9] Review request for JDK-8145173 HiDPI
  splash screen support on Windows

  On 3/2/2016 9:50 PM, Rajeev Chamyal wrote:


  Hello All,

  Please review the updated webrev.

  Added a free call for duplicate file name in
  splashscreen_sys.c :: SplashGetScaledImageName

  http://cr.openjdk.java.net/~rchamyal/8145173/webrev.02/
  <http://cr.openjdk.java.net/%7Erchamyal/8145173/webrev.02/>


 awt_Win32GraphicsDevice.cpp
   656 dpiX = GetScreenDpi(GetMonitor());
   657 if (dpiX > 0) {
   658 dpiX = dpiX >= 96 ? dpiX / 96 : dpiX;
   659 SetScale(dpiX, dpiX);

  The Windows HiDPI graphics support was designed to handle both DPI
  X and Y scales. The GetScreenDpi should return both values to be
  used in SetScale method.

  systemScale.cpp

 38

Re: [9] Review request for 8150844 [hidpi] [macosx] should -Dsun.java2d.uiScale be taken into account for OS X?

[Alexander Scherbatiy]

Could you review the updated fix:
   http://cr.openjdk.java.net/~alexsch/8150844/webrev.01/

 - the is2x() method is removed from the tests
 - imports are updated

On 16/03/16 13:25, Alexander Stepanov wrote:

Hello Alexandr,

just a minor remark about is2x() in tests:

do we still need this method in MultiresolutionIconTest?
The following simple check can be used in doTest() instead:

boolean is2x = "2".equals(scale);
Color
expected = is2x ? C2X : C1X,
unexpected = is2x ? C1X : C2X;

the same, probably, for MenuMultiresolutionIconTest, but here it is a 
matter of taste.


It seems also that the import for java.awt.geom.AffineTransform is 
unused for Corrupted2XImageTest (and, in case of changes, in 
MenuMultiresolutionIconTest).


Thanks,
Alexander

P.S. MultiresolutionIconTest: "@requires (os.family != "mac")" is 
connected to JDK-8151303, so the test will fail on OS X until it is 
fixed (having this line removed).
  It should be fine because bug number 8151303 is mentioned in the @bug 
tag and is counted as a known issue.


  Thanks,
  Alexandr.



On 3/16/2016 11:29 AM, Alexander Scherbatiy wrote:


Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8150844
  webrev: http://cr.openjdk.java.net/~alexsch/8150844/webrev.00

  The sun.java2d.uiScale property is now used in CGraphicsDevice.
  The requested tests are updated.

  Thanks,
  Alexandr.

Re: [9] RFR for 8151714: [TEST] add a manual test test for JOptionPane dialog multires. icons

[Alexander Scherbatiy]

The MultiResolutionJOptionPaneIconTest extends JFrame so it shoud be 
created and used on EDT as others Swing components.


Thanks,
Alexndr.

On 14/03/16 18:26, Alexander Stepanov wrote:

Hello Semyon,

> JOptionPane.showInternalInputDialog() is an utility method
Yes, the initial intention was to check the icons for all the dialogs 
created by means of these utilities. But in principle these options 
could be skipped, and the test could be limited by checking the icons 
for JOptionPane's createDialog() and createInternalFrame() methods.


In such a case the test indeed may be easily converted to the 
automated one.


> What doesn't work exactly, the dispose() call?
Please disregard. Simply shouldn't create and dispose the dialog at 
the same EDT.


So please see the updated webrev:
http://cr.openjdk.java.net/~avstepan/8151714/webrev.01/

Thanks,
Alexander

On 3/11/2016 8:38 PM, Semyon Sadetsky wrote:



On 3/11/2016 7:38 PM, Alexander Stepanov wrote:

> just call dispose().

1.
SwingUtilities.invokeAndWait(() -> { 
JOptionPane.showInternalInputDialog(...); });

r.waitForIdle();

dispose what? no reference to the dialog shown. or do you mean again 
use of component tree?

JOptionPane.showInternalInputDialog() is an utility method.


2.

this trivial way:

JOptionPane op = new JOptionPane(msg, info, opt, icon);
JDialog dlg = op.createDialog(parentPane, "test");
SwingUtilities.invokeAndWait(() -> { dlg.setVisible(true); });
r.waitForIdle(2000);
dlg.dispose();

- doesn't work as well until "ok" button isn't pressed by the user 
(and it differs from the initial test variety). not sure if it could 
be fixed by some thread tricks.

What doesn't work exactly, the dispose() call?

--Semyon


Thanks,
Alexander

On 3/11/2016 7:17 PM, Semyon Sadetsky wrote:



On 3/11/2016 6:33 PM, Alexander Stepanov wrote:

> traverse the component tree
Hm, interesting. But I'm not sure if everything is smoothly as, 
e.g., it is also required to close the dialogs one-by-one (for now 
the user does that), and I'm not sure if it is easy to implement.

Seriously? It is damn easy: just call dispose().


On 3/11/2016 5:55 PM, Semyon Sadetsky wrote:



On 3/11/2016 5:45 PM, Alexander Stepanov wrote:

*inside of the internal pane
parent pane

On 3/11/2016 5:42 PM, Alexander Stepanov wrote:

Hello Semyon,

I'm not sure if we can control the location of the dialogs 
shown (and of course we don't know the location of the icons on 
them), can we?
Yes, we can: Component# getLocationOnScreen() and traverse the 
component tree to find the Label component with the icon.


--Semyon
It is clear that the internal dialogs are located somewhere 
inside of the internal pane, but this is not true for the others.


So (probably I'm wrong) there is seemingly no an elegant way to 
predict the location of the icons, and simple iteration over 
the screen coordinates (checking for the pixel color) looks not 
very reliable.


Thanks,
Alexander

On 3/11/2016 5:24 PM, Semyon Sadetsky wrote:

Hi Alexandr,

Can the test be automated? It is possible to read color from 
the screen with AWT Robot.


--Semyon

On 3/11/2016 4:53 PM, Alexander Stepanov wrote:

Hello,

Could you please review the fix
http://cr.openjdk.java.net/~avstepan/8151714/webrev.00/
for
https://bugs.openjdk.java.net/browse/JDK-8151714
- just a single test added.

Thanks,
Alexander

Review request for 8151998 VS2010 ThemeReader.cpp(758) : error C3861: 'round': identifier not found

[Alexander Scherbatiy]

Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8151998
  webrev: http://cr.openjdk.java.net/~alexsch/8151998/webrev.00

  The fix uses the "ROUND_TO_INT(num) ((int) floor((num) + 0.5))" 
instead round function which is not compiled by VS2010.


Thanks,
Alexandr.

Re: [9] Review request for 8150844 [hidpi] [macosx] should -Dsun.java2d.uiScale be taken into account for OS X?

[Alexander Scherbatiy]

Hello,

Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8150844/webrev.02/

 The GTKLookAndFeel setting is changed to the system L in the 
HiDPIPropertiesUnixTest.


 Thanks,
 Alexandr.

On 16/03/16 20:34, Sergey Bylokhov wrote:

HiDPIPropertiesUnixTest will be noop on OSX:
 try {
  50 UIManager.setLookAndFeel(
  51 "com.sun.java.swing.plaf.gtk.GTKLookAndFeel");
  52 } catch (Exception e) {
  53 return;
  54 }

On 16.03.16 15:12, Alexander Scherbatiy wrote:


Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8150844/webrev.01/

  - the is2x() method is removed from the tests
  - imports are updated

On 16/03/16 13:25, Alexander Stepanov wrote:

Hello Alexandr,

just a minor remark about is2x() in tests:

do we still need this method in MultiresolutionIconTest?
The following simple check can be used in doTest() instead:

boolean is2x = "2".equals(scale);
Color
expected = is2x ? C2X : C1X,
unexpected = is2x ? C1X : C2X;

the same, probably, for MenuMultiresolutionIconTest, but here it is a
matter of taste.

It seems also that the import for java.awt.geom.AffineTransform is
unused for Corrupted2XImageTest (and, in case of changes, in
MenuMultiresolutionIconTest).

Thanks,
Alexander

P.S. MultiresolutionIconTest: "@requires (os.family != "mac")" is
connected to JDK-8151303, so the test will fail on OS X until it is
fixed (having this line removed).

   It should be fine because bug number 8151303 is mentioned in the @bug
tag and is counted as a known issue.

   Thanks,
   Alexandr.



On 3/16/2016 11:29 AM, Alexander Scherbatiy wrote:


Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8150844
  webrev: http://cr.openjdk.java.net/~alexsch/8150844/webrev.00

  The sun.java2d.uiScale property is now used in CGraphicsDevice.
  The requested tests are updated.

  Thanks,
  Alexandr.

Re: [9] RFR for 8151714: [TEST] add a manual test test for JOptionPane dialog multires. icons

[Alexander Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 17/03/16 11:15, Alexander Stepanov wrote:

Hello Alexandr,

> it should be created and used on EDT
the following methods affecting UI are run on EDT: UI(), 
showDialogOrFrame() and dispose() operations in doTest (the final 
'dispose' is indeed should be run on EDT as well). The robot 
operations/checks are executed on main thread using volatile/final 
references to get the coordinates.


But if this is still not safe enough, then this 'extends JFrame' could 
be easily removed as it doesn't make any special sense.


Please see the updated fix:
http://cr.openjdk.java.net/~avstepan/8151714/webrev.02/
(the transform-based scale check was also removed as it seems that the 
fix for JDK-8150844 is near to be pushed).


Thanks,
Alexander


On 3/16/2016 8:55 PM, Alexander Scherbatiy wrote:


The MultiResolutionJOptionPaneIconTest extends JFrame so it shoud be 
created and used on EDT as others Swing components.


Thanks,
Alexndr.

On 14/03/16 18:26, Alexander Stepanov wrote:

Hello Semyon,

> JOptionPane.showInternalInputDialog() is an utility method
Yes, the initial intention was to check the icons for all the 
dialogs created by means of these utilities. But in principle these 
options could be skipped, and the test could be limited by checking 
the icons for JOptionPane's createDialog() and createInternalFrame() 
methods.


In such a case the test indeed may be easily converted to the 
automated one.


> What doesn't work exactly, the dispose() call?
Please disregard. Simply shouldn't create and dispose the dialog at 
the same EDT.


So please see the updated webrev:
http://cr.openjdk.java.net/~avstepan/8151714/webrev.01/

Thanks,
Alexander

On 3/11/2016 8:38 PM, Semyon Sadetsky wrote:



On 3/11/2016 7:38 PM, Alexander Stepanov wrote:

> just call dispose().

1.
SwingUtilities.invokeAndWait(() -> { 
JOptionPane.showInternalInputDialog(...); });

r.waitForIdle();

dispose what? no reference to the dialog shown. or do you mean 
again use of component tree?

JOptionPane.showInternalInputDialog() is an utility method.


2.

this trivial way:

JOptionPane op = new JOptionPane(msg, info, opt, icon);
JDialog dlg = op.createDialog(parentPane, "test");
SwingUtilities.invokeAndWait(() -> { dlg.setVisible(true); });
r.waitForIdle(2000);
dlg.dispose();

- doesn't work as well until "ok" button isn't pressed by the user 
(and it differs from the initial test variety). not sure if it 
could be fixed by some thread tricks.

What doesn't work exactly, the dispose() call?

--Semyon


Thanks,
Alexander

On 3/11/2016 7:17 PM, Semyon Sadetsky wrote:



On 3/11/2016 6:33 PM, Alexander Stepanov wrote:

> traverse the component tree
Hm, interesting. But I'm not sure if everything is smoothly as, 
e.g., it is also required to close the dialogs one-by-one (for 
now the user does that), and I'm not sure if it is easy to 
implement.

Seriously? It is damn easy: just call dispose().


On 3/11/2016 5:55 PM, Semyon Sadetsky wrote:



On 3/11/2016 5:45 PM, Alexander Stepanov wrote:

*inside of the internal pane
parent pane

On 3/11/2016 5:42 PM, Alexander Stepanov wrote:

Hello Semyon,

I'm not sure if we can control the location of the dialogs 
shown (and of course we don't know the location of the icons 
on them), can we?
Yes, we can: Component# getLocationOnScreen() and traverse the 
component tree to find the Label component with the icon.


--Semyon
It is clear that the internal dialogs are located somewhere 
inside of the internal pane, but this is not true for the 
others.


So (probably I'm wrong) there is seemingly no an elegant way 
to predict the location of the icons, and simple iteration 
over the screen coordinates (checking for the pixel color) 
looks not very reliable.


Thanks,
Alexander

On 3/11/2016 5:24 PM, Semyon Sadetsky wrote:

Hi Alexandr,

Can the test be automated? It is possible to read color from 
the screen with AWT Robot.


--Semyon

On 3/11/2016 4:53 PM, Alexander Stepanov wrote:

Hello,

Could you please review the fix
http://cr.openjdk.java.net/~avstepan/8151714/webrev.00/
for
https://bugs.openjdk.java.net/browse/JDK-8151714
- just a single test added.

Thanks,
Alexander

Re: [9] Review request for 8150844 [hidpi] [macosx] should -Dsun.java2d.uiScale be taken into account for OS X?

[Alexander Scherbatiy]

On 3/16/2016 3:21 PM, Sergey Bylokhov wrote:

So fast, I just prepared the similar fix

Why the scale is "1" when isUIScaleEnabled is not enabled?
  Because sun.java2d.uiScale.enabled property explicitly disables the 
HiDPI graphics support.


  Thanks,
  Alexandr.



 252 private void initScaleFactor() {
 253 if (SunGraphicsEnvironment.isUIScaleEnabled()) {
 254 double debugScale = 
SunGraphicsEnvironment.getDebugScale();

 255 scale = (int) (debugScale >= 1
 256 ? Math.round(debugScale)
 257 : nativeGetScaleFactor(displayID));
 258 } else {
 259 scale = 1;
 260 }
 261 }
 262

On 16.03.16 11:29, Alexander Scherbatiy wrote:


Hello,

Could you review the fix:
   bug: https://bugs.openjdk.java.net/browse/JDK-8150844
   webrev: http://cr.openjdk.java.net/~alexsch/8150844/webrev.00

   The sun.java2d.uiScale property is now used in CGraphicsDevice.
   The requested tests are updated.

   Thanks,
   Alexandr.

[9] Review request for 8150844 [hidpi] [macosx] should -Dsun.java2d.uiScale be taken into account for OS X?

[Alexander Scherbatiy]

Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8150844
  webrev: http://cr.openjdk.java.net/~alexsch/8150844/webrev.00

  The sun.java2d.uiScale property is now used in CGraphicsDevice.
  The requested tests are updated.

  Thanks,
  Alexandr.

Re: [9] Review request for JDK-8145173 HiDPI splash screen support on Windows

[Alexander Scherbatiy]

On 15/03/16 16:56, Rajeev Chamyal wrote:

Hello All,

Please review the updated webrev.
http://cr.openjdk.java.net/~rchamyal/8145173/webrev.04/

Alexandr : I have build code with VS2013 and I didn't get any errors you 
mentioned.
Still I have updated the code as suggested.


  I would suggest to build the code with VS 2010 because it still 
complains on declaration like:

   619 free(dupFileName);
   620 FILE *fp = NULL;

 Thanks,
 Alexandr.


Added build team for make file review. JPRT build with fix was successful.

Regards,
Rajeev Chamyal

-Original Message-
From: Alexander Scherbatiy
Sent: 14 March 2016 17:28
To: Rajeev Chamyal
Cc: Sergey Bylokhov; awt-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8145173 HiDPI splash screen 
support on Windows

On 3/14/2016 8:03 AM, Rajeev Chamyal wrote:

Hello Sergey,

Could you please review the enhancement.

I have raised a new enhancement for unifying the splash screen image
names across platforms.

https://bugs.openjdk.java.net/browse/JDK-8151787


   575 float *scaleFactor)
   576 {
   577 *scaleFactor = 1.0;
   578 float dpiScaleX = -1.0f;
   579 float dpiScaleY = -1.0f;
   580 GetScreenDpi(getPrimaryMonitor(), , );

I have errors building the fix with VS2010:

* For target
support_native_java.desktop_libsplashscreen_splashscreen_sys.obj:
splashscreen_sys.c
jdk/src/java.desktop/windows/native/libsplashscreen/splashscreen_sys.c(578)
: error C2143: syntax error : missing ';' before 'type'
jdk/src/java.desktop/windows/native/libsplashscreen/splashscreen_sys.c(579)
: error C2143: syntax error : missing ';' before 'type'


You need to move variables declaration to the beginning of a statement.

Thanks,
Alexandr.


Regards,

Rajeev Chamyal

*From:*Alexandr Scherbatiy
*Sent:* 10 March 2016 18:46
*To:* Rajeev Chamyal; awt-dev@openjdk.java.net; Sergey Bylokhov
*Subject:* Re:  [9] Review request for JDK-8145173 HiDPI
splash screen support on Windows

The fix looks good to me.

Thanks,
Alexandr.

On 3/10/2016 3:05 AM, Rajeev Chamyal wrote:

 Hello Alexandr,

 Thanks for the review. Below is the updated webrev as per review
 comments.

 http://cr.openjdk.java.net/~rchamyal/8145173/webrev.03/
 <http://cr.openjdk.java.net/%7Erchamyal/8145173/webrev.03/>

 Regards,

 Rajeev Chamyal

 *From:*Alexandr Scherbatiy
 *Sent:* 10 March 2016 11:39
 *To:* Rajeev Chamyal; awt-dev@openjdk.java.net
 <mailto:awt-dev@openjdk.java.net>; Sergey Bylokhov
 *Subject:* Re:  [9] Review request for JDK-8145173 HiDPI
 splash screen support on Windows

 On 3/2/2016 9:50 PM, Rajeev Chamyal wrote:


 Hello All,

 Please review the updated webrev.

 Added a free call for duplicate file name in
 splashscreen_sys.c :: SplashGetScaledImageName

 http://cr.openjdk.java.net/~rchamyal/8145173/webrev.02/
 <http://cr.openjdk.java.net/%7Erchamyal/8145173/webrev.02/>


awt_Win32GraphicsDevice.cpp
  656 dpiX = GetScreenDpi(GetMonitor());
  657 if (dpiX > 0) {
  658 dpiX = dpiX >= 96 ? dpiX / 96 : dpiX;
  659 SetScale(dpiX, dpiX);

 The Windows HiDPI graphics support was designed to handle both DPI
 X and Y scales. The GetScreenDpi should return both values to be
 used in SetScale method.

 systemScale.cpp

38 float scale = -2.0f;

39 if (scale == -2) {

 Initially the scale variable was defined as static to avoid rereading the 
J2D_UISCALE test variable each time.

 It is better to preserve the "// for debug purposes" comment also.

   


 MultiResolutionSplashTest.java

 +   scaleFactor = (float)((SunGraphics2D) 
g).surfaceData.getDefaultScaleX();

   


 Now it is possible to get the the scale factor using
GraphicsEnvironment.getLocalGraphicsEnvironment().getDefaultScreenDevi
ce().getDefaultConfiguration().getDefaultTransform().getScaleX()

  


 Thanks,
  Alexandr.



 Regards,

 Rajeev Chamyal

 *From:*Rajeev Chamyal
 *Sent:* 01 March 2016 15:45
 *To:* awt-dev@openjdk.java.net
 <mailto:awt-dev@openjdk.java.net>; Sergey Bylokhov; Alexander
 Scherbatiy
 *Subject:* RE:  [9] Review request for JDK-8145173
 HiDPI splash screen support on Windows

 Hello All,

 Gentle reminder.

 Please review the updated webrev.


 http://cr.openjdk.java.net/~rchamyal/8145173/webrev.01/
 <http://cr.openjdk.java.net/%7Erchamyal/8145173/webrev.01/>

 Regards,

 Rajeev Chamyal

 *From:*Rajeev Chamyal
 *Sent:* 16 February 2016 16:01
 *To:* awt-dev@openjdk.java.net
 <mailto:awt-dev@openjdk.java.net>; Sergey Bylokhov; Alexander
 Scherbatiy
 *Subject:*  [9] 

Re: [9] Review request for JDK-8145173 HiDPI splash screen support on Windows

[Alexander Scherbatiy]

On 3/14/2016 8:03 AM, Rajeev Chamyal wrote:


Hello Sergey,

Could you please review the enhancement.

I have raised a new enhancement for unifying the splash screen image 
names across platforms.


https://bugs.openjdk.java.net/browse/JDK-8151787



 575 float *scaleFactor)
 576 {
 577 *scaleFactor = 1.0;
 578 float dpiScaleX = -1.0f;
 579 float dpiScaleY = -1.0f;
 580 GetScreenDpi(getPrimaryMonitor(), , );

I have errors building the fix with VS2010:

* For target 
support_native_java.desktop_libsplashscreen_splashscreen_sys.obj:

splashscreen_sys.c
jdk/src/java.desktop/windows/native/libsplashscreen/splashscreen_sys.c(578) 
: error C2143: syntax error : missing ';' before 'type'
jdk/src/java.desktop/windows/native/libsplashscreen/splashscreen_sys.c(579) 
: error C2143: syntax error : missing ';' before 'type'



You need to move variables declaration to the beginning of a statement.

Thanks,
Alexandr.


Regards,

Rajeev Chamyal

*From:*Alexandr Scherbatiy
*Sent:* 10 March 2016 18:46
*To:* Rajeev Chamyal; awt-dev@openjdk.java.net; Sergey Bylokhov
*Subject:* Re:  [9] Review request for JDK-8145173 HiDPI 
splash screen support on Windows


The fix looks good to me.

Thanks,
Alexandr.

On 3/10/2016 3:05 AM, Rajeev Chamyal wrote:

Hello Alexandr,

Thanks for the review. Below is the updated webrev as per review
comments.

http://cr.openjdk.java.net/~rchamyal/8145173/webrev.03/
<http://cr.openjdk.java.net/%7Erchamyal/8145173/webrev.03/>

Regards,

Rajeev Chamyal

*From:*Alexandr Scherbatiy
*Sent:* 10 March 2016 11:39
*To:* Rajeev Chamyal; awt-dev@openjdk.java.net
<mailto:awt-dev@openjdk.java.net>; Sergey Bylokhov
*Subject:* Re:  [9] Review request for JDK-8145173 HiDPI
splash screen support on Windows

On 3/2/2016 9:50 PM, Rajeev Chamyal wrote:


Hello All,

Please review the updated webrev.

Added a free call for duplicate file name in
splashscreen_sys.c :: SplashGetScaledImageName

http://cr.openjdk.java.net/~rchamyal/8145173/webrev.02/
<http://cr.openjdk.java.net/%7Erchamyal/8145173/webrev.02/>


   awt_Win32GraphicsDevice.cpp
 656 dpiX = GetScreenDpi(GetMonitor());
 657 if (dpiX > 0) {
 658 dpiX = dpiX >= 96 ? dpiX / 96 : dpiX;
 659 SetScale(dpiX, dpiX);

The Windows HiDPI graphics support was designed to handle both DPI
X and Y scales. The GetScreenDpi should return both values to be
used in SetScale method.

systemScale.cpp

   38 float scale = -2.0f;

   39 if (scale == -2) {

Initially the scale variable was defined as static to avoid rereading the 
J2D_UISCALE test variable each time.

It is better to preserve the "// for debug purposes" comment also.

  


MultiResolutionSplashTest.java

+   scaleFactor = (float)((SunGraphics2D) g).surfaceData.getDefaultScaleX();

  


Now it is possible to get the the scale factor using 
GraphicsEnvironment.getLocalGraphicsEnvironment().getDefaultScreenDevice().getDefaultConfiguration().getDefaultTransform().getScaleX()

 


Thanks,
 Alexandr.



Regards,

Rajeev Chamyal

*From:*Rajeev Chamyal
*Sent:* 01 March 2016 15:45
*To:* awt-dev@openjdk.java.net
<mailto:awt-dev@openjdk.java.net>; Sergey Bylokhov; Alexander
Scherbatiy
*Subject:* RE:  [9] Review request for JDK-8145173
HiDPI splash screen support on Windows

Hello All,

Gentle reminder.

Please review the updated webrev.


http://cr.openjdk.java.net/~rchamyal/8145173/webrev.01/
<http://cr.openjdk.java.net/%7Erchamyal/8145173/webrev.01/>

Regards,

Rajeev Chamyal

*From:*Rajeev Chamyal
*Sent:* 16 February 2016 16:01
*To:* awt-dev@openjdk.java.net
<mailto:awt-dev@openjdk.java.net>; Sergey Bylokhov; Alexander
Scherbatiy
*Subject:*  [9] Review request for JDK-8145173 HiDPI
splash screen support on Windows

Hello All,

Could you please review the following fix.

Bug : https://bugs.openjdk.java.net/browse/JDK-8145173

Webrev :
http://cr.openjdk.java.net/~rchamyal/8145173/webrev.00/
<http://cr.openjdk.java.net/%7Erchamyal/8145173/webrev.00/>

This is an enhancement to support HiDPI splash screen on windows.

As a part of this enhancement implementation to
splashscreen_sys.c::SplashGetScaledImageName method has been
provided.

System dpi and scale factor are used to determine the scaled
image name. Dpi value is read using GetDpiForMonitor API on
Windows 8 and GetDesktopDpi API on Windows 7.

Scale factor is calculated from the dpi value.

The naming conve

Re: [9] RFR for 8150724: [TEST] HiDPI: create a test for multiresolution icons

[Alexander Scherbatiy]

The test looks good to me.

Thanks,
Alexandr.

On 11/03/16 16:19, Alexander Stepanov wrote:

Hello Alexandr,

Thank you for the explanation. Could you please review the updated test?
http://cr.openjdk.java.net/~avstepan/8150724/webrev.03/

The icon size was reduced.

Thanks,
Alexander


On 3/11/2016 3:14 PM, Alexander Scherbatiy wrote:

On 10/03/16 16:22, Alexander Stepanov wrote:

Hello Alexandr,

Thank you for the comment. Please note that:

1. this hard-coded icon size for the tab is seemingly used only for 
Mac OS X L, please see
http://cr.openjdk.java.net/~avstepan/8150724/screenshots/screenshot-1.png 

(for other L the tab's size is adjusted to the icon's size. L 
are: Metal, Nimbus, CDE/Motif, Mac OS X)

the same for Win. / Ubuntu Linux)

// a loop over the L available should be added to the initial 
test code.


2. as it was mentioned earlier, if replace
 tabbedPane.addTab("", icon, p);
with
 tabbedPane.addTab("", p);
 tabbedPane.setTabComponentAt(0, new JLabel(icon));

in the code, then the user can see small but still blue icon:
http://cr.openjdk.java.net/~avstepan/8150724/screenshots/screenshot-2.png 



also here:
http://cr.openjdk.java.net/~avstepan/8150724/screenshots/screenshot-3.png 

it can be seen that the icons for the other components remain 
hi-res. even when going out of the component borders.


So all the above mentioned causes some feeling of uncoformity :)

Should this tab's icon for Mac OS X L be allowed to be so "special"?


   Aqua L has been designed to make UI components look in the same 
way as native Mac OS X controls.
   In this case the tabbed pane icons should look like icons for 
NSTabView control.


  Thanks,
  Alexandr.



Thanks,
Alexander

On 3/9/2016 7:10 PM, Alexander Scherbatiy wrote:



On 03/03/16 18:45, Alexander Stepanov wrote:

Could you please review the updated version of the test?
http://cr.openjdk.java.net/~avstepan/8150724/webrev.01

Line
 106 tabbedPane.addTab("", icon, p);
was replaced with
 109 tabbedPane.addTab("", p);
 110 tabbedPane.setTabComponentAt(0, new JLabel(icon));

- in such a case the test passes for OS X (as well as for Windows, 
Linux).

Probably these changes should be reverted after JDK-8151060 fix.


 There is no issue because the ImageIcon with base image size 35 
and high-resolution image size 70 is requested to be painted into 
region 32x32 for JTabbedPane icon. In this case the base image has 
enough quality to be painted in this region.


 To check that MultiResolutionImage is properly painted for 
JTabbedIcon it is possible to use BaseMultiResolutionImage with 
images with sizes 16x16 and 32x32.


Thanks,
Alexandr.



Thanks,
Alexander

On 3/2/2016 7:15 PM, Alexander Stepanov wrote:

Hello Sergey,

It fails because of
https://bugs.openjdk.java.net/browse/JDK-8151060

(plus we need here some tricky check for resolution by the 
analogy with 8150258 because of JDK-8150844).


Thanks,
Alexander

On 2/26/2016 4:25 PM, Sergey Bylokhov wrote:

Hi, Alexander.
The test failed on osx 10.11 + retina. Is it expected?

On 26.02.16 15:53, Alexander Stepanov wrote:

Hello,

Could you please review the following fix
http://cr.openjdk.java.net/~avstepan/8150724/webrev.00/
for
https://bugs.openjdk.java.net/browse/JDK-8150724 ?

Just a single test added.

Thanks,
Alexander

Re: thoughts on multiresolution images

[Alexander Scherbatiy]

 Hello Alan,

 Thank you for the feedback.

On 04/03/16 19:59, Alan Snyder wrote:

I am writing to share some thoughts based on recent experience using 
multiresolution images. My experience was not entirely pleasant. I am using JDK 
8, although I see no relevant differences in JDK 9.

One of the critical issues using multiresolution images is that the selection 
of a specific image is not made until the application attempts to draw the 
image. If the returned image is fully available at that time, then it is drawn 
with no problem. Otherwise, the image observer is called. Typically, this will 
call repaint() on a component.

There are two potential problems:

(1) If the component drawing the image is actually a cell renderer, then 
probably repaint() does nothing. The drawing will be incomplete and may not be 
fixed.
   Does the same problem exist if ordinary non multi-resolution image 
is used? Could you provide a code sample which illustrate the issue?


(2) Otherwise, if the resolution variant was created on the fly and not cached, 
then when the repainting occurs, a new resolution variant image will be 
created. Most likely, it will not be fully available, either, so the result is 
a possibly endless repaint loop.

I don't know of a solution to problem (1). It is not a new problem. However, 
what is new is that the common workaround of creating an ImageIcon may not work 
in this case, because only certain platform created multiresolution images are 
recognized by ImageIcon/MediaTracker/SunToolkit. In the general case, the 
component does not know which resolution variant is actually needed and thus is 
unable to wait for its full availability. The approach of waiting for all 
variants to be available is suboptimal and limiting (see below).

Problem (2) can be solved by caching. Given the importance of caching when 
arbitrary images might be in use, it is surprising that there is no public 
support for caching. The MultiResolutionCachedImage class is JDK internal, as 
is the supporting ImageCache class.

Another problem with multiresolution images is that anything that uses the 
getSource() method to obtain an ImageProducer will probably not do the right 
thing. An important example is using FilteredImageSource and an ImageFilter to 
create a derived image. There is no specific specification of what getSource() 
should return when invoked on a multiresolution image, but whatever it returns 
is a single-resolution image and therefore will not be the proper image in some 
circumstances.
   The ImageProducer does not contain information about Image 
resolution variants.

   Where is a discussion about it:
http://mail.openjdk.java.net/pipermail/2d-dev/2016-March/006448.html


Perhaps getSource() on a multiresolution image should thrown an exception.
  It could break existing applications which starts to use 
multi-resolution images and calls getSource() on it. In the current case 
these images are silently handled as ordinary images.


There seems to be an assumption that a multiresolution image should contain a "base 
image" at 1x. I do not see any basis for making that assumption. It seems reasonable 
to me to create a multiresolution image with a single, higher resolution image. The 
effective result is a dynamically scaled image, where the scaling factor is determined at 
the last possible moment, so that no resolution is lost unnecessarily. I also observe 
that in the future, 1x representations will be less and less useful.
  The base image is used to get size/properties/source/... from the 
main image. It is possible to override  them without the base image usage.


  The base image width and height are really necessary for the 
resolution variant size calculation. SunGraphics2D calls 
mrImage.getResolutionVariant(scale * baseImageWidth, scale * 
baseImageHeight) to obtain the high-resolution image.


 MultiResolutionCachedImage takes base image width and height as a 
parameter to avoid triggering base image creation.




I also question the rationale for the getResolutionVariants() method. This 
method assumes that a MultiResolutionImage contains a fixed number of variants. 
I do not see any reason to make that restriction. The resolution variants might 
be created from a scalable description, such as vector graphics. Even if the 
number of variants is fixed, if the image is served remotely, it might be very 
expensive to obtain them all. A lazy approach to creating derived 
multiresolution images is better.
  This is mostly used for native image construction when it is passed 
to the native system.


  For example to set a high-resolution cursor it is only necessary to call
Toolkit.getDefaultToolkit().createCustomCursor(mrImage, ...).

  Then the NSImage with given low and high resolution representation 
are created on Mac OS X.


To work around some of these problems, I created my own API that includes a 
method similar to the map() method of MultiResolutionCachedImage. It seems to 
me that 

Re: [9] RFR for 8150724: [TEST] HiDPI: create a test for multiresolution icons

[Alexander Scherbatiy]

On 10/03/16 16:22, Alexander Stepanov wrote:

Hello Alexandr,

Thank you for the comment. Please note that:

1. this hard-coded icon size for the tab is seemingly used only for 
Mac OS X L, please see

http://cr.openjdk.java.net/~avstepan/8150724/screenshots/screenshot-1.png
(for other L the tab's size is adjusted to the icon's size. L 
are: Metal, Nimbus, CDE/Motif, Mac OS X)

the same for Win. / Ubuntu Linux)

// a loop over the L available should be added to the initial test 
code.


2. as it was mentioned earlier, if replace
 tabbedPane.addTab("", icon, p);
with
 tabbedPane.addTab("", p);
 tabbedPane.setTabComponentAt(0, new JLabel(icon));

in the code, then the user can see small but still blue icon:
http://cr.openjdk.java.net/~avstepan/8150724/screenshots/screenshot-2.png

also here:
http://cr.openjdk.java.net/~avstepan/8150724/screenshots/screenshot-3.png
it can be seen that the icons for the other components remain hi-res. 
even when going out of the component borders.


So all the above mentioned causes some feeling of uncoformity :)

Should this tab's icon for Mac OS X L be allowed to be so "special"?


   Aqua L has been designed to make UI components look in the same 
way as native Mac OS X controls.
   In this case the tabbed pane icons should look like icons for 
NSTabView control.


  Thanks,
  Alexandr.



Thanks,
Alexander

On 3/9/2016 7:10 PM, Alexander Scherbatiy wrote:



On 03/03/16 18:45, Alexander Stepanov wrote:

Could you please review the updated version of the test?
http://cr.openjdk.java.net/~avstepan/8150724/webrev.01

Line
 106 tabbedPane.addTab("", icon, p);
was replaced with
 109 tabbedPane.addTab("", p);
 110 tabbedPane.setTabComponentAt(0, new JLabel(icon));

- in such a case the test passes for OS X (as well as for Windows, 
Linux).

Probably these changes should be reverted after JDK-8151060 fix.


 There is no issue because the ImageIcon with base image  size 35 and 
high-resolution image size 70 is requested to be painted into region 
32x32 for JTabbedPane icon. In this case the base image has enough 
quality to be painted in this region.


 To check that MultiResolutionImage is properly painted for 
JTabbedIcon it is possible to use BaseMultiResolutionImage with 
images with sizes 16x16 and 32x32.


Thanks,
Alexandr.



Thanks,
Alexander

On 3/2/2016 7:15 PM, Alexander Stepanov wrote:

Hello Sergey,

It fails because of
https://bugs.openjdk.java.net/browse/JDK-8151060

(plus we need here some tricky check for resolution by the analogy 
with 8150258 because of JDK-8150844).


Thanks,
Alexander

On 2/26/2016 4:25 PM, Sergey Bylokhov wrote:

Hi, Alexander.
The test failed on osx 10.11 + retina. Is it expected?

On 26.02.16 15:53, Alexander Stepanov wrote:

Hello,

Could you please review the following fix
http://cr.openjdk.java.net/~avstepan/8150724/webrev.00/
for
https://bugs.openjdk.java.net/browse/JDK-8150724 ?

Just a single test added.

Thanks,
Alexander

Re: [9] RFR for 8151269: [TEST] add test covering getSource() method for multiresolution image

[Alexander Scherbatiy]

 The test looks good to me.

 Thanks,
 Alexandr.

On 04/03/16 17:17, Alexander Stepanov wrote:

Hello,

Could you please review the following fix
http://cr.openjdk.java.net/~avstepan/8151269/webrev.00/
for
https://bugs.openjdk.java.net/browse/JDK-8151269 ?

Just a single test added.

Thanks,
Alexander

Re: [9] RFR for 8150724: [TEST] HiDPI: create a test for multiresolution icons

[Alexander Scherbatiy]

On 03/03/16 18:45, Alexander Stepanov wrote:

Could you please review the updated version of the test?
http://cr.openjdk.java.net/~avstepan/8150724/webrev.01

Line
 106 tabbedPane.addTab("", icon, p);
was replaced with
 109 tabbedPane.addTab("", p);
 110 tabbedPane.setTabComponentAt(0, new JLabel(icon));

- in such a case the test passes for OS X (as well as for Windows, 
Linux).

Probably these changes should be reverted after JDK-8151060 fix.


 There is no issue because the ImageIcon with base image  size 35 and 
high-resolution image size 70 is requested to be painted into region 
32x32 for JTabbedPane icon. In this case the base image has enough 
quality to be painted in this region.


 To check that MultiResolutionImage is properly painted for JTabbedIcon 
it is possible to use BaseMultiResolutionImage with images with sizes 
16x16 and 32x32.


Thanks,
Alexandr.



Thanks,
Alexander

On 3/2/2016 7:15 PM, Alexander Stepanov wrote:

Hello Sergey,

It fails because of
https://bugs.openjdk.java.net/browse/JDK-8151060

(plus we need here some tricky check for resolution by the analogy 
with 8150258 because of JDK-8150844).


Thanks,
Alexander

On 2/26/2016 4:25 PM, Sergey Bylokhov wrote:

Hi, Alexander.
The test failed on osx 10.11 + retina. Is it expected?

On 26.02.16 15:53, Alexander Stepanov wrote:

Hello,

Could you please review the following fix
http://cr.openjdk.java.net/~avstepan/8150724/webrev.00/
for
https://bugs.openjdk.java.net/browse/JDK-8150724 ?

Just a single test added.

Thanks,
Alexander

Re: [9] Review request for JDK-8145174 HiDPI splash screen support on Linux

[Alexander Scherbatiy]

On 2/23/2016 12:41 PM, Rajeev Chamyal wrote:


Hello Alexandr,

Thanks for the review.

I have updated the webrev as per review comments.

Webrev : http://cr.openjdk.java.net/~rchamyal/8145174/webrev.01/ 
<http://cr.openjdk.java.net/%7Erchamyal/8145174/webrev.01/>




  - splashscreen_sys.c
   Is it possible to specify the substring to copy in the snprintf 
using "%.*s" format to avoid copying of the file name to 
fileNameWithoutExt buffer?
   The returned error codes like in the snprintf should be properly 
handled.


 - systemScale.c
   The J2D_UISCALE property has been added for the testing purposes. It 
is better to include it into the getNativeScaleFactor method to use for 
splash screens too.


 - the copyright in the test need to be updated to 2016.


- It may be useful to have the same name convention for 
high-resolution splash screen on all platforms.
It allows to use only one  image.java-scale2x.ext file instead to 
have im...@2x.ext <mailto:im...@2x.ext> on Mac OS X and 
name.scale-200.ext on Windows.
   For windows we can have scale factor as float  value so it would be 
difficult to identify which image name to be displayed.


 I see. It can be an enhancement to support fractional scales too. 
For example image.java-scale150%.ext and image.java-scale144dpi.ext for 
scale factor 1.5.


Thanks,
Alexandr.


Regards,

Rajeev Chamyal

*From:*Alexander Scherbatiy
*Sent:* 18 February 2016 02:55
*To:* Rajeev Chamyal; awt-dev@openjdk.java.net; Sergey Bylokhov
*Subject:* Re:  [9] Review request for JDK-8145174 HiDPI 
splash screen support on Linux


On 12/02/16 16:21, Rajeev Chamyal wrote:

Hello All,

Could you please review the following fix.

Bug : https://bugs.openjdk.java.net/browse/JDK-8145174

Webrev : http://cr.openjdk.java.net/~rchamyal/8145174/webrev.00/
<http://cr.openjdk.java.net/%7Erchamyal/8145174/webrev.00/>

This is an enhancement to support HiDPI splash screen on Linux.

As a part of this enhancement implementation to
splashscreen_sys.c::SplashGetScaledImageName method has been
provided based on the GDK_SCALE environment variable set on
unix/linux system.

The new implementation checks for GDK_SCALE set on system and
returns the scaled image name, if GDK_SCALE=2 otherwise NULL.

The naming convention followed for scaled image is as follows:

Unscaled image name : image.ext

Scaled image name : image.java-scale2x.ext


  - It may be useful to have the same name convention for 
high-resolution splash screen on all platforms.
It allows to use only one  image.java-scale2x.ext file instead to 
have im...@2x.ext <mailto:im...@2x.ext> on Mac OS X and 
name.scale-200.ext on Windows.

Could you create an enhancement on it and send it to the review?

The automated jtreg test for this is currently failing due to
issues in robot.getPixelColor it is returning wrong pixel color
for GDK_SCALE=2.

Also fixed issues in following files.

1)splashscreen_impl.c::SplashInit() was resetting the scaleFactor
to 1.

- SplashSetScaleFactor should not be called from the 
SplashGetScaledImageName method because SplashInit has not been called 
yet.
  - The problem with setting the scale factor in SplashInit is that it 
is not clear is the high-resolution splash screen image provided or 
not. If the the high-resolution splash screen is not provided the 
scale factor should be set to 1.
  - The java.c uses the following sequence for the splash screen 
initialization:

--
 scaled_splash_name = DoSplashGetScaledImageName(
jar_name, file_name, _factor);
DoSplashInit();
DoSplashSetScaleFactor(scale_factor);
DoSplashLoadFile(scaled_splash_name);
--
  To make the SplashSetScaleFactor method work it should also be added 
to the make/mapfiles/libsplashscreen/mapfile-vers file.


 - There are two codes which detect the scale factor. One is in the 
splash screen (getNativeScaleFactor()  method)
  and another in the AWT 
(src/java.desktop/unix/native/libawt_xawt/awt/awt_GraphicsEnv.c file).
  Is it possible to move it one code that it will be used both from 
splash screen and from AWT?


  Thanks,
  Alexandr.

2)SplashScreen.java:: getBounds fixed the typo.

Regards,

Rajeev Chamyal

Re: [9] Review request for JDK-8145174 HiDPI splash screen support on Linux

[Alexander Scherbatiy]

On 12/02/16 16:21, Rajeev Chamyal wrote:


Hello All,

Could you please review the following fix.

Bug : https://bugs.openjdk.java.net/browse/JDK-8145174

Webrev : http://cr.openjdk.java.net/~rchamyal/8145174/webrev.00/ 



This is an enhancement to support HiDPI splash screen on Linux.

As a part of this enhancement implementation to 
splashscreen_sys.c::SplashGetScaledImageName method has been provided 
based on the GDK_SCALE environment variable set on unix/linux system.


The new implementation checks for GDK_SCALE set on system and returns 
the scaled image name, if GDK_SCALE=2 otherwise NULL.


The naming convention followed for scaled image is as follows:

Unscaled image name : image.ext

Scaled image name : image.java-scale2x.ext



  - It may be useful to have the same name convention for 
high-resolution splash screen on all platforms.
It allows to use only one  image.java-scale2x.ext file instead to 
have im...@2x.ext on Mac OS X and name.scale-200.ext on Windows.

Could you create an enhancement on it and send it to the review?


The automated jtreg test for this is currently failing due to issues 
in robot.getPixelColor it is returning wrong pixel color for GDK_SCALE=2.


Also fixed issues in following files.

1)splashscreen_impl.c::SplashInit() was resetting the scaleFactor to 1.

  - SplashSetScaleFactor should not be called from the 
SplashGetScaledImageName method because SplashInit has not been called yet.
  - The problem with setting the scale factor in SplashInit is that it 
is not clear is the high-resolution splash screen image provided or not. 
If the the high-resolution splash screen is not provided the scale 
factor should be set to 1.
  - The java.c uses the following sequence for the splash screen 
initialization:

--
   scaled_splash_name = DoSplashGetScaledImageName(
jar_name, file_name, _factor);
DoSplashInit();
DoSplashSetScaleFactor(scale_factor);
DoSplashLoadFile(scaled_splash_name);
--
To make the SplashSetScaleFactor method work it should also be added to 
the make/mapfiles/libsplashscreen/mapfile-vers file.


 - There are two codes which detect the scale factor. One is in the 
splash screen (getNativeScaleFactor()  method)
  and another in the AWT 
(src/java.desktop/unix/native/libawt_xawt/awt/awt_GraphicsEnv.c file).
  Is it possible to move it one code that it will be used both from 
splash screen and from AWT?


  Thanks,
  Alexandr.


2)SplashScreen.java:: getBounds fixed the typo.

Regards,

Rajeev Chamyal

Re: [OpenJDK 2D-Dev] [9] Review request for 8076545 Text size is twice bigger under Windows L on Win 8.1 with HiDPI display

[Alexander Scherbatiy]

On 2/5/2016 11:37 AM, Jim Graham wrote:

Hi Alexandr,

awt_DesktopProperties.cpp, line 300 - is the "1.0f /" a typo?

   Sorry.  Here is the updated fix without the typo:
 http://cr.openjdk.java.net/~alexsch/8076545/webrev.06/


Also, is there a still a need for the setFontProperty() variants that 
don't have a scale as the last parameter?
There are fonts like win.ansiFixed.font, win.ansiVar.font, and 
win.deviceDefault.font which size is the same for any display DPI.


  And there are fonts like win.oemFixed.font, win.system.font, and 
win.systemFixed.font which have one size for DPI 96 and another size for 
all other DPI.

   For example win.oemFixed.font:
   DPI  96, size: 12
   DPI 144, size: 18
   DPI 192, size: 18
   DPI 240, size: 18

  I left them unscaled but may be it is better to have one 
precalculated scale for this fonts which is used when DPI is not 96.


  I updated the setFontProperty() method without scale parameter usage 
to call with scale 1.0f.


  Thanks,
  Alexandr.



...jim

On 2/5/2016 2:12 AM, Alexandr Scherbatiy wrote:


Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8076545/webrev.05

   - The awt_DesktopProperties.cpp file is updated to use scaleX for
width rescaling and scaleY for height rescaling

   Thanks,
   Alexandr.

On 2/1/2016 5:51 PM, Jim Graham wrote:

Hi Alexandr,

In awt_DesktopProperties.cpp you are using the Y scaling factor to
scale widths still - see lines 287 (and others in that same function)
and then 322 in another function.  It looks like you'll need
getInvScaleX() and getInvScaleY().

I'll leave it to Phil to comment on the unit test...

...jim

On 2/1/16 4:27 AM, Alexandr Scherbatiy wrote:


Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8076545/webrev.04/

  - both LOGPIXELSX and Y are used for the theme size scaling.
  - LOGPIXELSY is used for text metrics height rescaling

   Thanks,
   Alexandr.

On 1/29/2016 1:16 PM, Jim Graham wrote:

Hi Alexandr,

Thanks for investigating the behaviors.

With respect to using LOGPIXELSX or Y, these methods are used to 
scale

both horizontal and vertical measurements so we really should have 2
scale values and rescale methods, one for horizontal use and one for
vertical...

...jim

On 1/29/2016 10:41 AM, Alexandr Scherbatiy wrote:

  Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8076545/webrev.03

  - LOGPIXELSX is changed to  LOGPIXELSY

On 11/16/2015 1:43 PM, Jim Graham wrote:

Note that LOGPIXELSY is global and static between reboots so it
doesn't really matter which monitor is used to get the value.

Also, the issue is that the measurements are in pixels, so if we
convert them into a resolution independent measurement then the 
rest
of the scaling in the AWT/2D will be correct regardless of any 
given

monitor's resolution.  We just have to make sure the
"de-pixelization"
of the measurement is an apples-to-apples calculation.

The question in my mind is whether the values they get from
GetTheme*() and SPI_GETNONCLIENTMETRICS are relative to
LOGPIXELSY, or
the more recent Per-Monitor aware DPI APIs, or ...? It would be
interesting to see what happens to those values when you change the
DPI settings on Windows 8.1 and not reboot. If they stay constant
until you reboot then LOGPIXELSY on the main screen would be the
value
to use to de-scale them...


I tried to use the "Change the size of all items" slider on
Windows
8.1 without rebooting.
GetDpiForMonitor() returns the updated DPI values: 192, 144, 96
LOGPIXELSY returns unchanged values:  192, 192, 192
SystemParametersInfo(SPI_GETNONCLIENTMETRICS,...) returns
unchanged
NONCLIENTMETRICS
GetThemePartSize() returns unchanged theme part sizes

It seems that theme part sizes behave in the same way as the
LOGPIXELSY in this case.

Thanks,
Alexandr.



...jim

On 11/16/2015 12:51 PM, Phil Race wrote:

That seems better. But one more question to get a point clarified.
You are using getDesktopWindow() which is for the primary monitor.
I assume that the 'inversion' results in the value being used 
to be

independent
of the monitor in a multi-mon situation and so when we move to 
a 2nd

monitor
that inverted size remains valid ?

If so looks good to me.

-phil.

On 11/16/2015 06:07 AM, Alexander Scherbatiy wrote:


  Hello,

  Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8076545/webrev.02

  - round is used instead of ceil
  - inverted scales are used

  Thanks,
  Alexandr.


On 10/30/2015 10:40 PM, Jim Graham wrote:

In this case round may be better. ceil() is more for cases
where you
needed "at least X amount of room", but I don't think a font
size is
an "at least this much" kind of case.

Also, I've been toying with the idea that use of ceil() and
floor()
in any DPI-adjustment equations should really be "ceil(val -
epsilon)"

Re: Review Request for 8142861: [TEST_BUG] MultiResolution image: add a manual test for two-display configuration (HiDPI + non-HiDPI)

[Alexander Scherbatiy]

 The fix looks good to me.

On 05/02/16 15:59, Alexander Stepanov wrote:

Hello Alexandr,

Thank you for the notes; yes, these lines are unnecessary. Please see 
the updated patch:

http://cr.openjdk.java.net/~avstepan/8142861/webrev.04/

It should be also mentioned that for now the test is still failing 
because of JDK-8143062. Should it be added to any exclude list?


  It is mentioned in the test @bug list. It should be enough because it 
means that the test should pass when the bug 8143062 is fixed.


  Thanks,
  Alexandr.



Thanks,
Alexander


On 2/5/2016 1:51 PM, Alexander Scherbatiy wrote:



 There are just small comments about lines:
 119 Graphics2D g = (Graphics2D) gr;
 120 if (g != null) { g.drawImage(IMG, 0, 0, this); }

  Is it necessary to cast the Graphics to Graphics2D because Graphics 
also has drawImage(...) method?
  Is it necessary to check the graphics to null? It looks like passed 
null graphics should be considered as a bug.


 Thanks,
 Alexandr.


On 20/01/16 20:32, Alexander Stepanov wrote:

Sorry, just a reminder...

Thanks,
Alexander

On 1/14/2016 6:00 PM, Alexander Stepanov wrote:

Hello Sergey,

> Note that MultiRes image can be created at runtime
Indeed, this case should be used for testing, as we have different 
naming conventions for OS X and Windows, so the write-read logic is 
thrown away (as we have such tests already), and now the 
multiresolution image is created at runtime.


Please see the updated webrev:
http://cr.openjdk.java.net/~avstepan/8142861/webrev.03/

Regards,
Alexander

On 11/16/2015 4:23 PM, Alexander Stepanov wrote:
P.S.: The behavior with Mac OS X mission control should also be 
checked which is definitely a manual job (it seems to be strange 
sometimes; not sure if buggy). So the test instructions should be 
appended a bit...


On 11/16/2015 3:24 PM, Alexander Stepanov wrote:

Hello Sergey,

Thank you for the notes.

> Do you have some thoughts why this test cannot be converted to 
auto test?


The following parts of the test scenario:

- "Please try to drag both parent and child, do it fast several 
times and check if no artifacts occur." (not very valuable part 
of the test, but I've seen these artifacts (only once) after fast 
and long dragging from one display to another, so probably it is 
better to save it, just in case).
- "For Mac OS X please check also the behavior for translucent 
windows appearing on the 2nd (non-active) display"


are hardly automated; especially for Mac OS X because the 
translucent window on the 2nd "inactive" display sometimes occurs 
but sometimes not, and I doubt if it could be predicted exactly 
where and when it should occur when dragging and when stopping 
dragging (and even change color; - e.g. we can see part  of the 
"1x" image on "2x" display, if its other part is still on the 
"1x" display together with cursor).  Please note also that the 
drag behavior for Mac OS X differs, from e.g., Ubuntu Linux: 
while dragging the parent dialog we also drag its child (whereas 
for Ubuntu this is not the case).


Moreover, the test requires not very trivial hardware 
configuration (HiDPI + non-HiDPI displays, ) which, probably, 
occurs rarely.


So I believe that the manual test has more chances to be run than 
automated (and is less prone to false negative); and attempts to 
automate it will bring more headache than gain.


> Note that MultiRes image can be created at runtime, it will be 
good to cover this also.

Will look at this, thanks.

Regards,
Alexander

P.S.: probably some simpler test cases could be automated: e.g., 
switching display resolution and checking correctness of the 
multires. image displayed, but I'd like to make that separately.


On 11/16/2015 11:35 AM, Sergey Bylokhov wrote:

Hi, Alexander.
 - Note that MultiRes image can be created at runtime, it will 
be good to cover this also.
- Do you have some thoughts why this test cannot be converted to 
auto test? Probably some api can be added to jdk to simplify 
creation of such tests? like already added 
"-Dsun.java2d.uiScale"(JDK-8073320).


On 13.11.15 14:34, Alexander Stepanov wrote:

webrev updated:
http://cr.openjdk.java.net/~anazarov/8142861/webrev.02/

Thanks,
Alexander

On 11/12/2015 7:51 PM, Alexander Stepanov wrote:

Hello,

Could you please review the following fix
http://cr.openjdk.java.net/~anazarov/8142861-3/webrev.00/
for
https://bugs.openjdk.java.net/browse/JDK-8142861

Just a single manual test added.
(sorry, I'll remove these unnecessary 'static' modifiers for
'parentName', 'childName')

Checked on Mac OS X 10.10 (2-display configuration + Quartz) 
with JDK9

b91

Thanks,
Alexander

Re: Review Request for 8142861: [TEST_BUG] MultiResolution image: add a manual test for two-display configuration (HiDPI + non-HiDPI)

[Alexander Scherbatiy]

 There are just small comments about lines:
 119 Graphics2D g = (Graphics2D) gr;
 120 if (g != null) { g.drawImage(IMG, 0, 0, this); }

  Is it necessary to cast the Graphics to Graphics2D because Graphics 
also has drawImage(...) method?
  Is it necessary to check the graphics to null? It looks like passed 
null graphics should be considered as a bug.


 Thanks,
 Alexandr.


On 20/01/16 20:32, Alexander Stepanov wrote:

Sorry, just a reminder...

Thanks,
Alexander

On 1/14/2016 6:00 PM, Alexander Stepanov wrote:

Hello Sergey,

> Note that MultiRes image can be created at runtime
Indeed, this case should be used for testing, as we have different 
naming conventions for OS X and Windows, so the write-read logic is 
thrown away (as we have such tests already), and now the 
multiresolution image is created at runtime.


Please see the updated webrev:
http://cr.openjdk.java.net/~avstepan/8142861/webrev.03/

Regards,
Alexander

On 11/16/2015 4:23 PM, Alexander Stepanov wrote:
P.S.: The behavior with Mac OS X mission control should also be 
checked which is definitely a manual job (it seems to be strange 
sometimes; not sure if buggy). So the test instructions should be 
appended a bit...


On 11/16/2015 3:24 PM, Alexander Stepanov wrote:

Hello Sergey,

Thank you for the notes.

> Do you have some thoughts why this test cannot be converted to 
auto test?


The following parts of the test scenario:

- "Please try to drag both parent and child, do it fast several 
times and check if no artifacts occur." (not very valuable part of 
the test, but I've seen these artifacts (only once) after fast and 
long dragging from one display to another, so probably it is better 
to save it, just in case).
- "For Mac OS X please check also the behavior for translucent 
windows appearing on the 2nd (non-active) display"


are hardly automated; especially for Mac OS X because the 
translucent window on the 2nd "inactive" display sometimes occurs 
but sometimes not, and I doubt if it could be predicted exactly 
where and when it should occur when dragging and when stopping 
dragging (and even change color; - e.g. we can see part  of the 
"1x" image on "2x" display, if its other part is still on the "1x" 
display together with cursor).  Please note also that the drag 
behavior for Mac OS X differs, from e.g., Ubuntu Linux: while 
dragging the parent dialog we also drag its child (whereas for 
Ubuntu this is not the case).


Moreover, the test requires not very trivial hardware configuration 
(HiDPI + non-HiDPI displays, ) which, probably, occurs rarely.


So I believe that the manual test has more chances to be run than 
automated (and is less prone to false negative); and attempts to 
automate it will bring more headache than gain.


> Note that MultiRes image can be created at runtime, it will be 
good to cover this also.

Will look at this, thanks.

Regards,
Alexander

P.S.: probably some simpler test cases could be automated: e.g., 
switching display resolution and checking correctness of the 
multires. image displayed, but I'd like to make that separately.


On 11/16/2015 11:35 AM, Sergey Bylokhov wrote:

Hi, Alexander.
 - Note that MultiRes image can be created at runtime, it will be 
good to cover this also.
- Do you have some thoughts why this test cannot be converted to 
auto test? Probably some api can be added to jdk to simplify 
creation of such tests? like already added 
"-Dsun.java2d.uiScale"(JDK-8073320).


On 13.11.15 14:34, Alexander Stepanov wrote:

webrev updated:
http://cr.openjdk.java.net/~anazarov/8142861/webrev.02/

Thanks,
Alexander

On 11/12/2015 7:51 PM, Alexander Stepanov wrote:

Hello,

Could you please review the following fix
http://cr.openjdk.java.net/~anazarov/8142861-3/webrev.00/
for
https://bugs.openjdk.java.net/browse/JDK-8142861

Just a single manual test added.
(sorry, I'll remove these unnecessary 'static' modifiers for
'parentName', 'childName')

Checked on Mac OS X 10.10 (2-display configuration + Quartz) 
with JDK9

b91

Thanks,
Alexander

Re: Review Request for 8062946 : Transparent JDialog will lose transparency upon iconify/deiconify sequence.

[Alexander Scherbatiy]

 The fix looks good to me.

 Thanks,
 Alexandr.

On 04/02/16 07:46, Prem Balakrishnan wrote:


Hi ,

Sorry my mistake.

Bug id in subject corrected 8062946.

Regards,
Prem

*From:*Alexandr Scherbatiy
*Sent:* Wednesday, February 03, 2016 8:44 PM
*To:* Prem Balakrishnan; Ambarish Rapte; Semyon Sadetsky; Sergey 
Bylokhov; Rajeev Chamyal; swing-...@openjdk.java.net; 
awt-dev@openjdk.java.net
*Subject:* Re: Review Request for 8062846 : Transparent JDialog will 
lose transparency upon iconify/deiconify sequence.



I just notice that the bug number in the subject 8062846 points to the 
different issue.


Could you resend the same fix the the correct bug id in the subject?

Thanks,
Alexandr.

On 2/2/2016 3:00 AM, Prem Balakrishnan wrote:

Hi Ambarish,

Updated test .

Webrev: http://cr.openjdk.java.net/~arapte/prem/8062946/webrev.01/
<http://cr.openjdk.java.net/%7Earapte/prem/8062946/webrev.01/>

Regards,
Prem

*From:*Ambarish Rapte
*Sent:* Tuesday, February 02, 2016 2:47 PM
*To:* Alexander Scherbatiy; Prem Balakrishnan; Semyon Sadetsky;
Sergey Bylokhov; Rajeev Chamyal; swing-...@openjdk.java.net
<mailto:swing-...@openjdk.java.net>; awt-dev@openjdk.java.net
<mailto:awt-dev@openjdk.java.net>
*Subject:* RE: Review Request for 8062846 : Transparent JDialog
will lose transparency upon iconify/deiconify sequence.

Hi Prem,

The test passes without including the fix.

The test should fail without fix, and should pass with fix.

Thanks,

Ambarish

*From:*Alexandr Scherbatiy
*Sent:* Monday, February 01, 2016 5:15 PM
*To:* Prem Balakrishnan; Semyon Sadetsky; Sergey Bylokhov;
Ambarish Rapte; Rajeev Chamyal; swing-...@openjdk.java.net
<mailto:swing-...@openjdk.java.net>; awt-dev@openjdk.java.net
<mailto:awt-dev@openjdk.java.net>
*Subject:* Re: Review Request for 8062846 : Transparent JDialog
will lose transparency upon iconify/deiconify sequence.


  The fix looks good to me.

  Thanks,
  Alexandr.

On 1/28/2016 9:45 PM, Prem Balakrishnan wrote:

Hi*,*

Please review fix for JDK9,

*Bug: *https://bugs.openjdk.java.net/browse/JDK-8062946**

*Webrev:
*http://cr.openjdk.java.net/~arapte/prem/8062946/webrev.00/
<http://cr.openjdk.java.net/%7Earapte/prem/8062946/webrev.00/>

Issue:

Transparent JDialog will lose transparency upon
iconify/deiconify sequence.

*Cause:*

Regression: due to 6780496: Javaw process taking up 80-90
percent of CPU time!

https://bugs.openjdk.java.net/browse/JDK-6780496

Intentionally when JDialog is iconified transparency is disabled,

And transparency is not enabled when JDialog is deiconified.

*Fix:*

Transparency is enabled when JDialog is deiconified.

Regards,
Prem

Re: [9] Review request for 8147440 HiDPI (Windows): Swing components have incorrect sizes after changing display resolution

[Alexander Scherbatiy]

On 05/02/16 12:59, Alexander Stepanov wrote:

Hello Alexandr,

WRT WindowResizingOnDPIChangingTest - I have quite similar manual test 
on review:

http://mail.openjdk.java.net/pipermail/awt-dev/2016-January/010569.html
(but it requires two-display configuration).

Could these tests coexist later on? I hope they are not full duplicates.
  Yes, the MultiDisplayTest always requires at least 2 display but 
WindowResizingOnDPIChangingTest can be used for display DPI changing 
only on one monitor. I think they both can coexist.


  Thanks,
  Alexandr.


(and now we come here, could please anyone review it?)

Thanks,
Alexander



On 2/5/2016 10:52 AM, Alexandr Scherbatiy wrote:


 Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8147440
  webrev: http://cr.openjdk.java.net/~alexsch/8147440/webrev.00/

  When Display DPI is changed the Windows OS rescales a native window 
size but leaves a native window location the same.

  Java frame size and location are calculated as
nativeWindow.location = scale * javaWindow.location
nativeWindow.size = scale * javaWindow.size

  The first approach is to rescale only frame size on native level so
newNativeWindow.size = newScale *  javaWindow.size
  This allows to leave the nativeWindow.location unchanged but the rule
nativeWindow.location = newScale * javaWindow.location
  will be broken in this case.

  The proposed fix explicitly rescales javaWindow.location in 
WWindowPeer so
  nativeWindow.location = prevScale * prevJavaWindow.location = 
newScale * newJavaWindow.location


 Thanks,
 Alexandr.

Re: [9] Review Request for 8139218: Dialog that opens and closes quickly changes focus in original focusowner

[Alexander Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 22/12/15 14:20, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8139218
webrev: http://cr.openjdk.java.net/~ssadetsky/8139218/webrev.00/

The root cause: when window gain focus come for window that is hidden 
already focus rollback to another window cannot be done synchronously. 
So number of consequent attempts to set focus to the previous 
component are usually failed and the focus target is shifted to the 
next in the focus traversal cycle.
Solution: try to wait a small time until the parent HW component gain 
the asynchronous focus from native platform.


--Semyon

Review request for 8139508 Debug option does not work in appletviewer

[Alexander Scherbatiy]

Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8139508
  webrev: http://cr.openjdk.java.net/~alexsch/8139508/jdk9-client/webrev.00

  The appletviewer debug support is removed.

Thanks,
Alexandr.

Re: Review Request for 8147966: [TEST] add a test for multiresolution image properties

[Alexander Scherbatiy]

The test looks good to me.

Thanks,
Alexandr.

On 22/01/16 20:41, Alexander Stepanov wrote:

Hello,

Could you please review the fix
http://cr.openjdk.java.net/~avstepan/8147966/webrev.00/
for
https://bugs.openjdk.java.net/browse/JDK-8147966

Just one simple test to cover BaseMultiResolutionImage.getProperty()

Thanks,
Alexander

Re: [9] Review Request for 8146162: [TEST_BUG] sun/awt/shell/ShellFolderMemoryLeak.java fails with Jigsaw

[Alexander Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 11/01/16 17:56, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8146162
webrev: http://cr.openjdk.java.net/~ssadetsky/8146162/webrev.00/

@modules corrected and 
-XaddExports:java.desktop/sun.awt.shell=ALL-UNNAMED is added to the 
spawn processes.

Should be commited to 9-repo-jigsaw repo only.

--Semyon

Re: Review request for 8143316 Crash Trend in 1.9.0-ea-b93 (sun.awt.DefaultMouseInfoPeer.fillPointWithCoords)

[Alexander Scherbatiy]

Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8143316/webrev.01

 - DefaultMouseInfoPeer is removed from the SunToolkit and 
WMouseInfoPeer is added to the WToolkit

 - Screen devices initialization is added to the WMouseInfoPeer
 - MONITOR_DEFAULTTONEAREST argument is changed to the 
MONITOR_DEFAULTTOPRIMARY for the MonitorFromPoint method

  to find a point which is not contained within any display monitor.

On 12/23/2015 5:59 PM, Sergey Bylokhov wrote:

Hi.
This issue is related to windows only, but we change the shared code 
in a way which can mislead someone, and the crash can be returned 
after some refactoring, or when someone will call 
fillPointWithCoords() before device initialization in some other place.
So it seems this code should be rewritten, am I missed something but 
it looks like this method incorrectly work in case of scaled displays. 
I guess fillPointWithCoords should return unscaled coords otherwise 
how we will figure out what scale should be used there(screen)? 
Because at the end of the method getPointerInfo we iterate over 
devices to find which one is correct.


Windows and Mac OS X use different strategies to return the mouse 
coordinates.
CGEventGetLocation function returns scaled mouse coordinates on Mac 
OS X.

GetCursorPos function returns unscaled mouse coordinates on Windows.
It looks like it is better to always return scaled coordinates from 
the native level.


   Thanks,
   Alexandr.



On 10/12/15 15:06, Alexander Scherbatiy wrote:


Hello,

Could you review the fix:
   bug: https://bugs.openjdk.java.net/browse/JDK-8143316
   webrev: http://cr.openjdk.java.net/~alexsch/8143316/webrev.00

   The the native MouseInfo.fillPointWithCoords() method can access the
devices before they are initialized.
   The fix moves the devices initialization before the
getMouseInfoPeer().fillPointWithCoords(point) call in the
   getPointerInfo() method.

   The MonitorFromPoint method is added to the native
fillPointWithCoords() method to find the device where the mouse cursor
is placed.

   Thanks,
   Alexandr.

Re: [9] Review Request for 8145795: [TEST_BUG][PIT] java/awt/Window/ScreenLocation/ScreenLocationTest.java fails (can assign Integer.MAX_VALUE to Window dimensions)

[Alexander Scherbatiy]

On 12/22/2015 6:41 PM, Semyon Sadetsky wrote:

Right. This is regression of 8073320.
Fix is annulled. Thanks.


  It would be better to update the test to take the scale factor into 
the account.

  Something like  A_BIG_SIZE = Integer.MAX_VALUE / SCALE_FACTOR.

   Thanks,
   Alexandr.



--Semyon

On 12/22/2015 6:03 PM, Yuri Nesterenko wrote:

Semyon,

but earlier builds of jdk9 could handle this bordercase properly,
didn't they?. E.g. b95?

-yan

On 12/22/2015 04:22 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8145795
webrev: http://cr.openjdk.java.net/~ssadetsky/8145795/webrev.00/

Windows do not accept  Integer.MAX_VALUE as dimension for internal
reasons. Twice less value works.

--Semyon

Re: [9] Review Request for 8145784: https://bugs.openjdk.java.net/browse/JDK-8145784

[Alexander Scherbatiy]

  The fix looks good to me.

 Thanks,
 Alexandr.

On 12/23/2015 5:48 PM, Semyon Sadetsky wrote:
The fix was changed because it is not possible to get individual 
screen windows under Xinerama correctly.

Please, review the updated solution:

http://cr.openjdk.java.net/~ssadetsky/8145784/webrev.01/

The pointer moment coordinates are changed to be relative to the 
origin of the chosen screen.


--Semyon

On 12/23/2015 1:41 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:
bug: https://bugs.openjdk.java.net/browse/JDK-8145784
webrev: http://cr.openjdk.java.net/~ssadetsky/8145784/webrev.00/

Robot used wrong root window for the mouse move because when the 
Xinerama is on JDK treats multiple screens as a single joint root 
window, and screen number obtained from the graphics config is always 
0. So mouse could not leave the main screen using AWT robot.

Solution: take the correct screen number from the device object.

--Semyon

Re: Review request for 8041928: MouseEvent.getModifiersEx gives wrong result

[Alexander Scherbatiy]

  The fix looks good to me.

  Thanks,
  Alexandr.

On 12/23/2015 9:23 AM, Rajeev Chamyal wrote:

Fix looks good to me.

Regards,
Rajeev Chamyal

-Original Message-
From: Ambarish Rapte
Sent: 23 December 2015 01:32
To: Ambarish Rapte; Rajeev Chamyal; Sergey Bylokhov; Semyon Sadetsky; Prasanta 
Sadhukhan; awt-dev@openjdk.java.net
Subject: RE:  Review request for 8041928: MouseEvent.getModifiersEx 
gives wrong result


Hi Rajeev,

Thanks for the review,
Please review the updated patch with required changes in test file at,
http://cr.openjdk.java.net/~arapte/8041928/webrev.02/



Regards,
Ambarish

-Original Message-
From: Ambarish Rapte
Sent: Tuesday, December 22, 2015 8:10 PM
To: Rajeev Chamyal; Sergey Bylokhov; Semyon Sadetsky; Prasanta Sadhukhan; 
awt-dev@openjdk.java.net
Subject: RE:  Review request for 8041928: MouseEvent.getModifiersEx 
gives wrong result

Hi Rajeev,

Thanks for the review,
Please review the updated test at,
http://cr.openjdk.java.net/~arapte/8041928/webrev.01/


Regards,
Ambarish

-Original Message-
From: Rajeev Chamyal
Sent: Tuesday, December 22, 2015 3:36 PM
To: Ambarish Rapte; Sergey Bylokhov; Semyon Sadetsky; Prasanta Sadhukhan; 
awt-dev@openjdk.java.net
Subject: RE:  Review request for 8041928: MouseEvent.getModifiersEx 
gives wrong result

Hello Ambarish,

The test instruction window is not getting closed after the test timeout on 
pressing close button/Ctrl-C.

Regards,
Rajeev Chamyal

-Original Message-
From: Ambarish Rapte
Sent: 22 December 2015 14:08
To: Sergey Bylokhov; Semyon Sadetsky; Prasanta Sadhukhan; 
awt-dev@openjdk.java.net
Subject: Re:  Review request for 8041928: MouseEvent.getModifiersEx 
gives wrong result

Hi,
Can I get one more review for this patch please.

Thanks,
Ambarish

-Original Message-
From: Ambarish Rapte
Sent: Monday, December 21, 2015 11:27 AM
To: Sergey Bylokhov; Semyon Sadetsky; Prasanta Sadhukhan; 
awt-dev@openjdk.java.net
Subject: RE:  Review request for 8041928: MouseEvent.getModifiersEx 
gives wrong result

Hi Sergey,
Thanks for the review.
AltGr+C → © is not supported by text component.
Tested with javax.swing.JTextArea & java.awt.TextArea.

Regards,
Ambarish

-Original Message-
From: Sergey Bylokhov
Sent: Saturday, December 19, 2015 3:37 AM
To: Ambarish Rapte; Semyon Sadetsky; Prasanta Sadhukhan; 
awt-dev@openjdk.java.net
Subject: Re:  Review request for 8041928: MouseEvent.getModifiersEx 
gives wrong result

Hi, Ambarish.
Fix looks fine. Can you please confirm that combination like AltGr+C → © 
(copyright sign) works in our text components?

On 09/12/15 18:30, Ambarish Rapte wrote:

Hi,

  Please review the fix for JDK9

  Bug: https://bugs.openjdk.java.net/browse/JDK-8041928

Webrev: http://cr.openjdk.java.net/~arapte/8041928/webrev.00/

Issue:

The 8041928 issue mentions below problems with the modifiers,

1.When clicking right button it gives Button3 and the Meta key modifiers.

2.Alt-Gr modifier is not set.

Cause:

1.When clicking right button it gives Button3 and the Meta key modifiers.

This is actually not an issue, the right mouse button (BUTTON3_MASK)
click and META key  (META_MASK) share same bit in modifiers.

Please check documentation here,

https://docs.oracle.com/javase/8/docs/api/java/awt/event/MouseEvent.ht
ml

The important statement from this doc:

(Note: Due to overlap in the values of ALT_MASK/BUTTON2_MASK and
META_MASK/BUTTON3_MASK, this is not always true for mouse events
involving modifier keys)

2.Alt-Gr modifier is not set.

This is an issue, and the patch fixes this issue for Windows.

Also added related manual test.

This bug does not happen on Linux,

But occurs with MACOSX,  Shall raise a new MACOSX specific bug to
continue work.

Fix:

Added check for key code VK_RMENU for windows, while creating the java
Modifiers of event.

The related test is manual because,

1.Robot cannot generate the Alt-Gr key event

2.There are different keyboard layouts, A keyboard may have Alt-Gr key
or might have to use Ctrl-Alt combination.

3.For Linux – ubuntu, The alternate graphics key needs to be enabled
explicitly from System Keyboard settings.

Thanks,

Ambarish



--
Best regards, Sergey.

Re: [9] Review Request for 8145795: [TEST_BUG][PIT] java/awt/Window/ScreenLocation/ScreenLocationTest.java fails (can assign Integer.MAX_VALUE to Window dimensions)

[Alexander Scherbatiy]

On 12/23/2015 7:08 PM, Semyon Sadetsky wrote:

Alexander, please clarify
1. will this fix the problem?

   It will still fails for example for the scale factor 3.

2. What if user uses value > Integer.MAX_VALUE / SCALE_FACTOR for 
window dimensions? If it should be prohibited now with your change, 
then we need to add this to all javadocs because window size does not 
meet its native limit in this case.


The sizes are scaled on the native system. I think that values that 
are greater than Integer.MAX_VALUE should be truncated to the 
Integer.MAX_VALUE.

If it does not checked it should be considered as a bug.

The Window.setSize() method have already had the documentation to 
cover such cases: "The method changes the geometry-related data. 
Therefore, the native windowing system may ignore such requests, or it 
may modify the requested data, so that the Window object is placed and 
sized in a way that corresponds closely to the desktop settings."


   Thanks,
   Alexandr.



--Semyon

On 12/23/2015 6:41 PM, Alexander Scherbatiy wrote:

On 12/22/2015 6:41 PM, Semyon Sadetsky wrote:

Right. This is regression of 8073320.
Fix is annulled. Thanks.


  It would be better to update the test to take the scale factor into 
the account.

  Something like  A_BIG_SIZE = Integer.MAX_VALUE / SCALE_FACTOR.

   Thanks,
   Alexandr.



--Semyon

On 12/22/2015 6:03 PM, Yuri Nesterenko wrote:

Semyon,

but earlier builds of jdk9 could handle this bordercase properly,
didn't they?. E.g. b95?

-yan

On 12/22/2015 04:22 PM, Semyon Sadetsky wrote:

Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8145795
webrev: http://cr.openjdk.java.net/~ssadetsky/8145795/webrev.00/

Windows do not accept  Integer.MAX_VALUE as dimension for internal
reasons. Twice less value works.

--Semyon

Re: [9] Review Request: 8143054 [macosx] KeyEvent modifiers do not contain information about mouse buttons

[Alexander Scherbatiy]

 The fix looks good to me.

 Thanks,
 Alexandr.


On 11/18/2015 6:50 PM, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk9.

On macosx when we create a KeyEvent we ignore the mouse state of all 
mouse buttons, which means that mouse modifiers are missing.
The only KeyEvent is affected, because when we convert NS modifiers to 
java modifiers we use nsToJavaKeyModifiers(), but in all other cases 
we use nsToJavaMouseModifiers() which is the same but adds mouse 
modifiers.


In the fix:
- The button parameter of nsToJavaMouseModifiers() was removed because 
it was unused.
- nsToJavaMouseModifiers() was renamed to nsToJavaModifiers() and now 
is used when keyEvent is generated.

- nsToJavaKeyModifiers() was removed because it unused now.

The similar bug for extended(id>=3) mouse buttons was filed for linux:
https://bugs.openjdk.java.net/browse/JDK-8143240

Bug: https://bugs.openjdk.java.net/browse/JDK-8143054
Webrev can be found at: 
http://cr.openjdk.java.net/~serb/8143054/webrev.00

Re: [9] Review request for 8134152: Public API for java 8 DataFlavor fields do not have @since tag

[Alexander Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.


On 30/11/15 18:40, Semyon Sadetsky wrote:

+1

--Semyon

On 11/30/2015 3:52 PM, Alexey Ivanov wrote:

Hello,

Please review the following documentation fix for jdk9:
bug: https://bugs.openjdk.java.net/browse/JDK-8134152
webrev: 
http://cr.openjdk.java.net/~aivanov/8134152/jdk9/webrev.00/


In JDK 8, three new fields were added to 
java.awt.datatransfer.DataFlavor class:

* selectionHtmlFlavor
* fragmentHtmlFlavor
* allHtmlFlavor

These fields do not have @since tag in their Javadoc.


Regards,
Alexey

Re: Review Request for 8055197: TextField deletes multiline strings

[Alexander Scherbatiy]

  The fix looks good to me.

  Thanks,
  Alexandr.

On 11/19/2015 11:43 AM, Ambarish Rapte wrote:

Dear Alex,

Please take a look at updated webrev,
http://cr.openjdk.java.net/~rchamyal/ambarish/8055197/webrev.03/


Updated the test code for testing de-serialization.


Many Thanks
Ambarish

-Original Message-
From: Alexander Scherbatiy
Sent: Friday, November 06, 2015 5:36 PM
To: Ambarish Rapte
Cc: awt-dev@openjdk.java.net
Subject: Re:  Review Request for 8055197: TextField deletes multiline 
strings

On 11/5/2015 2:34 PM, Alexander Scherbatiy wrote:

On 11/5/2015 1:33 PM, Ambarish Rapte wrote:

Hi Alex,

 I have updated the webrev with changes for deserialization,
 http://cr.openjdk.java.net/~rchamyal/ambarish/8055197/webrev.02/

 Please review.
 Verified that serialization & de-serialization of text field
object works fine.
 Also updated the test in this webrev to test de-serialization.

  There is one thing to note that readObject() in the fix first use
s.defaultReadObject() that can read a value with new lines into text field so 
there would be a small time when the TextField has an inconsistence state. It 
is recommended to read all fields by
fields.get(fieldName) to avoid this:

  ObjectInputStream.GetField fields = in.readFields();
  String name = (String) fields.get("name", DEFAULT);

Thanks,
Alexandr.


It is true if a serialized file was created by TextField
serialization. The text field does not contain new lines in this case
because they were removed by constructor or set method.
   The question was about serialized files which were generated
manually or changed after a serialization. In this case the text field
can be rewritten with a value which contains new lines.

Thanks,
Alexandr.


Many Thanks,
Ambarish

-Original Message-
From: Alexander Scherbatiy
Sent: Thursday, October 29, 2015 9:59 PM
To: Ambarish Rapte
Cc: Prasanta Sadhukhan; Semyon Sadetsky; awt-dev@openjdk.java.net;
Sergey Bylokhov
Subject: Re: Review Request for 8055197: TextField deletes multiline
strings

On 10/29/2015 5:31 PM, Ambarish Rapte wrote:

Hi Alexandr,

 Please review the updated webrev, with changes only in
TextField.java.

 Webrev:
http://cr.openjdk.java.net/~rchamyal/ambarish/8055197/webrev.01/


 replaceEOL() is declared static, as a class member method cannot
be called before call to super from Constructor.
 Also made changes, as per Sergey's comment for,
System.lineSeparator()

Thank you.

Could you also check the TextField deserialization? It always
should be kept in mind that fields in a deserialized object need to
passe the same checks which are applied in constructors and set methods.

  Thanks,
  Alexandr.

Many Thanks,
Ambarish

-Original Message-
From: Alexander Scherbatiy
Sent: Wednesday, October 28, 2015 8:48 PM
To: Ambarish Rapte
Cc: awt-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re: Review Request for 8055197: TextField deletes multiline
strings

On 10/26/2015 11:21 PM, Ambarish Rapte wrote:

Hi Alexandr,
 Yes, This is windows specific issue.
 Initially there was a native side fix, Please refer below
webrev for the earlier changes.
 
http://cr.openjdk.java.net/~psadhukhan/ambarish/8055197/webrev.00/


 But there was an issue pointed for this fix, and after
discussion with Phil,
 We arrived at this new solution, of replacing on java side.
 
http://cr.openjdk.java.net/~rchamyal/ambarish/8055197/webrev.00/




 A major review comment for this previous fix was,

 Sergey:
 Note that the fix replace the eol char on the peer level. So
there will
  be a difference in behavior of setText/GetText when the
peer exists or not

  I see the point now.

 Is it possible to add the 'String replaceEOL(String text)'
method to the TextField? In this case TextField constructors can
remove EOL from the given text and pass it to the super class. It
could avoid the TextComponent class changing.

   Thanks,
   Alexandr.


 However both the fix have no side effect, Have executed jtreg
on almost all tests which use or test TextField.

 There is confusion for most correct way to patch this. Please
guide

Thanks
Ambarish


-Original Message-
From: Alexander Scherbatiy
Sent: Wednesday, October 21, 2015 7:29 PM
To: Ambarish Rapte
Cc: awt-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re: Review Request for 8055197: TextField deletes
multiline strings



   Is this is Windows specific issue only? If so, it is better
to replace EOL on window text peer side or may be even better to do
it on the native side before setting a text to the single-line
RichEdit. May be there are methods that can remove EOL from a
string exactly in the same way as it is done by Edit control.

  Thanks,
  Alexandr.

On 10/19/2015 9:31 PM, Ambarish Rapte wrote:

Hi Alexander,

 In single line mode , th

Review request for 8143064 Icons are not properly rendered with Windows L on HiDPI display

[Alexander Scherbatiy]

Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8143064
  webrev: http://cr.openjdk.java.net/~alexsch/8143064/webrev.00

  Icon image sizes are scaled in sun.swing.CachedPainter.

  Thanks,
  Alexandr.