Skip to content
This repository has been archived by the owner. It is now read-only.

8258754: Gracefully fallback to the OpenGL rendering pipeline if Metal rendering pipeline initialization fails #147

Closed
wants to merge 10 commits into from
+245 −189
Closed

8258754: Gracefully fallback to the OpenGL rendering pipeline if Metal rendering pipeline initialization fails #147

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e752955
fallback to opengl if metal initialization fails
aghaisas Jan 6, 2021
b4bfed4
do metal framework availability check once and cache it
aghaisas Jan 7, 2021
9a6849a
Merge
aghaisas Jan 7, 2021
9856b1d
implement fallback for OpenGL pipeline as well
aghaisas Jan 8, 2021
d23bd72
Cover all combinations of OGL and Metal pipelines
aghaisas Jan 8, 2021
93ad231
Address review comments
aghaisas Jan 11, 2021
c36b416
Merge branch 'master' into fallback_to_opengl
aghaisas Jan 15, 2021
7822546
handle invalid requests to use default pipeline
aghaisas Jan 15, 2021
2600550
fix review comment
aghaisas Jan 18, 2021
617235c
remove redundant check
aghaisas Jan 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 src/java.desktop/macosx/classes/sun/awt/CGraphicsDevice.java
View file
Open in desktop
@@ -67,23 +67,18 @@ public final class CGraphicsDevice extends GraphicsDevice
public CGraphicsDevice(final int displayID) {
this.displayID = displayID;

// Check whether -Dsun.java2d.metal=true has been specified
if (MacOSFlags.isMetalEnabled()) {
Copy link
Member

@mrserb mrserb Jan 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to use metal and OGL at the same time on different devices? I think it is better to move these checks to the CGraphicsEnvironment.makeScreenDevice(), similar Win32GraphicsEnvironment.makeScreenDevice()

Copy link
Collaborator Author

@aghaisas aghaisas Jan 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why someone wants to use different pipelines on different devices? and should we support it?
On Windows we do that as a fallback to overcome a limitation of WGL - that's what I could infer from code in W32GraphicsDevice.getDefaultConfiguration()

I doubt whether we can add these checks in CGraphicsEnvironment.makeScreenDevice() - as we do not have specific MTLGraphicsDevice and CGLGraphicsDevice classes. That's the reason this method is currently unsupported.

Win32GraphicsEnvironment.makeScreenDevice() - creates either D3DGraphicsDevice or Win32GraphicsDevice based on whether isD3DEnabled.
Win32GraphicsDevice in turn uses either WGLGraphicsConfig or Win32GraphicsConfig based on whether OGL is enabled.

I felt CGraphicsDevice (similar to Win32GraphicsDevice) can differentiate between metal or OGL. Hence, I have added checks here.

Copy link
Member

@mrserb mrserb Jan 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is unsupported then why you should check that for every device creation? What happens if one device was created using metal and then another device will use OGL?

Copy link
Collaborator Author

@aghaisas aghaisas Jan 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have moved the Metal framework availability check to be done only once now.
There is still a check if MTLGraphicsConfig.getConfig() fails by any chance.

CGraphicsDevice contains GraphicsConfiguration - which is created in the constructor - It can be either MTLGraphicsConfiguration or OpenGLConfiguration. This check needs to be done while creating the config.
For Win32GraphicsDevice - the config is not created in the constructor, but at a later stage in getDefaultConfiguration(). So only the place of checking for OGL or falling back to GDI is different, but the concept is the same.

The whole idea of one device using metal and another device using OGL is hypothetical. The condition check cannot pass for one device and fail for another.

Copy link
Member

@kevinrushforth kevinrushforth Jan 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that it makes no sense to even consider using different pipelines on different devices. The change you made to do the check once seems best.

aghaisas marked this conversation as resolved.
Show resolved Hide resolved

// Check whether Metal framework is available on the system
if (MTLGraphicsConfig.isMetalAvailable()) {
// Try to get MTLGraphicsConfig
this.config = MTLGraphicsConfig.getConfig(this, displayID, 0);

// Try to get MTLGraphicsConfig
this.config = MTLGraphicsConfig.getConfig(this, displayID, 0);

// If MTLGraphicsConfig creation succeeds
if (this.config != null) {
metalPipelineEnabled = true;
} else {
// If MTLGraphicsConfig creation succeeds
if (this.config != null) {
metalPipelineEnabled = true;
} else {
if (MacOSFlags.isMetalVerbose()) {
System.out.println("Metal rendering pipeline initialization failed. Using OpenGL rendering pipeline.");
}
} else {
System.out.println("Metal framework is not available. Using OpenGL rendering pipeline.");
}
}

31 src/java.desktop/macosx/classes/sun/java2d/macos/MacOSFlags.java
View file
Open in desktop
@@ -26,6 +26,8 @@
package sun.java2d.macos;

import java.security.PrivilegedAction;
import sun.java2d.metal.MTLGraphicsConfig;


public class MacOSFlags {

@@ -36,6 +38,7 @@ public class MacOSFlags {
*/

private static boolean metalEnabled;
private static boolean metalVerbose;

static {
initJavaFlags();
@@ -66,6 +69,18 @@ private static boolean getBooleanProp(String p, boolean defaultVal) {
return returnVal;
}

private static boolean isBooleanPropTrueVerbose(String p) {
String propString = System.getProperty(p);
aghaisas marked this conversation as resolved.
Show resolved Hide resolved
if (propString != null) {
if (propString.equals("True") ||
propString.equals("T"))
{
return true;
}
}
return false;
}


private static boolean getPropertySet(String p) {
String propString = System.getProperty(p);
@@ -76,11 +91,27 @@ private static void initJavaFlags() {
java.security.AccessController.doPrivileged(
(PrivilegedAction<Object>) () -> {
metalEnabled = getBooleanProp("sun.java2d.metal", false);
if (metalEnabled) {
metalVerbose = isBooleanPropTrueVerbose("sun.java2d.metal");

// Check whether Metal framework is available on the system
if (!MTLGraphicsConfig.isMetalAvailable()) {
metalEnabled = false;

if (metalVerbose) {
System.out.println("Could not enable Metal pipeline (Metal framework not available)");
}
}
}
return null;
});
}

public static boolean isMetalEnabled() {
return metalEnabled;
}

public static boolean isMetalVerbose() {
return metalVerbose;
}
}
2 src/java.desktop/macosx/classes/sun/lwawt/macosx/LWCToolkit.java
View file
Open in desktop
@@ -501,7 +501,7 @@ public Insets getScreenInsets(final GraphicsConfiguration gc) {

@Override
public void sync() {
// flush the OGL pipeline (this is a no-op if OGL is not enabled)
// flush the rendering pipeline
if (CGraphicsDevice.useMetalPipeline()) {
MTLRenderQueue.sync();
Copy link
Member

@mrserb mrserb Jan 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this check mean that the MTLRenderQueue.sync() is not a noop if the metal is not enabled?

Copy link
Collaborator Author

@aghaisas aghaisas Jan 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this check mean that the MTLRenderQueue.sync() is not a noop if the metal is not enabled?

I don't know how else we will differentiate between Metal and OGL here.
MTLRenderQueue.sync() is still a noop - as there is null instance check inside it.
The comment above the if check has become stale - I will update it.

} else {