Re: [OpenJDK 2D-Dev] [8u-dev] RFR for JDK-8201240: Improve releasing native resources of BufImgSurfaceData.ICMColorData

[Sergey Bylokhov]

Looks fine.

On 23/04/2018 03:49, Alexey Ivanov wrote:

Hi,

Could you please review the following backport to 8u-dev?

jbs: https://bugs.openjdk.java.net/browse/JDK-8201240
review: 
http://mail.openjdk.java.net/pipermail/2d-dev/2018-April/009122.html

jdk11 changeset: http://hg.openjdk.java.net/jdk/client/rev/69f7e3ed043c


The patch does not apply cleanly to 8u and also requires changes to map 
files:

webrev: http://cr.openjdk.java.net/~aivanov/8201240/jdk8/webrev.0/

Thank you!

Regards,
Alexey



--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values

[Sergey Bylokhov]

Looks fine.

On 27/03/2018 04:41, Prahalad Kumar Narayanan wrote:

Hello Phil, Sergey and Jay

Thank you for your time in review.

I've incorporated your review suggestions.
In the updated code, ImageWriter gets disposed appropriately.

Kindly review the updated code at your convenience.
Link: http://cr.openjdk.java.net/~pnarayanan/4954348/webrev.01/

Thank you once again
Have a good day

Prahalad N.


-Original Message-
From: Philip Race
Sent: Tuesday, March 27, 2018 2:40 AM
To: Sergey Bylokhov
Cc: Prahalad Kumar Narayanan; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: 
JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values



On 3/23/18, 11:56 AM, Sergey Bylokhov wrote:

Hi, Prahalad.
A few small comments about the test:
  - Is it possible to test all installed ImageWriterSpi? It seems that
the test itself is not a  JPEG plugin specific?


None of the other plugins support thumbnails so they will all return 0 
regardless.
So I think that aspect of the test is OK



  - You will need to dispose the jpgWriter even in case of exception.


Probably should fix that.

Everything else seems fine.

-phil.


On 23/03/2018 01:10, Prahalad Kumar Narayanan wrote:

Kindly review the changes at your convenience
Link: http://cr.openjdk.java.net/~pnarayanan/4954348/webrev.00

Thank you
Have a good day

Prahalad N.







--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [11] JDK-8200526 Test sun/java2d/marlin/ClipShapeTest.java times out

[Sergey Bylokhov]

Looks fine.

On 30/03/2018 13:09, Laurent Bourgès wrote:

Phil,
Here is the ClipShapeTest change to fix timeout issue

JBS: https://bugs.openjdk.java.net/browse/JDK-8200526
webrev: http://cr.openjdk.java.net/~lbourges/marlin/marlin-8200526.0/

I increased the timeout to 300s per test.

I did a jtreg run on my machine (slowed down to 2Ghz) and it passed 
successfully although the ClipShapeTest finished in 1200s in total:


grep elapsed ClipShapeTest.jtr
elapsed=1209067 0\:20\:09.067
elapsed time (seconds): 1.131
elapsed time (seconds): 1.129
elapsed time (seconds): 70.291
elapsed time (seconds): 0.0
elapsed time (seconds): 244.185
elapsed time (seconds): 0.0
elapsed time (seconds): 127.729
elapsed time (seconds): 0.0
elapsed time (seconds): 187.525
elapsed time (seconds): 0.0
elapsed time (seconds): 60.818
elapsed time (seconds): 0.0
elapsed time (seconds): 200.825
elapsed time (seconds): 0.0
elapsed time (seconds): 126.312
elapsed time (seconds): 0.0
elapsed time (seconds): 190.248


Regards,
Laurent



--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [11] Review Request: 4912693 Behavior of null arguments not specified in Java Sound

[Sergey Bylokhov]

Hi, Phil.
Thank you for review.
An updated version:
http://cr.openjdk.java.net/~serb/4912693/webrev.02
CSR:
https://bugs.openjdk.java.net/browse/JDK-8199763

On 16/03/2018 08:57, Phil Race wrote:
- * are examples of typical and acceptable run time exceptions for such 
cases.

+ * are the examples of typical and acceptable run time exceptions for such
+ * cases.


all changes like this make the grammar WRONG.

"the examples" means you've enumerated all of them in which
case they are no longer examples. They are the full list.


So revert all of these in javax.print.

The sound ones look fine.

-phil.

On 03/08/2018 03:20 PM, Sergey Bylokhov wrote:

Thank you for review.
An updated version:
http://cr.openjdk.java.net/~serb/4912693/webrev.01/
I also have updated the text in the "javax/print" package

On 18/01/2018 16:18, Dan Rollo wrote:

Hi Sergey,

Looks good to me. One minor grammar thought: “an example of a”.

Maybe: {@code NullPointerException} is example of typical...
Should be: {@code NullPointerException} is an example of a typical…

-Dan

On Jan 17, 2018, at 10:55 PM, Sergey Bylokhov 
<sergey.bylok...@oracle.com> wrote:


Hello, Audio Guru.

Please review the fix for jdk11.

The text for behavior of null arguments is added to the package-info 
files as suggested in the first step in the bug report. The text is 
copied from the javax.print package:
https://docs.oracle.com/javase/9/docs/api/javax/print/package-summary.html 



The CSR will be filed after the technical review.

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


--
Best regards, Sergey.










--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR 8199789: Emit a warning message when t2k is selected via system property

[Sergey Bylokhov]

+1

On 20/03/2018 19:42, Prahalad Kumar Narayanan wrote:

Hello Phil

Good day to you.

Thank you for the explanation. I understand your point now.
The code changes look good.

Thank you
Have a good day

Prahalad N.


- Original Message -
From: Phil Race
Sent: Wednesday, March 21, 2018 12:14 AM
To: Prahalad Kumar Narayanan; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] RFR 8199789: Emit a warning message when t2k is 
selected via system property

I typed
+(!(FontUtilities.useT2K && !FontUtilities.useLegacy) ?

when I actually meant

+((!FontUtilities.useT2K && !FontUtilities.useLegacy) ?

ie the first "!" was supposed to be inside ..

I decided to add a debugging option to make it easier to tell what you'd got
and then further decided to add code that falls back to freetype if you
asked for T2K but it isn't there.


http://cr.openjdk.java.net/~prr/8199789.1

I've tested it with openjdk + oracle JDK builds.

I think it'll do for now until T2K is removed.

-phil.
On 03/20/2018 02:30 AM, Prahalad Kumar Narayanan wrote:
Hello Phil

Good day to you.

I imported your patch and checked the resulting build.
Warnings showed up as expected with use of "t2k" or "legacy" in the VM option 
-Dsun.java2d.font.scaler.

However, I'm unable to interpret this line and its intended outcome.
It also adds a new value for sun.java2d.font.scaler of "legacy" which means 
"t2k" but as it was used in by default.

In my observation with logs, setting VM option to
 "t2k" instantiates T2KFontScaler and
 "legacy" instantiates FreetypeFontScaler.
If this is the intended behavior, the code changes work as expected.

Thank you
Have a good day

Prahalad N.


-Original Message-
From: Phil Race
Sent: Tuesday, March 20, 2018 1:09 AM
To: 2d-dev
Subject: [OpenJDK 2D-Dev] RFR 8199789: Emit a warning message when t2k is 
selected via system property

Bug: https://bugs.openjdk.java.net/browse/JDK-8199789
Webrev : http://cr.openjdk.java.net/~prr/8199789/

https://bugs.openjdk.java.net/browse/JDK-8193017 made freetype the default font 
rasteriser for all JDK builds.

We plan to remove t2k completely including references to it from open sources, 
before JDK 11 GA's

But for now it is still there for debugging but to make it clear this small fix 
makes a warning get printed if you try to use it.

It also adds a new value for sun.java2d.font.scaler of "legacy" which means 
"t2k" but as it was used in by default.

The subtle issue is that "t2k" disables using GDI for LCD text.
"legacy" gets you exactly what JDK did by default in 6u10 -> JDK 10 inclusive.

So it may be more useful for a debugging comparison flag.

-phil.





--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: 8200129 : Remove D3D Performance Counter.

[Sergey Bylokhov]

Looks fine.

On 22/03/2018 13:33, Phil Race wrote:

bug: https://bugs.openjdk.java.net/browse/JDK-8200129
webrev: http://cr.openjdk.java.net/~prr/8200129/

It was noted in another thread [1] that we had a couple of qualified 
exports

from java.base to java.desktop that are platform-specific but are in the
main module-info.java.

It was suggested to move these to platform-specific locations.
But one of them doesn't seem to be at all important and we can just 
remove it.
It is a "counter" that basically tells you if D3D was used but I don't 
see any

standard tools that support it and I was able to build + run just fine.

The other platform-specific export is moved here as suggested since
it can't be just removed without some alternative.

NB I did build all platforms.

-phil.

[1] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-March/052198.html



--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values

[Sergey Bylokhov]

Hi, Prahalad.
A few small comments about the test:
 - Is it possible to test all installed ImageWriterSpi? It seems that 
the test itself is not a  JPEG plugin specific?

 - You will need to dispose the jpgWriter even in case of exception.

On 23/03/2018 01:10, Prahalad Kumar Narayanan wrote:

Kindly review the changes at your convenience
Link: http://cr.openjdk.java.net/~pnarayanan/4954348/webrev.00

Thank you
Have a good day

Prahalad N.




--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: 8199870: colorimaging.md needs to remove mention of KCMS

[Sergey Bylokhov]

+1

On 20/03/2018 09:54, Phil Race wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8199870
Diff: inline below

The legal attribution file src/java.desktop/share/legal/colorimaging.md
references the Kodak Color Management System which not only
was never in OpenJDK, but is not in Oracle JDK anymore either ..

This simple patch just updates the file to reflect this.
It still needs to be there since Kodak contributed to the implementation
of  number of the java/awt/image API classes.

Diff below.

-phil.

diff --git a/src/java.desktop/share/legal/colorimaging.md 
b/src/java.desktop/share/legal/colorimaging.md

--- a/src/java.desktop/share/legal/colorimaging.md
+++ b/src/java.desktop/share/legal/colorimaging.md
@@ -1,4 +1,4 @@
-## Eastman Kodak Company: Kodak Color Management System (kcms) and 
portions of color management and imaging software
+## Eastman Kodak Company: Portions of color management and imaging 
software


  ### Eastman Kodak Notice
  




--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [11] Upgrade to Marlin renderer 0.9.1

[Sergey Bylokhov]

Hi, Laurent.
On 05/03/2018 14:19, Laurent Bourgès wrote:
First Marlin 0.9.1 patch works well: no crash or bug with larger tiles 
128x64 even if d3d / opengl uses internally 32x32 texture caches.
Moreover xrender pipeline is the fastest compared to D3D (40% slower) or 
opengl (>250% slower) !


What types of GraphicsPrimitives were tested(in terms of java2d)? I 
guess that d3d/ogl may outperform other pipelines only in case of 
"native" blits, which are used in case of draw of cashed 
bufferedImage(OGL texture) or VolatileImage(FBO) to the window/volatile 
image(this also depends from the composite type and alpha).
In other cases it will be slower since it use an additional layer - 
RenderQueue, it would be good to compare xrender and gdi/X11.


Some unrelated question: it is interesting how xrender will work in Wayland.


Note 1. OGL is not officially supported on linux. We need to check ogl 
performance on macOS where it is used as a default pipeline.


Note 2. there are some other blit's related tiles like:
#define OGLC_BLIT_TILE_SIZE 128

Also please be care about different vendors:
OGLC_VENDOR_INTEL/OGLC_VENDOR_ATI/OGLC_VENDOR_NVIDIA because their 
native blits are implemented differently. The reason was in a 
performance of some OGL API(maybe this code is outdated).


I suggest to use some common testcases from J2DBench and SwingMark, so 
at some point later it will be possible to check other changes for 
possible regression, for example see:

https://bugs.openjdk.java.net/browse/JDK-8059944
Note that this fix in some cases decreased a performance by half but in 
other cases improved it by 25422.21%. You can see we can improve 
performance in one case but lose much more in another. This is why 
J2DBench is better because it can check all combinations of 
src/dst/composite/clip/size/etc..


Soory to insist but who could advice me and give me explanations on the 
RenderQueue & d3d / opengl backends.


I read JBS for RenderQueue buffering as I have several questions 
(asynchronous queue ?)

How to auto-tune such buffer capacity ?
It seems tricky as too small or too large buffers impacts performance as 
it depends on the GPU speed (drain the buffer).

Is there any design document ?


As far as I know there are no documents about tile tuning, most of 
decisions were made according to j2dbench results. But this code still 
uses ogl_2 and d3d_9 so it can be possible that the new versions of 
these API have a better alternative.


PS: I will once again look into java2d pipelines for tile constants (32) 
to see if other parts should be updated (TexturePaint ?)...
I also need help on testing such patches on many hw platforms to detect 
regressions (j2dBench, MapBench...)


I guess we can run these tests on at least supported configurations.

--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] Initial implementation for batched AA-mask upload for xrender

[Sergey Bylokhov]

Hi, Clemens.
On 21/02/2018 23:41, Clemens Eisserer wrote:

It is still in prototype state with a few rough edges and a few
corner-cases unimplemented (e.g. extra alpha with antialiasing),
but should be able to run most workloads:
http://93.83.133.214/webrev/
https://sourceforge.net/p/xrender-deferred/code/ref/default/

It is disabled by default, and can be enabled with -Dsun.java2d.xr.deferred=true
Shm upload is enabled with deferred and can be disabled with:
-Dsun.java2d.xr.shm=false

What would be the best way forward?
Would this have a chance to get into OpenJDK11 for platforms eith
XCB-based Xlib implementations?
Keeping in mind the dramatic performance increase,
even outperforming the current OpenGL pipeline, I really hope so.


Thank you for contribution! If there is a chance to implement it soon(at 
an early test stage) then it is possible to push this to jdk11 and 
enable it by default to expose all possible issues. If no issues will be 
found, then we can release it as-is, otherwise we can disable it by 
default via property.


--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] Crash in CGraphicsDevice.m

[Sergey Bylokhov]

Hi, Bill.

The similar bug was reported recently:
https://bugs.openjdk.java.net/browse/JDK-8211992

The root cause is how we use CoreGraphics display ID. This identifier can 
become non-valid at any time therefore methods, which is using this id should 
be ready to it.

And this bug found a few places which does not take care about the rule above.

I am working on the fix for jdk12.

On 10/10/2018 08:45, Bill York wrote:

2d-dev folks,

I work on a product called MATLAB and we have about 60 reports from customers 
on Mac related to a crash in CGraphicsDevice.m

Please let  me know if this is the right way to report a crash and discuss 
getting it resolved.

Here are the details:

CGraphicsDevice.m is a native file in support of Java drawing and gets called 
from Java_sun_awt_CGraphicsDevice_nativeGetDisplayMode

While I can’t reproduce the problem, it looks like the display pointer is 
becoming invalid for a time when the user’s laptop opens or closes.

Looking at this source code:

http://cr.openjdk.java.net/~serb/8000629/webrev.08/src/macosx/native/sun/awt/CGraphicsDevice.m.html

I see a direct call to CFStringCompare without a check for a NULL CFStringRef.

  * (CFStringCompare(mode, CFSTR(kIO30BitDirectPixels), 
kCFCompareCaseInsensitive) == kCFCompareEqualTo)

I believe if this code returned 0, the crash would not occur though there may 
be some other cleanup in the surrounding call frames.

Bill




--
Best regards, Sergey.

[OpenJDK 2D-Dev] [12] Review Request: 8211992 GraphicsConfiguration.getDevice().getDisplayMode() causes JVM crash on Mac

[Sergey Bylokhov]

Hello.
Please review the fix for jdk 12.

Bug: https://bugs.openjdk.java.net/browse/JDK-8211992
Webrev: http://cr.openjdk.java.net/~serb/8211992/webrev.00

Short description:
  The root cause is how we use CoreGraphics display ID. This identifier can 
become non-valid at any time therefore methods, which is using this id should 
be ready to it. And this bug found a few places which does not take care about 
the rule above.

Long description:
  In the CGraphicsDevice class we maintain the ID for the native screen. This 
ID usually changed when the user adds/removes monitors to the system. Since 
using invalid id can cause NULL result in the native code we tries to 
workaround it by two steps:

  Step (1) If the monitor is removed from the system, then we reassign its 
displayid to the primary screen. So if the user will holds the reference to 
this GDevice, it will use the MainDisplay. But note that if the user will 
remove the main screen again then the old screen, which were removed 
previously, will not be updated(but will use an invalid displayid).

  Step (2) Some of the native methods in CGraphicsDevice class are ready to 
NULL and provide some meaningful defaults, like 72 dpi. But some others methods 
are not ready for that, like nativeGetDisplayMode/etc.

Fix description:
 - I have minimized/dropped the usage of displayid outside of CGraphicsDevice 
class, in CRobot it was unused, and the code from CGraphicsConfig.m was moved 
to CGraphicsDevice.m.
 - In CGraphicsEnvironment class we now maintain the list of old devices, which 
is updated for all _displayReconfiguration events. This is the same 
implementation as on windows in Win32GraphicsEnvironment.java
 - I have added some default values to all native methods also. Just to cover 
the rare case when we call the method and displayid became invalid at the same 
moment.

--
Best regards, Sergey.

[OpenJDK 2D-Dev] [12] Review Request: 8212790 Javadoc cleanup of java.awt.color package

[Sergey Bylokhov]

Hello.
Please review a javadoc "weekend cleanup" in jdk 12.

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

To check the difference I suggest to use specdiff.

Some important changes:
 - The copyright header in CMMException.java is changed. I have checked the 
history and confirmed that this file had the same copyright as other files in 
jdk6. And I guess the header was not updated when it was moved to the open repo 
in JDK-6662775.
 - Descriptions of ICC_ProfileGray and ICC_ProfileRGB classes are unified. 
These are similar classes, but the javadoc was a little bit different. Any 
suggestions to improve it further are welcome.

Other changes:
 - description of the class/method/field should be followed by dot
 - @param, @return should not end with a dot, except a case when more than one 
sentences are used
 - empty line after description/before the first tag was added
 - unnecessary empty lines were removed
 - sets of spaces in the middle of text were deleted
 - @param, @throws, @return should be aligned, to be more readable
 - unnecessary imports were removed
 - the "true"/"false" were wrapped in {@code } when necessary
 - @exception is replaced by @throws

--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] Review Request: 8212790 Javadoc cleanup of java.awt.color package

[Sergey Bylokhov]

Hi, Krishna.
Thank you for review!


1894, 1895, 1902, 1904.

The text on the lines above is a block comment not a javadoc, the tags are not 
necessary there.

1570, 1601, 


I have fixed these:
http://cr.openjdk.java.net/~serb/8212790/webrev.01

--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] Speed of drawPolyline on JDK11

[Sergey Bylokhov]

I have no comments about the current proposal(results is good), but is that 
really necessary to have this implementation in native code?

On 23/10/2018 13:37, Laurent Bourgès wrote:

Phil,
I quickly modified the final update & sort loop to:
- move sort in another block
- use qsort() using a new comparator sortSegmentsByCurX

This improves performance in PolyLineTest by 3 times: ~1s vs 3.5s !
Apparently qsort() is not optimal (comparator can not be inlined by c) so it 
may explain why Marlin (0x0 sampling) is still 2 times faster with its custom 
merge-sort (in-place).

Any idea to improve C sort ?
Is it good enough ?

- USE_QSORT_X: 1
oct. 23, 2018 10:15:29 PM polylinetest.Canvas paintComponent
INFOS: Paint Time: 1,081s
INFOS: Paint Time: 1,058s
INFOS: Paint Time: 1,067s

- USE_QSORT_X: 0
oct. 23, 2018 10:18:50 PM polylinetest.Canvas paintComponent
INFOS: Paint Time: 3,318s
INFOS: Paint Time: 3,258s
INFOS: Paint Time: 3,273s

Patch:
diff -r 297450fcab26 
src/java.desktop/share/native/libawt/java2d/pipe/ShapeSpanIterator.c
--- a/src/java.desktop/share/native/libawt/java2d/pipe/ShapeSpanIterator.c    
Tue Oct 16 23:21:05 2018 +0530
+++ b/src/java.desktop/share/native/libawt/java2d/pipe/ShapeSpanIterator.c    
Tue Oct 23 22:31:00 2018 +0200
@@ -1243,6 +1243,18 @@
  }
  }

+/* LBO: enable (1) / disable (0) qsort on curx */
+#define USE_QSORT_X (0)
+
+static int CDECL
+sortSegmentsByCurX(const void *elem1, const void *elem2)
+{
+    jint x1 = (*(segmentData **)elem1)->curx;
+    jint x2 = (*(segmentData **)elem2)->curx;
+
+    return (x1 - x2);
+}
+
  static jboolean
  ShapeSINextSpan(void *state, jint spanbox[])
  {
@@ -1378,16 +1390,28 @@
  seg->curx = x0;
  seg->cury = y0;
  seg->error = err;
+    }

-    /* Then make sure the segment is sorted by x0 */
-    for (new = cur; new > lo; new--) {
-    segmentData *seg2 = segmentTable[new - 1];
-    if (seg2->curx <= x0) {
-    break;
+    if (USE_QSORT_X && (hi - lo) > 100)
+    {
+    /* use quick sort on [lo - hi] range */
+    qsort(&(segmentTable[lo]), (hi - lo), sizeof(segmentData *),
+    sortSegmentsByCurX);
+    } else {
+    for (cur = lo; cur < hi; cur++) {
+    seg = segmentTable[cur];
+    x0 = seg->curx;
+
+    /* Then make sure the segment is sorted by x0 */
+    for (new = cur; new > lo; new--) {
+    segmentData *seg2 = segmentTable[new - 1];
+    if (seg2->curx <= x0) {
+    break;
+    }
+    segmentTable[new] = seg2;
  }
-    segmentTable[new] = seg2;
+    segmentTable[new] = seg;
  }
-    segmentTable[new] = seg;
  }
  cur = lo;
  }

Cheers,
Laurent

Le mar. 23 oct. 2018 à 08:30, Laurent Bourgès mailto:bourges.laur...@gmail.com>> a écrit :

Phil,
Yesterday I started hacking ShapeSpanIterator.c to add stats: the last 
stage (sort by x0) is the bottleneck.
In this case every sort takes up to 15ms per pixel row !

I will see if I can adapt Marlin's MergeSort.java to C to have an efficient 
sort in-place.
Do you know if libawt has already an efficient sort instead of porting mine 
?

PS: "To save the planet, make software more efficient" is my quote of the 
day !

Cheers,
Laurent




--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] Review Request: 8212790 Javadoc cleanup of java.awt.color package

[Sergey Bylokhov]

Hi, Phil.
See my comments inline:

On 27/10/2018 13:02, Philip Race wrote:

I am wondering why you took out the unordered list here :
https://docs.oracle.com/javase/9/docs/api/java/awt/color/ICC_ProfileRGB.html

The specdiff *maybe* doesn't really tell me what it looks like now but I don't 
see how
it can still be a list ...


I have changed it from the list to the plain text(this text is similar as in 
ICC_ProfileGray), the reason is that the first sentence in the class 
description is used as a summary of the class in the package javadoc:
https://docs.oracle.com/javase/9/docs/api/java/awt/color/package-summary.html

And before the fix it was appeared as a plain text(w/o list). So I unified it 
for all places:
http://cr.openjdk.java.net/~serb/8212790/specdiff.00/java.desktop/java/awt/color/package-summary.html



May I assume you tested out any newly added links ?
eg

73 * spaces via {@link ColorSpace#getInstance}.


yes.



Looks straightforward so I don't expect any issues, just checking.

Also it looks as if some lines got longer due to reformatting, are they are 
still within bounds ?


Correct, this is one of the goal of the fix.



eg

- *
- * A subclass of the ICC_Profile class which represents profiles
- * which meet the following criteria: the color space type of the
- * profile is TYPE_GRAY and the profile includes the grayTRCTag and
- * mediaWhitePointTag tags. Examples of this kind of profile are
- * monochrome input profiles, monochrome display profiles, and
- * monochrome output profiles. The getInstance methods in the
- * ICC_Profile class will
- * return an ICC_ProfileGray object when the above conditions are
- * met. The advantage of this class is that it provides a lookup
- * table that Java or native methods may be able to use directly to
- * optimize color conversion in some cases.
+ * The {@code ICC_ProfileGray} class is a subclass of the {@code ICC_Profile}
+ * class that represents profiles which meet the following criteria: the color
+ * space type of the profile is {@code TYPE_GRAY} and the profile includes the
+ * {@code grayTRCTag} and {@code mediaWhitePointTag} tags. The
+ * {@code getInstance} methods in the {@code ICC_Profile} class will return an
+ * {@code ICC_ProfileGray} object when the above conditions are met. Examples 
of
+ * this kind of profile are monochrome input profiles, monochrome display
+ * profiles, and monochrome output profiles.
+ * 
+ * The advantage of this class is that it provides a lookup table that Java
+ * or native methods can use directly to optimize color conversion in some
+ * cases.


-phil

On 10/24/18, 9:36 PM, Sergey Bylokhov wrote:

Hi, Krishna.
Thank you for review!


1894, 1895, 1902, 1904.

The text on the lines above is a block comment not a javadoc, the tags are not 
necessary there.

1570, 1601, 


I have fixed these:
http://cr.openjdk.java.net/~serb/8212790/webrev.01




--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] Review Request: 8211992 GraphicsConfiguration.getDevice().getDisplayMode() causes JVM crash on Mac

[Sergey Bylokhov]

Any volunteers to review? =)

On 15/10/2018 18:11, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk 12.

Bug: https://bugs.openjdk.java.net/browse/JDK-8211992
Webrev: http://cr.openjdk.java.net/~serb/8211992/webrev.00

Short description:
   The root cause is how we use CoreGraphics display ID. This identifier can 
become non-valid at any time therefore methods, which is using this id should 
be ready to it. And this bug found a few places which does not take care about 
the rule above.

Long description:
   In the CGraphicsDevice class we maintain the ID for the native screen. This 
ID usually changed when the user adds/removes monitors to the system. Since 
using invalid id can cause NULL result in the native code we tries to 
workaround it by two steps:

   Step (1) If the monitor is removed from the system, then we reassign its 
displayid to the primary screen. So if the user will holds the reference to 
this GDevice, it will use the MainDisplay. But note that if the user will 
remove the main screen again then the old screen, which were removed 
previously, will not be updated(but will use an invalid displayid).

   Step (2) Some of the native methods in CGraphicsDevice class are ready to 
NULL and provide some meaningful defaults, like 72 dpi. But some others methods 
are not ready for that, like nativeGetDisplayMode/etc.

Fix description:
  - I have minimized/dropped the usage of displayid outside of CGraphicsDevice 
class, in CRobot it was unused, and the code from CGraphicsConfig.m was moved 
to CGraphicsDevice.m.
  - In CGraphicsEnvironment class we now maintain the list of old devices, 
which is updated for all _displayReconfiguration events. This is the same 
implementation as on windows in Win32GraphicsEnvironment.java
  - I have added some default values to all native methods also. Just to cover 
the rare case when we call the method and displayid became invalid at the same 
moment.




--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] Review Request: 8211992 GraphicsConfiguration.getDevice().getDisplayMode() causes JVM crash on Mac

[Sergey Bylokhov]

On 30/10/2018 09:59, Phil Race wrote:

Looks reasonable. A test is difficult, but I presume you did some testing.
Can you report the scenarios you tested and on which OS version ?
eg,

Did you try remove/add/remove, or add/remove/add ?
Did you try wake from sleep ?
Did you try removing a monitor whilst it was sleeping and then waking ?


I was not able to reproduce this bug on the multimonitor config,
but in a single monitor the bug is reproduced when I switch
the user while the program is executed.

For the test purpose and to prove that the updated methods will not crash,
I have hardcoded the wrong display id at the end of the constructor
of the GraphicsDevice, and run our tests.



All whilst a Java app was running of course - on the monitor being removed.

-phil.

On 10/15/18 6:11 PM, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk 12.

Bug: https://bugs.openjdk.java.net/browse/JDK-8211992
Webrev: http://cr.openjdk.java.net/~serb/8211992/webrev.00

Short description:
  The root cause is how we use CoreGraphics display ID. This identifier can 
become non-valid at any time therefore methods, which is using this id should 
be ready to it. And this bug found a few places which does not take care about 
the rule above.

Long description:
  In the CGraphicsDevice class we maintain the ID for the native screen. This 
ID usually changed when the user adds/removes monitors to the system. Since 
using invalid id can cause NULL result in the native code we tries to 
workaround it by two steps:

  Step (1) If the monitor is removed from the system, then we reassign its 
displayid to the primary screen. So if the user will holds the reference to 
this GDevice, it will use the MainDisplay. But note that if the user will 
remove the main screen again then the old screen, which were removed 
previously, will not be updated(but will use an invalid displayid).

  Step (2) Some of the native methods in CGraphicsDevice class are ready to 
NULL and provide some meaningful defaults, like 72 dpi. But some others methods 
are not ready for that, like nativeGetDisplayMode/etc.

Fix description:
 - I have minimized/dropped the usage of displayid outside of CGraphicsDevice 
class, in CRobot it was unused, and the code from CGraphicsConfig.m was moved 
to CGraphicsDevice.m.
 - In CGraphicsEnvironment class we now maintain the list of old devices, which 
is updated for all _displayReconfiguration events. This is the same 
implementation as on windows in Win32GraphicsEnvironment.java
 - I have added some default values to all native methods also. Just to cover 
the rare case when we call the method and displayid became invalid at the same 
moment.






--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: 8210863: Remove Xrandr include files from JDK sources

[Sergey Bylokhov]

Looks fine, I assume mach5 is green.

On 31/10/2018 11:57, Phil Race wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8210863
Webrev: http://cr.openjdk.java.net/~prr/8210863/

JDK imported Xrandr headers into it's source a really long time ago because
old build systems did not have it. We no longer have that problem, so we can
remove these.

I am not changing the code to remove the dynamic loading of the functions and
change it to compile time link against them.
We can probably do that too at a later time,  but the immediate goal is to
remove these source files since they are imported 3rd party sources.

-phil.





--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] RFR JDK-8213138: Update ProblemList.txt for mac

[Sergey Bylokhov]

On 30/10/2018 22:30, Prasanta Sadhukhan wrote:

What is the syntax of this? I tried

-nativepath:

but it gave "unexpected exit from test code"

Can we put this option in the test itself in @run option? since we do not run 
with nativepath option neither in PIT not in test sprint, it will always fail.


Before run this test, you need to compile the native part of the tests by this 
command:
  make test-image
and then
  make run-test TEST="java/awt/Window/MainKeyWindowTest/TestMainKeyWindow.java"

Samples are here:
http://hg.openjdk.java.net/jdk/jdk/raw-file/e38473506688/doc/testing.html

Or you can pass this option to jtreg directly:
  jtreg/bin/jtreg -nativepath:./build/macosx-x64/images/test/jdk/jtreg/native/ 
-jdk:./build/macosx-x64/images/jdk  
open/test/jdk/java/awt/Window/MainKeyWindowTest/TestMainKeyWindow.java

The native test-image is also produced by our RE, so you can use pre-build 
test-image if you wish.



Regards
Prasanta
On 31-Oct-18 4:27 AM, Sergey Bylokhov wrote:

Hi, Prasanta.
On 30/10/2018 02:58, Prasanta Sadhukhan wrote:

java/awt/Window/MainKeyWindowTest/TestMainKeyWindow.java: JDK-8213126 
<https://bugs.openjdk.java.net/browse/JDK-8213126>


The failure of the TestMainKeyWindow.java is not a bug, you need to provide 
-nativepath to the jtreg, because this test has a native code.


javax/swing/JTree/6263446/bug6263446.java: JDK-8213125 
<https://bugs.openjdk.java.net/browse/JDK-8213125>
javax/swing/JEditorPane/6917744/bug6917744.java JDK-8213124 
<https://bugs.openjdk.java.net/browse/JDK-8213124>
javax/swing/JButton/4368790/bug4368790.java JDK-8213123 
<https://bugs.openjdk.java.net/browse/JDK-8213123>
javax/swing/GraphicsConfigNotifier/StalePreferredSize.java JDK-8213121 
<https://bugs.openjdk.java.net/browse/JDK-8213121>
java/awt/TextArea/AutoScrollOnSelectAndAppend/AutoScrollOnSelectAndAppend.java 
JDK-8213120 <https://bugs.openjdk.java.net/browse/JDK-8213120>
java/awt/GraphicsDevice/CheckDisplayModes.java JDK-8213119 
<https://bugs.openjdk.java.net/browse/JDK-8213119>

Bug: https://bugs.openjdk.java.net/browse/JDK-8213138
webrev: http://cr.openjdk.java.net/~psadhukhan/8213138/webrev/

Regards
Prasanta








--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] RFR JDK-8213138: Update ProblemList.txt for mac

[Sergey Bylokhov]

Hi, Prasanta.
On 30/10/2018 02:58, Prasanta Sadhukhan wrote:

java/awt/Window/MainKeyWindowTest/TestMainKeyWindow.java: JDK-8213126 



The failure of the TestMainKeyWindow.java is not a bug, you need to provide 
-nativepath to the jtreg, because this test has a native code.


javax/swing/JTree/6263446/bug6263446.java: JDK-8213125 

javax/swing/JEditorPane/6917744/bug6917744.java JDK-8213124 

javax/swing/JButton/4368790/bug4368790.java JDK-8213123 

javax/swing/GraphicsConfigNotifier/StalePreferredSize.java JDK-8213121 

java/awt/TextArea/AutoScrollOnSelectAndAppend/AutoScrollOnSelectAndAppend.java 
JDK-8213120 
java/awt/GraphicsDevice/CheckDisplayModes.java JDK-8213119 


Bug: https://bugs.openjdk.java.net/browse/JDK-8213138
webrev: http://cr.openjdk.java.net/~psadhukhan/8213138/webrev/

Regards
Prasanta



--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] Speed of drawPolyline on JDK11

[Sergey Bylokhov]

On 26/10/2018 00:41, Laurent Bourgès wrote:

I suppose a JEP or RFE is required, isnt it ?


It depends from the changes, but such contributions are welcome!



Cheers,
Laurent


On 23/10/2018 13:37, Laurent Bourgès wrote:
 > Phil,
 > I quickly modified the final update & sort loop to:
 > - move sort in another block
 > - use qsort() using a new comparator sortSegmentsByCurX
 >
 > This improves performance in PolyLineTest by 3 times: ~1s vs 3.5s !
 > Apparently qsort() is not optimal (comparator can not be inlined by c) 
so it may explain why Marlin (0x0 sampling) is still 2 times faster with its 
custom merge-sort (in-place).
 >
 > Any idea to improve C sort ?
 > Is it good enough ?
 >
 > - USE_QSORT_X: 1
 > oct. 23, 2018 10:15:29 PM polylinetest.Canvas paintComponent
 > INFOS: Paint Time: 1,081s
 > INFOS: Paint Time: 1,058s
 > INFOS: Paint Time: 1,067s
 >
 > - USE_QSORT_X: 0
 > oct. 23, 2018 10:18:50 PM polylinetest.Canvas paintComponent
 > INFOS: Paint Time: 3,318s
 > INFOS: Paint Time: 3,258s
 > INFOS: Paint Time: 3,273s
 >
 > Patch:
 > diff -r 297450fcab26 
src/java.desktop/share/native/libawt/java2d/pipe/ShapeSpanIterator.c
 > --- 
a/src/java.desktop/share/native/libawt/java2d/pipe/ShapeSpanIterator.c    Tue Oct 
16 23:21:05 2018 +0530
 > +++ 
b/src/java.desktop/share/native/libawt/java2d/pipe/ShapeSpanIterator.c    Tue Oct 
23 22:31:00 2018 +0200
 > @@ -1243,6 +1243,18 @@
 >   }
 >   }
 >
 > +/* LBO: enable (1) / disable (0) qsort on curx */
 > +#define USE_QSORT_X (0)
 > +
 > +static int CDECL
 > +sortSegmentsByCurX(const void *elem1, const void *elem2)
 > +{
 > +    jint x1 = (*(segmentData **)elem1)->curx;
 > +    jint x2 = (*(segmentData **)elem2)->curx;
 > +
 > +    return (x1 - x2);
 > +}
 > +
 >   static jboolean
 >   ShapeSINextSpan(void *state, jint spanbox[])
 >   {
 > @@ -1378,16 +1390,28 @@
 >   seg->curx = x0;
 >   seg->cury = y0;
 >   seg->error = err;
 > +    }
 >
 > -    /* Then make sure the segment is sorted by x0 */
 > -    for (new = cur; new > lo; new--) {
 > -    segmentData *seg2 = segmentTable[new - 1];
 > -    if (seg2->curx <= x0) {
 > -    break;
 > +    if (USE_QSORT_X && (hi - lo) > 100)
 > +    {
 > +    /* use quick sort on [lo - hi] range */
 > +    qsort(&(segmentTable[lo]), (hi - lo), sizeof(segmentData *),
 > +    sortSegmentsByCurX);
 > +    } else {
 > +    for (cur = lo; cur < hi; cur++) {
 > +    seg = segmentTable[cur];
 > +    x0 = seg->curx;
 > +
 > +    /* Then make sure the segment is sorted by x0 */
 > +    for (new = cur; new > lo; new--) {
 > +    segmentData *seg2 = segmentTable[new - 1];
 > +    if (seg2->curx <= x0) {
 > +    break;
 > +    }
 > +    segmentTable[new] = seg2;
 >   }
 > -    segmentTable[new] = seg2;
 > +    segmentTable[new] = seg;
 >   }
 > -    segmentTable[new] = seg;
 >   }
 >   cur = lo;
 >   }
 >
 > Cheers,
 > Laurent
 >
 > Le mar. 23 oct. 2018 à 08:30, Laurent Bourgès mailto:bourges.laur...@gmail.com> >> a écrit :
 >
 >     Phil,
 >     Yesterday I started hacking ShapeSpanIterator.c to add stats: the 
last stage (sort by x0) is the bottleneck.
 >     In this case every sort takes up to 15ms per pixel row !
 >
 >     I will see if I can adapt Marlin's MergeSort.java to C to have an 
efficient sort in-place.
 >     Do you know if libawt has already an efficient sort instead of 
porting mine ?
 >
 >     PS: "To save the planet, make software more efficient" is my quote 
of the day !
 >
 >     Cheers,
 >     Laurent
 >


-- 
Best regards, Sergey.





--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] RFR JDK-8176556: java/awt/dnd/ImageTransferTest/ImageTransferTest.java fails for JFIF

[Sergey Bylokhov]

On more question about "image/gif" format, as far as I understand this format
do not support the alpha as well, then why it works w/o exceptions?

On 07/11/2018 11:17, Sergey Bylokhov wrote:

Hi, Jay.

As far as I understand the "png" still support alpha,
and it is tested by this test?

On 07/11/2018 02:11, Jayathirth D V wrote:

Hello All,

Please review the following fix in JDK12 :

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

Webrev : http://cr.openjdk.java.net/~jdv/8176556/webrev.00/

Issue : When we run java/awt/dnd/ImageTransferTest/ImageTransferTest.java if 
fails to get ImageWriter(Service provider) capable of encoding JFIF format.

Root cause: Support for alpha channel in JPEG is removed in JDK-8204187 
<https://bugs.openjdk.java.net/browse/JDK-8204187> and this causes 
DataTransferer to throw exception saying unable to get Service provided for ARGB 
channel in for Jpeg.

Solution : Use RGB image without Alpha channel. This regression test was 
written to verify image data before and after DnD, so removing alpha channel 
will not affect its purpose.

Thanks,

Jay







--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] RFR JDK-8176556: java/awt/dnd/ImageTransferTest/ImageTransferTest.java fails for JFIF

[Sergey Bylokhov]

Hi, Jay.

As far as I understand the "png" still support alpha,
and it is tested by this test?

On 07/11/2018 02:11, Jayathirth D V wrote:

Hello All,

Please review the following fix in JDK12 :

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

Webrev : http://cr.openjdk.java.net/~jdv/8176556/webrev.00/

Issue : When we run java/awt/dnd/ImageTransferTest/ImageTransferTest.java if 
fails to get ImageWriter(Service provider) capable of encoding JFIF format.

Root cause: Support for alpha channel in JPEG is removed in JDK-8204187 
 and this causes 
DataTransferer to throw exception saying unable to get Service provided for ARGB 
channel in for Jpeg.

Solution : Use RGB image without Alpha channel. This regression test was 
written to verify image data before and after DnD, so removing alpha channel 
will not affect its purpose.

Thanks,

Jay




--
Best regards, Sergey.

[OpenJDK 2D-Dev] [12] Review Request: 8213110 Remove the use of applets in automatic tests

[Sergey Bylokhov]

Hello.
Please review the fix for jdk 12.

Bug: https://bugs.openjdk.java.net/browse/JDK-8213110
Webrev: http://cr.openjdk.java.net/~serb/8213110/webrev.03

Description of the bug:
  A number of our tests in jdk still launch by applets via "@run applet".
Usually we drop this usage when we update the test for some reason, and
in this fix we will drop them completely for automated tests from the open repo.
I did not update the tests which are specific for Applet API, manual tests, or 
tests
which are currently under review for some other bugs.
 
Note: the "@run applet" means, that the jtreg will do these steps:

 - Creates a frame as a holder for the Applet
 - Creates an applet(which is undecorated panel) and adds it to the frame
 - Sets the size of the frame
 - Place the frame to the center of the screen
 - Make the frame visible
 - Call init() and start() methods of the Applet
 - Waits for 2 seconds
 - Call stop() and destroy() methods of the Applet
 - Dispose the frame


Description of the fix:

 - In all cases the usage of the Applet API was dropped
 - In the common case when the applet was used as launcher, this code now used 
instead:
   public static void main(final String[] args) {
  TestName app = new TestName();
  app.init();
  app.start();
   }
   Example:
   
http://cr.openjdk.java.net/~serb/8213110/webrev.03/test/jdk/java/awt/Choice/PopdownGeneratesMouseEvents/PopdownGeneratesMouseEvents.java.sdiff.html

 - In some cases it was possible to replace init()/start() by the simple main() 
method.
   Example:
   
http://cr.openjdk.java.net/~serb/8213110/webrev.03/test/jdk/java/awt/Choice/PopupPosTest/PopupPosTest.java.sdiff.html

 - Some of the tests were used the "extend Applet" w/o a reasons:
   Example:
   
http://cr.openjdk.java.net/~serb/8213110/webrev.03/test/jdk/java/awt/Focus/ActualFocusedWindowTest/ActualFocusedWindowBlockingTest.java.sdiff.html

 - Some of the tests shows the applet window(which was unrelated to the test 
itself) in the center of the screen.
   Example:
   
http://cr.openjdk.java.net/~serb/8213110/webrev.03/test/jdk/java/awt/Focus/ActualFocusedWindowTest/ActualFocusedWindowRetaining.java.sdiff.html

 - Some of the tests uses the applet as a place holder for the test components.
   In this case it was necessary to change the Applet API to the Frame API, and 
complete
   the steps which were done by the jtreg:
   Example:
   
http://cr.openjdk.java.net/~serb/8213110/webrev.03/test/jdk/java/awt/Focus/AppletInitialFocusTest/AppletInitialFocusTest.java.sdiff.html


Notes:
 - To simplify the review I tried to not change the logic of the tests,
   so if the test fail before the fix, then it will fail after the fix.
   I would like to create the separate CRs for all additional(if any) cleanup 
of these tests.
 - Just a few exception from the above is additional calls to:
   "setLocationRelativeTo" to place the window to the center of the screen
   "robot.waitForIdle()"/"setUndecorated()" to make the tests more stable(when 
I compared the tests before/after the fix)


--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] RFR JDK-8176556: java/awt/dnd/ImageTransferTest/ImageTransferTest.java fails for JFIF

[Sergey Bylokhov]

Hi, Jay.
So you leaved the code which checks the full color including alpha, but
the source image does not have an alpha so the check for alpha is not strictly 
necessary.
May be we can use RGB only for JPEG and for all others we can use ARGB?

BTW Looking at this test I assume that it was expected that jpeg/gif images 
might be changed during DND operation,
 - so maybe DND of the argb image using jpeg format should drop the alpha 
channel?
 - or it should be somehow be reported as unsupported?
instead of thrown exception somewhere in DND code?

On 08/11/2018 01:13, Jayathirth D V wrote:

Hello All,

I am replying to the last mail in this thread.

 From the test case we can see that it is not strict about pixel data which was 
transferred during DnD. It is just making sure we are able to do DnD for all 
native image formats.

In test case ImageTransferer. areImagesIdentical() function we can see that for 
JPEG & GIF(lossy formats) they are ignoring the pixel data.

And for all other image formats except PNG they are not checking alpha channel 
and only checking RGB data.

So I have simplified areImagesIdentical() function also.

I have verified this latest test case in Windows(native image flavours : PNG JFIF 
DIB ENHMETAFILE METAFILEPICT), Ubuntu(native image flavours : image/png image/x-png 
image/tiff image/gif PNG JFIF) & Mac(native image flavours : PNG JFIF TIFF) and 
it passes.

Please find updated webrev for review:

http://cr.openjdk.java.net/~jdv/8176556/webrev.02/

Thanks,

Jay

*From:*Philip Race
*Sent:* Thursday, November 08, 2018 6:05 AM
*To:* Brian Burkhalter
*Cc:* awt-...@openjdk.java.net; 2d-dev
*Subject:* Re: [OpenJDK 2D-Dev]  [12] RFR JDK-8176556: 
java/awt/dnd/ImageTransferTest/ImageTransferTest.java fails for JFIF

Right. It supports alpha of 0 and 255, but nothing in between :-)

-phil.

On 11/7/18, 12:41 PM, Brian Burkhalter wrote:

GIF 89a supports a transparent color index in the palette.

Brian



On Nov 7, 2018, at 11:25 AM, Sergey Bylokhov mailto:sergey.bylok...@oracle.com>> wrote:

On more question about "image/gif" format, as far as I understand this 
format
do not support the alpha as well, then why it works w/o exceptions?




--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] Review Request: 8213110 Remove the use of applets in automatic tests

[Sergey Bylokhov]

Note that for some of the tests I'll need to update the problem list and 
replace the .html by the .java.
But since we actively updates the problem lists right now I postponed this task 
for a weekend.

On 07/11/2018 15:47, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk 12.

Bug: https://bugs.openjdk.java.net/browse/JDK-8213110
Webrev: http://cr.openjdk.java.net/~serb/8213110/webrev.03

Description of the bug:
   A number of our tests in jdk still launch by applets via "@run applet".
Usually we drop this usage when we update the test for some reason, and
in this fix we will drop them completely for automated tests from the open repo.
I did not update the tests which are specific for Applet API, manual tests, or 
tests
which are currently under review for some other bugs.

Note: the "@run applet" means, that the jtreg will do these steps:
  - Creates a frame as a holder for the Applet
  - Creates an applet(which is undecorated panel) and adds it to the frame
  - Sets the size of the frame
  - Place the frame to the center of the screen
  - Make the frame visible
  - Call init() and start() methods of the Applet
  - Waits for 2 seconds
  - Call stop() and destroy() methods of the Applet
  - Dispose the frame


Description of the fix:

  - In all cases the usage of the Applet API was dropped
  - In the common case when the applet was used as launcher, this code now used 
instead:
    public static void main(final String[] args) {
   TestName app = new TestName();
   app.init();
   app.start();
    }
    Example:
    
http://cr.openjdk.java.net/~serb/8213110/webrev.03/test/jdk/java/awt/Choice/PopdownGeneratesMouseEvents/PopdownGeneratesMouseEvents.java.sdiff.html

  - In some cases it was possible to replace init()/start() by the simple 
main() method.
    Example:
    
http://cr.openjdk.java.net/~serb/8213110/webrev.03/test/jdk/java/awt/Choice/PopupPosTest/PopupPosTest.java.sdiff.html

  - Some of the tests were used the "extend Applet" w/o a reasons:
    Example:
    
http://cr.openjdk.java.net/~serb/8213110/webrev.03/test/jdk/java/awt/Focus/ActualFocusedWindowTest/ActualFocusedWindowBlockingTest.java.sdiff.html

  - Some of the tests shows the applet window(which was unrelated to the test 
itself) in the center of the screen.
    Example:
    
http://cr.openjdk.java.net/~serb/8213110/webrev.03/test/jdk/java/awt/Focus/ActualFocusedWindowTest/ActualFocusedWindowRetaining.java.sdiff.html

  - Some of the tests uses the applet as a place holder for the test components.
    In this case it was necessary to change the Applet API to the Frame API, 
and complete
    the steps which were done by the jtreg:
    Example:
    
http://cr.openjdk.java.net/~serb/8213110/webrev.03/test/jdk/java/awt/Focus/AppletInitialFocusTest/AppletInitialFocusTest.java.sdiff.html


Notes:
  - To simplify the review I tried to not change the logic of the tests,
    so if the test fail before the fix, then it will fail after the fix.
    I would like to create the separate CRs for all additional(if any) cleanup 
of these tests.
  - Just a few exception from the above is additional calls to:
    "setLocationRelativeTo" to place the window to the center of the screen
    "robot.waitForIdle()"/"setUndecorated()" to make the tests more stable(when 
I compared the tests before/after the fix)





--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] RFR JDK-8176556: java/awt/dnd/ImageTransferTest/ImageTransferTest.java fails for JFIF

[Sergey Bylokhov]

Looks fine.

On 09/11/2018 03:31, Jayathirth D V wrote:

Hi Sergey,

I have made changes to specifically use RGB channel for Jpeg and ARGB for all 
others.
Please find updated webrev for review:
http://cr.openjdk.java.net/~jdv/8176556/webrev.03/

In DataTransferer. imageToStandardBytesImpl(), 
JpegImageWriterSpi.canEncodeImage() returns false.
And since none of registered service providers can encode ARGB Jpeg. 
DataTransferer. imageToStandardBytesImpl() throws its own IOException as:

if (ioe == null) {
 ioe = new IOException("Registered service providers failed to encode 
"
   + renderedImage + " to " + mimeType);
  }

1) so maybe DND of the argb image using jpeg format should drop the alpha 
channel?
- I think it we should leave it to user on what kind on image should be 
DnDed for jpeg format. Dropping alpha channel explicitly during DnD would not 
be a good option.

2) or it should be somehow be reported as unsupported?
- It is throwing IOException mentioning properly that there are no 
ImageIO spi available for image/jpeg with bands information for encoding an 
image . I think this has enough information, but if we need to change content 
of IOException or throw something like UnsupportedOperationException we can do 
it in separate bug. Please let me know I will raise separate bug for the same.

Thanks,
Jay

-Original Message-
From: Sergey Bylokhov
Sent: Friday, November 09, 2018 12:41 AM
To: Jayathirth D V; Philip Race; Brian Burkhalter
Cc: awt-...@openjdk.java.net; 2d-dev
Subject: Re: [OpenJDK 2D-Dev]  [12] RFR JDK-8176556: 
java/awt/dnd/ImageTransferTest/ImageTransferTest.java fails for JFIF

Hi, Jay.
So you leaved the code which checks the full color including alpha, but the 
source image does not have an alpha so the check for alpha is not strictly 
necessary.
May be we can use RGB only for JPEG and for all others we can use ARGB?

BTW Looking at this test I assume that it was expected that jpeg/gif images 
might be changed during DND operation,
   - so maybe DND of the argb image using jpeg format should drop the alpha 
channel?
   - or it should be somehow be reported as unsupported?
instead of thrown exception somewhere in DND code?

On 08/11/2018 01:13, Jayathirth D V wrote:

Hello All,

I am replying to the last mail in this thread.

  From the test case we can see that it is not strict about pixel data which 
was transferred during DnD. It is just making sure we are able to do DnD for 
all native image formats.

In test case ImageTransferer. areImagesIdentical() function we can see that for 
JPEG & GIF(lossy formats) they are ignoring the pixel data.

And for all other image formats except PNG they are not checking alpha channel 
and only checking RGB data.

So I have simplified areImagesIdentical() function also.

I have verified this latest test case in Windows(native image flavours : PNG JFIF 
DIB ENHMETAFILE METAFILEPICT), Ubuntu(native image flavours : image/png image/x-png 
image/tiff image/gif PNG JFIF) & Mac(native image flavours : PNG JFIF TIFF) and 
it passes.

Please find updated webrev for review:

http://cr.openjdk.java.net/~jdv/8176556/webrev.02/

Thanks,

Jay

*From:*Philip Race
*Sent:* Thursday, November 08, 2018 6:05 AM
*To:* Brian Burkhalter
*Cc:* awt-...@openjdk.java.net; 2d-dev
*Subject:* Re: [OpenJDK 2D-Dev]  [12] RFR JDK-8176556:
java/awt/dnd/ImageTransferTest/ImageTransferTest.java fails for JFIF

Right. It supports alpha of 0 and 255, but nothing in between :-)

-phil.

On 11/7/18, 12:41 PM, Brian Burkhalter wrote:

 GIF 89a supports a transparent color index in the palette.

 Brian



 On Nov 7, 2018, at 11:25 AM, Sergey Bylokhov mailto:sergey.bylok...@oracle.com>> wrote:

 On more question about "image/gif" format, as far as I understand this 
format
 do not support the alpha as well, then why it works w/o exceptions?




--
Best regards, Sergey.




--
Best regards, Sergey.

[OpenJDK 2D-Dev] [12] Review Request: 8213568 Typo in java/awt/GraphicsEnvironment/LoadLock/GE_init5.java

[Sergey Bylokhov]

Hello.
Please review the fix for jdk 12.

Bug: https://bugs.openjdk.java.net/browse/JDK-8213568
Webrev: http://cr.openjdk.java.net/~serb/8213568/webrev.00

I have accidentally found a typo in this test, looks like
it was copied from the GE_init4, but the tag was not updated
properly, so it runs another test, and compilation error in
this test was not found.


--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] RFR JDK-8212116: IIOException "tEXt chunk length is not proper" on opening png file

[Sergey Bylokhov]

Looks fine.

On 12/11/2018 20:22, Jayathirth D V wrote:

Hello All,

Gentle reminder for review.

Thanks,

Jay

*From:* Jayathirth D V
*Sent:* Tuesday, October 16, 2018 6:37 PM
*To:* 2d-dev
*Subject:* [OpenJDK 2D-Dev] [12] RFR JDK-8212116: IIOException "tEXt chunk length is 
not proper" on opening png file

Hello All,

Please review the following fix in JDK12:

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

Webrev: http://cr.openjdk.java.net/~jdv/8212116/webrev.00/

Issue : When we try to read PNG image with no text in tEXt chunk we throw 
IIOException.

Root cause : While fixing JDK-8191023 
 we added tighter condition 
for chunk length in text chunks and we throw IIOException even when text length is 0. 
But text chunks can contain null text with just keyword and other data according to 
PNG spec.

Solution : Allow text chunks with text length 0 in tEXt chunk. As part of this fix I have 
made changes to zTXt and iTXt chunk also as we will have same problem there also. In 
JDK-8191023  we added tighter 
checks for iCCP and sPLT chunk also, but in these chunks we should not allow them to have 
null compressed profile or null palette. So no changes are made in iCCP & sPLT parse 
functions.

Thanks,

Jay




--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] RFR JDK-8211795: ArrayIndexOutOfBoundsException in PNGImageReader after JDK-6788458

[Sergey Bylokhov]

Hi, Jay.

Can you please provide some more detail about this bug.


Root cause : In JDK-6788458 we are adding extra alpha channel for destination 
whenever we have tRNS chunk. But the number of bands in bitDepth scale array 
was not changed when we have tRNS chunk. This is causing 
ArrayIndexOutOfBoundsException for scale array.


As far as I understand the AIOOB is occurred when we access ps[b] at line 1308 not when 
we access the scale array, because the scale array is created as "scale = new 
int[numBands][]".  So maybe numBands should depends on the passRow? or the creation 
of scale[][xxx] should be updated?
BTW this code uses +1/-1 in a lot of places already, and it is not always clear 
why.


--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] RFR JDK-8211422: Reading PNG with corrupt CRC for IEND chunk throws IIOException

[Sergey Bylokhov]

Hi, Jay.

Probably it will be better to not to change the code inside switch, and only 
add the check below
around of reading CRC:
  "if (chunkType != IEND_TYPE) {"

The current fix may throw "Invalid chunk length" when seek/flush the data for 
IEND_TYPE,
which is not correct.


On 12/11/2018 20:22, Jayathirth D V wrote:

Hello All,

Gentle reminder for review.

Thanks,

Jay

*From:*Jayathirth Rao
*Sent:* Tuesday, October 23, 2018 7:09 PM
*To:* 2d-dev@openjdk.java.net
*Subject:* [OpenJDK 2D-Dev] [12] RFR JDK-8211422: Reading PNG with corrupt CRC 
for IEND chunk throws IIOException

Hello All,

Please review the following fix in JDK12:

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

Webrev: http://cr.openjdk.java.net/~jdv/8211422/webrev.00/

Issue : When we try to read PNG image with corrupt/no 4 byte CRC data for IEND chunk 
we throw IIOException. We see this issue only after JDK-8164971 
.

Root cause : In JDK-8164971  fix we 
made changes to continue reading metadata until we reach IEND chunk. Before JDK-8164971 
 change we used to stop reading 
metadata as soon as we hit first IDAT chunk. According to PNG spec there can be more than 
one IDAT chunk/ more headers after IDAT chunk. So we need to read metadata until we hit 
IEND chunk. But in case of this bug we have IEND chunk but it has corrupt/no CRC chunk, so 
we throw IIOException(According to PNG spec every PNG chunk should contain 4 byte CRC). But 
lot of other decoders accept these kind of images which has no CRC chunk for IEND chunk.

Solution : There is no need to verify CRC for IEND chunk(Chunk data length for 
IEND is 0). Stop reading metadata once we hit Chunk type info for IEND chunk.

Thanks,

Jay




--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] RFR JDK-8211422: Reading PNG with corrupt CRC for IEND chunk throws IIOException

[Sergey Bylokhov]

Looks fine.

On 14/11/2018 04:35, Jayathirth D V wrote:

Hi Sergey,

Thanks for the review.

I have updated the code to not move the changes out of switch. Apart from that 
I have refined comments to make it clear why we are not reading CRC for IEND 
chunk. Also chunkCRC value needs to be initialized because of this update, 
initialized value of chunkCRC will not be used anywhere in the logic.

Please find updated webrev for review:
http://cr.openjdk.java.net/~jdv/8211422/webrev.01/

Thanks,
Jay

-Original Message-
From: Sergey Bylokhov
Sent: Wednesday, November 14, 2018 4:31 AM
To: Jayathirth D V; 2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [12] RFR JDK-8211422: Reading PNG with corrupt 
CRC for IEND chunk throws IIOException

Hi, Jay.

Probably it will be better to not to change the code inside switch, and only 
add the check below around of reading CRC:
"if (chunkType != IEND_TYPE) {"

The current fix may throw "Invalid chunk length" when seek/flush the data for 
IEND_TYPE, which is not correct.


On 12/11/2018 20:22, Jayathirth D V wrote:

Hello All,

Gentle reminder for review.

Thanks,

Jay

*From:*Jayathirth Rao
*Sent:* Tuesday, October 23, 2018 7:09 PM
*To:* 2d-dev@openjdk.java.net
*Subject:* [OpenJDK 2D-Dev] [12] RFR JDK-8211422: Reading PNG with
corrupt CRC for IEND chunk throws IIOException

Hello All,

Please review the following fix in JDK12:

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

Webrev: http://cr.openjdk.java.net/~jdv/8211422/webrev.00/

Issue : When we try to read PNG image with corrupt/no 4 byte CRC data for IEND chunk 
we throw IIOException. We see this issue only after JDK-8164971 
<https://bugs.openjdk.java.net/browse/JDK-8164971>.

Root cause : In JDK-8164971 <https://bugs.openjdk.java.net/browse/JDK-8164971> fix we 
made changes to continue reading metadata until we reach IEND chunk. Before JDK-8164971 
<https://bugs.openjdk.java.net/browse/JDK-8164971> change we used to stop reading 
metadata as soon as we hit first IDAT chunk. According to PNG spec there can be more than 
one IDAT chunk/ more headers after IDAT chunk. So we need to read metadata until we hit 
IEND chunk. But in case of this bug we have IEND chunk but it has corrupt/no CRC chunk, so 
we throw IIOException(According to PNG spec every PNG chunk should contain 4 byte CRC). But 
lot of other decoders accept these kind of images which has no CRC chunk for IEND chunk.

Solution : There is no need to verify CRC for IEND chunk(Chunk data length for 
IEND is 0). Stop reading metadata once we hit Chunk type info for IEND chunk.

Thanks,

Jay




--
Best regards, Sergey.




--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR JDK-8211300: Convert C-style array declarations in Java code

[Sergey Bylokhov]

Looks fine to me.
I have checked the patch, have build the change and run some tests on 
our supported platforms.


On 30/09/2018 20:29, Tagir Valeev wrote:

Hello!

Please review JDK-8211300 [1] this change: [2]. Also it needs a
sponsor as I have only JDK author status in OpenJDK Census [3].

The discussion in core-libs-dev [4] shows that it's desired to get rid
of old style array declarations like `int array[]` and massively
convert them to `int[] array`. I volunteered to work on this. As Alan
Bateman noted [5], java.desktop module is pushed to separate repo, so
it would be better to process it separately, thus I'd like to start
with it.

The changeset was created in the following steps:
* Import JDK sources to IntelliJ IDEA
* Mark java.desktop/aix/classes, java.desktop/macosx/classes,
java.desktop/unix/classes, java.desktop/solaris/classes,
java.desktop/windows/classes and java.desktop/share/classes as source
roots
* Disable automatic formatting on the whole project
* Run the inspection "C-style array declaration"
* Apply the quick-fix massively
* Perform regex replace over changed files only:
Search: Copyright \(c\) (\d+), (\d+), Oracle and/or its affiliates.
All rights reserved.
Replace: Copyright \(c\) $1, 2018, Oracle and/or its affiliates. All
rights reserved.
* Perform regex replace over changed files only:
Search: Copyright \(c\) (\d+), Oracle and/or its affiliates. All
rights reserved.
Replace: Copyright \(c\) $1, 2018, Oracle and/or its affiliates. All
rights reserved.
* It appeared that compound declarations like `byte r[], g[], b[];`
are converted to `byte[] r;byte[] g; byte[] b;` which does not look
nice while compilable. I found three such cases via simple regexp
search in BMPImageReader#658, BMPImageWriter#325 and
AreaAveragingScaleFilter#66 and fixed them manually.

In total 339 files were changed with 1335 lines of array declaration
updates and 310 lines of copyright updates. I reviewed the generated
patch by eyes, but only partially, because it's too big. Also I
performed some kind of simple sanity check using regexps:

$ grep '^+[^+]' jdk.patch | grep -v 'Oracle and/or its affiliates. All
rights reserved'|grep -v '\[\]'|wc
   0   0   0
(check that every updated line contains either copyright message or [])

With best regards,
Tagir Valeev.

[1] https://bugs.openjdk.java.net/browse/JDK-8211300
[2] http://cr.openjdk.java.net/~tvaleev/webrev/8211300/r1/
[3] http://openjdk.java.net/census#tvaleev
[4] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-September/055390.html
[5] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-September/055759.html




--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR JDK-8211300: Convert C-style array declarations in Java code

[Sergey Bylokhov]

On 03/10/2018 16:03, Phil Race wrote:
http://cr.openjdk.java.net/~tvaleev/webrev/8211300/r1/src/java.desktop/share/classes/javax/swing/tree/FixedHeightLayoutCache.java.udiff.html 

http://cr.openjdk.java.net/~tvaleev/webrev/8211300/r1/src/java.desktop/share/classes/javax/swing/tree/VariableHeightLayoutCache.java.udiff.html 



Sergey, are you going to commit this, or am I ?


I can push it.



The changes in demos + accessibility are best handled under a different 
bug id since

this one is quite far along ..

-phil.

On 10/03/2018 03:06 AM, Tagir Valeev wrote:

Hello!


Given what Sergey has done, I don't need to repeat that work.
I am now just looking for formatting anomalies, but there is a lot to
look at.
I should be able to OK this tomorrow and we can then commit it on your
behalf.

Thanks, that would be great!

Have  you looked at the other client modules ? datatransfer, 
accessibility,

jdk.unsupported.desktop, or the client demos ?

No, I haven't. I was not aware that there are other client modules. I
assume that client demos are in src/demo/share directory, right?
I found no hits in datatransfer and jdk.unsupported.desktop, 4 files
in accessibility and 71 files in demos. Here's separate webrev for
these 75 files:
http://cr.openjdk.java.net/~tvaleev/patches/c_style_arrays_demos_accessibility/ 


Should I post a separate JBS issue for this (or two issues) or it
could be incorporated within existing issue?
Should I post separate reviews for accessibility and demos or it's
fine to have them together?


If you are crazy (just an idea) then maybe even tests !
But that latter one is a low priority.

Let's postpone tests. I'd like to move to some core modules like
java.base until my volunteering energy runs out :-)


  > It appeared that compound declarations like `byte r[], g[], b[];`
  > are converted to `byte[] r;byte[] g; byte[] b;

Seems like someone should fix their tool :-)

Yeah, this is what I'm thinking about (I'm IDEA developer). See, IDEA
processes every hit separately, and we have three independent hits
here. So when it processes r[] it doesn't know that I want to convert
others as well, thus it has to split the compound declaration into
`byte[] r;byte g[], b[];` (when formatting is active, they are placed
on separate lines, but I switched off the formatting to simplify the
changeset). The same is repeated for `g` and `b` independently and we
have unpleasant result. Probably we can fix this issuing single
warning per declaration and suggesting to fix the whole declaration or
nothing (it's unlikely that somebody would like to fix only some of
these arrays), but then the question is which source element should be
highlighted. Now we highlight three pairs of brackets in three
individual warnings, but if we have a single warning, we should
highlight something continuous. We may highlight the whole
declaration, but what if it also contains non-convertible things like
`byte r[], g, b[]`. Looks like a simple problem, but I don't see the
trivial solution. But ok, it's an off-topic :-)

With best regards,
Tagir Valeev


-phil.


On 10/1/18, 6:24 PM, Sergey Bylokhov wrote:

Looks fine to me.
I have checked the patch, have build the change and run some tests on
our supported platforms.

On 30/09/2018 20:29, Tagir Valeev wrote:

Hello!

Please review JDK-8211300 [1] this change: [2]. Also it needs a
sponsor as I have only JDK author status in OpenJDK Census [3].

The discussion in core-libs-dev [4] shows that it's desired to get rid
of old style array declarations like `int array[]` and massively
convert them to `int[] array`. I volunteered to work on this. As Alan
Bateman noted [5], java.desktop module is pushed to separate repo, so
it would be better to process it separately, thus I'd like to start
with it.

The changeset was created in the following steps:
* Import JDK sources to IntelliJ IDEA
* Mark java.desktop/aix/classes, java.desktop/macosx/classes,
java.desktop/unix/classes, java.desktop/solaris/classes,
java.desktop/windows/classes and java.desktop/share/classes as source
roots
* Disable automatic formatting on the whole project
* Run the inspection "C-style array declaration"
* Apply the quick-fix massively
* Perform regex replace over changed files only:
Search: Copyright \(c\) (\d+), (\d+), Oracle and/or its affiliates.
All rights reserved.
Replace: Copyright \(c\) $1, 2018, Oracle and/or its affiliates. All
rights reserved.
* Perform regex replace over changed files only:
Search: Copyright \(c\) (\d+), Oracle and/or its affiliates. All
rights reserved.
Replace: Copyright \(c\) $1, 2018, Oracle and/or its affiliates. All
rights reserved.
* It appeared that compound declarations like `byte r[], g[], b[];`
are converted to `byte[] r;byte[] g; byte[] b;` which does not look
nice while compilable. I found three such cases via simple regexp
search in BMPImageReader#658, BMPImageWriter#325 and
AreaAveragingScaleFilter#66 and fixed them manually.

In total 339 files were changed

Re: [OpenJDK 2D-Dev] RFR: JDK-8211693 Convert C-style array declarations in client demos and jdk.accessibility

[Sergey Bylokhov]

Looks fine.
I there are no objections I'll push the fix on Monday.

On 03/10/2018 20:46, Tagir Valeev wrote:

Please review and sponsor the conversion of C-style arrays in other
client modules: demos and jdk.accessibility (a left-over after
JDK-8211300):
http://cr.openjdk.java.net/~tvaleev/webrev/8211693/r1/
https://bugs.openjdk.java.net/browse/JDK-8211693

This patch is much smaller, only 75 files are affected (71 in demos
and 4 in accessibility). The procedure was the same as previously. I
manually fixed these two compound declarations:

1. Font2DTest.java:
-String fileText, textLines[];
+String fileText;
+String[] textLines;

2. java2d/Intro.java
-private Shape shapes[], txShapes[];
+private Shape[] shapes, txShapes;

Everything else was nicely converted automatically. This time I
reviewed the whole patch by eyes and haven't noticed any formatting
issues.

Other two client modules java.datatransfer and jdk.unsupported.desktop
were also checked and no C-style array declarations were found there.

With best regards,
Tagir Valeev.




--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR [12] Clipping problems with complex affine transforms: negative scaling factors or small scaling factors

[Sergey Bylokhov]

Looks fine.

On 18/09/2018 00:22, Laurent Bourgès wrote:

Hi,

Please could a second reviewer have a look ?
Phil approved 2 weeks ago.
I would like this bug done asap.

I will propose for review the javafx patch (almost the same fix) in a 
moment.


Cheers,
Laurent

Le mar. 11 sept. 2018 à 08:55, Laurent Bourgès 
mailto:bourges.laur...@gmail.com>> a écrit :


Hi,
Can I have a second review, please ?

I would like to make a jdk11 updates fix request asap...

Laurent

Le jeu. 6 sept. 2018 à 09:31, Laurent Bourgès
mailto:bourges.laur...@gmail.com>> a écrit :

Phil,
Thanks for your review.

Le jeu. 6 sept. 2018 à 01:39, Philip Race
mailto:philip.r...@oracle.com>> a écrit :

This looks good to me.
I've run all our automated tests + done some manual testing
as well as building on all platforms and reviewing the
source changes.


Do you have more closed-source tests that could be opened in
OpenJDK ?


  >  PS: What is the process to ask for backport to JDK11
updates ?

If you think this important enough to backport, then this is
the process :

http://openjdk.java.net/projects/jdk-updates/approval.html


I fixed these bugs as I was contacted on the Marlin mailing list
by an end user testing the migration of its Map viewer app from
jdk8 to OpenJDK11.

I made this patch as small as possible that is compatible with
OpenJDK 11/12 and is well tested: low risk.
For 12, I will propose a more important patch later to upgrade
to Marlin 0.9.3

As JDK11 is LTS and this bug is a regression (P3 ?) since 10, I
think it is worth fixing it in 11 too.

Any other opinion ?

PS: I will fix OpenJFX 11/12 soon

Cheers,
Laurent




--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: 8130264 : change the mechanism by which JDK loads the platform-specific PrinterJob implementation

[Sergey Bylokhov]

Looks fine.

On 15/11/2018 13:41, Phil Race wrote:

bug: http://cr.openjdk.java.net/~prr/8130264/
webrev: http://cr.openjdk.java.net/~prr/8130264/

Currently java launcher code embeds the name of the java.desktop module's 
PrinterJob
implementation class for each platform in a system property which is later
read by the java.desktop code to use to reflectively locate the class and 
instantiate it.

This fix removes that entirely from the launcher code and the desktop module
now looks it up internally via a simple platform proxy class.

This builds on all platforms and we rely on existing printing tests to verify
that we can still locate the implementation class.

The new regression test just verifies the system property name space is no 
longer polluted.
I didn't find any test (apart from this new one) that references it.

Since that system property has been around for a long time I am thinking I 
should file a CSR
to document its removal .. unless there is a concensus it is not necessary.

-phil.



--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] RFR JDK-8211795: ArrayIndexOutOfBoundsException in PNGImageReader after JDK-6788458

[Sergey Bylokhov]

Looks fine.

On 16/11/2018 02:16, Jayathirth D V wrote:

Hi Sergey,

As discussed offline I did more analysis on whether we can use common variable to determine number 
of bands. Since we have "outputSampleSize.length - 1" and "inputBands + 1" kind 
of things.

Actually scale array will be used on input data(ps[]), so we should use input 
bands value to create and use scale array. Before JDK-6788458 there was no 
difference between input bands and output bands so we didn't see any issue. But 
after JDK-6788458 number of bands data is different between input and output 
data for PNG_COLOR_RGB/GRAY having tRNS chunk.

So I have made change to use inputBands data for creation and use of scale 
array. Ran complete imageio jtreg and JCK tests there are no failures.
Please find updated webrev for review:
http://cr.openjdk.java.net/~jdv/8211795/webrev.01/

Thanks,
Jay

-Original Message-
From: Jayathirth D V
Sent: Wednesday, November 14, 2018 1:09 PM
To: Sergey Bylokhov; 2d-dev
Subject: RE: [OpenJDK 2D-Dev] [12] RFR JDK-8211795: 
ArrayIndexOutOfBoundsException in PNGImageReader after JDK-6788458

Hi Sergey,

Thanks for the review.

As you pointed out, yes the AIOOB is happening for ps[b]. I have updated the 
analysis in JBS bug also.

Basically the calculation of numBands is not proper because we take numBands 
value from destination image. This destination image will have extra alpha 
channel for Gray or RGB input data(ps[]) which will throw AIOOB.

So we need to update the logic of how we calculate numBands different for PNG 
Gray/RGB image havng tRNS chunk. Fortunately, webrev.00 is actually doing this 
job.

Regarding whether we need to change scale array logic : We expect first 3 
channel to be RGB and first channel to be Gray for PNG_COLOR_RGB and 
PNG_COLOR_GRAY respectively. So just updating numBands information will create 
proper scale array. So there is no need to change the scale array logic.

History JDK-6788458 : Toolkit was able to show transparent color information 
for RGB/Gray PNG image when it has tRNS chunk, but ImageIO didn't support it. 
To use tRNS data and show transparent color in output image we needed to add 
extra alpha channel for PNG RGB/Gray image with tRNS chunk. But fix present in 
JDK-6788458 didn't handle the case where bitDepth adjustment is needed and we 
are using band information from output image(having extra alpha channel) on 
input image which has no alpha channel. Change in numBands logic for this bug 
fixes that issue.


Thanks,
Jay

-Original Message-
From: Sergey Bylokhov
Sent: Wednesday, November 14, 2018 4:07 AM
To: Jayathirth D V; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [12] RFR JDK-8211795: 
ArrayIndexOutOfBoundsException in PNGImageReader after JDK-6788458

Hi, Jay.

Can you please provide some more detail about this bug.


Root cause : In JDK-6788458 we are adding extra alpha channel for destination 
whenever we have tRNS chunk. But the number of bands in bitDepth scale array 
was not changed when we have tRNS chunk. This is causing 
ArrayIndexOutOfBoundsException for scale array.


As far as I understand the AIOOB is occurred when we access ps[b] at line 1308 not when 
we access the scale array, because the scale array is created as "scale = new 
int[numBands][]".  So maybe numBands should depends on the passRow? or the creation 
of scale[][xxx] should be updated?
BTW this code uses +1/-1 in a lot of places already, and it is not always clear 
why.





--
Best regards, Sergey.

[OpenJDK 2D-Dev] [13] Review Request: 8216592 Drop of sun.awt.AWTSecurityManager class

[Sergey Bylokhov]

Hello.
Please review the fix for jdk 13.

Bug: https://bugs.openjdk.java.net/browse/JDK-8216592
Webrev: http://cr.openjdk.java.net/~serb/8216592/webrev.01

The AWTSecurityManager class is an abstract class which provided
the AppContext objects through SecurityManager extensions. The plugin
provided a concrete implementation of this class. Since plugin was
removed the AWTSecurityManager itself can be removed as well.

Since we currently never set AWTSecurityManager some of our checks like:
"sm instanceof sun.awt.AWTSecurityManager"
are always "false". And I dropped the code which uses such
checks(mostly in the SunFontManager)


--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: 8216965 [JDK12] : crash in freetypeScaler.c CopyBW2Grey8

[Sergey Bylokhov]

Looks fine.

On 18/01/2019 13:44, Phil Race wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8216965
Webrev: http://cr.openjdk.java.net/~prr/8216965/

Starting with JDK 12 b24 we are calling FT_Render_Glyph(..) but there's no
check of an error return. Seems that on some very rare occasions
freetype errors out on pt sz=1 so we need this check.

This fix is intended for JDK 12 so we don't ship with a new regression.

-phil



--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] Review Request: 8214076 Cleanup the code related to GraphicsEnvironment/Device/Configuration

[Sergey Bylokhov]

Yes, I have build/tested it manually on win/lin/mac platforms, and using mach5 
for all supported platforms.

On 11/12/2018 19:23, Philip Race wrote:

Looks OK. Please confirm it builds on all platforms + headful + headless tests 
pass on all platforms ..

-phil.

On 11/30/18, 6:21 PM, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk 12.

Bug: https://bugs.openjdk.java.net/browse/JDK-8214076
Webrev: http://cr.openjdk.java.net/~serb/8214076/webrev.01

While I am working on the:
https://bugs.openjdk.java.net/browse/JDK-8076313
I have started to unify the code between the platforms, and found
that the fix became bigger than the small cleanup.
So I decided to extract it to this separate CR.

Change description:
 - Unused java/native methods were removed
 - Name of the variables are unified:
   1. GConfig will refer GDevice using "device" variable
   2. GDevice will refer GConfig using "config" variable
 - @Override is used in all classes
 - Empty javadoc spec was removed
 - Most classes were marked as "final" to prove that it is not necessary to
   update "some other" platform specific subclasses





--
Best regards, Sergey.

[OpenJDK 2D-Dev] [12] Review Request: 8214076 Cleanup the code related to GraphicsEnvironment/Device/Configuration

[Sergey Bylokhov]

Hello.
Please review the fix for jdk 12.

Bug: https://bugs.openjdk.java.net/browse/JDK-8214076
Webrev: http://cr.openjdk.java.net/~serb/8214076/webrev.01

While I am working on the:
https://bugs.openjdk.java.net/browse/JDK-8076313
I have started to unify the code between the platforms, and found
that the fix became bigger than the small cleanup.
So I decided to extract it to this separate CR.

Change description:
 - Unused java/native methods were removed
 - Name of the variables are unified:
   1. GConfig will refer GDevice using "device" variable
   2. GDevice will refer GConfig using "config" variable
 - @Override is used in all classes
 - Empty javadoc spec was removed
 - Most classes were marked as "final" to prove that it is not necessary to
   update "some other" platform specific subclasses


--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: 8214558: bad @run tag in CheckPrinterJobSystemProperty.java

[Sergey Bylokhov]

Looks fine.

On 30/11/2018 14:50, Phil Race wrote:

Bug : https://bugs.openjdk.java.net/browse/JDK-8214558
As noted in the bug this test has a bad @run line and the fix is to remove it
diff inline below :

diff --git 
a/test/jdk/java/awt/print/PrinterJob/CheckPrinterJobSystemProperty.java 
b/test/jdk/java/awt/print/PrinterJob/CheckPrinterJobSystemProperty.java
--- a/test/jdk/java/awt/print/PrinterJob/CheckPrinterJobSystemProperty.java
+++ b/test/jdk/java/awt/print/PrinterJob/CheckPrinterJobSystemProperty.java
@@ -23,8 +23,7 @@

  /**
   * @test
- * @run CheckPrinterJobSystemProperty
- * @bug 8130264 8214552
+ * @bug 8130264 8214552 8214558
   * @summary verify the PrinterJob implementation class name is not
   *  polluting system properties
   */


-phil.



--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] RFR JDK-8213051:Invalid use of HTML5 in javax.print files

[Sergey Bylokhov]

+1

On 30/11/2018 13:16, Alexey Ivanov wrote:

Hi Prasanta,

Looks good to me.

Regards,
Alexey

On 30/11/2018 17:23, Prasanta Sadhukhan wrote:

Hi Alexey,

Ok. Modified webrev:
http://cr.openjdk.java.net/~psadhukhan/8213051/webrev.2/

Regards
Prasanta
On 30-Nov-18 10:12 PM, Alexey Ivanov wrote:

Hi Prasanta,

Adding style="text-align: center" attribute to  element is cleaner and achieves the 
same result as it applies to all  (as well  which are center-aligned by default 
anyway). If the content of the table is modified in the future, there will be no need to add style 
attributes to new cells which will have non-empty content.

Does it look reasonable?

Regards,
Alexey

On 30/11/2018 15:25, Prasanta Sadhukhan wrote:

Hi Alexey,

I have modified to keep the "X" centred. Please find the modified webrev:
http://cr.openjdk.java.net/~psadhukhan/8213051/webrev.1/

Regards
Prasanta
On 30-Nov-18 4:50 PM, Alexey Ivanov wrote:

Hi Prasanta,

Would the presentation benefit from

to keep X centred and thus to preserve the visual appearance?

Do you mind updating the copyright?

Otherwise, the change looks good to me.

Regards,
Alexey

On 30/11/2018 07:41, Prasanta Sadhukhan wrote:

Hi All,

Please review a doc-fix to comply with HTML5 standard by removing the obsolete  
attribute "align"
Bug: https://bugs.openjdk.java.net/browse/JDK-8213051
webrev: http://cr.openjdk.java.net/~psadhukhan/8213051/webrev.0/

Regards
Prasanta













--
Best regards, Sergey.

[OpenJDK 2D-Dev] [12] Review Request: 8214461 Some unused classes may be removed

[Sergey Bylokhov]

Hello.
Please review the fix for jdk 12.

Bug: https://bugs.openjdk.java.net/browse/JDK-8214461
Webrev: http://cr.openjdk.java.net/~serb/8214461/webrev.00

We have a few internal classes/interfaces which currently unused and may be 
removed:
 - sun.awt.image.BadDepthException.java: looks like it was never used
 - sun.awt.TracedEventQueue.java: was used for logging
 - sun.awt.Graphics2Delegate.java: currently unused


--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732

[Sergey Bylokhov]

Hi, Shashi.


The big problem was that I was not able to reproduce this problem neither at my 
end nor at the systems used for PIT testing. Only Sergey had produced this NPE 
till now.

If you cannot reproduce this exception on the system w/o
printers, then it is interesting what will return this code:

 259 ::EnumPrinters(PRINTER_ENUM_LOCAL | PRINTER_ENUM_CONNECTIONS,
 260NULL, 4, pPrinterEnum, cbNeeded, ,
 261);
 262
 263 if (cReturned > 0) {}

what will happen if you try to print something to the printer
which is returned in the case above?

--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] RFR(XS) JDK-8212875: ftp: links for tiff/TTN2.draft.txt do not respond

[Sergey Bylokhov]

Looks fine.

On 29/11/2018 01:29, Jayathirth D V wrote:

Hi Sergey,

Please find updated webrev for review:
http://cr.openjdk.java.net/~jdv/8212875/webrev.02/

Thanks,
Jay

-Original Message-
From: Sergey Bylokhov
Sent: Tuesday, November 27, 2018 1:29 AM
To: Jayathirth D V; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [12] RFR(XS) JDK-8212875: ftp: links for 
tiff/TTN2.draft.txt do not respond

On 21/11/2018 23:00, Jayathirth D V wrote:

I was following  ZLib like content in the table, but as you mentioned we can 
keep at least text mentioning that we need to look up stuff related to TIFF 
Technical Note #2. Also I have refined comment in BaselineTIFFTagSet.


The text about zlib broke the table as well(looks like it was caused y 
JDK-8189702), please fix it as well.






--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] Speed of drawPolyline on JDK11

[Sergey Bylokhov]

Hi, Laurent.

I do not think that we should postpone it to the next version.
Just send the changes when they are ready, if the review fails
to take place at the right time, then the fix will be
moved to the jdk13.

On 20/11/2018 00:28, Laurent Bourgès wrote:

As OpenJDK12 RDP1 is coming soon, I propose this plan:
- integrate this basic fix in ShapeSpanIterator.c code to use stdlib sort 
(mergesort on linux)
- integrate a very simple patch in Marlin renderer to disable insertion sort 
for large arrays: 0.5s to 0.25s, few LOC
- postpone my changes to Marlin sort & Marlin nonAA renderer integration in 
OpenJDK 13

Will you have time to review 2 small patchs on time ?





Cheers,
Laurent

Le mar. 23 oct. 2018 à 22:37, Laurent Bourgès mailto:bourges.laur...@gmail.com>> a écrit :

Phil,
I quickly modified the final update & sort loop to:
- move sort in another block
- use qsort() using a new comparator sortSegmentsByCurX

This improves performance in PolyLineTest by 3 times: ~1s vs 3.5s !
Apparently qsort() is not optimal (comparator can not be inlined by c) so 
it may explain why Marlin (0x0 sampling) is still 2 times faster with its 
custom merge-sort (in-place).

Any idea to improve C sort ?
Is it good enough ?

- USE_QSORT_X: 1
oct. 23, 2018 10:15:29 PM polylinetest.Canvas paintComponent
INFOS: Paint Time: 1,081s
INFOS: Paint Time: 1,058s
INFOS: Paint Time: 1,067s

- USE_QSORT_X: 0
oct. 23, 2018 10:18:50 PM polylinetest.Canvas paintComponent
INFOS: Paint Time: 3,318s
INFOS: Paint Time: 3,258s
INFOS: Paint Time: 3,273s

Patch:
diff -r 297450fcab26 
src/java.desktop/share/native/libawt/java2d/pipe/ShapeSpanIterator.c
--- a/src/java.desktop/share/native/libawt/java2d/pipe/ShapeSpanIterator.c  
  Tue Oct 16 23:21:05 2018 +0530
+++ b/src/java.desktop/share/native/libawt/java2d/pipe/ShapeSpanIterator.c  
  Tue Oct 23 22:31:00 2018 +0200
@@ -1243,6 +1243,18 @@
  }
  }

+/* LBO: enable (1) / disable (0) qsort on curx */
+#define USE_QSORT_X (0)
+
+static int CDECL
+sortSegmentsByCurX(const void *elem1, const void *elem2)
+{
+    jint x1 = (*(segmentData **)elem1)->curx;
+    jint x2 = (*(segmentData **)elem2)->curx;
+
+    return (x1 - x2);
+}
+
  static jboolean
  ShapeSINextSpan(void *state, jint spanbox[])
  {
@@ -1378,16 +1390,28 @@
  seg->curx = x0;
  seg->cury = y0;
  seg->error = err;
+    }

-    /* Then make sure the segment is sorted by x0 */
-    for (new = cur; new > lo; new--) {
-    segmentData *seg2 = segmentTable[new - 1];
-    if (seg2->curx <= x0) {
-    break;
+    if (USE_QSORT_X && (hi - lo) > 100)
+    {
+    /* use quick sort on [lo - hi] range */
+    qsort(&(segmentTable[lo]), (hi - lo), sizeof(segmentData *),
+    sortSegmentsByCurX);
+    } else {
+    for (cur = lo; cur < hi; cur++) {
+    seg = segmentTable[cur];
+    x0 = seg->curx;
+
+    /* Then make sure the segment is sorted by x0 */
+    for (new = cur; new > lo; new--) {
+    segmentData *seg2 = segmentTable[new - 1];
+    if (seg2->curx <= x0) {
+    break;
+    }
+    segmentTable[new] = seg2;
  }
-    segmentTable[new] = seg2;
+    segmentTable[new] = seg;
  }
-    segmentTable[new] = seg;
  }
  cur = lo;
  }

Cheers,
Laurent

Le mar. 23 oct. 2018 à 08:30, Laurent Bourgès mailto:bourges.laur...@gmail.com>> a écrit :

Phil,
Yesterday I started hacking ShapeSpanIterator.c to add stats: the last 
stage (sort by x0) is the bottleneck.
In this case every sort takes up to 15ms per pixel row !

I will see if I can adapt Marlin's MergeSort.java to C to have an 
efficient sort in-place.
Do you know if libawt has already an efficient sort instead of porting 
mine ?

PS: "To save the planet, make software more efficient" is my quote of 
the day !

Cheers,
Laurent




--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] RFR(XS) JDK-8212875: ftp: links for tiff/TTN2.draft.txt do not respond

[Sergey Bylokhov]

Hi,  Jay.

In a few places you changed the tables which contains 3 columns,
and you actually drop the last column in the row.

BTW why we cannot preserve some of the text like:
"TIFF Technical Note 2"

On 16/11/2018 03:31, Jayathirth D V wrote:

Hello All,

Please review the following fix in JDK12:

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

Webrev : http://cr.openjdk.java.net/~jdv/8212875/webrev.00/

TIFF Technical Note #2 link is broken at many places in our code. I tried 
finding replacement for broken link, unfortunately there are no reliable 
source. So I have removed broken links.

Thanks,

Jay




--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] Review Request: 8213110 Remove the use of applets in automatic tests

[Sergey Bylokhov]

The "ProblemList.txt" is updated in the new version:
http://cr.openjdk.java.net/~serb/8213110/webrev.04

On 08/11/2018 10:57, Sergey Bylokhov wrote:

Note that for some of the tests I'll need to update the problem list and 
replace the .html by the .java.
But since we actively updates the problem lists right now I postponed this task 
for a weekend.

On 07/11/2018 15:47, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk 12.

Bug: https://bugs.openjdk.java.net/browse/JDK-8213110
Webrev: http://cr.openjdk.java.net/~serb/8213110/webrev.03

Description of the bug:
   A number of our tests in jdk still launch by applets via "@run applet".
Usually we drop this usage when we update the test for some reason, and
in this fix we will drop them completely for automated tests from the open repo.
I did not update the tests which are specific for Applet API, manual tests, or 
tests
which are currently under review for some other bugs.

Note: the "@run applet" means, that the jtreg will do these steps:
  - Creates a frame as a holder for the Applet
  - Creates an applet(which is undecorated panel) and adds it to the frame
  - Sets the size of the frame
  - Place the frame to the center of the screen
  - Make the frame visible
  - Call init() and start() methods of the Applet
  - Waits for 2 seconds
  - Call stop() and destroy() methods of the Applet
  - Dispose the frame


Description of the fix:

  - In all cases the usage of the Applet API was dropped
  - In the common case when the applet was used as launcher, this code now used 
instead:
    public static void main(final String[] args) {
   TestName app = new TestName();
   app.init();
   app.start();
    }
    Example:
    
http://cr.openjdk.java.net/~serb/8213110/webrev.03/test/jdk/java/awt/Choice/PopdownGeneratesMouseEvents/PopdownGeneratesMouseEvents.java.sdiff.html

  - In some cases it was possible to replace init()/start() by the simple 
main() method.
    Example:
    
http://cr.openjdk.java.net/~serb/8213110/webrev.03/test/jdk/java/awt/Choice/PopupPosTest/PopupPosTest.java.sdiff.html

  - Some of the tests were used the "extend Applet" w/o a reasons:
    Example:
    
http://cr.openjdk.java.net/~serb/8213110/webrev.03/test/jdk/java/awt/Focus/ActualFocusedWindowTest/ActualFocusedWindowBlockingTest.java.sdiff.html

  - Some of the tests shows the applet window(which was unrelated to the test 
itself) in the center of the screen.
    Example:
    
http://cr.openjdk.java.net/~serb/8213110/webrev.03/test/jdk/java/awt/Focus/ActualFocusedWindowTest/ActualFocusedWindowRetaining.java.sdiff.html

  - Some of the tests uses the applet as a place holder for the test components.
    In this case it was necessary to change the Applet API to the Frame API, 
and complete
    the steps which were done by the jtreg:
    Example:
    
http://cr.openjdk.java.net/~serb/8213110/webrev.03/test/jdk/java/awt/Focus/AppletInitialFocusTest/AppletInitialFocusTest.java.sdiff.html


Notes:
  - To simplify the review I tried to not change the logic of the tests,
    so if the test fail before the fix, then it will fail after the fix.
    I would like to create the separate CRs for all additional(if any) cleanup 
of these tests.
  - Just a few exception from the above is additional calls to:
    "setLocationRelativeTo" to place the window to the center of the screen
    "robot.waitForIdle()"/"setUndecorated()" to make the tests more stable(when 
I compared the tests before/after the fix)








--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] RFR(XS) JDK-8212875: ftp: links for tiff/TTN2.draft.txt do not respond

[Sergey Bylokhov]

On 21/11/2018 23:00, Jayathirth D V wrote:

I was following  ZLib like content in the table, but as you mentioned we can 
keep at least text mentioning that we need to look up stuff related to TIFF 
Technical Note #2. Also I have refined comment in BaselineTIFFTagSet.


The text about zlib broke the table as well(looks like it was caused y 
JDK-8189702), please fix it as well.



--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] Pixel-perfect drawing on JPanel

[Sergey Bylokhov]

Hi, Michał.

You can create a bug report here:
https://bugreport.java.com/bugreport

Please additionally to the test provide the system
information, and information about screen(resolutions and default scale factor).


On 20/12/2018 03:11, Michał Błaszak wrote:

Hi All,

I hope this is a known problem but just in case it’s not.

Trying to make an app drawing something on JPanel I noticed some annoying 
inaccuracies.

The attached piece of code is supposed to draw a series of lines one below the 
other (swapping colors are to demonstrate what's happening).

The result is presented in the following picture:

That means that some pixels are unreachable.

I tested it on several jdk releases. Results are:

OpenJDK Runtime Environment (build 1.8.0_40-b25): Works correctly

OpenJDK Runtime Environment (build 9+181): Wrong behavior

OpenJDK Runtime Environment 18.3 (build 10.0.2+13): Wrong behavior

OpenJDK Runtime Environment 18.9 (build 11.0.1+13): Wrong behavior

The same inaccuracies are observable when using drawRect() or fillRect().

I wanted to report is a bug but I do not know how to do it.

Thank you for looking at it.

Best regards

*Michał Błaszak*

Section Manager, Research & Development




--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [13] RFR 8214579: JFrame does not paint content in XVFB / X11vnc environment

[Sergey Bylokhov]

Hi, Dmitry.
On 03/01/2019 10:29, Dmitry Markov wrote:

Fix:
It is necessary to use X11SurfaceType instead of XRSurfaceType inside 
createData() for XRWindowSurfaceData


Can you please provide some more details why it is necessary? From the
first point of view the XRSurfaceType should be used for XRWindowSurfaceData,
because all this code is implementation of the XRender pipeline.


--
Best regards, Sergey.

[OpenJDK 2D-Dev] [13] Review Request: 8211885 Duplicate id declarations in java.awt.geom.Path2D

[Sergey Bylokhov]

Hello.
Please review the fix for jdk 13.

Bug: https://bugs.openjdk.java.net/browse/JDK-8211885
Webrev: http://cr.openjdk.java.net/~serb/8211885/webrev.00

Duplicated and unused id were removed from the javadoc.

--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: 8218077: Move src/java.desktop/share/legal/mesa3d.md to unix location

[Sergey Bylokhov]

Hi, Phil.

Not sure that we can move it from the shared folder, because we use this 
license in the
java.desktop/share/native/common/java2d/opengl/J2D_GL/gl.h

On 23/03/2019 12:13, Philip Race wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8218077
Webrev : http://cr.openjdk.java.net/~prr/8218077/

This fix does not change the file's contents, it just moves
it to the unix specific location to match the usage.

-phil.





--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [13] RFR: JDK-8221263: [TEST_BUG] RemotePrinterStatusRefresh test is hard to use

[Sergey Bylokhov]

On 03/04/2019 09:35, Alexey Ivanov wrote:
We can make minRefreshTime equal to 2 minutes but leave the default refresh time of 4 minutes. Does it sound good? This way the time of waiting for two refresh cycles would be reduced to 4 minutes only> 

What do you think about this suggestion?
If you agree, I'll submit a new CR to update both the implementation and the 
test.


But why do you need a separate CR? The "sun.java2d.print.minRefreshTime" is a 
java property, we can set it in this test to 1 minute or so, and suggest the user to wait 
2 minutes, no?



If not, then I would rather leave the test and its instructions as they are in 
webrev.1.

Are there any other comments?


On 29/03/2019 08:28, Alexey Ivanov wrote:

Please take a look at the updated webrev where I've removed @ignore tag from 
the test:
http://cr.openjdk.java.net/~aivanov/8221263/webrev.1/

On 29/03/2019 01:09, Philip Race wrote:

Are you looking for a reason other than that the implementation
is set to refresh the printer list every 4 minutes ?


Yes, remote printer list is refreshed every 4 minutes on Windows.

The reason why I added step 6 is that the user has no way of knowing when the 
refresh occurred. The refresh could happen just before the printer is added. 
Waiting for another refresh is to prevent false failures. If the list isn't 
updated in two refresh cycles, likely it won't be updated. In this case, the 
user will click Fail.

Regards,
Alexey



-phil.


On 3/28/19, 5:52 PM, Sergey Bylokhov wrote:

Hi, Alexey.

I think it is ok to drop @ignore tag, but I wonder why did you add the step6 to 
the instruction.
197 +  "updated.\n"
198 + "Step 6: If the list is not updated, wait for another 
"
199 +  "4 minutes, and then click Refresh again.\n"





--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [13] RFR: JDK-8221263: [TEST_BUG] RemotePrinterStatusRefresh test is hard to use

[Sergey Bylokhov]

On 05/04/2019 15:30, Sergey Bylokhov wrote:

On 03/04/2019 09:35, Alexey Ivanov wrote:
We can make minRefreshTime equal to 2 minutes but leave the default refresh time of 4 minutes. Does it sound good? This way the time of waiting for two refresh cycles would be reduced to 4 minutes only> 

What do you think about this suggestion?
If you agree, I'll submit a new CR to update both the implementation and the 
test.


But why do you need a separate CR? The "sun.java2d.print.minRefreshTime" is a 
java property, we can set it in this test to 1 minute or so, and suggest the user to wait 
2 minutes, no?



If not, then I would rather leave the test and its instructions as they are in 
webrev.1.


I just got it, we cannot set the timeout less than DEFAULT_MINREFRESH, yes 
please file a separate CR.



Are there any other comments?


On 29/03/2019 08:28, Alexey Ivanov wrote:

Please take a look at the updated webrev where I've removed @ignore tag from 
the test:
http://cr.openjdk.java.net/~aivanov/8221263/webrev.1/

On 29/03/2019 01:09, Philip Race wrote:

Are you looking for a reason other than that the implementation
is set to refresh the printer list every 4 minutes ?


Yes, remote printer list is refreshed every 4 minutes on Windows.

The reason why I added step 6 is that the user has no way of knowing when the 
refresh occurred. The refresh could happen just before the printer is added. 
Waiting for another refresh is to prevent false failures. If the list isn't 
updated in two refresh cycles, likely it won't be updated. In this case, the 
user will click Fail.

Regards,
Alexey



-phil.


On 3/28/19, 5:52 PM, Sergey Bylokhov wrote:

Hi, Alexey.

I think it is ok to drop @ignore tag, but I wonder why did you add the step6 to 
the instruction.
197 +  "updated.\n"
198 + "Step 6: If the list is not updated, wait for another 
"
199 +  "4 minutes, and then click Refresh again.\n"








--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [13] RFR: JDK-8221263: [TEST_BUG] RemotePrinterStatusRefresh test is hard to use

[Sergey Bylokhov]

But it looks like an overkill to wait 8 minutes for one test, isn't it?
Probably we can force the less timeout by the "sun.java2d.print.minRefreshTime"?

On 29/03/2019 08:28, Alexey Ivanov wrote:

Please take a look at the updated webrev where I've removed @ignore tag from 
the test:
http://cr.openjdk.java.net/~aivanov/8221263/webrev.1/

On 29/03/2019 01:09, Philip Race wrote:

Are you looking for a reason other than that the implementation
is set to refresh the printer list every 4 minutes ?


Yes, remote printer list is refreshed every 4 minutes on Windows.

The reason why I added step 6 is that the user has no way of knowing when the 
refresh occurred. The refresh could happen just before the printer is added. 
Waiting for another refresh is to prevent false failures. If the list isn't 
updated in two refresh cycles, likely it won't be updated. In this case, the 
user will click Fail.

Regards,
Alexey



-phil.


On 3/28/19, 5:52 PM, Sergey Bylokhov wrote:

Hi, Alexey.

I think it is ok to drop @ignore tag, but I wonder why did you add the step6 to 
the instruction.
197 +  "updated.\n"
198 + "Step 6: If the list is not updated, wait for another 
"
199 +  "4 minutes, and then click Refresh again.\n"

On 28/03/2019 09:07, Alexey Ivanov wrote:

Hi,

Please review the following fix for jdk 13:

bug: https://bugs.openjdk.java.net/browse/JDK-8221263
webrev: http://cr.openjdk.java.net/~aivanov/8221263/webrev.0/

This is a complete re-write of the test.

It's semi-automatic: the tester is to add or remove a remote printer and to make sure the change is 
reflected in the list of printers returned from PrintServiceLookup.lookupPrintServices(null, null). 
Added or removed printers are highlighted in the "Before" and "After" lists.

I've attached the screenshots to the JBS issue.

I'm adding 8221263 and 8221412 to @bug tag.


Shall I remove @ignore tag from the test? If it's there, jtreg does not start 
the test, likely it will never be run.

Thank you in advance.

Regards,
Alexey 



--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [13] RFR: JDK-8221263: [TEST_BUG] RemotePrinterStatusRefresh test is hard to use

[Sergey Bylokhov]

Hi, Alexey.

I think it is ok to drop @ignore tag, but I wonder why did you add the step6 to 
the instruction.
197 +  "updated.\n"
198 + "Step 6: If the list is not updated, wait for another 
"
199 +  "4 minutes, and then click Refresh again.\n"

On 28/03/2019 09:07, Alexey Ivanov wrote:

Hi,

Please review the following fix for jdk 13:

bug: https://bugs.openjdk.java.net/browse/JDK-8221263
webrev: http://cr.openjdk.java.net/~aivanov/8221263/webrev.0/

This is a complete re-write of the test.

It's semi-automatic: the tester is to add or remove a remote printer and to make sure the change is 
reflected in the list of printers returned from PrintServiceLookup.lookupPrintServices(null, null). 
Added or removed printers are highlighted in the "Before" and "After" lists.

I've attached the screenshots to the JBS issue.

I'm adding 8221263 and 8221412 to @bug tag.


Shall I remove @ignore tag from the test? If it's there, jtreg does not start 
the test, likely it will never be run.

Thank you in advance.

Regards,
Alexey



--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: 8220528: [AIX] Fix basic Xinerama and Xrender functionality

[Sergey Bylokhov]

v4 looks fine.

On 25/03/2019 11:42, Volker Simonis wrote:

Hi Thomas, Christoph,

thanks for looking into my fix. I hope I've addressed all your
concerns with my new webrev:

http://cr.openjdk.java.net/~simonis/webrevs/2019/8220528.v4

By the way, I've tested the new implementation by simulating a
multi-screen environment on my Linux box with Xephyr:

$ Xephyr +xinerama -screen 1024x768 -screen 1024x768+1024+0 -ac -listen tcp :1

And started a Swing application from AIX where I've redirected the
DISPLAY to myhost:1:

$ DISPLAY=myhost:1 ./images/jdk/bin/java -showversion
-Dsun.awt.nativedebug=true -Dawtdebug.trace=true -Dawtdebug.on=true
-Dawtdebug.ctrace=true -cp ~/Java HelloSwing

The various debug properties will lead to the following output:

openjdk version "13-internal" 2019-09-17
OpenJDK Runtime Environment (slowdebug build 13-internal+0-adhoc.xxx.jdk-jdk)
OpenJDK 64-Bit Server VM (slowdebug build
13-internal+0-adhoc.xxx.jdk-jdk, mixed mode)
Xinerama extension is available
calling XineramaQueryScreens func
Enabling Xinerama support
  num screens = 2
allocating 2 screens

The Swing frame will appear on one of the Xinerama screens and can be
move with the mouse between the two available screens.

If I hear no further objections, I plan to push this by the end of the week.

Thank you and best regards,
Volker



On Mon, Mar 18, 2019 at 8:59 PM Thomas Stüfe  wrote:


Sorry, not awt/2d group, but to me it looks fine too. Only nit: the error 
printout at 495 I would put right after the associated dlsym at 456. You want 
to make sure dlerror() is not stale at the point where you call it, since it 
would return NULL.

Cheers, Thomas



On Mon, Mar 18, 2019 at 3:17 PM Volker Simonis  wrote:


Ping...

Can I please also get a review from the awt/2d group?

Thanks,
Volker

On Wed, Mar 13, 2019 at 11:09 AM Volker Simonis
 wrote:


Hi,

can I please have a review for the following change which fixes
Xinerama and Xrender support on AIX:

http://cr.openjdk.java.net/~simonis/webrevs/2019/8220528/
https://bugs.openjdk.java.net/browse/JDK-8220528

The change basically fixes the way how the corresponding libraries
(libXext and libXrender) get loaded on AIX and does some cleanups with
regards to the various platform macros. I've also added some debug
tracing to the Xrender initialization code similar to the Xinerama
initialization.

Thank you and best regards,
Volker



--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [13] RFR: JDK-8221412: lookupPrintServices() does not always update the list of Windows remote printers

[Sergey Bylokhov]

Looks fine.

On 26/03/2019 13:22, Alexey Ivanov wrote:

Please see the updated webrev:
http://cr.openjdk.java.net/~aivanov/8221412/webrev.1/

The difference between .0 and .1 is in some minor white-space adjustments, like 
parameter alignment.

On 26/03/2019 17:01, Alexey Ivanov wrote:

Hi,

Please review the fix for jdk 13:

bug: https://bugs.openjdk.java.net/browse/JDK-8221412
webrev: http://cr.openjdk.java.net/~aivanov/8221412/webrev.0/

Description:
The list of printers is not updated in the situation where there are no remote 
printers on the system and later the user adds a remote printer.

Root cause:
JDK-8153732 [1] added a new thread which polls remote printers on the system 
and updates the list if it detects a change. In this case it does not update 
the print services because of this condition:
449 if (prevRemotePrinters != null && prevRemotePrinters.length > 0)
prevRemotePrinters is not null but its length is zero because no remote 
printers were initially present on the system.

Fix:
I removed this if. We have to update the list printers if doCompare() returns 
true. Either prevRemotePrinters or currentRemotePrinters, or both can be null; 
doCompare() handles this situation gracefully after JDK-8212202 [2].

The listener called getRemotePrintersNames() twice: in constructor and in run() 
before entering the first sleep. Now it's done only once.

I consolidated the implementation of getAllPrinterNames() and 
getRemotePrintersNames(). They were very similar. The old implementation of 
getRemotePrintersNames() got the list of both local and remote printers and 
then filtered out local ones. The new implementation gets the list of remote 
printers only. So the difference between the two is limited to the flags passed 
to EnumPrinters.

If there are no remote printers, the new version getRemotePrintersNames() 
returns null instead of empty array. As mentioned earlier, doCompare() handles 
this situation.

I'll add the bugid (8221412) to the regression test when fixing JDK-8221263 
[3]. I'm currently working on it.


Thank you in advance.

Regards,
Alexey

[1] https://bugs.openjdk.java.net/browse/JDK-8153732
[2] https://bugs.openjdk.java.net/browse/JDK-8212202
[3] https://bugs.openjdk.java.net/browse/JDK-8221263



--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR 8218854: FontMetrics.getMaxAdvance may be less than the maximum FontMetrics.charWidth

[Sergey Bylokhov]

Hi, Martin.>   * getAllFonts NPE handled
The current version is fine, but note that this null-check is not necessary.
Previously it was possible because the loadFonts() method might skip initialization of the instance 
field "fonts"(which was null by default) as a result NPE could occurred when you try to 
iterate over "fonts"

--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: 8210782: Upgrade HarfBuzz to the latest 2.3.1

[Sergey Bylokhov]

Hi, Phil.
Should we update the "harfbuzz.md" file as well?

On 28/02/2019 16:12, Philip Race wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8210782
Webrev: http://cr.openjdk.java.net/~prr/8210782/

This change upgrades JDK 13 from using harfbuzz v 1.8.1 to v 2.3.1 which is 
currently the latest release of harfbuzz

harfbuzz is the 3rd party (external) C++ library used by OpenJDK for OpenType 
text layout.

In this large upgrade
- Many files were renamed following the pattern of "hb-foo-private.cc" becoming 
"hb-foo.cc"
- Many new files were added
- A couple of files were deleted.

There are two additional changes on top of this
I needed to import a published but un-released fix to enable building with 
Oracle Studio 12.6 on Solaris
See 
https://github.com/harfbuzz/harfbuzz/commit/1e06282105a2d579aab32094cc7abc10ed231978
I needed to reapply a fix made in JDK as JDK-8218965 that mirrors upstream to 
support building with the latest AIX compilers
See 
https://github.com/harfbuzz/harfbuzz/commit/5c2bb1de8de31fecf0dae2ef905b896e42d39f1d
This doesn't show up as a "diff" in the JDK webrev which demonstrates that it 
is correctly re-applied as previously.

There are two JDK files changed :
(1)
- The makefile has been updated to disable several new warnings.
- To prevent harfbuzz from enabling warnings that were disabled - and avoid 
unknown pragma warnings we now define

-DHB_NO_PRAGMA_GCC_DIAGNOSTIC


See hb.hh for what harfbuzz is doing there, but without this -D option we cannot
disable warnings from the build since they are enabled in the code itself.

(2)
- A couple of harfbuzz APIs were deprecated so I needed to make code changes in 
hb-jdk-font.cc to use
the new API.

I have tested that this builds cleanly with all the current devkits (via the 
build servers) and that it also builds
with the in progress work to provide a gcc 8.2 devkit for Linux
I have also run all the related automated tests and performed some manual 
testing too.

-phil.



--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: 8210782: Upgrade HarfBuzz to the latest 2.3.1

[Sergey Bylokhov]

Looks fine.

On 28/02/2019 17:55, Philip Race wrote:

Oh yes we should :-). Good catch.
The license isn't changed so its a small matter of changing what version we 
list in the file.
http://cr.openjdk.java.net/~prr/8210782.1

-phil.

On 2/28/19, 4:58 PM, Sergey Bylokhov wrote:

Hi, Phil.
Should we update the "harfbuzz.md" file as well?

On 28/02/2019 16:12, Philip Race wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8210782
Webrev: http://cr.openjdk.java.net/~prr/8210782/

This change upgrades JDK 13 from using harfbuzz v 1.8.1 to v 2.3.1 which is 
currently the latest release of harfbuzz

harfbuzz is the 3rd party (external) C++ library used by OpenJDK for OpenType 
text layout.

In this large upgrade
- Many files were renamed following the pattern of "hb-foo-private.cc" becoming 
"hb-foo.cc"
- Many new files were added
- A couple of files were deleted.

There are two additional changes on top of this
I needed to import a published but un-released fix to enable building with 
Oracle Studio 12.6 on Solaris
See 
https://github.com/harfbuzz/harfbuzz/commit/1e06282105a2d579aab32094cc7abc10ed231978
I needed to reapply a fix made in JDK as JDK-8218965 that mirrors upstream to 
support building with the latest AIX compilers
See 
https://github.com/harfbuzz/harfbuzz/commit/5c2bb1de8de31fecf0dae2ef905b896e42d39f1d
This doesn't show up as a "diff" in the JDK webrev which demonstrates that it 
is correctly re-applied as previously.

There are two JDK files changed :
(1)
- The makefile has been updated to disable several new warnings.
- To prevent harfbuzz from enabling warnings that were disabled - and avoid 
unknown pragma warnings we now define

-DHB_NO_PRAGMA_GCC_DIAGNOSTIC


See hb.hh for what harfbuzz is doing there, but without this -D option we cannot
disable warnings from the build since they are enabled in the code itself.

(2)
- A couple of harfbuzz APIs were deprecated so I needed to make code changes in 
hb-jdk-font.cc to use
the new API.

I have tested that this builds cleanly with all the current devkits (via the 
build servers) and that it also builds
with the in progress work to provide a gcc 8.2 devkit for Linux
I have also run all the related automated tests and performed some manual 
testing too.

-phil.






--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: JDK-8218682 + JDK-8198411: [TEST_BUG] DashOffset fails in mach5 + Two java2d tests are unstable in mach5

[Sergey Bylokhov]

On 21/02/2019 04:23, Alexey Ivanov wrote:

Thank you, Sergey, for looking into this.
I guess this case requires more investigation. At first sight, it does not look 
right, does it?


Right, I am not sure that "IndexColorModel" is selected intentionally.


--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [13] Review Request: 8216592 Drop of sun.awt.AWTSecurityManager class

[Sergey Bylokhov]

On 06/02/2019 04:47, Laurent Bourgès wrote:

Few questions about this patch.

I wonder if IcedTeaWeb will be broken by such change removing the support of 
multiple AppContexts in FontManage> What are the possible side effects if this 
change is applied on application using multiple AppContexts ?


All depends of how it is used in the IcedTeaWeb. But I suggest to stop its 
usage, and use some default context( in fact the MainAppxontext will be used).

Some additional information:
https://mail.openjdk.java.net/pipermail/awt-dev/2018-October/014561.html

--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR 8218854: FontMetrics.getMaxAdvance may be less than the maximum FontMetrics.charWidth

[Sergey Bylokhov]

Hi, Martin.

What exceptions do you expect below? I think any exception will means a bug in 
jdk, no?

try {
GraphicsEnvironment e =
GraphicsEnvironment.getLocalGraphicsEnvironment();
fonts = e.getAllFonts();
} catch (Exception e) {}

Even if you skip an Exception, you will get NPE later at line "103 for (Font f : 
fonts) {"

BTW probably the test can be moved to the "java/awt/FontMetrics/" folder.

On 21/02/2019 13:27, Martin Balao wrote:

Hi Phil,

On 2/19/19 10:34 PM, Philip Race wrote:

One more thing about the test, I am not sure why you need to use
sun.font.FontDesignMetrics directly ?

Isn't it enough to create a BufferedImage and get an appropriate
FRC and FM by setting the properties on the graphics for that ?



Yes, that's better, there's no need to use an internal API here.

Webrev.02:

  * http://cr.openjdk.java.net/~mbalao/webrevs/8218854/8218854.webrev.02/
  * http://cr.openjdk.java.net/~mbalao/webrevs/8218854/8218854.webrev.02.zip

Are we good to go with Webrev.02?

Thanks,
Martin.-




--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [PATCH] 8218914: Support fonts installed per-user on Windows 10

[Sergey Bylokhov]

Looks fine.
Thank you for contribution!

On 12/03/2019 23:38, Mikhail Filippov wrote:

I looking for the second reviewer and sponsor for my font patch: 
http://cr.openjdk.java.net/~dbatrak/8218914/webrev.03/
--
Mikhail Filippov
Software Developer
JetBrains
http://jetbrains.com
“The Drive To Develop"



On 4 Mar 2019, at 21:45, Mikhail Filippov mailto:mikhail.filip...@jetbrains.com>> wrote:

Webrev with line-length fix: 
http://cr.openjdk.java.net/~dbatrak/8218914/webrev.03/
--
Mikhail Filippov
Software Developer
JetBrains
http://jetbrains.com 
“The Drive To Develop"


On 28 Feb 2019, at 00:35, Phil Race mailto:philip.r...@oracle.com>> wrote:

I have made sure this builds OK and that it passes at least basic tests.
Line 514 is MUCH more than 80 chars. Please break it.

Once you get a 2nd review, your sponsor can push it to jdk/client.

-phil

On 2/19/19 5:57 AM, Mikhail Filippov wrote:

New webrev with fixes: http://cr.openjdk.java.net/~dbatrak/8218914/webrev.02/



On 15 Feb 2019, at 19:31, Phil Race mailto:philip.r...@oracle.com>> wrote:

> 8218914: Handle the case when fonts are installed into user registry key. 
This is the default behaviour since Windows 10 1809.

When you get to the point of preparing a changeset, this line should have the 
bug synopsis.
The text you have here is better placed on the "Summary:" line.
You seem to have lines > 80 chars. Please fix.

I attached new webrev without commit message.


What does Windows do if a user installs a different version of a font already 
installed on the system ?
- Refuse to install it ?
- Use the system one ?
- Use the user one ?

If it refuses to install it, we can ignore that problem. If it prefers one, we 
should make sure
we do the same.

I check this case. If you have installed system-wide and user fonts system-wide 
font preferred. I changed call order in the patch to match this behaviour.



I think the comment

/* Starting from Windows 10 Preview Build 17704 fonts are installed into user's 
home folder by default,

can be misconstrued. It could be read as ALL fonts are installed into a user 
folder and
there is no more system location. I think you actually mean

/* Starting from Windows 10 Preview Build 17704, when a user installs 
non-system fonts, * then by default they are installed in a new per-user 
location as specified in a * per user registry entry. */

Comment fixed.


Have you tested this on a machine with at least several user fonts installed and
verified we still get ALL the same system fonts as well as the new user fonts ?

Have you verified what this does on older OS versions ?

On previous Windows version user fonts key not exists and fonts loading only 
from the system-wide key.



-phil.

On 2/15/19 6:23 AM, Mikhail Filippov wrote:

Hi. Please review the fix.

patch: attached to message.
bug: https://bugs.openjdk.java.net/browse/JDK-8218914
webrev: http://cr.openjdk.java.net/~dbatrak/8218914/webrev.01/

Description:
Starting from Windows 10 Preview Build 17704 fonts are installed into the user's home 
folder by default, and are listed in user's registry section. This is Microsoft blog post 
about it: 
"https://blogs.windows.com/windowsexperience/2018/06/27/announcing-windows-10-insider-preview-build-17704/;
 I this patch I extract function for registry access and call it for two keys: 
HKEY_LOCAL_MACHINE and HKEY_CURRENT_USER. In original code fonts loading only from 
HKEY_LOCAL_MACHINE.



--
Mikhail Filippov
Software Developer
JetBrains
http://jetbrains.com 
“The Drive To Develop"




--
Mikhail Filippov
Software Developer
JetBrains
http://jetbrains.com 
“The Drive To Develop"









--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR(S): 8214343: Handle the absence of Xrandr more generically

[Sergey Bylokhov]

Hi, Volker.

On 27/11/2018 05:52, Volker Simonis wrote:

Change JDK-8213944 fixed the build on AIX (which has no Xrandr) by
conditionally excluding the relevant parts on AIX with the help of
preprocessor defines. On the mailing lists the wish was expressed to
handle the absence of Xrandr more generically during the configure
step, so here comes the corresponding change.


I am working on the fix for the multiscreen support in jdk,
currently our code is based on the xinerama and has some issues. I have an
option to migrate to xrandr.

Can you please provide some info about xinerama support in the AIX or
it is unsupported in the same way as xrandr?

--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] CFV: New 2d Group Member : Jayathirth DV

[Sergey Bylokhov]

Vote: yes

Re: [OpenJDK 2D-Dev] RFR: 8221304: Problem list java/awt/FontMetrics/MaxAdvanceIsMax.java

[Sergey Bylokhov]

Hi, Phil.

The fix looks fine, but I guess the ProblemList should use 8221305 instead of 
8221304.

On 21/03/2019 21:20, Philip Race wrote:

Problem list a test that fails on Mac + Solaris on just those platforms :

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

diff --git a/test/jdk/ProblemList.txt b/test/jdk/ProblemList.txt
--- a/test/jdk/ProblemList.txt
+++ b/test/jdk/ProblemList.txt
@@ -219,6 +219,7 @@
  java/awt/font/TextLayout/TextLayoutBounds.java 8169188 generic-all
  java/awt/font/StyledMetrics/BoldSpace.java 8198422 linux-all
  java/awt/FontMetrics/FontCrash.java 8198336 windows-all
+java/awt/FontMetrics/MaxAdvanceIsMax.java 8221304 solaris-all,macosx-all
  java/awt/image/DrawImage/IncorrectAlphaSurface2SW.java 8056077 generic-all
  java/awt/image/DrawImage/IncorrectClipXorModeSW2Surface.java 8196025 
windows-all
  java/awt/image/DrawImage/IncorrectClipXorModeSurface2Surface.java 8196025 
windows-all

https://bugs.openjdk.java.net/browse/JDK-8221305 has been filed to
follow up on this.

-phil.



--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: 8221304: Problem list java/awt/FontMetrics/MaxAdvanceIsMax.java

[Sergey Bylokhov]

+1

On 21/03/2019 21:30, Philip Race wrote:

Ah yes ..
diff --git a/test/jdk/ProblemList.txt b/test/jdk/ProblemList.txt
--- a/test/jdk/ProblemList.txt
+++ b/test/jdk/ProblemList.txt
@@ -219,6 +219,7 @@
  java/awt/font/TextLayout/TextLayoutBounds.java 8169188 generic-all
  java/awt/font/StyledMetrics/BoldSpace.java 8198422 linux-all
  java/awt/FontMetrics/FontCrash.java 8198336 windows-all
+java/awt/FontMetrics/MaxAdvanceIsMax.java 8221305 solaris-all,macosx-all
  java/awt/image/DrawImage/IncorrectAlphaSurface2SW.java 8056077 generic-all
  java/awt/image/DrawImage/IncorrectClipXorModeSW2Surface.java 8196025 
windows-all
  java/awt/image/DrawImage/IncorrectClipXorModeSurface2Surface.java 8196025 
windows-all

-phil.


On 3/21/19, 9:27 PM, Sergey Bylokhov wrote:

Hi, Phil.

The fix looks fine, but I guess the ProblemList should use 8221305 instead of 
8221304.

On 21/03/2019 21:20, Philip Race wrote:

Problem list a test that fails on Mac + Solaris on just those platforms :

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

diff --git a/test/jdk/ProblemList.txt b/test/jdk/ProblemList.txt
--- a/test/jdk/ProblemList.txt
+++ b/test/jdk/ProblemList.txt
@@ -219,6 +219,7 @@
  java/awt/font/TextLayout/TextLayoutBounds.java 8169188 generic-all
  java/awt/font/StyledMetrics/BoldSpace.java 8198422 linux-all
  java/awt/FontMetrics/FontCrash.java 8198336 windows-all
+java/awt/FontMetrics/MaxAdvanceIsMax.java 8221304 solaris-all,macosx-all
  java/awt/image/DrawImage/IncorrectAlphaSurface2SW.java 8056077 generic-all
  java/awt/image/DrawImage/IncorrectClipXorModeSW2Surface.java 8196025 
windows-all
  java/awt/image/DrawImage/IncorrectClipXorModeSurface2Surface.java 8196025 
windows-all

https://bugs.openjdk.java.net/browse/JDK-8221305 has been filed to
follow up on this.

-phil.






--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [12] Review Request: 8213110 Remove the use of applets in automatic tests

[Sergey Bylokhov]

On 30/01/2019 14:53, Phil Race wrote:

Oh .. and I meant to ask what probably is an obvious question but needs to be 
asked anyway.
You say you did not try to fix tests that already fail - this is just a 
conversion - but did you
confirm there are no NEW failures on any platform ?


Yes, there are no tests which passed before the fix and fails after.



-phil.

On 1/30/19 2:29 PM, Phil Race wrote:


You discuss the life cycle of an applet in jtreg
---
  - Call init() and start() methods of the Applet
  - Waits for 2 seconds
  - Call stop() and destroy() methods of the Applet
  - Dispose the frame
--

I see init() and start() being used but I don't see the rest replaced. Why not ?
Do you really mean jtreg() kills the applet just two seconds after start() 
returns ?


For the tests where you do something like
 main(...) {

obj.init();
obj.start();

eg here :
http://cr.openjdk.java.net/~serb/8213110/webrev.04/test/jdk/java/awt/Focus/AppletInitialFocusTest/AppletInitialFocusTest.java.sdiff.html
would it make sense to invoke that code on the EDT instead of the main thread ?

EventQueue.invokeLater(new Runnable() {
Like already happens here :
http://cr.openjdk.java.net/~serb/8213110/webrev.04/test/jdk/java/awt/FileDialog/FilenameFilterTest/FilenameFilterTest.java.sdiff.html
We have lots of AWT tests that use the main thread so not a blocker but just a 
question and
likely could mean further updates to some tests are needed if an exception no 
longer propagates
on an expected thread.


-phil.

On 11/26/18 6:29 PM, Sergey Bylokhov wrote:

The "ProblemList.txt" is updated in the new version:
http://cr.openjdk.java.net/~serb/8213110/webrev.04

On 08/11/2018 10:57, Sergey Bylokhov wrote:

Note that for some of the tests I'll need to update the problem list and 
replace the .html by the .java.
But since we actively updates the problem lists right now I postponed this task 
for a weekend.

On 07/11/2018 15:47, Sergey Bylokhov wrote:

Hello.
Please review the fix for jdk 12.

Bug: https://bugs.openjdk.java.net/browse/JDK-8213110
Webrev: http://cr.openjdk.java.net/~serb/8213110/webrev.03

Description of the bug:
   A number of our tests in jdk still launch by applets via "@run applet".
Usually we drop this usage when we update the test for some reason, and
in this fix we will drop them completely for automated tests from the open repo.
I did not update the tests which are specific for Applet API, manual tests, or 
tests
which are currently under review for some other bugs.

Note: the "@run applet" means, that the jtreg will do these steps:
  - Creates a frame as a holder for the Applet
  - Creates an applet(which is undecorated panel) and adds it to the frame
  - Sets the size of the frame
  - Place the frame to the center of the screen
  - Make the frame visible
  - Call init() and start() methods of the Applet
  - Waits for 2 seconds
  - Call stop() and destroy() methods of the Applet
  - Dispose the frame


Description of the fix:

  - In all cases the usage of the Applet API was dropped
  - In the common case when the applet was used as launcher, this code now used 
instead:
    public static void main(final String[] args) {
   TestName app = new TestName();
   app.init();
   app.start();
    }
    Example:
http://cr.openjdk.java.net/~serb/8213110/webrev.03/test/jdk/java/awt/Choice/PopdownGeneratesMouseEvents/PopdownGeneratesMouseEvents.java.sdiff.html

  - In some cases it was possible to replace init()/start() by the simple 
main() method.
    Example:
http://cr.openjdk.java.net/~serb/8213110/webrev.03/test/jdk/java/awt/Choice/PopupPosTest/PopupPosTest.java.sdiff.html

  - Some of the tests were used the "extend Applet" w/o a reasons:
    Example:
http://cr.openjdk.java.net/~serb/8213110/webrev.03/test/jdk/java/awt/Focus/ActualFocusedWindowTest/ActualFocusedWindowBlockingTest.java.sdiff.html

  - Some of the tests shows the applet window(which was unrelated to the test 
itself) in the center of the screen.
    Example:
http://cr.openjdk.java.net/~serb/8213110/webrev.03/test/jdk/java/awt/Focus/ActualFocusedWindowTest/ActualFocusedWindowRetaining.java.sdiff.html

  - Some of the tests uses the applet as a place holder for the test components.
    In this case it was necessary to change the Applet API to the Frame API, 
and complete
    the steps which were done by the jtreg:
    Example:
http://cr.openjdk.java.net/~serb/8213110/webrev.03/test/jdk/java/awt/Focus/AppletInitialFocusTest/AppletInitialFocusTest.java.sdiff.html


Notes:
  - To simplify the review I tried to not change the logic of the tests,
    so if the test fail before the fix, then it will fail after the fix.
    I would like to create the separate CRs for all additional(if any) cleanup 
of these tests.
  - Just a few exception from the above is additional calls to:
    "setLocationRelativeTo" to place the window to the center of the screen
    "robot.waitForIdle()&qu

Re: [OpenJDK 2D-Dev] [12] Review Request: 8213110 Remove the use of applets in automatic tests

[Sergey Bylokhov]

Hi, Phil.

Thank you for review, see my comments inline:


You discuss the life cycle of an applet in jtreg
---
   - Call init() and start() methods of the Applet
   - Waits for 2 seconds
   - Call stop() and destroy() methods of the Applet
   - Dispose the frame
--

I see init() and start() being used but I don't see the rest replaced. Why not ?


Because most of our tests override init() and start() and skip stop()/destroy().


Do you really mean jtreg() kills the applet just two seconds after start() 
returns ?


After two seconds it will call stop()/destroy() then dispose the frame and exit.
https://hg.openjdk.java.net/code-tools/jtreg/file/6f00c63c0a98/src/share/classes/com/sun/javatest/regtest/agent/AppletWrapper.java#l157


For the tests where you do something like
  main(...) {

obj.init();
obj.start();

eg here :
http://cr.openjdk.java.net/~serb/8213110/webrev.04/test/jdk/java/awt/Focus/AppletInitialFocusTest/AppletInitialFocusTest.java.sdiff.html
would it make sense to invoke that code on the EDT instead of the main thread ?


In this awt test it is not necessary, but some other tests(for Swing library) 
should be updated at some point.


EventQueue.invokeLater(new Runnable() {

Like already happens here :
http://cr.openjdk.java.net/~serb/8213110/webrev.04/test/jdk/java/awt/FileDialog/FilenameFilterTest/FilenameFilterTest.java.sdiff.html
We have lots of AWT tests that use the main thread so not a blocker but just a 
question and
likely could mean further updates to some tests are needed if an exception no 
longer propagates
on an expected thread.


In this test it is needed because otherwise "fd.setVisible(true)" will block the 
"main()" thread.


--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: JDK-8218682 + JDK-8198411: [TEST_BUG] DashOffset fails in mach5 + Two java2d tests are unstable in mach5

[Sergey Bylokhov]

It looks fine.
BTW do we sure that the usage of IndexColorModel is not a bug?

On 14/02/2019 06:07, Alexey Ivanov wrote:

Hi Sergey,

Do you have any comments for the latest webrev:
http://cr.openjdk.java.net/~aivanov/8218682-8198411/webrev.01/

Do I push the fix?

Regards,
Alexey

On 12/02/2019 18:33, Phil Race wrote:

+1

-phil.

On 2/12/19 6:24 AM, Alexey Ivanov wrote:

Hi Phil,

On 11/02/2019 18:32, Phil Race wrote:


On 2/11/19 1:44 AM, Alexey Ivanov wrote:

Hi Phil,

On 08/02/2019 21:02, Phil Race wrote:

can you add
@key headful

to all these tests ?


Yes, I can if you think it's required.


Yes. I think it is required.


Please see the updated webrev:
http://cr.openjdk.java.net/~aivanov/8218682-8198411/webrev.01/

These three test are not run in mach5.

Regards,
Alexey




I was thinking whether I shall add a diagnostic message for skipping 
VolatileImage when IndexColorModel is in effect…


sure.

-phil.




Regards,
Alexey



-phil.

On 2/8/19 12:13 PM, Alexey Ivanov wrote:

Hi,

Please review the fix for jdk 13:

bugs:
https://bugs.openjdk.java.net/browse/JDK-8218682
https://bugs.openjdk.java.net/browse/JDK-8198411

webrev:
http://cr.openjdk.java.net/~aivanov/8218682-8198411/webrev.00/

Description:
The updated DashOffset test proved to fail in mach5.
It passed BufferedImage test and then failed VolatileImage test.

VolatileImage had different colours instead of the expected white, blue and 
green.

Root cause:
The host uses IndexColorModel; the image uses the closest colour.
Thus the colours do not match.

It's also the reason why DashScaleMinWidth.java and DashZeroWidth.java fail in 
mach5.

Fix:
Skip testing VolatileImage where default graphics configuration uses 
IndexColorModel.

I'm removing DashScaleMinWidth.java and DashZeroWidth.java from ProblemList.txt 
as the tests pass now.


Regards,
Alexey







--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: JDK-8218682 + JDK-8198411: [TEST_BUG] DashOffset fails in mach5 + Two java2d tests are unstable in mach5

[Sergey Bylokhov]

On 11/02/2019 09:28, Alexey Ivanov wrote:

No, it does not look like it's possible directly.
In the case of DashOffset, VolatileImage contains #F8F8F8 instead of #FF, 
#CC instead of #FF, #00CC00 instead of #00FF00. In other 
configurations, the colours could be different. BufferedImage contains the 
expected colours.


But I thought the colors should be the same if the BI was created using 
IndexColorModel(or any other used by VI) instead of default color model?


--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: JDK-8218682 + JDK-8198411: [TEST_BUG] DashOffset fails in mach5 + Two java2d tests are unstable in mach5

[Sergey Bylokhov]

Hi, Alexey.

On 11/02/2019 01:44, Alexey Ivanov wrote:

Isn't isHeadless() used by jtreg itself?
These tests seem to have failed only in Windows where there's no true headless 
environment. Recent versions of Windows Server allow installing the OS without 
support for desktop environment, it could be the reason why we see the limited 
IndexColorModel.

I was thinking whether I shall add a diagnostic message for skipping 
VolatileImage when IndexColorModel is in effect…


Is it possible in case of non-default color model to use BufferedImages as gold 
version to compare?


--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: JDK-8218682 + JDK-8198411: [TEST_BUG] DashOffset fails in mach5 + Two java2d tests are unstable in mach5

[Sergey Bylokhov]

On 11/02/2019 10:41, Alexey Ivanov wrote:


Let me experiment with this.
Would it be acceptable if I submit a new CR to improve these tests?


Ok, fine.


--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: JDK-8218682 + JDK-8198411: [TEST_BUG] DashOffset fails in mach5 + Two java2d tests are unstable in mach5

[Sergey Bylokhov]

On 14/02/2019 13:02, Alexey Ivanov wrote:

BTW do we sure that the usage of IndexColorModel is not a bug?


I don't think it's a bug. Likely Windows Server OS is installed in Server Code 
mode which has limited GUI support.


I was able to get IndexColorModel on my desktop by something like this:

User1:
 - sleep for 10 sec - > run java program -> print color model for the current GC 
-> pause
 - win+L
User2:
 - Login to User2 -> wait 10 sec
User1:
 - IndexColorModel will be printed, instead of DirectColorModel.


--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [13] RFR: JDK-8217263: Automate DashOffset test

[Sergey Bylokhov]

Looks fine.

On 06/02/2019 14:55, Alexey Ivanov wrote:

Hi,

Please review the fix for jdk 13.

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

Description:
The test has been re-worked to an automatic test.
It uses only 2D API. When run in headful environment, it also tests with 
VolatileImage.

I have ensured that the updated test fails before JDK-4469881 was fixed and 
passes after it was fixed (jdk 1.4.0).

This is a request to move the updated test to the open ws.


Regards,
Alexey



--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [13] Review Request: 8216592 Drop of sun.awt.AWTSecurityManager class

[Sergey Bylokhov]

Hi, Phil.

On 30/01/2019 12:52, Phil Race wrote:

private static boolean maybeMultiAppContext() { // Do we have multiple app 
contexts any more ? // Or do we need a new way to check for multiple app 
contexts ? // If not, can we remove this code + its downstream dependencies too 
in a separate fix ? return false; } Are there any tests that are also obsoleted 
?


There are no open tests which are affected by this fix.


--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR. Repeated words typos in java.desktop

[Sergey Bylokhov]

Hi, Andrey.

I found only one questionable place in the fix:
==
 /**
  * Returns an object created with id=key. If the object is not of
- * type type, this will throw an exception.
+ * that type, this will throw an exception.
  */
 private Object lookup(String key, Class type) throws SAXException {
 Object value;
==
The first "type" is a synonym to "class" and the second "type" is the name of 
the method parameter.

Instead of first "type" you can use "an instance of type" or 
"class".



On 31/01/2019 14:28, Andrey Turbanov wrote:

Hello.
I would like to contribute small patch to fix repeated words typos.

Andrey Turbanov


diff --git 
a/src/java.desktop/macosx/classes/com/apple/laf/AquaInternalFrameManager.java
b/src/java.desktop/macosx/classes/com/apple/laf/AquaInternalFrameManager.java
index c2ce10b0e4a..ccd65105699 100644
--- 
a/src/java.desktop/macosx/classes/com/apple/laf/AquaInternalFrameManager.java
+++ 
b/src/java.desktop/macosx/classes/com/apple/laf/AquaInternalFrameManager.java
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 2011, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2019, Oracle and/or its affiliates. All rights reserved.
   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
   *
   * This code is free software; you can redistribute it and/or modify it
@@ -43,7 +43,7 @@ import javax.swing.*;
   * This class implements a DesktopManager which more closely follows
   * the MDI model than the DefaultDesktopManager.  Unlike the
   * DefaultDesktopManager policy, MDI requires that the selected
- * and activated child frames are the same, and that that frame
+ * and activated child frames are the same, and that this frame
   * always be the top-most window.
   * 
   * The maximized state is managed by the DesktopManager with MDI,
diff --git 
a/src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFIFD.java
b/src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFIFD.java
index bc21d5b8118..e1270283d4e 100644
--- a/src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFIFD.java
+++ b/src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFIFD.java
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 2005, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2019, Oracle and/or its affiliates. All rights reserved.
   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
   *
   * This code is free software; you can redistribute it and/or modify it
@@ -786,7 +786,7 @@ public class TIFFIFD extends TIFFDirectory {
  // The IFD entry value is a pointer to the actual field value.
  long offset = stream.readUnsignedInt();

-// Check whether the the field value is within the stream.
+// Check whether the field value is within the stream.
  if (haveStreamLength && offset + size > streamLength) {
  continue;
  }
diff --git 
a/src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFRenderedImage.java
b/src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFRenderedImage.java
index 77e0c9a109c..d5ebebb0857 100644
--- 
a/src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFRenderedImage.java
+++ 
b/src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFRenderedImage.java
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 2005, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2019, Oracle and/or its affiliates. All rights reserved.
   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
   *
   * This code is free software; you can redistribute it and/or modify it
@@ -86,7 +86,7 @@ public class TIFFRenderedImage implements RenderedImage {

  /**
   * Creates a copy of {@code param}. The source subsampling and
- * and bands settings and the destination bands and offset settings
+ * bands settings and the destination bands and offset settings
   * are copied. If {@code param} is a {@code TIFFImageReadParam}
   * then the {@code TIFFDecompressor} and
   * {@code TIFFColorConverter} settings are also copied; otherwise
diff --git 
a/src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKLookAndFeel.java
b/src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKLookAndFeel.java
index 0e18f4d55a9..35e4d901305 100644
--- 
a/src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKLookAndFeel.java
+++ 
b/src/java.desktop/share/classes/com/sun/java/swing/plaf/gtk/GTKLookAndFeel.java
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 2002, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2002, 2019, Oracle and/or its affiliates. All rights reserved.
   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
   *
   * This code is free software; you can redistribute it and/or modify it
@@ -68,7 +68,7 @@ public 

Re: [OpenJDK 2D-Dev] [13] RFR 8214579: JFrame does not paint content in XVFB / X11vnc environment

[Sergey Bylokhov]

Hi, Dmitry.

Can you please check your test on top of this fix?
https://mail.openjdk.java.net/pipermail/awt-dev/2019-January/014940.html

The description of the bug looks similar to this one.

On 23/01/2019 03:23, Dmitry Markov wrote:

Hi Phil,

I have found that the problem occurrence depends on depth setting of VNC 
server. XVFB/X11VNC configuration (where the problem takes place) uses 16 bpp 
and at the same time I cannot reproduce the issue on X11VNC with 32 bpp. In 
other words the issue depends on pixel size used by the configuration: it takes 
place if the pixel size is 16 and does not happen if the pixel size is 24. I am 
not an expert in XRender but the following seems correct: current 
implementation of XRSurfaceData.getSurfaceType() always returns INT (32-bit) 
surface type which might not work properly for the configuration where pixel 
size is 16.

Also the problem is not reproducible on XVNC4 (default depth value is 16) 
because XRender pipeline cannot be enabled there for some reasons. That may 
explain why the issue is  not observed on other XVNC configurations. The root 
cause of XRender pipeline failure for XVNC4 is currently unclear but I think it 
is out of scope for this bug.

Thanks,
Dmitry


On 8 Jan 2019, at 18:44, Phil Race mailto:philip.r...@oracle.com>> wrote:

I don't really understand why this only affects XVFB/X11VNC ?
The bug evaluation is vague in explaining the root cause.
What are they doing that is different ?
Is there an unexpected alpha channel ?
If so,
- are we then selecting a loop which is supplying a zero value alpha
channel instead of an opaque one ?
- why is it only for X11VNC ?
- Why was this not seen on Solaris ? Most if not all testing there uses Xvnc.

-phil.

On 1/8/19 2:24 AM, Dmitry Markov wrote:

Hi Sergey,

We started using XRSurfaceType (surface type from XRSurfaceData) after 
integration of JDK-8204931 [1]. Before that fix getSurfaceType() was not 
overridden in XRGraphicsConfig and surface type from 
X11GraphicsConfig/X11SurfaceData, (i.e. X11SurfaceType) was used for 
XRWindowSurfaceData and XRPixmapSurfaceData. If I got it right JDK-8204931was 
intended for fixing problems with XRPixmapSurfaceData and it solved them by 
introducing surface type with PixelConverter in XRSurfaceData and overriding 
getSurfaceType() in XRGraphicsConfig. These changes are correct for 
XRPixmapSurfaceData but they accidentally broke XRWindowSurfaceData and caused 
this problem.

In proposed fix I restored the previous behaviour for XRWindowSurfaceData, 
(i.e. use surface type from X11SurfaceData instead there one from 
XRSurfaceData).

Thanks,
Dmitry

[1] - https://bugs.openjdk.java.net/browse/JDK-8204931



On 7 Jan 2019, at 23:14, Sergey Bylokhov mailto:sergey.bylok...@oracle.com>> wrote:

Hi, Dmitry.
On 03/01/2019 10:29, Dmitry Markov wrote:

Fix:
It is necessary to use X11SurfaceType instead of XRSurfaceType inside 
createData() for XRWindowSurfaceData


Can you please provide some more details why it is necessary? From the
first point of view the XRSurfaceType should be used for XRWindowSurfaceData,
because all this code is implementation of the XRender pipeline.


--
Best regards, Sergey.









--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: [JDK12] 8218020 Fix version number in mesa.md 3rd party legal file

[Sergey Bylokhov]

Looks fine.

On 30/01/2019 09:39, Prasanta Sadhukhan wrote:

+1

Regards
Prasanta
On 30-Jan-19 10:59 PM, Phil Race wrote:

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

This is a trivial fix to a license file for JDK 12 needed because v5.0 is what
we are tracking as using, but the license file isn't updated from v4.1
The licensed header file (just one file !) is identical between v4.1 + v5.0 - ie
the source file in Mesa 5.0 claims to still be v4.1 - which is probably why we 
have a mismatch

So this is purely a license file change

Diff :

diff --git a/src/java.desktop/share/legal/mesa3d.md 
b/src/java.desktop/share/legal/mesa3d.md
--- a/src/java.desktop/share/legal/mesa3d.md
+++ b/src/java.desktop/share/legal/mesa3d.md
@@ -1,10 +1,10 @@
-## Mesa 3-D Graphics Library v4.1
+## Mesa 3-D Graphics Library v5.0

 ### Mesa License
 

 Mesa 3-D graphics library
-Version:  4.1
+Version:  5.0

 Copyright (C) 1999-2002  Brian Paul   All Rights Reserved.

-phil.





--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [13] RFR: JDK-8221263: [TEST_BUG] RemotePrinterStatusRefresh test is hard to use

[Sergey Bylokhov]

On 08/04/2019 03:14, Alexey Ivanov wrote:

Exactly! I would've done it if it had been possible.
Changing the minRefreshTime requires changes to both the implementation and the 
test, so it's better to do it under separate CR, I've submitted one:
https://bugs.openjdk.java.net/browse/JDK-8222108


yes, it looks fine, thank you for clarification.



--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [13] RFR: JDK-8217707: JNICALL declaration breaks Splash screen functions

[Sergey Bylokhov]

Looks fine.

On 24/03/2019 12:18, Alexey Ivanov wrote:

Hi,

Please review the fix for jdk 13.

bug: https://bugs.openjdk.java.net/browse/JDK-8217707
webrev: http://cr.openjdk.java.net/~aivanov/8217707/webrev.0/

Description:
Splash screen functionality is broken in 32 bit Windows. It's because the 
functions in splashscreen.dll are exported with decorated names now, yet 
they're looked up by the undecorated names.

The easiest way to reproduce the problem is to run SwingSet2:
java -jar SwingSet2.jar


Fix:
The fix removes JNICALL (__stdcall) declarations from splash screen functions. 
Thus the functions are exported undecorated.

I've run SplashScreen jtreg tests, all tests pass.

This is a follow-up fix to
https://bugs.openjdk.java.net/browse/JDK-8201226
https://bugs.openjdk.java.net/browse/JDK-8202476
which re-enabled 32 bit Windows post removal of mapfiles / compiler options.


Thank you in advance.

Regards,
Alexey



--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [PATCH] 8220231: Cache HarfBuzz face object for same font's text layout calls

[Sergey Bylokhov]

+1

On 01/04/2019 02:12, Alexey Ushakov wrote:

Looks good for me.

Best Regards,
Alexey

On Mon, Apr 1, 2019 at 10:46 AM Dmitry Batrak mailto:dmitry.bat...@jetbrains.com>> wrote:

 > Sorry for the delay. I've now finished verifying this and it is a +1 
from me.
Thanks!

Anyone else, please? A second reviewer is required.

On Mon, Mar 25, 2019 at 11:19 PM Philip Race mailto:philip.r...@oracle.com>> wrote:

Sorry for the delay. I've now finished verifying this and it is a +1 
from me.

-phil.

On 3/12/19, 1:32 AM, Dmitry Batrak wrote:

> This looks good to me, if I understand correctly that we now create
> the face on first use and cache it fin Java or as long as the Font2D 
is
> alive.
> And the JNIEnv is always found on
That's correct. The assumption is that HarfBuzz doesn't create its own 
threads,
so HarfBuzz-related native code will always be invoked from a Java 
thread
(as part of 'shape' call), and so JNIEnv will be available in that 
context.

I've updated the webrev by including a stress test for multi-threaded 
behaviour
testing. Apart from the test, webrev also has some cosmetic differences
from the previous version (like a comment fix or parameter order 
changing),
appeared during 'splitting' process. To simplify the review, I'm also 
providing
the links to the 'split' version of the same webrev - three parts that 
produce
the same result when combined. I've not tested the changes separately
(except basic compilation check).

Complete change:
http://cr.openjdk.java.net/~dbatrak/8220231/webrev.01/ 

Part 1 (caching hb_face_t):
http://cr.openjdk.java.net/~dbatrak/8220231/webrev.01-1/ 

Part 2 (tables caching removal):
http://cr.openjdk.java.net/~dbatrak/8220231/webrev.01-2/ 

Part 3 (scaler pointer passing removal):
http://cr.openjdk.java.net/~dbatrak/8220231/webrev.01-3/ 


Best regards,
Dmitry Batrak

On Fri, Mar 8, 2019 at 3:21 AM Philip Race mailto:philip.r...@oracle.com>> wrote:

This looks good to me, if I understand correctly that we now create
the face on first use and cache it fin Java or as long as the 
Font2D is
alive.
And the JNIEnv is always found on

I think you are right that we don't need the caching of the tables 
since
now the face will do it. The unfortunate thing is that the removal 
of
this code is
well over half the changes and as such it greatly muddied finding 
the
changes
that make the performance difference so my review was harder and 
less
certain
because of that.
It could have been separated into a follow-on change I think so 
that it
would have
been easier to review the important change.

The pScaler parameter looks like it is unused these days which is 
why I
expect you removed it but also not directly relevant.

I have run builds + some tests - but I'm not in a position to run 
more tests
for a couple of weeks.

A (mild) stress test re-using the same font from multiple threads 
eachmaking
multiple calls into layout would be a good addition here. That 
should
help tell
us if there are any MT or re-entrancy problems. Can you provide 
such a
test ?
It will be a good thing to have automatically run to catch any 
problems
introduced later either on the Java side or by an update to 
harfbuzz.

-phil.



On 3/6/19, 5:45 PM, Dmitry Batrak wrote:
> Hello,
>
> I'd like to submit a patch for JDK-8220231. I'm not a Committer, 
so
> I'll need someone to sponsor this change.
>
> The proposed approach is used without known issues in 
OpenJDK-based
> JetBrains Runtime for almost three years now. I've mentioned it
> previously on this mailing list
> 
(https://mail.openjdk.java.net/pipermail/2d-dev/2017-August/008497.html).
> The change has been refactored as compared to the version 
mentioned
> above (the logic has been moved to SunLayoutEngine), and includes 
the
> removal of font tables caching (JDK-8186317). The latter, I 
believe,
> becomes redundant with this fix.
>
> Issue: https://bugs.openjdk.java.net/browse/JDK-8220231
> Webrev: http://cr.openjdk.java.net/~dbatrak/8220231/webrev.00/ 

Re: [OpenJDK 2D-Dev] [13] RFR JDK-8225368:broken links in java.desktop files

[Sergey Bylokhov]

On 06/06/2019 12:50, Jonathan Gibbons wrote:

You should be able to use {@link} to refer to other Java elements;


It is possible even across the different modules?(I remember there was some 
related issue)



-- Jon


On 06/06/2019 12:46 PM, Sergey Bylokhov wrote:

Hi, Prasanta.

Can you please double check is it possible to use {@link } instead of  in 
the java files?
For example in 
src/java.desktop/share/classes/javax/print/attribute/package-info.java

Note that some of these docs use 80 chars per line alignment, please use the 
same style.

On 06/06/2019 11:34, Prasanta Sadhukhan wrote:

Hi All,

Please review a doc-fix to fix broken links in java.desktop files

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

webrev: http://cr.openjdk.java.net/~psadhukhan/8225368/webrev.0/

Regards
Prasanta








--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [13] RFR JDK-8225368:broken links in java.desktop files

[Sergey Bylokhov]

Looks fine.

On 07/06/2019 01:08, Prasanta Sadhukhan wrote:

Modified webrev http://cr.openjdk.java.net/~psadhukhan/8225368/webrev.1/

to use @link for cases where it points to public class/API.  For 
Font/FontMetrics/DocFlavor, the link was pointing to some heading and not to any API, 
where @link was not working so kept  for those cases.

Regards
Prasanta
On 07-Jun-19 6:16 AM, Jonathan Gibbons wrote:

You should be able to link to public API in other modules.

There was an issue with hard-coded '` links when javadoc added the 
extra level of module directory into the output hierarchy, but those issues have now 
been sorted out.

-- Jon

On 06/06/2019 05:31 PM, Sergey Bylokhov wrote:

On 06/06/2019 12:50, Jonathan Gibbons wrote:

You should be able to use {@link} to refer to other Java elements;


It is possible even across the different modules?(I remember there was some 
related issue)



-- Jon


On 06/06/2019 12:46 PM, Sergey Bylokhov wrote:

Hi, Prasanta.

Can you please double check is it possible to use {@link } instead of  in 
the java files?
For example in 
src/java.desktop/share/classes/javax/print/attribute/package-info.java

Note that some of these docs use 80 chars per line alignment, please use the 
same style.

On 06/06/2019 11:34, Prasanta Sadhukhan wrote:

Hi All,

Please review a doc-fix to fix broken links in java.desktop files

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

webrev: http://cr.openjdk.java.net/~psadhukhan/8225368/webrev.0/

Regards
Prasanta















--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: 8225007: java/awt/print/PrinterJob/LandscapeStackOverflow.java may hang

[Sergey Bylokhov]

On 29/05/2019 18:06, Philip Race wrote:

This test does not check. Nor does (IIRC) the implementation,


[1] I guess it is checked in the RasterPrintJob.print() where the code call the 
getPrintService(). And getPrintService() returns null if there are no printers installed, as a 
result "new PrinterException("No print service found.") is thrown.

I checked this test on win10 w/o printers installed and it pass but throw the 
next exception to the log[1]. But if I tries to print from the notepad it 
really suggest to install the printer.


because you want an experience like what you would get if you
tried to print from notepad, word, IE, whatever.
Which was to prompt you to install a printer if there is none.
That was the idea "back in the day" and is what happened on XP for sure.>>  - 
When the test is executed the printer promts the location to where store the pdf file(probably 
this location can be set to some default and we will able to test printing as well at someday)

Even on a system with physical printers configured, if a pdf printer is the 
default
you could get prompted, but there is no API that I know of that can tell you 
this.

I guess we need to documents this, since such "additional" windows from 
printers/onenotes etc may affect other tests.



-phil


BTW this windows system was s slow so I was not able to move the mouse to 
select the file location, and have to reboot it



-Phil.


On May 29, 2019, at 3:40 PM, Phil Race  wrote:

If you try to print when there is no printer the behavior is not strictly 
specified.

-Phil.


On May 29, 2019, at 3:25 PM, Sergey Bylokhov  wrote:

On 29/05/2019 15:16, Philip Race wrote:
It doesn't hang there. It hangs in print().


I guess it is even more strange, it hangs when tries to print to non-existent printer. I guess in 
this case the "new PrinterException("No print service found.")" should be 
thrown. It looks like a bug, no?


-phil.

On 5/29/19, 3:17 PM, Sergey Bylokhov wrote:
On 29/05/2019 14:09, Phil Race wrote:
think the mystery is not why it times out now, but why it did not do so earlier.


But I assume it if the system does not have the printer then 
PrinterJob.getPrinterJob() should not hang?
It looks like the test correctly assume that .getPrinterJob() should returns 
something even if there are no printers, and also tries to catch any exceptions 
in the print(); I guess its expectation are according the specification, isn't?




--
Best regards, Sergey.










--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [13] RFR JDK-8225368:broken links in java.desktop files

[Sergey Bylokhov]

Hi, Prasanta.

Can you please double check is it possible to use {@link } instead of  in 
the java files?
For example in 
src/java.desktop/share/classes/javax/print/attribute/package-info.java

Note that some of these docs use 80 chars per line alignment, please use the 
same style.

On 06/06/2019 11:34, Prasanta Sadhukhan wrote:

Hi All,

Please review a doc-fix to fix broken links in java.desktop files

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

webrev: http://cr.openjdk.java.net/~psadhukhan/8225368/webrev.0/

Regards
Prasanta



--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [13] RFR JDK-8224824:ProblemList java/awt/Color/AlphaColorTest failure in linux_x64

[Sergey Bylokhov]

Looks fine.

On 27/05/2019 02:33, Prasanta Sadhukhan wrote:

Hi All,

AlphaColorTest is failing in mach5 linux headful nightly system consistently 
and it seems to be a product bug, so needs to be problemlisted.

diff -r cb2628a4f33f test/jdk/ProblemList.txt
--- a/test/jdk/ProblemList.txt  Wed May 15 15:17:24 2019 +0530
+++ b/test/jdk/ProblemList.txt  Mon May 27 15:02:28 2019 +0530
@@ -114,6 +114,7 @@

  # jdk_awt

+java/awt/Color/AlphaColorTest.java 8224825 linux-all
  java/awt/event/MouseEvent/MouseClickTest/MouseClickTest.java 8168389 
windows-all,macosx-all
  java/awt/Focus/ActualFocusedWindowTest/ActualFocusedWindowBlockingTest.java 
8168408 windows-all,macosx-all
  java/awt/Focus/FocusOwnerFrameOnClick/FocusOwnerFrameOnClick.java 8081489 
generic-all

Regards
Prasanta



--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: 8225007: java/awt/print/PrinterJob/LandscapeStackOverflow.java may hang

[Sergey Bylokhov]

On 29/05/2019 14:09, Phil Race wrote:

think the mystery is not why it times out now, but why it did not do so earlier.


But I assume it if the system does not have the printer then 
PrinterJob.getPrinterJob() should not hang?
It looks like the test correctly assume that .getPrinterJob() should returns 
something even if there are no printers, and also tries to catch any exceptions 
in the print(); I guess its expectation are according the specification, isn't?

--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: 8225007: java/awt/print/PrinterJob/LandscapeStackOverflow.java may hang

[Sergey Bylokhov]

On 29/05/2019 15:16, Philip Race wrote:

It doesn't hang there. It hangs in print().


I guess it is even more strange, it hangs when tries to print to non-existent printer. I guess in 
this case the "new PrinterException("No print service found.")" should be 
thrown. It looks like a bug, no?



-phil.

On 5/29/19, 3:17 PM, Sergey Bylokhov wrote:

On 29/05/2019 14:09, Phil Race wrote:

think the mystery is not why it times out now, but why it did not do so earlier.


But I assume it if the system does not have the printer then 
PrinterJob.getPrinterJob() should not hang?
It looks like the test correctly assume that .getPrinterJob() should returns 
something even if there are no printers, and also tries to catch any exceptions 
in the print(); I guess its expectation are according the specification, isn't?




--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: 8225007: java/awt/print/PrinterJob/LandscapeStackOverflow.java may hang

[Sergey Bylokhov]

On 29/05/2019 15:42, Phil Race wrote:

For example windows some times pops up a dialog prompting you to install a 
printer.


But in this case we should not even try to print something, since we checked at 
the beginning that there are no printers.

I decided to check by eyes what happens on the system:
 - The system has a few printers, and default one is "print to pdf".
 - When the test is executed the printer promts the location to where store the 
pdf file(probably this location can be set to some default and we will able to 
test printing as well at someday)
 
BTW this windows system was s slow so I was not able to move the mouse to select the file location, and have to reboot it




-Phil.


On May 29, 2019, at 3:40 PM, Phil Race  wrote:

If you try to print when there is no printer the behavior is not strictly 
specified.

-Phil.


On May 29, 2019, at 3:25 PM, Sergey Bylokhov  wrote:

On 29/05/2019 15:16, Philip Race wrote:
It doesn't hang there. It hangs in print().


I guess it is even more strange, it hangs when tries to print to non-existent printer. I guess in 
this case the "new PrinterException("No print service found.")" should be 
thrown. It looks like a bug, no?


-phil.

On 5/29/19, 3:17 PM, Sergey Bylokhov wrote:
On 29/05/2019 14:09, Phil Race wrote:
think the mystery is not why it times out now, but why it did not do so earlier.


But I assume it if the system does not have the printer then 
PrinterJob.getPrinterJob() should not hang?
It looks like the test correctly assume that .getPrinterJob() should returns 
something even if there are no printers, and also tries to catch any exceptions 
in the print(); I guess its expectation are according the specification, isn't?




--
Best regards, Sergey.







--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: 8225007: java/awt/print/PrinterJob/LandscapeStackOverflow.java may hang

[Sergey Bylokhov]

Looks fine.

On 29/05/2019 12:30, Phil Race wrote:

Bug : https://bugs.openjdk.java.net/browse/JDK-8225007
Webrev : http://cr.openjdk.java.net/~prr/8225007/

This is just adding the printer keyword to two jtreg tests.
This is useful for both selecting & excluding the tests and should prevent
one of these tests from hanging in our nightly headful testing.

-phil.



--
Best regards, Sergey.

[OpenJDK 2D-Dev] [13] Review Request: 8225372 accessibility errors in tables in java.desktop files

[Sergey Bylokhov]

Hello.
Please review the fix for JDK 13.

Bug: https://bugs.openjdk.java.net/browse/JDK-8225372
Fix: http://cr.openjdk.java.net/~serb/8225372/webrev.00
Doc: 
http://cr.openjdk.java.net/~serb/8225372/docs/api/java.desktop/module-summary.html

In the fix, some(most) of the issues which were reported by the accessibility 
checker were fixed.

Changes description:

 - In the simplest case, the "scope=col/row" attribute was added to the tables:
 
http://cr.openjdk.java.net/~serb/8225372/webrev.00/src/java.desktop/share/classes/javax/swing/plaf/synth/doc-files/componentProperties.html.sdiff.html

 - The tables which are used for a layout were replaced by the .(In fact, this 
is just emulation of ):
 
http://cr.openjdk.java.net/~serb/8225372/webrev.00/src/java.desktop/share/classes/javax/swing/JScrollPane.java.sdiff.html

 - Some tables do not have the meaningful cell to be row header, so I have added an 
additional column "index" and use it cells as row header:
 
http://cr.openjdk.java.net/~serb/8225372/webrev.00/src/java.desktop/share/classes/javax/imageio/metadata/doc-files/gif_metadata.html.sdiff.html

 - In one place I have added a special role to the table "role=presentation" 
because the table was used just for layout and its content can be read without 
information about this table:
 
http://cr.openjdk.java.net/~serb/8225372/docs/api/java.desktop/java/awt/doc-files/Modality.html#Examples

 - In some cases I have dropped the table, and replace it by the list of 
elements:
 
http://cr.openjdk.java.net/~serb/8225372/webrev.00/src/java.desktop/share/classes/javax/print/attribute/standard/Finishings.java.sdiff.html
 New: 
http://cr.openjdk.java.net/~serb/8225372/docs/api/java.desktop/javax/print/attribute/standard/Finishings.html
 Old: 
https://docs.oracle.com/en/java/javase/11/docs/api/java.desktop/javax/print/attribute/standard/Finishings.html

Note that I cannot fix two reported issues:
   Take a look to the "Common dialog" and "HTML Content of example above" on 
the links below:
 
http://cr.openjdk.java.net/~serb/8225372/docs/api/java.desktop/javax/swing/JOptionPane.html
 
http://cr.openjdk.java.net/~serb/8225372/docs/api/java.desktop/javax/swing/text/html/HTMLDocument.html

I tried to mark these tables as "role=presentation" or "aria-hidden=true", but 
it does not work because Javadoc tool generates HTML4 and these attributes are supported by the 
HTML5.

--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] [13] Review Request: 8224171 The cleanup multi-font related code in the XFontPeer

[Sergey Bylokhov]

Hi, Semyon.

On 07/06/2019 13:24, Semyon Sadetsky wrote:

I don't know how timely it is to cleanup this code. Anyway, you've removed 
global awtJNI_GetFontData() function but left its external declaration in 
open/src/java.desktop/unix/native/common/awt/awt_p.h:

http://hg.openjdk.java.net/jdk/client/file/f680bedc0dcb/src/java.desktop/unix/native/common/awt/awt_p.h#l122


I'll remove it as well:
http://cr.openjdk.java.net/~serb/8224171/webrev.01



Since native code was modified can you provide the prove that the change 
doesn't brake the build?


Mach5 is green for both: version.00 and version.01



--Semyon

On 5/18/19 3:10 PM, Sergey Bylokhov wrote:

Hello.
Please review the fix for JDK 13.

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

This change is the part of my effort to clean up the native initialization code 
in awt/2d. Initially, I have dropped the native code inside initIds() which was 
unused(JDK-8223766[1]). Now I tried to check is the code inside initIDs() is 
used for some meaningful purpose.

I found that XFontPeer.initIds() uses XFontPeer.xfsname field which is always 
null, so in this fix, I have dropped the field itself, the initIds(), the 
methods which depend on this field, and methods which calls methods which 
depends on this field, etc. Next step will be clean up the native code for the 
Fonts itself, there is some unused code as well.

Note that this field was unused since 2009[2]

[1] https://bugs.openjdk.java.net/browse/JDK-8223766
[2] http://hg.openjdk.java.net/jdk/client/rev/a79da6a9d184




--
Best regards, Sergey.

Re: [OpenJDK 2D-Dev] RFR: 8217731: Font rendering and glyph spacing changed from jdk-8 to jdk-11

[Sergey Bylokhov]

Looks fine.

On 11/06/2019 05:39, Philip Race wrote:

Hi,

I am still looking for a code review on this - needed today !

-phil.

On 6/5/19, 11:43 AM, Phil Race wrote:

bug: https://bugs.openjdk.java.net/browse/JDK-8217731
webrev: http://cr.openjdk.java.net/~prr/8217731/

This is intended to "help" but cannot completely cure, with
some of the rendering differences in JDK11 vs JDK 8.
In a typical Swing app on Windows using LCD rendering
it manifests as subtle adjustments in the spacing between glyphs.
There isn't an easy regression test for this, and it is subjective
as to how bad it was before and how much this improves it,
even if you were to accept that 8 is "better" .. and not just different ..

-phil.



--
Best regards, Sergey.