Re: [OpenJDK 2D-Dev] Heads up : JDK 17 b19 through b22 will use Metal instead of OpenGL for Java 2D rendering on macOS.

2021-05-06 Thread Philip Race
> I’ve only tried a couple of apps and only this test program has shown 
the problem.


Did you try to attach something ? the lists strip attachments.
Sent it directly to me and I'll see if I can add it to a bug report.

-phil.

On 5/6/21 2:50 PM, Alan Snyder wrote:



On May 6, 2021, at 1:45 PM, Philip Race  wrote:

Alan,

I am not sure this is a known issue. We'll need a lot more details.

I figured you would. :-)



What is your h+w and OS update ?

iMac 27 inch 2020 Radeon Pro 5500 XT 8 GB
11.3.1



Is this all windows in an app or just the first one ?

Definitely not just the first one, but not all of them, either.



Does it matter what the window content is ?

It might. The app is a test program that can create any of 30 different kinds 
of windows on demand.

So far, I’ve seen the problem in 11 kinds of windows but not in the others. No 
obvious pattern in the content.

The problem is most likely to happen the first time a given window is shown, 
but it can also happen on later instances of the same kind of window.
I just tried a new kind of window and it happened the first 3 times, but not 
the 4th, then about 50% of the time.
I tried creating a different window about 25 times, and it happened on #1, #4, 
and #25.


Any app or some specific app ?

I’ve only tried a couple of apps and only this test program has shown the 
problem.




-phil.


On 4/29/21 7:18 PM, Alan Snyder wrote:

I am seeing some unusual behavior (in b20) that I do not see using OpenGL (or 
using JDK 16).

Sometimes when I open a new window, the window appears blank (except for the 
title bar) for about two seconds before the content appears.

This behavior is not consistent. Opening another instance of the same window 
might be fast or slow. It happens with a variety of window classes.

In JDK16 and using OpenGL, the content always appears immediately.






On Apr 23, 2021, at 1:13 PM, Philip Race  wrote:

FYI to the wider community that may not subscribe to the client mailing lists, 
nor appreciate too much cross-posting.

-phil.


 Forwarded Message 
Subject:Heads up : JDK 17 b19 through b22 will use Metal instead of 
OpenGL for Java 2D rendering on macOS.
Date:   Fri, 23 Apr 2021 13:10:46 -0700
From:   Philip Race 
To: 2d-dev@openjdk.java.net <2d-dev@openjdk.java.net>
CC: lanai-...@openjdk.java.net, swing-...@openjdk.java.net 
, awt-...@openjdk.java.net 





Heads up to anyone who is testing JDK 17 for running apps on macOS.
Starting with build 19 [1], JDK 17 for macOS is *temporarily* switched from 
using OpenGL
to using Apple's Metal API for Java 2D rendering. This should be invisible to 
applications.
We expect to revert this temporary switch in JDK 17 build 23,meaning b22 will 
be the last build with Metal as default.

See JEP 382 [2] for more information about how Metal is used by JDK.

If you are running any kind of 2D / Swing/ AWT UI application on macOS, and see 
any rendering related problems
starting with JDK 17 b19, please do report them to us at either the usual bug 
submission channel [3],
or on the 2d-dev@openjdk.java.net OpenJDK mailing list [4]
Please be ready to provide us with a test case and screen shots.

You may also set "-Dsun.java2d.opengl=true" to re-enable OpenGL - which  
implicitly disables Metal -
to confirm that any problem you see is a Metal related rendering glitch.

I will also forward this email to jdk-...@openjdk.java.net

-Phil.

[1] https://jdk.java.net/17/
[2] https://openjdk.java.net/jeps/382 
[3] https://bugreport.java.com/bugreport/
[4] https://mail.openjdk.java.net/mailman/listinfo/2d-dev





Re: [OpenJDK 2D-Dev] Heads up : JDK 17 b19 through b22 will use Metal instead of OpenGL for Java 2D rendering on macOS.

2021-05-06 Thread Alan Snyder



> On May 6, 2021, at 1:45 PM, Philip Race  wrote:
> 
> Alan,
> 
> I am not sure this is a known issue. We'll need a lot more details.

I figured you would. :-)


> What is your h+w and OS update ?

iMac 27 inch 2020 Radeon Pro 5500 XT 8 GB
11.3.1


> Is this all windows in an app or just the first one ?

Definitely not just the first one, but not all of them, either.


> Does it matter what the window content is ?

It might. The app is a test program that can create any of 30 different kinds 
of windows on demand.

So far, I’ve seen the problem in 11 kinds of windows but not in the others. No 
obvious pattern in the content.

The problem is most likely to happen the first time a given window is shown, 
but it can also happen on later instances of the same kind of window.
I just tried a new kind of window and it happened the first 3 times, but not 
the 4th, then about 50% of the time.
I tried creating a different window about 25 times, and it happened on #1, #4, 
and #25.

> Any app or some specific app ?

I’ve only tried a couple of apps and only this test program has shown the 
problem.



> -phil.
> 
> 
> On 4/29/21 7:18 PM, Alan Snyder wrote:
>> I am seeing some unusual behavior (in b20) that I do not see using OpenGL 
>> (or using JDK 16).
>> 
>> Sometimes when I open a new window, the window appears blank (except for the 
>> title bar) for about two seconds before the content appears.
>> 
>> This behavior is not consistent. Opening another instance of the same window 
>> might be fast or slow. It happens with a variety of window classes.
>> 
>> In JDK16 and using OpenGL, the content always appears immediately.
>> 
>> 
>> 
>> 
>> 
>>> On Apr 23, 2021, at 1:13 PM, Philip Race  wrote:
>>> 
>>> FYI to the wider community that may not subscribe to the client mailing 
>>> lists, nor appreciate too much cross-posting.
>>> 
>>> -phil.
>>> 
>>> 
>>>  Forwarded Message 
>>> Subject:Heads up : JDK 17 b19 through b22 will use Metal instead of 
>>> OpenGL for Java 2D rendering on macOS.
>>> Date:   Fri, 23 Apr 2021 13:10:46 -0700
>>> From:   Philip Race 
>>> To: 2d-dev@openjdk.java.net <2d-dev@openjdk.java.net>
>>> CC: lanai-...@openjdk.java.net, swing-...@openjdk.java.net 
>>> , awt-...@openjdk.java.net 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> Heads up to anyone who is testing JDK 17 for running apps on macOS.
>>> Starting with build 19 [1], JDK 17 for macOS is *temporarily* switched from 
>>> using OpenGL
>>> to using Apple's Metal API for Java 2D rendering. This should be invisible 
>>> to applications.
>>> We expect to revert this temporary switch in JDK 17 build 23,meaning b22 
>>> will be the last build with Metal as default.
>>> 
>>> See JEP 382 [2] for more information about how Metal is used by JDK.
>>> 
>>> If you are running any kind of 2D / Swing/ AWT UI application on macOS, and 
>>> see any rendering related problems
>>> starting with JDK 17 b19, please do report them to us at either the usual 
>>> bug submission channel [3],
>>> or on the 2d-dev@openjdk.java.net OpenJDK mailing list [4]
>>> Please be ready to provide us with a test case and screen shots.
>>> 
>>> You may also set "-Dsun.java2d.opengl=true" to re-enable OpenGL - which  
>>> implicitly disables Metal -
>>> to confirm that any problem you see is a Metal related rendering glitch.
>>> 
>>> I will also forward this email to jdk-...@openjdk.java.net
>>> 
>>> -Phil.
>>> 
>>> [1] https://jdk.java.net/17/
>>> [2] https://openjdk.java.net/jeps/382 
>>> [3] https://bugreport.java.com/bugreport/
>>> [4] https://mail.openjdk.java.net/mailman/listinfo/2d-dev
>>> 
> 



Re: [OpenJDK 2D-Dev] RFR: 8240756: [macos] SwingSet2:TableDemo:Printed Japanese characters were garbled

2021-05-06 Thread Phil Race
On Thu, 22 Apr 2021 09:21:20 GMT, Toshio Nakamura  wrote:

> Hi,
> 
> Could you review the fix?
> When non-English characters were printed from JTable on MacOS, 
> CTextPipe.doDrawGlyphs was called by OSXSurfaceData.drawGlyphs. However, 
> CTextPipe seems not support glyph with slot number of composite fonts.
> 
> The slot data mask of GlyphVector is 0xff00. In my environment, Japanese 
> font was loaded at slot 4, and glyph data is like [0x40003e5]. Then, 
> unexpected glyph was drawn. 
> 
> This patch checks slot data of each character. If slot data exists, it will 
> branch to GlyphVector's drawing path.
> 
> Well, I couldn't create automatic test for this fix. This method seems to be 
> called for printing only. I appreciate any advice.
> Tested java/awt and javax/swing on MacOS BigSur, and there was no regression.
> 
> Regards,
> Toshio Nakamura

The immediately next method in this file, drawGlyphVector(..) surely would have 
the same problem ?
And then the drawChars method too ?

Does this not in fact affect all code points for which the slot != 0 ?
Do we really want to print all code points from slot > 0 as shapes ? 

And if this code isn't expecting to handle composite fonts, how did we get here 
with one ?
Or maybe the problem is that the printing path code needs to handle this 
downstream not upstream ?

-

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


Re: [OpenJDK 2D-Dev] Heads up : JDK 17 b19 through b22 will use Metal instead of OpenGL for Java 2D rendering on macOS.

2021-05-06 Thread Philip Race

Alan,

I am not sure this is a known issue. We'll need a lot more details.

What is your h+w and OS update ?
Is this all windows in an app or just the first one ?
Does it matter what the window content is ?
Any app or some specific app ?

-phil.


On 4/29/21 7:18 PM, Alan Snyder wrote:

I am seeing some unusual behavior (in b20) that I do not see using OpenGL (or 
using JDK 16).

Sometimes when I open a new window, the window appears blank (except for the 
title bar) for about two seconds before the content appears.

This behavior is not consistent. Opening another instance of the same window 
might be fast or slow. It happens with a variety of window classes.

In JDK16 and using OpenGL, the content always appears immediately.






On Apr 23, 2021, at 1:13 PM, Philip Race  wrote:

FYI to the wider community that may not subscribe to the client mailing lists, 
nor appreciate too much cross-posting.

-phil.


 Forwarded Message 
Subject:Heads up : JDK 17 b19 through b22 will use Metal instead of 
OpenGL for Java 2D rendering on macOS.
Date:   Fri, 23 Apr 2021 13:10:46 -0700
From:   Philip Race 
To: 2d-dev@openjdk.java.net <2d-dev@openjdk.java.net>
CC: lanai-...@openjdk.java.net, swing-...@openjdk.java.net 
, awt-...@openjdk.java.net 





Heads up to anyone who is testing JDK 17 for running apps on macOS.
Starting with build 19 [1], JDK 17 for macOS is *temporarily* switched from 
using OpenGL
to using Apple's Metal API for Java 2D rendering. This should be invisible to 
applications.
We expect to revert this temporary switch in JDK 17 build 23,meaning b22 will 
be the last build with Metal as default.

See JEP 382 [2] for more information about how Metal is used by JDK.

If you are running any kind of 2D / Swing/ AWT UI application on macOS, and see 
any rendering related problems
starting with JDK 17 b19, please do report them to us at either the usual bug 
submission channel [3],
or on the 2d-dev@openjdk.java.net OpenJDK mailing list [4]
Please be ready to provide us with a test case and screen shots.

You may also set "-Dsun.java2d.opengl=true" to re-enable OpenGL - which  
implicitly disables Metal -
to confirm that any problem you see is a Metal related rendering glitch.

I will also forward this email to jdk-...@openjdk.java.net

-Phil.

[1] https://jdk.java.net/17/
[2] https://openjdk.java.net/jeps/382 
[3] https://bugreport.java.com/bugreport/
[4] https://mail.openjdk.java.net/mailman/listinfo/2d-dev





Re: [OpenJDK 2D-Dev] Heads up : JDK 17 b19 through b22 will use Metal instead of OpenGL for Java 2D rendering on macOS.

2021-05-06 Thread Alan Snyder
I am seeing some unusual behavior (in b20) that I do not see using OpenGL (or 
using JDK 16).

Sometimes when I open a new window, the window appears blank (except for the 
title bar) for about two seconds before the content appears.

This behavior is not consistent. Opening another instance of the same window 
might be fast or slow. It happens with a variety of window classes.

In JDK16 and using OpenGL, the content always appears immediately.





> On Apr 23, 2021, at 1:13 PM, Philip Race  wrote:
> 
> FYI to the wider community that may not subscribe to the client mailing 
> lists, nor appreciate too much cross-posting.
> 
> -phil.
> 
> 
>  Forwarded Message 
> Subject:  Heads up : JDK 17 b19 through b22 will use Metal instead of 
> OpenGL for Java 2D rendering on macOS.
> Date: Fri, 23 Apr 2021 13:10:46 -0700
> From: Philip Race 
> To:   2d-dev@openjdk.java.net <2d-dev@openjdk.java.net>
> CC:   lanai-...@openjdk.java.net, swing-...@openjdk.java.net 
> , awt-...@openjdk.java.net 
> 
> 
> 
> 
> 
> Heads up to anyone who is testing JDK 17 for running apps on macOS.
> Starting with build 19 [1], JDK 17 for macOS is *temporarily* switched from 
> using OpenGL
> to using Apple's Metal API for Java 2D rendering. This should be invisible to 
> applications.
> We expect to revert this temporary switch in JDK 17 build 23,meaning b22 will 
> be the last build with Metal as default.
> 
> See JEP 382 [2] for more information about how Metal is used by JDK.
> 
> If you are running any kind of 2D / Swing/ AWT UI application on macOS, and 
> see any rendering related problems
> starting with JDK 17 b19, please do report them to us at either the usual bug 
> submission channel [3],
> or on the 2d-dev@openjdk.java.net OpenJDK mailing list [4]
> Please be ready to provide us with a test case and screen shots.
> 
> You may also set "-Dsun.java2d.opengl=true" to re-enable OpenGL - which  
> implicitly disables Metal -
> to confirm that any problem you see is a Metal related rendering glitch.
> 
> I will also forward this email to jdk-...@openjdk.java.net
> 
> -Phil.
> 
> [1] https://jdk.java.net/17/
> [2] https://openjdk.java.net/jeps/382 
> [3] https://bugreport.java.com/bugreport/
> [4] https://mail.openjdk.java.net/mailman/listinfo/2d-dev
> 



Re: [OpenJDK 2D-Dev] RFR: 8263583: Emoji rendering on macOS [v2]

2021-05-06 Thread Phil Race
On Fri, 19 Mar 2021 13:48:12 GMT, Dmitry Batrak  wrote:

>> This is the implementation used by JetBrains Runtime for the last 4 years, 
>> after some cleanup, and with one problem,
>> found while preparing the pull request, fixed.
>> Even though typical scenarios for a UI application should be covered, it's 
>> not a complete solution. In particular, emoji-s
>> still won't be rendered for large font sizes (more than 100pt), and for 
>> non-trivial composite/painting modes.
>> Notable implementation details are listed below.
>> 
>> **Glyph image generation**
>> 
>> Deprecated CGContextShowGlyphsAtPoint function, used by JDK on macOS to 
>> render text, cannot render emojis,
>> CTFontDrawGlyphs is used instead. It ignores the scale component of text 
>> transformation matrix, so a 'real-sized'
>> CTFont object should be passed to it. The same sizing procedure is done when 
>> calculating glyph metrics, because they
>> are not scaled proportionally with font size (as they do for vector fonts).
>> 
>> **Glyph image storage**
>> 
>> Existing GlyphInfo structure is used to store color glyph image. Color glyph 
>> can be distinguished by having 4 bytes
>> of storage per pixel. Color components are stored in pre-multiplied alpha 
>> format.
>> 
>> **Glyph rendering**
>> 
>> Previously, GlyphList instance always contained glyphs in the same format 
>> (solid, grayscale or LCD), determined by the
>> effective rendering hint. Now the renderers must be prepared to GlyphList 
>> having 'normal' glyphs interspersed with
>> color glyphs (they can appear due to font fallback). This isn't a problem 
>> for OpenGL renderer (used for on-screen painting),
>> but GlyphListLoopPipe-based renderers (used for off-screen painting) needed 
>> an adjustment to be able to operate on
>> specific segments of GlyphList.
>> As an incidental optimization, calculation of GlyphList bounds ('getBounds' 
>> method) is performed now only when needed
>> (most text renderers don't need this information).
>> Speaking of the actual rendering of the glyph image, it's done by the 
>> straightforward glDrawPixels call in OpenGL renderer,
>> and by re-using existing Blit primitive in off-screen renderers.
>> 
>> **Testing**
>> 
>> There's no good way to test the new functionality automatically, but I've 
>> added a test verifying that 'something' is
>> rendered for the emoji character, when painting to BufferedImage.
>> 
>> Existing tests pass after the change.
>
> Dmitry Batrak has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - 8263583: Emoji rendering on macOS
>
>extended test case to cover different types of target surfaces
>  - 8263583: Emoji rendering on macOS
>
>implementation for Metal rendering pipeline

Marked as reviewed by prr (Reviewer).

src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLTextRenderer.m line 
327:

> 325: }
> 326: 
> 327: MTLPaint* storedPaint = nil;

should this be static ?

-

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


Re: [OpenJDK 2D-Dev] RFR: JDK-8263467: Incorrect double-checked locking in sun.java2d.CRenderer [v2]

2021-05-06 Thread Phil Race
On Fri, 12 Mar 2021 07:22:26 GMT, Aleksey Shipilev  wrote:

>> SonarCloud reports multiple incorrect double-checked locking cases in 
>> `sun.java2d.CRenderer`. For example:
>> 
>> 
>>   Arc2D arcToShape;
>> 
>>   ...
>> if (arcToShape == null) {
>> synchronized (this) {
>> if (arcToShape == null) {
>> arcToShape = new Arc2D.Float();
>> }
>> }
>> }
>> 
>> 
>> `Arc2D` contains fields that are not `final`. `arcToShape` is not 
>> `volatile`. This makes it an incorrect DCL.
>> This code is used by Mac OS, do it would probably blow up some time later on 
>> M1.
>> 
>> This is the candidate fix that preserves the semantics. I am doing this fix 
>> blindly so far, without testing on real Mac OS. But maybe there is no need 
>> to do any of this, because the setter methods overwrite the contents of all 
>> these objects under their own sync. So, maybe we can just allocate those 
>> objects without DCL-backed caching and pass them in?
>
> Aleksey Shipilev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Drop DCL and useless synchronization completely

LGTM

-

Marked as reviewed by prr (Reviewer).

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


Re: [OpenJDK 2D-Dev] RFR: 8265062: Remove duplication constant MaxTextureSize

2021-05-06 Thread Phil Race
On Thu, 15 Apr 2021 14:42:55 GMT, Denis Konoplev  wrote:

> I've removed MaxTextureSize and replaced its usages with 
> MTL_GPU_FAMILY_MAC_TXT_SIZE

Marked as reviewed by prr (Reviewer).

-

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


Re: [OpenJDK 2D-Dev] RFR: 8226384: Implement a better logic to switch between OpenGL and Metal pipeline [v2]

2021-05-06 Thread Phil Race
On Wed, 5 May 2021 07:37:23 GMT, Jayathirth D V  wrote:

>> We have many if else conditions to select OpenGL/Metal pipeline objects.
>> Apart from initialization phase we should not fetch these objects everytime 
>> checking whether we are using OpenGL/Metal pipeline.
>
> Jayathirth D V has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Move peer and getters to CFLayer

Marked as reviewed by prr (Reviewer).

-

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


Re: [OpenJDK 2D-Dev] RFR: 8266545: 8261169 broke Harfbuzz build with gcc 7 and 8 [v2]

2021-05-06 Thread Thomas Stuefe
On Thu, 6 May 2021 16:38:52 GMT, Phil Race  wrote:

>> Thomas Stuefe has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   switch off warning in build instead of fixing it
>
> The policy is to do what you ended up doing. We (almost) never modify 3rd 
> party sources in such cases.
> About the only case is if a fix has already been pushed upstream then we 
> might copy that fix.
> You did the right thing notifying upstream but still best to just disable 
> this warning for now.

Thanks @prrace. I agree this is the simplest way. Good that Aleksey had this 
idea.

-

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


Re: [OpenJDK 2D-Dev] RFR: 8266545: 8261169 broke Harfbuzz build with gcc 7 and 8 [v2]

2021-05-06 Thread Phil Race
On Thu, 6 May 2021 06:58:16 GMT, Thomas Stuefe  wrote:

>> Harfbuzz upgrade broke Linux x64 build on older gccs. For details see JBS 
>> issue.
>> 
>> I fixed the issue in the harfbuzz sources, but I am not sure of the policy 
>> here. Do we modify the harfbuzz sources or leave them untouched? Advice is 
>> welcome.
>> 
>> The patch fixes linux x64 fastdebug and opt build for me.
>
> Thomas Stuefe has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   switch off warning in build instead of fixing it

The policy is to do what you ended up doing. We (almost) never modify 3rd party 
sources in such cases.
About the only case is if a fix has already been pushed upstream then we might 
copy that fix.
You did the right thing notifying upstream but still best to just disable this 
warning for now.

-

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


[OpenJDK 2D-Dev] Integrated: 8252758: Lanai: Optimize index calculation while copying glyphs

2021-05-06 Thread Jayathirth D V
On Wed, 5 May 2021 13:50:47 GMT, Jayathirth D V  wrote:

> Loop optimization while copying glyph content.
> J2DDemo, SwingSet2 and Font2DTest are green.

This pull request has now been integrated.

Changeset: 2438498a
Author:Jayathirth D V 
URL:   
https://git.openjdk.java.net/jdk/commit/2438498a3f6dfa53966a0f5b28af28617ca00e6b
Stats: 15 lines in 1 file changed: 0 ins; 0 del; 15 mod

8252758: Lanai: Optimize index calculation while copying glyphs

Reviewed-by: aghaisas, pbansal

-

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


[OpenJDK 2D-Dev] Integrated: JDK-8263362: Avoid division by 0 in java/awt/font/TextJustifier.java justify

2021-05-06 Thread Matthias Baesken
On Wed, 10 Mar 2021 12:31:31 GMT, Matthias Baesken  wrote:

> In java/awt/font/TextJustifier.java justify-method there is a potential code 
> path where divison by zero might happen , see also the Sonar finding 
> https://sonarcloud.io/project/issues?id=shipilev_jdk=AXcqMwpm8sPJZZzONu1k=false=CRITICAL=BUG
> 
> 
> boolean hitLimit = (weight == 0) || (!lastPass && ((delta < 0) == 
> (delta < gslimit)));
> boolean absorbing = hitLimit && absorbweight > 0;
> // predivide delta by weight
> float weightedDelta = delta / weight; // not used if weight == 0
> 
> In case of (weight == 0) the division should not be done because the value of 
> weightedDelta is unused in this case anyway.

This pull request has now been integrated.

Changeset: ea30bd66
Author:Matthias Baesken 
URL:   
https://git.openjdk.java.net/jdk/commit/ea30bd6684fa3003889062a129a5aee1bc9b0024
Stats: 6 lines in 1 file changed: 3 ins; 0 del; 3 mod

8263362: Avoid division by 0 in java/awt/font/TextJustifier.java justify

Reviewed-by: psadhukhan

-

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


Re: [OpenJDK 2D-Dev] RFR: 8252758: Lanai: Optimize index calculation while copying glyphs

2021-05-06 Thread Pankaj Bansal
On Wed, 5 May 2021 13:50:47 GMT, Jayathirth D V  wrote:

> Loop optimization while copying glyph content.
> J2DDemo, SwingSet2 and Font2DTest are green.

Marked as reviewed by pbansal (Reviewer).

-

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


Re: [OpenJDK 2D-Dev] RFR: JDK-8263362: Avoid division by 0 in java/awt/font/TextJustifier.java justify [v2]

2021-05-06 Thread Prasanta Sadhukhan
On Wed, 5 May 2021 14:11:16 GMT, Matthias Baesken  wrote:

>> In java/awt/font/TextJustifier.java justify-method there is a potential code 
>> path where divison by zero might happen , see also the Sonar finding 
>> https://sonarcloud.io/project/issues?id=shipilev_jdk=AXcqMwpm8sPJZZzONu1k=false=CRITICAL=BUG
>> 
>> 
>> boolean hitLimit = (weight == 0) || (!lastPass && ((delta < 0) 
>> == (delta < gslimit)));
>> boolean absorbing = hitLimit && absorbweight > 0;
>> // predivide delta by weight
>> float weightedDelta = delta / weight; // not used if weight == 0
>> 
>> In case of (weight == 0) the division should not be done because the value 
>> of weightedDelta is unused in this case anyway.
>
> Matthias Baesken has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   adjust absorbweight check

Marked as reviewed by psadhukhan (Reviewer).

-

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


Re: [OpenJDK 2D-Dev] RFR: 8252758: Lanai: Optimize index calculation while copying glyphs

2021-05-06 Thread Ajit Ghaisas
On Wed, 5 May 2021 13:50:47 GMT, Jayathirth D V  wrote:

> Loop optimization while copying glyph content.
> J2DDemo, SwingSet2 and Font2DTest are green.

Marked as reviewed by aghaisas (Committer).

-

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


[OpenJDK 2D-Dev] Integrated: 8266545: 8261169 broke Harfbuzz build with gcc 7 and 8

2021-05-06 Thread Thomas Stuefe
On Wed, 5 May 2021 07:54:20 GMT, Thomas Stuefe  wrote:

> Harfbuzz upgrade broke Linux x64 build on older gccs. For details see JBS 
> issue.
> 
> I fixed the issue in the harfbuzz sources, but I am not sure of the policy 
> here. Do we modify the harfbuzz sources or leave them untouched? Advice is 
> welcome.
> 
> The patch fixes linux x64 fastdebug and opt build for me.

This pull request has now been integrated.

Changeset: a86ee9b3
Author:Thomas Stuefe 
URL:   
https://git.openjdk.java.net/jdk/commit/a86ee9b3f370b59caea2ae78169d13498560cd8e
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod

8266545: 8261169 broke Harfbuzz build with gcc 7 and 8

Reviewed-by: mbaesken, rrich

-

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


Re: [OpenJDK 2D-Dev] RFR: 8266545: 8261169 broke Harfbuzz build with gcc 7 and 8 [v2]

2021-05-06 Thread Thomas Stuefe
On Thu, 6 May 2021 07:53:52 GMT, Richard Reingruber  wrote:

> Looks good to me, thanks!
> Personally I'm building with gcc 9. I've test your fix successfully with gcc 
> 7.5.
> 
> Cheers, Richard.

Thanks Richard! I tested the build on Debian with gcc 7 and gcc 9, it worked.

Since I now have enough reviews, I'' push after the GHA are through

-

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


Re: [OpenJDK 2D-Dev] RFR: 8266545: 8261169 broke Harfbuzz build with gcc 7 and 8 [v2]

2021-05-06 Thread Richard Reingruber
On Thu, 6 May 2021 06:58:16 GMT, Thomas Stuefe  wrote:

>> Harfbuzz upgrade broke Linux x64 build on older gccs. For details see JBS 
>> issue.
>> 
>> I fixed the issue in the harfbuzz sources, but I am not sure of the policy 
>> here. Do we modify the harfbuzz sources or leave them untouched? Advice is 
>> welcome.
>> 
>> The patch fixes linux x64 fastdebug and opt build for me.
>
> Thomas Stuefe has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   switch off warning in build instead of fixing it

Looks good to me, thanks!
Personally I'm building with gcc 9. I've test your fix successfully with gcc 
7.5.

Cheers, Richard.

-

Marked as reviewed by rrich (Reviewer).

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


Re: [OpenJDK 2D-Dev] RFR: 8266545: 8261169 broke Harfbuzz build with gcc 7 and 8 [v2]

2021-05-06 Thread Thomas Stuefe
On Thu, 6 May 2021 06:16:36 GMT, Aleksey Shipilev  wrote:

>> Thomas Stuefe has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   switch off warning in build instead of fixing it
>
> FWIW, current jdk master builds fine with gcc 9.3.0. It fails with gcc 6.3.0 
> to me.
> 
> I am unaware of the policy of changing the Harfbuzz sources too. If we are 
> changing the Harfbuzz sources, I prefer the multi-line form you did in 
> Harfbuzz PR, though: https://github.com/harfbuzz/harfbuzz/pull/2973 -- it 
> would also simplify the merge. 
> 
> But there is an alternative, disable the warning and then wait for Harfbuzz 
> fix to drop:
> 
> 
> diff --git a/make/modules/java.desktop/lib/Awt2dLibraries.gmk 
> b/make/modules/java.desktop/lib/Awt2dLibraries.gmk
> index ff5fa00c720..627aa51ebdf 100644
> --- a/make/modules/java.desktop/lib/Awt2dLibraries.gmk
> +++ b/make/modules/java.desktop/lib/Awt2dLibraries.gmk
> @@ -465,7 +465,7 @@ else
>  
> HARFBUZZ_DISABLED_WARNINGS_gcc := type-limits missing-field-initializers 
> strict-aliasing
> HARFBUZZ_DISABLED_WARNINGS_CXX_gcc := reorder delete-non-virtual-dtor 
> strict-overflow \
> -maybe-uninitialized class-memaccess unused-result
> +maybe-uninitialized class-memaccess unused-result extra
> HARFBUZZ_DISABLED_WARNINGS_clang := unused-value 
> incompatible-pointer-types \
>  tautological-constant-out-of-range-compare int-to-pointer-cast \
>  undef missing-field-initializers range-loop-analysis \

I changed the fix according to @shipilev suggestion. This avoids all the hassle 
of changing downstream HB sources.

@shipilev can we consider this as trivial?

-

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


Re: [OpenJDK 2D-Dev] RFR: 8266545: 8261169 broke Harfbuzz build with gcc 7 and 8

2021-05-06 Thread Thomas Stuefe
On Wed, 5 May 2021 07:54:20 GMT, Thomas Stuefe  wrote:

> Harfbuzz upgrade broke Linux x64 build on older gccs. For details see JBS 
> issue.
> 
> I fixed the issue in the harfbuzz sources, but I am not sure of the policy 
> here. Do we modify the harfbuzz sources or leave them untouched? Advice is 
> welcome.
> 
> The patch fixes linux x64 fastdebug and opt build for me.

> FWIW, current jdk master builds fine with gcc 9.3.0. It fails with gcc 6.3.0 
> to me.
> 
> I am unaware of the policy of changing the Harfbuzz sources too. If we are 
> changing the Harfbuzz sources, I prefer the multi-line form you did in 
> Harfbuzz PR, though: 
> [harfbuzz/harfbuzz#2973](https://github.com/harfbuzz/harfbuzz/pull/2973) -- 
> it would also simplify the merge.
> 
> But there is an alternative, disable the warning and then wait for Harfbuzz 
> fix to drop:
> 
> ```
> diff --git a/make/modules/java.desktop/lib/Awt2dLibraries.gmk 
> b/make/modules/java.desktop/lib/Awt2dLibraries.gmk
> index ff5fa00c720..627aa51ebdf 100644
> --- a/make/modules/java.desktop/lib/Awt2dLibraries.gmk
> +++ b/make/modules/java.desktop/lib/Awt2dLibraries.gmk
> @@ -465,7 +465,7 @@ else
>  
> HARFBUZZ_DISABLED_WARNINGS_gcc := type-limits missing-field-initializers 
> strict-aliasing
> HARFBUZZ_DISABLED_WARNINGS_CXX_gcc := reorder delete-non-virtual-dtor 
> strict-overflow \
> -maybe-uninitialized class-memaccess unused-result
> +maybe-uninitialized class-memaccess unused-result extra
> HARFBUZZ_DISABLED_WARNINGS_clang := unused-value 
> incompatible-pointer-types \
>  tautological-constant-out-of-range-compare int-to-pointer-cast \
>  undef missing-field-initializers range-loop-analysis \
> ```

You are right, that is much better. Why did this not occur to me. I will do 
this.

-

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


Re: [OpenJDK 2D-Dev] RFR: 8266545: 8261169 broke Harfbuzz build with gcc 7 and 8 [v2]

2021-05-06 Thread Thomas Stuefe
> Harfbuzz upgrade broke Linux x64 build on older gccs. For details see JBS 
> issue.
> 
> I fixed the issue in the harfbuzz sources, but I am not sure of the policy 
> here. Do we modify the harfbuzz sources or leave them untouched? Advice is 
> welcome.
> 
> The patch fixes linux x64 fastdebug and opt build for me.

Thomas Stuefe has updated the pull request incrementally with one additional 
commit since the last revision:

  switch off warning in build instead of fixing it

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3873/files
  - new: https://git.openjdk.java.net/jdk/pull/3873/files/dd0f4d29..bcdc7a54

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

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

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


Re: [OpenJDK 2D-Dev] RFR: 8266545: 8261169 broke Harfbuzz build with gcc 7 and 8

2021-05-06 Thread Aleksey Shipilev
On Wed, 5 May 2021 07:54:20 GMT, Thomas Stuefe  wrote:

> Harfbuzz upgrade broke Linux x64 build on older gccs. For details see JBS 
> issue.
> 
> I fixed the issue in the harfbuzz sources, but I am not sure of the policy 
> here. Do we modify the harfbuzz sources or leave them untouched? Advice is 
> welcome.
> 
> The patch fixes linux x64 fastdebug and opt build for me.

FWIW, current jdk master builds fine with gcc 9.3.0. It fails with gcc 6.3.0 to 
me.

I am unaware of the policy of changing the Harfbuzz sources too. If we are 
changing the Harfbuzz sources, I prefer the multi-line form you did in Harfbuzz 
PR, though: https://github.com/harfbuzz/harfbuzz/pull/2973 -- it would also 
simplify the merge. 

But there is an alternative, disable the warning and then wait for Harfbuzz fix 
to drop:


diff --git a/make/modules/java.desktop/lib/Awt2dLibraries.gmk 
b/make/modules/java.desktop/lib/Awt2dLibraries.gmk
index ff5fa00c720..627aa51ebdf 100644
--- a/make/modules/java.desktop/lib/Awt2dLibraries.gmk
+++ b/make/modules/java.desktop/lib/Awt2dLibraries.gmk
@@ -465,7 +465,7 @@ else
 
HARFBUZZ_DISABLED_WARNINGS_gcc := type-limits missing-field-initializers 
strict-aliasing
HARFBUZZ_DISABLED_WARNINGS_CXX_gcc := reorder delete-non-virtual-dtor 
strict-overflow \
-maybe-uninitialized class-memaccess unused-result
+maybe-uninitialized class-memaccess unused-result extra
HARFBUZZ_DISABLED_WARNINGS_clang := unused-value incompatible-pointer-types 
\
 tautological-constant-out-of-range-compare int-to-pointer-cast \
 undef missing-field-initializers range-loop-analysis \

-

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