Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for EGL_EXT_gl_colorspace_bt2020_pq #18

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
+52 −5
Draft

Add support for EGL_EXT_gl_colorspace_bt2020_pq #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1a36e40
define EGL_EXT_gl_colorspace_bt2020_*
tmm1 Jun 29, 2020
684d67b
implement EGL_GL_COLORSPACE_BT2020_PQ_EXT for Metal
tmm1 Jun 29, 2020
f0fe6f8
expose MGLDrawableColorFormatRGBA16 and MGLDrawableColorSpaceBT2020PQ
tmm1 Jun 29, 2020
c5334b4
add to MGLKView also
tmm1 Jun 29, 2020
f5d8be7
switch back to 8bit window format
tmm1 Jun 29, 2020
4f44fe1
add missing validation checks for EGL_GL_COLORSPACE_BT2020_PQ_EXT
tmm1 Jun 29, 2020
c9b9642
remove MGLDrawableColorSpace
tmm1 Jun 30, 2020
b9a86f9
add MGLDrawableColorFormatRGBA16BT2020PQ
tmm1 Jun 30, 2020
3503d77
move metal layer colorspace into SurfaceMtl
tmm1 Jun 30, 2020
247bf59
fix mMetalLayerColorSpace lifecycle
tmm1 Jun 30, 2020
075dbc4
support 16-bit configs
tmm1 Jun 30, 2020
7a8cd44
remove unused header
tmm1 Jun 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ios/xcode/MGLKit/MGLLayer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ typedef enum MGLDrawableColorFormat : int
MGLDrawableColorFormatRGBA8888 = 32,
MGLDrawableColorFormatSRGBA8888 = -32,
MGLDrawableColorFormatRGB565 = 16,

MGLDrawableColorFormatRGBA16 = 64,
MGLDrawableColorFormatRGBA16BT2020PQ = 65,
} MGLDrawableColorFormat;

typedef enum MGLDrawableStencilFormat : int
Expand Down
8 changes: 8 additions & 0 deletions ios/xcode/MGLKit/MGLLayer.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,14 @@ - (void)ensureSurfaceCreated
red = green = blue = alpha = 8;
colorSpace = EGL_GL_COLORSPACE_SRGB_KHR;
break;
case MGLDrawableColorFormatRGBA16:
red = green = blue = alpha = 16;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using 16 causes Failed to call eglChooseConfig() exception. I think it is probably unnecessary since we are setting the metal pixel format and color space correctly.

Copy link
Owner

Choose a reason for hiding this comment

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

For a new config to be accepted (such as RGBA 16 bits), it must be added to the supported list inside DisplayMtl::generateConfigs()

Copy link
Contributor Author

@tmm1 tmm1 Jun 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Oh okay, I see. That function is a bit confusing to me, but I will try to figure out how to add the 16bit formats in there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was able to make this work. But I still don't understand what effect this has, as opposed to fbo format and texture formats used.

Copy link
Owner

Choose a reason for hiding this comment

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

Currently SurfaceMtl assumes users always create a window surface with 8 bits format (this is actually guaranteed by DisplayMtl::generateConfigs()). Hence in this code, there wasn't any color channels' configured bits size's verification, thus it always chooses MTLPixelFormatBGRA8Unorm metal format for default colorspace:

metalangle/src/libANGLE/renderer/metal/SurfaceMtl.mm

Line 219 in 78017f0

mColorFormat.metalFormat = MTLPixelFormatBGRA8Unorm;

If you add a new 16 bits config with default colorspace, the code should examine the color channels's configured bits size inside egl::SurfaceState parameter and choose appropriate metal format such as MTLPixelFormatBGRA8Unorm or MTLPixelFormatRGBA16Float similar to how depth & stencil bits configuration is handled:

metalangle/src/libANGLE/renderer/metal/SurfaceMtl.mm

Line 228 in 78017f0

depthBits = state.config->depthSize;

metalangle/src/libANGLE/renderer/metal/SurfaceMtl.mm

Line 232 in 78017f0

if (depthBits && stencilBits)

However, I just realized you have no use for RGBA16 with default color space, maybe we can just omit MGLDrawableColorFormatRGBA16 and only keep its BT.2020 variant. Thus further hassles would be avoided.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I will reduce scope to BT2020 variant only as you suggested.

colorSpace = EGL_GL_COLORSPACE_LINEAR_KHR;
break;
case MGLDrawableColorFormatRGBA16BT2020PQ:
red = green = blue = alpha = 16;
colorSpace = EGL_GL_COLORSPACE_BT2020_PQ_EXT;
break;
default:
UNREACHABLE();
break;
Expand Down
2 changes: 2 additions & 0 deletions src/libANGLE/Caps.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1222,6 +1222,8 @@ std::vector<std::string> DisplayExtensions::getStrings() const
InsertExtensionString("EGL_EXT_gl_colorspace_display_p3", glColorspaceDisplayP3, &extensionStrings);
InsertExtensionString("EGL_EXT_gl_colorspace_display_p3_linear", glColorspaceDisplayP3Linear, &extensionStrings);
InsertExtensionString("EGL_EXT_gl_colorspace_display_p3_passthrough", glColorspaceDisplayP3Passthrough, &extensionStrings);
InsertExtensionString("EGL_EXT_gl_colorspace_bt2020_linear", glColorspaceBT2020, &extensionStrings);
InsertExtensionString("EGL_EXT_gl_colorspace_bt2020_pq", glColorspaceBT2020PQ, &extensionStrings);
InsertExtensionString("EGL_KHR_gl_texture_2D_image", glTexture2DImage, &extensionStrings);
InsertExtensionString("EGL_KHR_gl_texture_cubemap_image", glTextureCubemapImage, &extensionStrings);
InsertExtensionString("EGL_KHR_gl_texture_3D_image", glTexture3DImage, &extensionStrings);
Expand Down
6 changes: 6 additions & 0 deletions src/libANGLE/Caps.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,12 @@ struct DisplayExtensions

// EGL_EXT_gl_colorspace_display_p3_passthrough
bool glColorspaceDisplayP3Passthrough = false;

// EGL_EXT_gl_colorspace_bt2020_linear
bool glColorspaceBT2020 = false;

// EGL_EXT_gl_colorspace_bt2020_pq
bool glColorspaceBT2020PQ = false;
};

struct DeviceExtensions
Expand Down
1 change: 1 addition & 0 deletions src/libANGLE/formatutils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ ANGLE_INLINE bool ColorspaceFormatOverride(const EGLenum colorspace, GLenum *ren
case EGL_GL_COLORSPACE_DISPLAY_P3_PASSTHROUGH_EXT: // App, not the HW, will specify the
// transfer function
case EGL_GL_COLORSPACE_SCRGB_EXT: // App, not the HW, will specify the transfer function
case EGL_GL_COLORSPACE_BT2020_PQ_EXT:
// No translation
return true;
case EGL_GL_COLORSPACE_SRGB_KHR:
Expand Down
4 changes: 4 additions & 0 deletions src/libANGLE/renderer/gl/egl/DisplayEGL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ void DisplayEGL::generateExtensions(egl::DisplayExtensions *outExtensions) const
mEGL->hasExtension("EGL_EXT_gl_colorspace_scrgb_linear");
outExtensions->glColorspaceDisplayP3Passthrough =
mEGL->hasExtension("EGL_EXT_gl_colorspace_display_p3_passthrough");
outExtensions->glColorspaceBT2020 =
mEGL->hasExtension("EGL_EXT_gl_colorspace_bt2020_linear");
outExtensions->glColorspaceBT2020PQ =
mEGL->hasExtension("EGL_EXT_gl_colorspace_bt2020_pq");
}

outExtensions->imageNativeBuffer = mEGL->hasExtension("EGL_ANDROID_image_native_buffer");
Expand Down
11 changes: 7 additions & 4 deletions src/libANGLE/renderer/metal/DisplayMtl.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ bool IsMetalDisplayAvailable()
outExtensions->fenceSync = true;
outExtensions->waitSync = true;
outExtensions->glColorspace = true;
outExtensions->glColorspaceBT2020PQ = true;
}

void DisplayMtl::generateCaps(egl::Caps *outCaps) const {}
Expand Down Expand Up @@ -301,17 +302,19 @@ bool IsMetalDisplayAvailable()
config.colorComponentType = EGL_COLOR_COMPONENT_TYPE_FIXED_EXT;

constexpr int samplesSupported[] = {0, 4};
constexpr int sizesSupported[] = {8, 16};

for (int size : sizesSupported)
for (int samples : samplesSupported)
{
config.samples = samples;
config.sampleBuffers = (samples == 0) ? 0 : 1;

// Buffer sizes
config.redSize = 8;
config.greenSize = 8;
config.blueSize = 8;
config.alphaSize = 8;
config.redSize = size;
Copy link
Contributor Author

@tmm1 tmm1 Jun 30, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Based on your explanation I understand now this is not correct. Instead I should advertise only one RGBA16 format, and set allowed depth/stencil to 0 for that format.

What should I use for renderTargetFormat and depthStencilFormat for this new config?

config.greenSize = size;
config.blueSize = size;
config.alphaSize = size;
config.bufferSize = config.redSize + config.greenSize + config.blueSize + config.alphaSize;

// With DS
Expand Down
1 change: 1 addition & 0 deletions src/libANGLE/renderer/metal/SurfaceMtl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ class SurfaceMtl : public SurfaceImpl
const mtl::TextureRef &oldDepthTexture,
const mtl::TextureRef &oldStencilTexture);

CGColorSpaceRef mMetalLayerColorSpace = NULL;
mtl::AutoObjCObj<CAMetalLayer> mMetalLayer = nil;
CALayer *mLayer;
mtl::AutoObjCPtr<id<CAMetalDrawable>> mCurrentDrawable = nil;
Expand Down
15 changes: 14 additions & 1 deletion src/libANGLE/renderer/metal/SurfaceMtl.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,13 @@ void StopFrameCapture()
{
mColorFormat = display->getPixelFormat(angle::FormatID::B8G8R8A8_UNORM_SRGB);
}
else if (attribs.get(EGL_GL_COLORSPACE, EGL_GL_COLORSPACE_LINEAR) == EGL_GL_COLORSPACE_BT2020_PQ_EXT)
{
mColorFormat.intendedFormatId = mColorFormat.actualFormatId =
angle::FormatID::R16G16B16A16_FLOAT;
mColorFormat.metalFormat = MTLPixelFormatRGBA16Float;
mMetalLayerColorSpace = CGColorSpaceCreateWithName(kCGColorSpaceITUR_2020_PQ);
}
else
{
// https://developer.apple.com/metal/Metal-Feature-Set-Tables.pdf says that BGRA8Unorm is
Expand Down Expand Up @@ -254,7 +261,12 @@ void StopFrameCapture()
}
}

SurfaceMtl::~SurfaceMtl() {}
SurfaceMtl::~SurfaceMtl() {
if (mMetalLayerColorSpace) {
CGColorSpaceRelease(mMetalLayerColorSpace);
mMetalLayerColorSpace = NULL;
}
}

void SurfaceMtl::destroy(const egl::Display *display)
{
Expand Down Expand Up @@ -302,6 +314,7 @@ void StopFrameCapture()

mMetalLayer.get().device = metalDevice;
mMetalLayer.get().pixelFormat = mColorFormat.metalFormat;
mMetalLayer.get().colorspace = mMetalLayerColorSpace;
mMetalLayer.get().framebufferOnly = NO; // Support blitting and glReadPixels

#if TARGET_OS_OSX || TARGET_OS_MACCATALYST
Expand Down
6 changes: 6 additions & 0 deletions src/libANGLE/validationEGL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,12 @@ Error ValidateColorspaceAttribute(const DisplayExtensions &displayExtensions, EG
return EglBadAttribute() << "EXT_gl_colorspace_scrgb_linear is not available.";
}
break;
case EGL_GL_COLORSPACE_BT2020_PQ_EXT:
if (!displayExtensions.glColorspaceBT2020PQ)
{
return EglBadAttribute() << "EXT_gl_colorspace_bt2020_pq is not available.";
}
break;
default:
return EglBadAttribute();
}
Expand Down