Re: [OpenJDK 2D-Dev] RFR: 8264318 Lanai: DrawHugeImageTest.java fails on apple M1

[Sergey Bylokhov]

On Wed, 7 Apr 2021 19:56:43 GMT, Sergey Bylokhov  wrote:

>> Check if blit sizes are less than MTL_GPU_FAMILY_MAC_TXT_SIZE.
>> 
>> It's safe since we copy tile from the image with memcpy. 
>> // copy src pixels inside src bounds to buff
>> for (int row = 0; row < sh; row++) {
>> memcpy(buff.contents + (row * sw * srcInfo->pixelStride), raster, sw * 
>> srcInfo->pixelStride);
>> raster += (NSUInteger)srcInfo->scanStride;
>> }
>
> src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLBlitLoops.m line 
> 162:
> 
>> 160: const int sh = MIN(srcInfo->bounds.y2 - srcInfo->bounds.y1, 
>> MTL_GPU_FAMILY_MAC_TXT_SIZE);
>> 161: const int dw = MIN(dx2 - dx1, MTL_GPU_FAMILY_MAC_TXT_SIZE);
>> 162: const int dh = MIN(dy2 - dy1, MTL_GPU_FAMILY_MAC_TXT_SIZE);
> 
> Just curious why such big coordinates are passed here. We should not be able 
> to create a window of such size, as well as a volatile image.

Missed the MIN usage, initially read it as max.

-

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

Re: [OpenJDK 2D-Dev] RFR: 8264666: Reuse Math.multiplyExact/addExact in the LCMSImageLayout class

[Sergey Bylokhov]

On Wed, 7 Apr 2021 20:14:06 GMT, Phil Race  wrote:

> I don't understand the exception handling here. multiplyExact and addExact 
> will throw a runtime exception - ArithmeticException. And I don't see where 
> it is caught. The change for ImageLayoutException to extend 
> ArithmeticException doesn't seem like it can help here. Where is the 
> ArithmeticException being turned into ImageLayoutException ?

O_o for some reason... missed that.

-

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

[OpenJDK 2D-Dev] Withdrawn: 8258788: incorrect response to change in window insets [lanai]

[Alexey Ushakov]

On Thu, 1 Apr 2021 14:38:52 GMT, Alexey Ushakov  wrote:

> Dynamically change MTLLayer insets depending on FULL_WINDOW_CONTENT property. 
> MTLLayer.h header cleanup.

This pull request has been closed without being integrated.

-

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

Re: [OpenJDK 2D-Dev] RFR: 8258788: incorrect response to change in window insets [lanai] [v4]

[Alexey Ushakov]

> Dynamically change MTLLayer insets depending on FULL_WINDOW_CONTENT property. 
> MTLLayer.h header cleanup.

Alexey Ushakov has refreshed the contents of this pull request, and previous 
commits have been removed. The incremental views will show differences compared 
to the previous content of the PR. The pull request contains one new commit 
since the last revision:

  8258788: incorrect response to change in window insets [lanai]
  
  Perform replaceSurfaceData on insets change

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3308/files
  - new: https://git.openjdk.java.net/jdk/pull/3308/files/52fc75e7..ed5e6814

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

  Stats: 35 lines in 8 files changed: 17 ins; 17 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3308.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3308/head:pull/3308

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

Re: [OpenJDK 2D-Dev] RFR: 8264666: Reuse Math.multiplyExact/addExact in the LCMSImageLayout class

[Alexander Zvegintsev]

On Sun, 4 Apr 2021 07:58:44 GMT, Sergey Bylokhov  wrote:

>> - The hand-crafted methods for addition and multiplication are replaced by 
>> the "Math" versions.
>> - Cleanup: the usage of do/while(false) is removed
>
> src/java.desktop/share/classes/sun/java2d/cmm/lcms/LCMSImageLayout.java line 
> 241:
> 
>> 239: l.imageAtOnce = true;
>> 240: }
>> 241: } while (false);
> 
> Any idea why the "do/while(false)" was used here?

Probably it is just a way to highlight and improve readability of a block of 
code, but simply `{}` would be enough in this case.

-

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

Re: [OpenJDK 2D-Dev] RFR: 8258788: incorrect response to change in window insets [lanai]

[Alexey Ushakov]

On Tue, 6 Apr 2021 12:51:47 GMT, Alexey Ushakov  wrote:

>>> Is it possible to automatically test it?
>> 
>> Yes, just added the test.
>
>> @avu Test passes without fix also.
> @jayathirthrao Could you provide the details about your configuration along 
> with parameters passed to jtreg ?

> @avu I am running test in 13 inch Macbook Early 2015 with integrated Intel 
> Iris Graphics 6100.
> jtreg command i am using "jtreg -jdk: -Dsun.java2d.metal=true 
> "
@jayathirthrao  I'm not sure that -Dsun.java2d.metal had been passed to the 
test JVM. I rechecked with slightly different command line and the test fails 
without the fix
jtreg -ignore:quiet -v -a -xml -vmoptions:"-Dsun.java2d.metal=True "  
-testjdk:path_to_jdk path_to_test

-

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

Re: [OpenJDK 2D-Dev] RFR: 8264666: Reuse Math.multiplyExact/addExact in the LCMSImageLayout class

[Phil Race]

On Fri, 2 Apr 2021 23:06:43 GMT, Sergey Bylokhov  wrote:

>> - The hand-crafted methods for addition and multiplication are replaced by 
>> the "Math" versions.
>> - Cleanup: the usage of do/while(false) is removed
>
> The nice PR number: !

I don't understand the exception handling here. multiplyExact and addExact will 
throw a runtime exception - ArithmeticException. And I don't see where it is 
caught. The change for ImageLayoutException to extend ArithmeticException 
doesn't seem like it can help here. Where is the ArithmeticException being 
turned into ImageLayoutException ?

-

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

Re: [OpenJDK 2D-Dev] RFR: 8264318 Lanai: DrawHugeImageTest.java fails on apple M1

[Sergey Bylokhov]

On Wed, 7 Apr 2021 05:24:30 GMT, Denis Konoplev  wrote:

> Check if blit sizes are less than MTL_GPU_FAMILY_MAC_TXT_SIZE.
> 
> It's safe since we copy tile from the image with memcpy. 
> // copy src pixels inside src bounds to buff
> for (int row = 0; row < sh; row++) {
> memcpy(buff.contents + (row * sw * srcInfo->pixelStride), raster, sw * 
> srcInfo->pixelStride);
> raster += (NSUInteger)srcInfo->scanStride;
> }

I wonder why we have two similar but different constants:
#define MaxTextureSize 16384
#define MTL_GPU_FAMILY_MAC_TXT_SIZE 16384

src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLBlitLoops.m line 
162:

> 160: const int sh = MIN(srcInfo->bounds.y2 - srcInfo->bounds.y1, 
> MTL_GPU_FAMILY_MAC_TXT_SIZE);
> 161: const int dw = MIN(dx2 - dx1, MTL_GPU_FAMILY_MAC_TXT_SIZE);
> 162: const int dh = MIN(dy2 - dy1, MTL_GPU_FAMILY_MAC_TXT_SIZE);

Just curious why such big coordinates are passed here. We should not be able to 
create a window of such size, as well as a volatile image.

-

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

[OpenJDK 2D-Dev] RFR: 8264666: Reuse Math.multiplyExact/addExact in the LCMSImageLayout class

[Sergey Bylokhov]

- The hand-crafted methods for addition and multiplication are replaced by the 
"Math" versions.
- Cleanup: the usage of do/while(false) is removed

-

Commit messages:
 - delete "do{...} while (false);"
 - safeXX -> xxExact

Changes: https://git.openjdk.java.net/jdk/pull//files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk==00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8264666
  Stats: 110 lines in 1 file changed: 5 ins; 36 del; 69 mod
  Patch: https://git.openjdk.java.net/jdk/pull/.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull//head:pull/

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

Re: [OpenJDK 2D-Dev] RFR: 8264666: Reuse Math.multiplyExact/addExact in the LCMSImageLayout class

[Sergey Bylokhov]

On Fri, 2 Apr 2021 23:02:50 GMT, Sergey Bylokhov  wrote:

> - The hand-crafted methods for addition and multiplication are replaced by 
> the "Math" versions.
> - Cleanup: the usage of do/while(false) is removed

The nice PR number: !

src/java.desktop/share/classes/sun/java2d/cmm/lcms/LCMSImageLayout.java line 
241:

> 239: l.imageAtOnce = true;
> 240: }
> 241: } while (false);

Any idea why the "do/while(false)" was used here?

-

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

Re: [OpenJDK 2D-Dev] RFR: 8264143 Lanai: RenderPerfTest.BgrSwBlitImage has artefacts on apple M1 [v3]

[Denis Konoplev]

> There was no code to check num of work items in compute shader.
> Also, I've replaced four similar shaders.

Denis Konoplev has updated the pull request incrementally with one additional 
commit since the last revision:

  8264143: Change uint8_t to unsigned char

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3368/files
  - new: https://git.openjdk.java.net/jdk/pull/3368/files/51eaa139..8cc9463d

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

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

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

[OpenJDK 2D-Dev] Integrated: 8263984: Invalidate printServices when there are no printers

[Alexey Ivanov]

On Tue, 23 Mar 2021 13:45:33 GMT, Alexey Ivanov  wrote:

> When `getAllPrinterNames()` returns null, the list of `printServices` is 
> assigned a new empty array without invalidating old services which were in 
> the array before.
> 
> The old print services should be invalidated.

This pull request has now been integrated.

Changeset: 9d650397
Author:Alexey Ivanov 
URL:   https://git.openjdk.java.net/jdk/commit/9d650397
Stats: 8 lines in 1 file changed: 7 ins; 1 del; 0 mod

8263984: Invalidate printServices when there are no printers

Reviewed-by: serb, jdv

-

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

Re: [OpenJDK 2D-Dev] RFR: 8264428: Cleanup usages of StringBuffer in java.desktop [v3]

[Alexey Ivanov]

On Wed, 7 Apr 2021 06:39:48 GMT, Andrey Turbanov 
 wrote:

>> There are few possible cleanups in java.desktop related to legacy 
>> StringBuffer usages:
>> 1. In few places StringBuffer can be replaced with plain String 
>> concatenation.
>> 2. StringBuffer can be replaced with StringBuilder. StringBuilder has better 
>> performance as it is not thread-safe.
>> 3. There are few places where result of string concatenation is passed to 
>> StringBuffer.append method. Using separate `.append` calls is more clear.
>> 4. In few places primitives are unnecessary converted to String before 
>> `.append` call. They can be replaced with specialized methods (like 
>> `.append(int)` calls.
>
> Andrey Turbanov has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   [PATCH] Replace uses of StringBuffer with StringBuilder in java.desktop
>   
>   fix copyright year

Marked as reviewed by aivanov (Reviewer).

src/java.desktop/unix/classes/sun/awt/X11/XCreateWindowParams.java line 88:

> 86: while (eIter.hasNext()) {
> 87: Map.Entry entry = eIter.next();
> 88: buf.append(entry.getKey()).append(": 
> ").append(entry.getValue()).append("\n");

Would it be clearer if each append was on its own line?
Suggestion:

buf.append(entry.getKey())
   .append(": ")
   .append(entry.getValue())
   .append("\n");

-

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

[OpenJDK 2D-Dev] Integrated: 8255800: Raster creation methods need some specification clean up

[Phil Race]

On Fri, 26 Mar 2021 19:53:44 GMT, Phil Race  wrote:

> https://bugs.openjdk.java.net/browse/JDK-8255800 could have been a one line 
> spec clean up but
> it didn't take a lot of looking to realize there were many more 
> inconsistencies between spec and implementation.
> I've spent a lot of time on what is just small number of factory methods in 
> Raster because there are so
> many possible exceptions and in some cases they rely on other API they call 
> to generate exceptions and
> these may have not been documented or documented acc. to some long lost 
> behavior.
> I've mostly tried to ONLY change spec. But I couldn't help myself when some 
> checks were missed that
> ended up with bizarre and dubious behavior - throwing 
> NegativeArrayIndexException which just about
> always has to be an internal bug !
> 
> The supplied test passes on JDK 16 as well as this code, because the 
> (relatively) small number of
> cases where JDK 16 threw NegativeArrayIndexException are caught and allowed 
> only for releases < 17
> So where you see those in the test it corresponds to the behavioral changes 
> from NegativeArrayIndexException
> to IllegalArgumentException.
> JCK conformance tests still pass so they must not test those conditions.

This pull request has now been integrated.

Changeset: adb860ec
Author:Phil Race 
URL:   https://git.openjdk.java.net/jdk/commit/adb860ec
Stats: 1494 lines in 5 files changed: 1424 ins; 35 del; 35 mod

8255800: Raster creation methods need some specification clean up

Reviewed-by: serb

-

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

Re: [OpenJDK 2D-Dev] RFR: 8263984: Invalidate printServices when there are no printers [v3]

[Prasanta Sadhukhan]

I have not looked at entire file but since this function was modified, I made 
that suggestion. I am not that particular if you wish to ignore that.

Regards
Prasanta

Get Outlook for Android

From: 2d-dev <2d-dev-r...@openjdk.java.net> on behalf of Alexey Ivanov 

Sent: Wednesday, April 7, 2021 4:22:29 PM
To: 2d-dev@openjdk.java.net <2d-dev@openjdk.java.net>
Subject: Re: [OpenJDK 2D-Dev] RFR: 8263984: Invalidate printServices when there 
are no printers [v3]

On Wed, 7 Apr 2021 04:12:55 GMT, Prasanta Sadhukhan  
wrote:

>> [Code Conventions for 
>> Java](https://www.oracle.com/java/technologies/javase/codeconventions-indentation.html#248)
>>  say, “Line wrapping for `if` statements should generally use the 8-space 
>> rule, since conventional (4 space) indentation makes seeing the body 
>> difficult.” (It's the second to last block on the page.)
>
> If we are adding a new line, then I think we should need to add at l129, l138 
> otherwise it will look odd doing it at one place only.

I haven't touched that code at all.
Not that odd because it's isolated to the new function now.

What is your suggestion? Refactor all if statements in these two functions? 
Submit a separate bug for refactor all if statements the entire file? Revert 
back to no new line, leaving this particular if untouched as well?

-

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

Re: [OpenJDK 2D-Dev] RFR: 8264428: Cleanup usages of StringBuffer in java.desktop [v3]

[Alexander Zvegintsev]

On Wed, 7 Apr 2021 06:39:48 GMT, Andrey Turbanov 
 wrote:

>> There are few possible cleanups in java.desktop related to legacy 
>> StringBuffer usages:
>> 1. In few places StringBuffer can be replaced with plain String 
>> concatenation.
>> 2. StringBuffer can be replaced with StringBuilder. StringBuilder has better 
>> performance as it is not thread-safe.
>> 3. There are few places where result of string concatenation is passed to 
>> StringBuffer.append method. Using separate `.append` calls is more clear.
>> 4. In few places primitives are unnecessary converted to String before 
>> `.append` call. They can be replaced with specialized methods (like 
>> `.append(int)` calls.
>
> Andrey Turbanov has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   [PATCH] Replace uses of StringBuffer with StringBuilder in java.desktop
>   
>   fix copyright year

Marked as reviewed by azvegint (Reviewer).

-

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

Re: [OpenJDK 2D-Dev] RFR: 8263984: Invalidate printServices when there are no printers [v3]

[Alexey Ivanov]

On Wed, 7 Apr 2021 04:12:55 GMT, Prasanta Sadhukhan  
wrote:

>> [Code Conventions for 
>> Java](https://www.oracle.com/java/technologies/javase/codeconventions-indentation.html#248)
>>  say, “Line wrapping for `if` statements should generally use the 8-space 
>> rule, since conventional (4 space) indentation makes seeing the body 
>> difficult.” (It's the second to last block on the page.)
>
> If we are adding a new line, then I think we should need to add at l129, l138 
> otherwise it will look odd doing it at one place only.

I haven't touched that code at all.
Not that odd because it's isolated to the new function now.

What is your suggestion? Refactor all if statements in these two functions? 
Submit a separate bug for refactor all if statements the entire file? Revert 
back to no new line, leaving this particular if untouched as well?

-

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

Re: [OpenJDK 2D-Dev] RFR: 8264143 Lanai: RenderPerfTest.BgrSwBlitImage has artefacts on apple M1 [v2]

[Denis Konoplev]

> There was no code to check num of work items in compute shader.
> Also, I've replaced four similar shaders.

Denis Konoplev has updated the pull request incrementally with one additional 
commit since the last revision:

  8264143: Lanai: RenderPerfTest.BgrSwBlitImage has artefacts on apple M1
  
  Add stdint include to fix x64 build

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3368/files
  - new: https://git.openjdk.java.net/jdk/pull/3368/files/40760c12..51eaa139

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

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

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

Re: [OpenJDK 2D-Dev] RFR: 8264428: Cleanup usages of StringBuffer in java.desktop [v3]

[Andrey Turbanov]

> There are few possible cleanups in java.desktop related to legacy 
> StringBuffer usages:
> 1. In few places StringBuffer can be replaced with plain String concatenation.
> 2. StringBuffer can be replaced with StringBuilder. StringBuilder has better 
> performance as it is not thread-safe.
> 3. There are few places where result of string concatenation is passed to 
> StringBuffer.append method. Using separate `.append` calls is more clear.
> 4. In few places primitives are unnecessary converted to String before 
> `.append` call. They can be replaced with specialized methods (like 
> `.append(int)` calls.

Andrey Turbanov has updated the pull request incrementally with one additional 
commit since the last revision:

  [PATCH] Replace uses of StringBuffer with StringBuilder in java.desktop
  
  fix copyright year

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3251/files
  - new: https://git.openjdk.java.net/jdk/pull/3251/files/80bff1db..963dc56e

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

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

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

Re: [OpenJDK 2D-Dev] RFR: 8264428: Cleanup usages of StringBuffer in java.desktop [v3]

[Andrey Turbanov]

On Tue, 30 Mar 2021 15:46:56 GMT, Alexander Zvegintsev  
wrote:

>> Andrey Turbanov has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   [PATCH] Replace uses of StringBuffer with StringBuilder in java.desktop
>>   
>>   fix copyright year
>
> src/java.desktop/windows/classes/sun/java2d/d3d/D3DContext.java line 139:
> 
>> 137: @Override
>> 138: public String toString() {
>> 139: StringBuilder buf = new StringBuilder(super.toString());
> 
> Looks like this is the only file where copyright year is not updated.

Thanks. Fixed.

-

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

[OpenJDK 2D-Dev] Integrated: 8264680: Use the blessed modifier order in java.desktop

[Alex Blewitt]

On Sat, 3 Apr 2021 22:03:44 GMT, Alex Blewitt 
 wrote:

> 8264680: Use the blessed modifier order in java.desktop

This pull request has now been integrated.

Changeset: 92fad1b4
Author:Alex Blewitt 
Committer: Sergey Bylokhov 
URL:   https://git.openjdk.java.net/jdk/commit/92fad1b4
Stats: 132 lines in 11 files changed: 0 ins; 114 del; 18 mod

8264680: Use the blessed modifier order in java.desktop

Reviewed-by: serb, kizune, azvegint

-

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