Skip to content

Commit

Permalink
Vulkan: Consolidate colorspace override states
Browse files Browse the repository at this point in the history
ColorspaceState struct is now used to cache colorspace related states
and used to determine the colorspace of Vulkan image views.
ImageViewHelper methods are called during initialization and when
colorspace related states are toggled dynamically which in turn process
these states and determine the final read and write colorspaces.

We can now fully support rendering to EGLImages, with colorspace
overrides, via texture or renderbuffer EGLImage targets

Bug: angleproject:40644776
Tests: ImageTest*Colorspace*Vulkan
       MultithreadingTestES3.SharedSrgbTextureMultipleContexts*Vulkan
       SRGBTextureTest.SRGB*TextureParameter*Vulkan
       SRGBTextureTestES3.SRGBDecodeTexelFetch*Vulkan
       ReadPixelsPBOTest.SrgbUnorm*Vulkan
Change-Id: I1cc2b5bd834b519b83deab4d80a2fcaabeb271d6
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5841290
Reviewed-by: Shahbaz Youssefi <[email protected]>
Reviewed-by: Charlie Lao <[email protected]>
Commit-Queue: mohan maiya <[email protected]>
  • Loading branch information
Mohan Maiya authored and Angle LUCI CQ committed Sep 20, 2024
1 parent 167b9e8 commit bffcd23
Show file tree
Hide file tree
Showing 19 changed files with 929 additions and 411 deletions.
1 change: 1 addition & 0 deletions src/libANGLE/Image.h
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ class Image final : public ThreadSafeRefCountObject, public LabeledObject
size_t getSamples() const;
GLuint getLevelCount() const;
bool hasProtectedContent() const;
EGLenum getColorspaceAttribute() const { return mState.colorspace; }

Error initialize(const Display *display, const gl::Context *context);

Expand Down
10 changes: 2 additions & 8 deletions src/libANGLE/renderer/vulkan/ContextVk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7447,15 +7447,9 @@ angle::Result ContextVk::updateActiveTextures(const gl::Context *context, gl::Co
// The new parameter, TEXTURE_SRGB_DECODE_EXT controls whether the
// decoding happens at sample time. It only applies to textures with an
// internal format that is sRGB and is ignored for all other textures.
const vk::ImageHelper &image = textureVk->getImage();
ASSERT(image.valid());
if (image.getActualFormat().isSRGB && samplerState.getSRGBDecode() == GL_SKIP_DECODE_EXT)
{
// Make sure we use the MUTABLE bit for the storage. Because the "skip decode" is a
// Sampler state we might not have caught this setting in TextureVk::syncState.
ANGLE_TRY(textureVk->ensureMutable(this));
}
ANGLE_TRY(textureVk->updateSrgbDecodeState(this, samplerState));

const vk::ImageHelper &image = textureVk->getImage();
if (image.hasInefficientlyEmulatedImageFormat())
{
ANGLE_VK_PERF_WARNING(
Expand Down
68 changes: 48 additions & 20 deletions src/libANGLE/renderer/vulkan/FramebufferVk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,7 @@ angle::Result FramebufferVk::clearImpl(const gl::Context *context,
gl::DrawBuffersArray<VkClearColorValue> adjustedClearColorValues;
const gl::DrawBufferMask colorAttachmentMask = mState.getColorAttachmentsMask();
const auto &colorRenderTargets = mRenderTargetCache.getColors();
bool anyAttachmentWithColorspaceOverride = false;
for (size_t colorIndexGL = 0; colorIndexGL < colorAttachmentMask.size(); ++colorIndexGL)
{
if (colorAttachmentMask[colorIndexGL])
Expand All @@ -559,6 +560,12 @@ angle::Result FramebufferVk::clearImpl(const gl::Context *context,
adjustedClearColorValues[colorIndexGL].float32[1] = clearColorValue.float32[0];
adjustedClearColorValues[colorIndexGL].float32[2] = clearColorValue.float32[1];
}
else if (colorRenderTarget->hasColorspaceOverrideForWrite())
{
// If a rendertarget has colorspace overrides, we need to clear with a draw
// to make sure the colorspace override is honored.
anyAttachmentWithColorspaceOverride = true;
}
else if (contextVk->getRenderer()->getFeatures().adjustClearColorPrecision.enabled)
{
const angle::FormatID colorRenderTargetFormat =
Expand Down Expand Up @@ -602,7 +609,8 @@ angle::Result FramebufferVk::clearImpl(const gl::Context *context,
mActiveColorComponentMasksForClear;
const bool maskedClearStencil = clearStencil && stencilMask != 0xFF;

bool clearColorWithDraw = clearColor && (maskedClearColor || scissoredClear);
bool clearColorWithDraw =
clearColor && (maskedClearColor || scissoredClear || anyAttachmentWithColorspaceOverride);
bool clearDepthWithDraw = clearDepth && scissoredClear;
bool clearStencilWithDraw = clearStencil && (maskedClearStencil || scissoredClear);

Expand Down Expand Up @@ -753,7 +761,7 @@ angle::Result FramebufferVk::clearImpl(const gl::Context *context,
// revert to vkCmdClearAttachments. This is not currently deemed necessary.
if (((clearColorBuffers.any() && !mEmulatedAlphaAttachmentMask.any() && !maskedClearColor) ||
clearDepthWithDraw || (clearStencilWithDraw && !maskedClearStencil)) &&
!preferDrawOverClearAttachments)
!preferDrawOverClearAttachments && !anyAttachmentWithColorspaceOverride)
{
if (!contextVk->hasActiveRenderPass())
{
Expand Down Expand Up @@ -1360,9 +1368,9 @@ angle::Result FramebufferVk::blit(const gl::Context *context,
bool canBlitWithCommand =
!isColorResolve && noClip && (noFlip || !disableFlippingBlitWithCommand) &&
HasSrcBlitFeature(renderer, readRenderTarget) && rotation == SurfaceRotation::Identity;
// If we need to reinterpret the colorspace then the blit must be done through a shader
bool reinterpretsColorspace =
mCurrentFramebufferDesc.getWriteControlMode() != gl::SrgbWriteControlMode::Default;
// If we need to reinterpret the colorspace of read RenderTarget
// then the blit must be done through a shader
bool reinterpretsColorspace = readRenderTarget->hasColorspaceOverrideForRead();
bool areChannelsBlitCompatible = true;
bool areFormatsIdentical = true;
for (size_t colorIndexGL : mState.getEnabledDrawBuffers())
Expand All @@ -1375,6 +1383,10 @@ angle::Result FramebufferVk::blit(const gl::Context *context,
AreSrcAndDstColorChannelsBlitCompatible(readRenderTarget, drawRenderTarget);
areFormatsIdentical = areFormatsIdentical &&
AreSrcAndDstFormatsIdentical(readRenderTarget, drawRenderTarget);
// If we need to reinterpret the colorspace of the draw RenderTarget
// then the blit must be done through a shader
reinterpretsColorspace =
reinterpretsColorspace || drawRenderTarget->hasColorspaceOverrideForWrite();
}

// Now that all flipping is done, adjust the offsets for resolve and prerotation
Expand Down Expand Up @@ -2280,6 +2292,19 @@ angle::Result FramebufferVk::updateColorAttachment(const gl::Context *context,
return angle::Result::Continue;
}

void FramebufferVk::updateColorAttachmentColorspace(gl::SrgbWriteControlMode srgbWriteControlMode)
{
// Update colorspace of color attachments.
const auto &colorRenderTargets = mRenderTargetCache.getColors();
const gl::DrawBufferMask colorAttachmentMask = mState.getColorAttachmentsMask();
for (size_t colorIndexGL : colorAttachmentMask)
{
RenderTargetVk *colorRenderTarget = colorRenderTargets[colorIndexGL];
ASSERT(colorRenderTarget);
colorRenderTarget->updateWriteColorspace(srgbWriteControlMode);
}
}

angle::Result FramebufferVk::updateDepthStencilAttachment(const gl::Context *context)
{
ANGLE_TRY(mRenderTargetCache.updateDepthStencilRenderTarget(context, mState));
Expand Down Expand Up @@ -2397,9 +2422,8 @@ angle::Result FramebufferVk::syncState(const gl::Context *context,
gl::DrawBufferMask dirtyColorAttachments;
bool dirtyDepthStencilAttachment = false;

bool shouldUpdateColorMaskAndBlend = false;
bool shouldUpdateLayerCount = false;
bool shouldUpdateSrgbWriteControlMode = false;
bool shouldUpdateColorMaskAndBlend = false;
bool shouldUpdateLayerCount = false;

// Cache new foveation state, if any
const gl::FoveationState *newFoveationState = nullptr;
Expand Down Expand Up @@ -2435,7 +2459,6 @@ angle::Result FramebufferVk::syncState(const gl::Context *context,
releaseCurrentFramebuffer(contextVk);
break;
case gl::Framebuffer::DIRTY_BIT_FRAMEBUFFER_SRGB_WRITE_CONTROL_MODE:
shouldUpdateSrgbWriteControlMode = true;
break;
case gl::Framebuffer::DIRTY_BIT_DEFAULT_LAYERS:
shouldUpdateLayerCount = true;
Expand Down Expand Up @@ -2494,15 +2517,12 @@ angle::Result FramebufferVk::syncState(const gl::Context *context,
}
}

if (shouldUpdateSrgbWriteControlMode)
{
// Framebuffer colorspace state has been modified, so refresh the current framebuffer
// descriptor to reflect the new state.
gl::SrgbWriteControlMode newSrgbWriteControlMode = mState.getWriteControlMode();
mCurrentFramebufferDesc.setWriteControlMode(newSrgbWriteControlMode);
// mRenderPassDesc will be updated later in updateRenderPassDesc() in case if
// mCurrentFramebufferDesc was changed.
}
// A shared attachment's colospace could have been modified in another context, update
// colorspace of all attachments to reflect current context's colorspace.
gl::SrgbWriteControlMode srgbWriteControlMode = mState.getWriteControlMode();
updateColorAttachmentColorspace(srgbWriteControlMode);
// Update current framebuffer descriptor to reflect the new state.
mCurrentFramebufferDesc.setWriteControlMode(srgbWriteControlMode);

if (shouldUpdateColorMaskAndBlend)
{
Expand Down Expand Up @@ -2609,8 +2629,16 @@ void FramebufferVk::updateRenderPassDesc(ContextVk *contextVk)
}
else
{
mRenderPassDesc.packColorAttachment(
colorIndexGL, colorRenderTarget->getImageForRenderPass().getActualFormatID());
// Account for attachments with colorspace override
angle::FormatID actualFormat =
colorRenderTarget->getImageForRenderPass().getActualFormatID();
if (colorRenderTarget->hasColorspaceOverrideForWrite())
{
actualFormat =
colorRenderTarget->getColorspaceOverrideFormatForWrite(actualFormat);
}

mRenderPassDesc.packColorAttachment(colorIndexGL, actualFormat);
// Add the resolve attachment, if any.
if (colorRenderTarget->hasResolveAttachment())
{
Expand Down
1 change: 1 addition & 0 deletions src/libANGLE/renderer/vulkan/FramebufferVk.h
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ class FramebufferVk : public FramebufferImpl
void updateActiveColorMasks(size_t colorIndex, bool r, bool g, bool b, bool a);
void updateRenderPassDesc(ContextVk *contextVk);
angle::Result updateColorAttachment(const gl::Context *context, uint32_t colorIndex);
void updateColorAttachmentColorspace(gl::SrgbWriteControlMode srgbWriteControlMode);
angle::Result updateDepthStencilAttachment(const gl::Context *context);
void updateDepthStencilAttachmentSerial(ContextVk *contextVk);
angle::Result flushColorAttachmentUpdates(const gl::Context *context,
Expand Down
16 changes: 7 additions & 9 deletions src/libANGLE/renderer/vulkan/RenderTargetVk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,45 +183,43 @@ const vk::ImageHelper &RenderTargetVk::getResolveImageForRenderPass() const

angle::Result RenderTargetVk::getImageViewImpl(vk::Context *context,
const vk::ImageHelper &image,
gl::SrgbWriteControlMode mode,
vk::ImageViewHelper *imageViews,
const vk::ImageView **imageViewOut) const
{
ASSERT(image.valid() && imageViews);
vk::LevelIndex levelVk = image.toVkLevel(getLevelIndexForImage(image));
if (mLayerCount == 1)
{
return imageViews->getLevelLayerDrawImageViewWithSrgbWriteControlMode(
context, image, levelVk, mLayerIndex, mode, imageViewOut);
return imageViews->getLevelLayerDrawImageView(context, image, levelVk, mLayerIndex,
imageViewOut);
}

// Layered render targets view the whole level or a handful of layers in case of multiview.
return imageViews->getLevelDrawImageView(context, image, levelVk, mLayerIndex, mLayerCount,
mode, imageViewOut);
imageViewOut);
}

angle::Result RenderTargetVk::getImageView(vk::Context *context,
const vk::ImageView **imageViewOut) const
{
ASSERT(mImage);
return getImageViewImpl(context, *mImage, gl::SrgbWriteControlMode::Default, mImageViews,
imageViewOut);
return getImageViewImpl(context, *mImage, mImageViews, imageViewOut);
}

angle::Result RenderTargetVk::getImageViewWithColorspace(vk::Context *context,
gl::SrgbWriteControlMode mode,
const vk::ImageView **imageViewOut) const
{
ASSERT(mImage);
return getImageViewImpl(context, *mImage, mode, mImageViews, imageViewOut);
mImageViews->updateSrgbWiteControlMode(*mImage, mode);
return getImageViewImpl(context, *mImage, mImageViews, imageViewOut);
}

angle::Result RenderTargetVk::getResolveImageView(vk::Context *context,
const vk::ImageView **imageViewOut) const
{
ASSERT(mResolveImage);
return getImageViewImpl(context, *mResolveImage, gl::SrgbWriteControlMode::Default,
mResolveImageViews, imageViewOut);
return getImageViewImpl(context, *mResolveImage, mResolveImageViews, imageViewOut);
}

bool RenderTargetVk::isResolveImageOwnerOfData() const
Expand Down
23 changes: 22 additions & 1 deletion src/libANGLE/renderer/vulkan/RenderTargetVk.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,13 +174,34 @@ class RenderTargetVk final : public FramebufferAttachmentRenderTarget
reset();
}

// Helpers to update rendertarget colorspace
void updateWriteColorspace(gl::SrgbWriteControlMode srgbWriteControlMode)
{
ASSERT(mImage && mImage->valid() && mImageViews);
mImageViews->updateSrgbWiteControlMode(*mImage, srgbWriteControlMode);
}
bool hasColorspaceOverrideForRead() const
{
ASSERT(mImage && mImage->valid() && mImageViews);
return mImageViews->hasColorspaceOverrideForRead(*mImage);
}
bool hasColorspaceOverrideForWrite() const
{
ASSERT(mImage && mImage->valid() && mImageViews);
return mImageViews->hasColorspaceOverrideForWrite(*mImage);
}
angle::FormatID getColorspaceOverrideFormatForWrite(angle::FormatID format) const
{
ASSERT(mImage && mImage->valid() && mImageViews);
return mImageViews->getColorspaceOverrideFormatForWrite(format);
}

private:
void invalidateImageAndViews();
void reset();

angle::Result getImageViewImpl(vk::Context *context,
const vk::ImageHelper &image,
gl::SrgbWriteControlMode mode,
vk::ImageViewHelper *imageViews,
const vk::ImageView **imageViewOut) const;

Expand Down
20 changes: 16 additions & 4 deletions src/libANGLE/renderer/vulkan/RenderbufferVk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,16 @@ angle::Result RenderbufferVk::setStorageEGLImageTarget(const gl::Context *contex
mImageObserverBinding.bind(mImage);
mImageViews.init(renderer);

const vk::Format &vkFormat = renderer->getFormat(image->getFormat().info->sizedInternalFormat);
const angle::Format &textureFormat = vkFormat.getActualRenderableImageFormat();

VkImageAspectFlags aspect = vk::GetFormatAspectFlags(textureFormat);
// Update ImageViewHelper's colorspace related state
EGLenum imageColorspaceAttribute = image->getColorspaceAttribute();
if (imageColorspaceAttribute != EGL_GL_COLORSPACE_DEFAULT_EXT)
{
egl::ImageColorspace imageColorspace =
(imageColorspaceAttribute == EGL_GL_COLORSPACE_SRGB_KHR) ? egl::ImageColorspace::SRGB
: egl::ImageColorspace::Linear;
ASSERT(mImage != nullptr);
mImageViews.updateEglImageColorspace(*mImage, imageColorspace);
}

// Transfer the image to this queue if needed
if (mImage->isQueueFamilyChangeNeccesary(contextVk->getDeviceQueueIndex()))
Expand All @@ -208,6 +214,12 @@ angle::Result RenderbufferVk::setStorageEGLImageTarget(const gl::Context *contex
vk::CommandBufferAccess access;
access.onExternalAcquireRelease(mImage);
ANGLE_TRY(contextVk->getOutsideRenderPassCommandBuffer(access, &commandBuffer));

const vk::Format &vkFormat =
renderer->getFormat(image->getFormat().info->sizedInternalFormat);
const angle::Format &textureFormat = vkFormat.getActualRenderableImageFormat();
VkImageAspectFlags aspect = vk::GetFormatAspectFlags(textureFormat);

mImage->changeLayoutAndQueue(contextVk, aspect, vk::ImageLayout::ColorWrite,
contextVk->getDeviceQueueIndex(), commandBuffer);

Expand Down
Loading

0 comments on commit bffcd23

Please sign in to comment.