mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-09-07 19:58:32 +09:00
[Fix] (MG_Backend): read back the stencil half, and clear an sRGB target to the value asked for
Two reasons a framebuffer's contents came back wrong, both on the read/clear side rather than the write side. Stencil, on both backends. The CTS reads stencil with glReadPixels(GL_STENCIL_INDEX, GL_INT), which is as legal as the unsigned widths, and neither backend accepted it: DirectGLES's ReadPixelsStencilViaNative rejected every signed type, after which the call fell through to a native ES read the driver refuses and nothing was written at all, so the caller kept its zeros; DirectVulkan's pack switch had no GL_INT case, and of the cases it did have only GL_UNSIGNED_INT sourced the stencil plane - GL_FLOAT and GL_UNSIGNED_SHORT emitted a depth value, which is meaningless for a stencil-only image. Both now take the signed and float widths, and DirectVulkan decides "this is a stencil read" once rather than per type. DirectGLES also gains the GL_FLOAT_32_UNSIGNED_INT_24_8_REV fallback a DEPTH32F_STENCIL8 attachment needs, which rejects the 24_8 packed type. sRGB, on DirectVulkan. Every other write path goes through the UNORM twin view while GL_FRAMEBUFFER_SRGB is off, storing the raw value GL asked for, but a deferred clear is materialised with vkCmdClearColorImage - which names the image, so the driver applied the sRGB transfer function and a clear to 0.25 landed at 0.537. PreCompensateSrgbClearColor hands it the linear colour whose encoding is the requested value instead. It is a no-op for non-sRGB destinations, for integer clear encodings, and when GL_FRAMEBUFFER_SRGB is on and GL really does want the encode. Takes renderbuffers_storage from failing to passing on both backends, plus renderbuffers_storage_multisample and framebuffers_blit on Espryt.
This commit is contained in:
@@ -4566,14 +4566,27 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
Vector<Uint32> packed(outStencil.size(), 0);
|
||||
ClearGLErrors();
|
||||
g_GLESFuncs.glReadPixels(x, y, width, height, GL_DEPTH_STENCIL, GL_UNSIGNED_INT_24_8, packed.data());
|
||||
if (g_GLESFuncs.glGetError() == GL_NO_ERROR) {
|
||||
for (SizeT i = 0; i < outStencil.size(); ++i) {
|
||||
outStencil[i] = static_cast<Uint8>(packed[i] & 0xFFu);
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
// A DEPTH32F_STENCIL8 attachment rejects the 24_8 type: its packed layout is a float depth
|
||||
// followed by a padded stencil byte, eight bytes per pixel with the index at offset 4.
|
||||
Vector<Uint8> packed32f(outStencil.size() * 8u, 0);
|
||||
ClearGLErrors();
|
||||
g_GLESFuncs.glReadPixels(x, y, width, height, GL_DEPTH_STENCIL, GL_FLOAT_32_UNSIGNED_INT_24_8_REV,
|
||||
packed32f.data());
|
||||
const GLenum packedError = g_GLESFuncs.glGetError();
|
||||
if (packedError != GL_NO_ERROR) {
|
||||
MGLOG_E("ReadPixels: neither GL_STENCIL_INDEX nor GL_DEPTH_STENCIL readback is available: %s",
|
||||
MGLOG_E("ReadPixels: no stencil readback path is available: %s",
|
||||
MG_Util::ConvertGLEnumToString(packedError).c_str());
|
||||
return false;
|
||||
}
|
||||
for (SizeT i = 0; i < outStencil.size(); ++i) {
|
||||
outStencil[i] = static_cast<Uint8>(packed[i] & 0xFFu);
|
||||
outStencil[i] = packed32f[i * 8u + 4u];
|
||||
}
|
||||
return true;
|
||||
}
|
||||
@@ -4583,11 +4596,20 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
// values themselves are always 8 bits.
|
||||
static Bool ReadPixelsStencilViaNative(GLint x, GLint y, GLsizei width, GLsizei height, GLenum type,
|
||||
void* pixels) {
|
||||
// GL 4.6 core 18.2.8: a stencil index is written unconverted into whichever integer width
|
||||
// the client asked for, and converted to a float value for GL_FLOAT. The signed widths are
|
||||
// as legal as the unsigned ones - the CTS reads stencil with GL_INT - and rejecting them
|
||||
// here used to let the call fall through to a native ES read the driver refuses, after
|
||||
// which nothing was written at all and the caller kept its zeros.
|
||||
SizeT dstPixelBytes = 0;
|
||||
switch (type) {
|
||||
case GL_UNSIGNED_BYTE: dstPixelBytes = sizeof(Uint8); break;
|
||||
case GL_UNSIGNED_SHORT: dstPixelBytes = sizeof(Uint16); break;
|
||||
case GL_UNSIGNED_INT: dstPixelBytes = sizeof(Uint32); break;
|
||||
case GL_UNSIGNED_BYTE:
|
||||
case GL_BYTE: dstPixelBytes = sizeof(Uint8); break;
|
||||
case GL_UNSIGNED_SHORT:
|
||||
case GL_SHORT: dstPixelBytes = sizeof(Uint16); break;
|
||||
case GL_UNSIGNED_INT:
|
||||
case GL_INT: dstPixelBytes = sizeof(Uint32); break;
|
||||
case GL_FLOAT: dstPixelBytes = sizeof(GLfloat); break;
|
||||
default: return false;
|
||||
}
|
||||
if (width <= 0 || height <= 0) {
|
||||
@@ -4621,11 +4643,16 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
for (GLsizei col = 0; col < width; ++col) {
|
||||
switch (type) {
|
||||
case GL_UNSIGNED_BYTE:
|
||||
case GL_BYTE:
|
||||
rowBuf[static_cast<SizeT>(col)] = srcRow[col];
|
||||
break;
|
||||
case GL_UNSIGNED_SHORT:
|
||||
case GL_SHORT:
|
||||
reinterpret_cast<Uint16*>(rowBuf.data())[col] = srcRow[col];
|
||||
break;
|
||||
case GL_FLOAT:
|
||||
reinterpret_cast<GLfloat*>(rowBuf.data())[col] = static_cast<GLfloat>(srcRow[col]);
|
||||
break;
|
||||
default:
|
||||
reinterpret_cast<Uint32*>(rowBuf.data())[col] = srcRow[col];
|
||||
break;
|
||||
|
||||
@@ -8,9 +8,13 @@
|
||||
|
||||
#include "VkClearManager.h"
|
||||
|
||||
#include "MG_State/GLState/Core.h"
|
||||
#include "MG_Util/Converters/MGToStr/FramebufferEnumConverter.h"
|
||||
#include "MG_Util/Converters/MGToStr/TextureEnumConverter.h"
|
||||
|
||||
#include <algorithm>
|
||||
#include <cmath>
|
||||
|
||||
namespace MobileGL::MG_Backend::DirectVulkan {
|
||||
static Bool IsCubeMapFaceUploadTarget(TextureUploadTarget target) {
|
||||
return target >= TextureUploadTarget::CubeMapPositiveX &&
|
||||
@@ -42,6 +46,23 @@ namespace MobileGL::MG_Backend::DirectVulkan {
|
||||
return clearValue;
|
||||
}
|
||||
|
||||
void PreCompensateSrgbClearColor(ClearAttachmentPayload& payload, VkFormat destinationFormat) {
|
||||
if (payload.colorEncoding != ClearColorEncoding::Float) return;
|
||||
// With GL_FRAMEBUFFER_SRGB enabled GL performs the encoding itself, so the driver doing it
|
||||
// is exactly right and there is nothing to undo.
|
||||
if (MG_State::pGLContext->IsCapabilityEnabled(MobileGL::CapabilityInput::FramebufferSrgb)) return;
|
||||
if (ResolveSrgbAttachmentWriteFormat(destinationFormat, false) == destinationFormat) return;
|
||||
|
||||
// sRGB -> linear (GL 4.6 core 8.24), applied to the colour channels only: alpha is stored
|
||||
// linearly in an sRGB format and must pass through untouched.
|
||||
const auto toLinear = [](Float encoded) {
|
||||
const Float value = std::clamp(encoded, 0.0f, 1.0f);
|
||||
return value <= 0.04045f ? value / 12.92f : std::pow((value + 0.055f) / 1.055f, 2.4f);
|
||||
};
|
||||
payload.color = FloatVec4(toLinear(payload.color.x()), toLinear(payload.color.y()),
|
||||
toLinear(payload.color.z()), payload.color.w());
|
||||
}
|
||||
|
||||
void ForceOpaqueClearAlpha(ClearAttachmentPayload& payload) {
|
||||
switch (payload.colorEncoding) {
|
||||
case ClearColorEncoding::Int:
|
||||
|
||||
@@ -50,6 +50,15 @@ namespace MobileGL::MG_Backend::DirectVulkan {
|
||||
// the destination is known.
|
||||
void ForceOpaqueClearAlpha(ClearAttachmentPayload& payload);
|
||||
|
||||
// vkCmdClearColorImage names the image, so the driver applies the destination format's transfer
|
||||
// function to whatever value it is handed. Every other write path in this backend goes through
|
||||
// the UNORM twin view while GL_FRAMEBUFFER_SRGB is off (ResolveSrgbAttachmentWriteFormat) and
|
||||
// therefore stores the raw value GL asked for. Rewrites `payload` to the linear colour whose
|
||||
// encoding is that raw value, so a direct image clear of an sRGB destination agrees with them.
|
||||
// A no-op for every other format, for integer clear encodings, and when GL is doing the
|
||||
// encoding itself.
|
||||
void PreCompensateSrgbClearColor(ClearAttachmentPayload& payload, VkFormat destinationFormat);
|
||||
|
||||
struct PendingClearKey {
|
||||
MG_State::GLState::ITextureObject* texture = nullptr;
|
||||
Uint64 textureLifetimeId = 0;
|
||||
|
||||
@@ -5914,9 +5914,10 @@ void main() {
|
||||
subresourceRange.baseArrayLayer = pendingClear.key.baseArrayLayer;
|
||||
subresourceRange.layerCount = pendingClear.key.layerCount;
|
||||
|
||||
const auto& clearPayload = pendingClear.payload;
|
||||
auto clearPayload = pendingClear.payload;
|
||||
if ((resource->aspect & VK_IMAGE_ASPECT_COLOR_BIT) != 0) {
|
||||
subresourceRange.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT;
|
||||
PreCompensateSrgbClearColor(clearPayload, resource->format);
|
||||
const VkClearColorValue clearValue =
|
||||
MakeVkClearColorValue(clearPayload, ColorFormatLacksAlpha(&texture));
|
||||
vkCmdClearColorImage(commandBuffer, resource->image, VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
|
||||
@@ -6000,6 +6001,7 @@ void main() {
|
||||
VkImageLayout steadyLayout;
|
||||
if ((resource->aspect & VK_IMAGE_ASPECT_COLOR_BIT) != 0) {
|
||||
subresourceRange.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT;
|
||||
PreCompensateSrgbClearColor(clearPayload, resource->format);
|
||||
// RGB renderbuffers are backed by an RGBA image; the missing alpha reads as 1.
|
||||
const VkClearColorValue clearValue = MakeVkClearColorValue(
|
||||
clearPayload,
|
||||
@@ -7632,13 +7634,16 @@ void main() {
|
||||
switch (type) {
|
||||
case GL_FLOAT:
|
||||
case GL_UNSIGNED_INT:
|
||||
case GL_INT:
|
||||
case GL_UNSIGNED_INT_24_8:
|
||||
dstPixelBytes = 4;
|
||||
break;
|
||||
case GL_UNSIGNED_SHORT:
|
||||
case GL_SHORT:
|
||||
dstPixelBytes = 2;
|
||||
break;
|
||||
case GL_UNSIGNED_BYTE:
|
||||
case GL_BYTE:
|
||||
dstPixelBytes = 1;
|
||||
break;
|
||||
case GL_FLOAT_32_UNSIGNED_INT_24_8_REV:
|
||||
@@ -7649,29 +7654,40 @@ void main() {
|
||||
return;
|
||||
}
|
||||
|
||||
// GL 4.6 core 18.2.8: a GL_STENCIL_INDEX read reports the index itself, unconverted, in
|
||||
// whatever width the client asked for. Only the packed types mix depth in. Deciding this
|
||||
// once - rather than per type, where GL_FLOAT and GL_UNSIGNED_SHORT used to emit a depth
|
||||
// value that is meaningless for a stencil-only image - is what makes the CTS's
|
||||
// (GL_STENCIL_INDEX, GL_INT) read return 7 instead of nothing.
|
||||
const Bool stencilOnly = format == GL_STENCIL_INDEX;
|
||||
|
||||
Vector<Uint8> packed(pixelCount * dstPixelBytes);
|
||||
for (SizeT i = 0; i < pixelCount; ++i) {
|
||||
Uint8* dst = packed.data() + i * dstPixelBytes;
|
||||
switch (type) {
|
||||
case GL_FLOAT: {
|
||||
const Float value = depthValueAt(i);
|
||||
const Float value = stencilOnly ? static_cast<Float>(stencilSrc[i]) : depthValueAt(i);
|
||||
Memcpy(dst, &value, sizeof(value));
|
||||
break;
|
||||
}
|
||||
case GL_UNSIGNED_SHORT: {
|
||||
const Uint16 value =
|
||||
static_cast<Uint16>(std::lround(static_cast<double>(depthValueAt(i)) * 65535.0));
|
||||
case GL_UNSIGNED_SHORT:
|
||||
case GL_SHORT: {
|
||||
const Uint16 value = stencilOnly
|
||||
? static_cast<Uint16>(stencilSrc[i])
|
||||
: static_cast<Uint16>(std::lround(static_cast<double>(depthValueAt(i)) * 65535.0));
|
||||
Memcpy(dst, &value, sizeof(value));
|
||||
break;
|
||||
}
|
||||
case GL_UNSIGNED_INT: {
|
||||
const Uint32 value = format == GL_STENCIL_INDEX
|
||||
case GL_UNSIGNED_INT:
|
||||
case GL_INT: {
|
||||
const Uint32 value = stencilOnly
|
||||
? stencilSrc[i]
|
||||
: static_cast<Uint32>(static_cast<double>(depthValueAt(i)) * 4294967295.0);
|
||||
Memcpy(dst, &value, sizeof(value));
|
||||
break;
|
||||
}
|
||||
case GL_UNSIGNED_BYTE: {
|
||||
case GL_UNSIGNED_BYTE:
|
||||
case GL_BYTE: {
|
||||
dst[0] = stencilSrc[i];
|
||||
break;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user