<AWT Dev> RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v3]

[Gerard Ziemski]

On Mon, 8 Feb 2021 12:28:07 GMT, Ajit Ghaisas <aghaisas at 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)
>
> Ajit Ghaisas has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Lanai PR#175 - 8261304 - aghaisas

> > General comment - I am not sure I like the `MTL` prefix acronym in names (ex. `sun.java2d.metal.MTLVolatileSurfaceManager`).
> > I think you tried to match the `CGL`, but that is a real acronym that stands for "Core Graphics Layer" (I think).
> > `MTL` on the other hand is no acronym. I can see `ML` for "Metal Layer" I suppose, but also just `Metal` would work just fine.
> 
> `MTL` is the abbreviation that Apple uses for Metal in all of their APIs. The only potential issue I might see with this prefix is in the native code where there could be name collisions between Java2D's names and Apple's names. Since we haven't run into such a collision, I don't think this needs to change, and wouldn't necessarily affect the Java class names anyway. If we were to consider it, `METAL` seems better than `ML` (which is too short to be descriptive and might suggest machine learning).

If Apple itself uses `MTL` then we are good.

src/java.desktop/macosx/classes/sun/awt/CGraphicsConfig.java line 35:

> 33: 
> 34: import sun.java2d.SurfaceData;
> 35: import sun.java2d.opengl.CGLLayer;

Not needed import anymore?

src/java.desktop/macosx/classes/sun/awt/CGraphicsDevice.java line 113:

> 111:             // This indicates fallback to other rendering pipeline also failed.
> 112:             // Should never reach here
> 113:             throw new InternalError("Error - unable to initialize any rendering pipeline.");

There is no software based renderer to fall back here?

src/java.desktop/macosx/classes/sun/java2d/MacOSFlags.java line 66:

> 64:                        propString.equals("f") ||
> 65:                        propString.equals("False") ||
> 66:                        propString.equals("F"))

Shouldn't `1` and `0` be also allowed here?

src/java.desktop/macosx/classes/sun/java2d/MacOSFlags.java line 100:

> 98:                         oglState = PropertyState.ENABLED; // Enable default pipeline
> 99:                         metalState = PropertyState.DISABLED; // Disable non-default pipeline
> 100:                     }

This matches JEP 382 specification, but even when both GL is `false` and Metal is `false` we still get GL? There is no software based pipeline anymore?

src/java.desktop/macosx/classes/sun/java2d/metal/MTLContext.java line 131:

> 129:         @Native
> 130:         static final int CAPS_EXT_GRAD_SHADER  = (FIRST_PRIVATE_CAP << 3);
> 131:         /** Indicates the presence of the GL_ARB_texture_rectangle extension. */

Reference to `GL_ARB_texture_rectangle` extension in Metal pipeline?

src/java.desktop/macosx/classes/sun/java2d/metal/MTLContext.java line 134:

> 132:         @Native
> 133:         static final int CAPS_EXT_TEXRECT      = (FIRST_PRIVATE_CAP << 4);
> 134:         /** Indicates the presence of the GL_NV_texture_barrier extension. */

Reference to `GL_NV_texture_barrier` extension in Metal pipeline?

src/java.desktop/macosx/classes/sun/java2d/metal/MTLRenderQueue.java line 97:

> 95:     public static void disposeGraphicsConfig(long pConfigInfo) {
> 96:         MTLRenderQueue rq = getInstance();
> 97:         rq.lock();

Is it allowed to have multiple `MTLRenderQueue` instances?

If not, then I see this pattern everywhere:

        MTLRenderQueue rq = getInstance();
        rq.lock();
        {
          ...
        }
        rq.unlock();
why not just do:

        MTLRenderQueue.lock();
        {
          ...
        }
        MTLRenderQueue.unlock();

and have `MTLRenderQueue.lock()` and `MTLRenderQueue.unlock()` implement getting the actual lock instance and locking/unlocking it?

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

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