RFR: 8264318 Lanai: DrawHugeImageTest.java fails on apple M1

[Denis Konoplev-2]

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;
}

-------------

Commit messages:
 - 8264318: DrawHugeImageTest.java fails on apple M1

Changes: https://git.openjdk.java.net/jdk/pull/3369/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3369&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8264318
  Stats: 5 lines in 1 file changed: 1 ins; 0 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3369.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3369/head:pull/3369

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

[Sergey Bylokhov-2]

On Wed, 7 Apr 2021 05:24:30 GMT, Denis Konoplev <[hidden email]> 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

[Sergey Bylokhov-2]

On Wed, 7 Apr 2021 19:56:43 GMT, Sergey Bylokhov <[hidden email]> 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

[Denis Konoplev-2]

On Wed, 7 Apr 2021 19:54:03 GMT, Sergey Bylokhov <[hidden email]> wrote:

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

I can remove MaxTextureSize and replace its usages.
Should I file a new bug or put changes in this PR?

-------------

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

[Sergey Bylokhov-2]

On Thu, 8 Apr 2021 16:45:23 GMT, Denis Konoplev <[hidden email]> wrote:

> I can remove MaxTextureSize and replace its usages.
> Should I file a new bug or put changes in this PR?

Yes please file a bug.

-------------

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

[Denis Konoplev-2]

On Wed, 7 Apr 2021 05:24:30 GMT, Denis Konoplev <[hidden email]> 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;
> }

Bug: [Remove duplication constant MaxTextureSize](https://bugs.openjdk.java.net/browse/JDK-8265062)

-------------

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

[Denis Konoplev-2]

On Sat, 10 Apr 2021 20:31:31 GMT, Sergey Bylokhov <[hidden email]> wrote:

>>> I wonder why we have two similar but different constants:
>>> #define MaxTextureSize 16384
>>> #define MTL_GPU_FAMILY_MAC_TXT_SIZE 16384
>>
>> I can remove MaxTextureSize and replace its usages.
>> Should I file a new bug or put changes in this PR?
>
>> I can remove MaxTextureSize and replace its usages.
>> Should I file a new bug or put changes in this PR?
>
> Yes please file a bug.

@mrserb maybe you know someone who can review my PR: [3368](https://github.com/openjdk/jdk/pull/3368)

-------------

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

[Sergey Bylokhov-2]

On Wed, 7 Apr 2021 05:24:30 GMT, Denis Konoplev <[hidden email]> 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;
> }

Marked as reviewed by serb (Reviewer).

-------------

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

[Jayathirth D V-2]

On Wed, 7 Apr 2021 05:24:30 GMT, Denis Konoplev <[hidden email]> 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;
> }

Marked as reviewed by jdv (Reviewer).

I will sponsor this PR, waiting for complete test run to finish.

-------------

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

[Denis Konoplev-2]

On Wed, 7 Apr 2021 05:24:30 GMT, Denis Konoplev <[hidden email]> 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;
> }

This pull request has now been integrated.

Changeset: 0fee6eca
Author:    Denis Konoplev <[hidden email]>
Committer: Jayathirth D V <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/0fee6eca
Stats:     5 lines in 1 file changed: 1 ins; 0 del; 4 mod

8264318: Lanai: DrawHugeImageTest.java fails on apple M1

Reviewed-by: serb, jdv

-------------

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