On Mon, 1 Mar 2021 11:17:39 GMT, Ajit Ghaisas <aghai...@openjdk.org> wrote:
>> **Description :** >> This is the implementation of [JEP 382 : New macOS Rendering >> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8238361) >> It implements a Java 2D internal rendering pipeline for macOS using the >> Apple Metal API. >> The entire work on this was done under [OpenJDK Project - >> Lanai](http://openjdk.java.net/projects/lanai/) >> >> We iterated through several Early Access (EA) builds and have reached a >> stage where it is ready to be integrated to openjdk/jdk. The latest EA build >> is available at - https://jdk.java.net/lanai/ >> >> A new option -Dsun.java2d.metal=true | True needs to be used to use this >> pipeline. >> >> **Testing :** >> This implementation has been tested with the tests present at - [Test Plan >> for JEP 382: New macOS Rendering >> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8251396) >> >> **Note to reviewers :** >> 1) Default rendering pipeline on macOS has not been changed by this PR. >> OpenGL still stays as the default rendering pipeline and Metal rendering >> pipeline is optional to choose. >> >> 2) To apply and test this PR - >> To enable the metal pipeline you must specify on command line >> -Dsun.java2d.metal=true (No message will be printed in this case) or >> -Dsun.java2d.metal=True (A message indicating Metal rendering pipeline is >> enabled gets printed) >> >> 3) Review comments (including some preliminary informal review comments) are >> tracked with JBS issues - https://bugs.openjdk.java.net/issues/?filter=40598 > > Ajit Ghaisas has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains 36 additional > commits since the last revision: > > - Lanai PR#206 - 8262729 - aghaisas > - Lanai PR#205 - 8262496 - avu > - Lanai PR#203 - 8262313 - jdv > - Lanai PR#202 - 8262293 - avu > - Lanai PR#201 - 8261891 - avu > - Lanai PR#200 - 8262115 - aghaisas > - Merge branch 'master' into 8260931_lanai_JEP_branch > - Lanai PR#199 - 8262091 - aghaisas > - Lanai PR#198 - 8261646 - avu > - Lanai PR#197 - 8261960 - jdv > - ... and 26 more: > https://git.openjdk.java.net/jdk/compare/49503c0d...5cb1fd91 src/java.desktop/macosx/classes/sun/awt/CGraphicsDevice.java line 81: > 79: // Try falling back to OpenGL pipeline > 80: if (MacOSFlags.isMetalVerbose()) { > 81: System.out.println("Metal rendering pipeline > initialization failed. Using OpenGL rendering pipeline."); Please split the long lines, here and there. Most of these files use 80 chars per line. src/java.desktop/macosx/classes/sun/java2d/metal/MTLBlitLoops.java line 499: > 497: } > 498: > 499: // We can convert argb_pre data from MTL surface in two places: Does anybody check that this is true for the metal pipeline? or It is just copied from the OGL? src/java.desktop/macosx/classes/sun/java2d/metal/MTLContext.java line 58: > 56: * Makes the given GraphicsConfig's context current to its associated > 57: * "scratch surface". Each GraphicsConfig maintains a native context > 58: * (MTLDevice) as well as a native pbuffer "pbuffer"? src/java.desktop/macosx/classes/sun/java2d/metal/MTLContext.java line 68: > 66: * when disposing a texture-based surface). > 67: */ > 68: public static void setScratchSurface(long pConfigInfo) { How the scratch surface is used in the metal pipeline? Why it is not enough to set the "context current"? src/java.desktop/macosx/classes/sun/java2d/metal/MTLGraphicsConfig.java line 168: > 166: } > 167: > 168: ContextCapabilities caps = new MTLContext.MTLContextCaps( CAPS_DOUBLEBUFFERED is not included? src/java.desktop/macosx/classes/sun/java2d/metal/MTLContext.java line 140: > 138: StringBuilder sb = new StringBuilder(super.toString()); > 139: if ((caps & CAPS_DOUBLEBUFFERED) != 0) { > 140: sb.append("CAPS_DOUBLEBUFFERED|"); Related to other questions, we do not include CAPS_DOUBLEBUFFERED, but anyway, report the surface as double buffered. src/java.desktop/macosx/classes/sun/java2d/metal/MTLLayer.java line 44: > 42: > 43: // Pass the insets to native code to make adjustments in blitTexture > 44: private static native void nativeSetInsets(long layerPtr, int top, > int left); Probably I have asked this already, but why we need to use insets? Why insets is not necessary for ogl? src/java.desktop/macosx/classes/sun/java2d/metal/MTLRenderer.java line 25: > 23: * questions. > 24: */ > 25: package sun.java2d.metal; Looks like other files use blank line befor "package". src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line 146: > 144: return MTLSurfaceRTT; > 145: default: > 146: return MTLSurface; Do we really support 3 different types of surface? I guess only two of them are different: textures used for manageable images, and surfaces used by the volatile image. src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line 194: > 192: * Creates a SurfaceData object representing an off-screen buffer > 193: */ > 194: public static MTLOffScreenSurfaceData createData(MTLGraphicsConfig > gc, Something wrong in the formatting. src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line 221: > 219: protected native boolean initRTexture(long pData, boolean isOpaque, > int width, int height); > 220: > 221: protected native boolean initFlipBackbuffer(long pData); flip back buffer is supported? src/java.desktop/macosx/classes/sun/lwawt/macosx/CWarningWindow.java line 304: > 302: }; > 303: } > 304: public MTLLayer createMTLLayer() { TODO to test that this functionality works src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLBlitLoops.m line 36: > 34: #include "GraphicsPrimitiveMgr.h" > 35: > 36: #include <stdlib.h> // malloc malloc is unused in this file? src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLBlitLoops.m line 676: > 674: height = srcInfo.bounds.y2 - srcInfo.bounds.y1; > 675: > 676: pDst = PtrAddBytes(pDst, dstx * dstInfo.pixelStride); Here and in other places use the PtrPixelsRow instead, do not use multiplication, see the history of the OGLBlitLoops_SurfaceToSwBlit method. src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLSurfaceData.m line 284: > 282: */ > 283: jboolean > 284: MTLSD_InitMTLWindow(JNIEnv *env, BMTLSDOps *bmtlsdo) When the MTLWindow is used? src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLSurfaceData.m line 316: > 314: > 315: void > 316: MTLSD_SwapBuffers(JNIEnv *env, jlong pPeerData) Copied from OGL and can be removed? src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLSurfaceDataBase.h line 109: > 107: #define MTLSD_TEXTURE sun_java2d_pipe_hw_AccelSurface_TEXTURE > 108: #define MTLSD_FLIP_BACKBUFFER > sun_java2d_pipe_hw_AccelSurface_FLIP_BACKBUFFER > 109: #define MTLSD_RT_TEXTURE > sun_java2d_pipe_hw_AccelSurface_RT_TEXTURE Only two of them are supported? MTLSD_TEXTURE and MTLSD_RT_TEXTURE? ------------- PR: https://git.openjdk.java.net/jdk/pull/2403
