On Mon, 8 Feb 2021 16:53:16 GMT, Gerard Ziemski <gziem...@openjdk.org> wrote:

>> Ajit Ghaisas has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Lanai PR#175 - 8261304 - aghaisas
>
> 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?

OpenGL was the sole rendering pipeline on macOS. Now we are adding Metal 
rendering pipeline. There is no software based fall back renderer.

> 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?

No.
0 and 1 are not supported for existing VM options for rendering pipelines. We 
won't be adding it for metal pipeline either.

> 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?

OpenGL was the sole rendering pipeline on macOS. Now we are adding Metal 
rendering pipeline. There is no software based fall back renderer.

If user requests both - GL and Metal - as false - we shall use 'the default' 
pipeline.
Current default render pipeline on macOS is OpenGL.
Changing default render pipeline on macOS is not in the scope of this JEP.

> src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line 61:
> 
>> 59: 
>> 60: public abstract class MTLSurfaceData extends SurfaceData
>> 61:         implements AccelSurface {
> 
> `MTLSurfaceData` and `OGLSurfaceData` seem to share lots of the same code, 
> isn't there a way to refactor the common code out?
> 
> There are other files that structually look identical, except for the names 
> of classes they use, ex `MTLMaskBlit.java` and `OGLMaskBlit.java`.

Although it is a good suggestion, I do not recommend this refactoring - 
- We do not wish to destabilize proven OpenGL pipeline by refactoring - 
additional testing on OpenGL pipeline also would be needed.
- OpenGL pipeline on macOS might be removed once Apple removes OpenGL support 
from macOS - this will render this refactoring pointless.

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

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

Reply via email to