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.

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

Handling of SRGB vertex formats is confusing #2214

Open
etang-cw opened this issue Sep 7, 2023 · 6 comments
Open

Handling of SRGB vertex formats is confusing #2214

etang-cw opened this issue Sep 7, 2023 · 6 comments

Comments

@etang-cw
Copy link

etang-cw commented Sep 7, 2023

Section 21.3.1 of the spec makes no mention of running an sRGB to linear conversion for SRGB formats. The CTS seems to take this as an indication that SRGB vertex formats should be treated identically to UNORM formats. MoltenVK and RADV both support SRGB vertex buffers, and I assume have implemented them in this way (confirmed for MoltenVK).

On the other hand, the VkFormat description makes no mention of this special handling, and states that the formats are all "stored with sRGB nonlinear encoding". In addition, I think most people would expect that the same color data with the same format would come up the same regardless of whether you loaded it from a texel buffer or a vertex attribute.

I think one of the following changes would improve clarity:

  • Update VkFormat's docs to specifically mention that SRGB formats are treated as linear when used with vertex attributes
  • Update 21.3.1 to say that sRGB to linear conversion should happen on vertex buffers, and update CTS accordingly
@Tobski
Copy link
Contributor

Tobski commented Sep 13, 2023

Thanks for your feedback! We'll have a look at this, it does seem that treating it as UNORM is what the spec states, we're having a look at our options and will let you know when we figure what we want to do.

@HansKristian-Work
Copy link
Contributor

RADV both support SRGB vertex buffers

does it?

I've tried a bunch of implementations on desktop and I've found no driver that supports RGBA8_SRGB vertex buffers.

  • RADV
  • AMD proprietary Linux/Windows
  • NV Linux/Windows
  • Intel Mesa/Windows

Looking at RADV source, there's an explicit "if srgb return false" in VBO feature query.

@HansKristian-Work
Copy link
Contributor

Tried looking at CTS for this and there is
dEQP-VK.pipeline.monolithic.vertex_input.single_attribute.vec4.as_r8g8b8a8_srgb_rate_vertex and friends, but I cannot figure out what it's actually trying to test.

@etang-cw
Copy link
Author

etang-cw commented Sep 18, 2023

does it?

I was going off https://vulkan.gpuinfo.org/listdevicescoverage.php?bufferformat=R8G8B8A8_SRGB&featureflagbit=VERTEX_BUFFER&platform=linux for that one, the only one I've actually confirmed support for is MoltenVK

Edit: Looking more closely, it looks like it's only actually supported by some older version of RADV, which makes it show up in the list even though newer drivers don't support it.

And yeah, that's the test that seems to verify that R8G8B8A8_SRGB loads identically to _UNORM (and failed when I accidentally made MoltenVK load it as sRGB)

@HansKristian-Work
Copy link
Contributor

HansKristian-Work commented Sep 25, 2023

the sRGB vertex format couldn't be captured in RDoc (probably blocks sRGB vertex support), but the UNORM test could be captured.

inputs

I think this means it's indeed trying values that are not 0 or 1, so it should at least be sensitive to sRGB decode happening or not.

@HansKristian-Work
Copy link
Contributor

Found the snippet that generates GLSL and UNORM and sRGB have the same code. There is no sRGB curve applied to verification here, so I think the CTS intends for sRGB to be decoded as UNORM for attributes, indeed.

			else if (isVertexFormatUnorm(attributeInfo.vkType) || isVertexFormatSRGB(attributeInfo.vkType))
			{
				const float representableDiff = isVertexFormatPacked(attributeInfo.vkType) ? getRepresentableDifferenceUnormPacked(attributeInfo.vkType, orderNdx) : getRepresentableDifferenceUnorm(attributeInfo.vkType);

				if (isVertexFormatPacked(attributeInfo.vkType))
					glslCode << indentStr << "if (abs(" << accessStr << " - " << "clamp((" << representableDiff << " * (" << totalComponentCount << ".0 * float(" << indexId << ") + " << componentIndex << ".0)), 0.0, 1.0)) < " << threshold[orderNdx] << ")\n";
				else
					glslCode << indentStr << "if (abs(" << accessStr << " - " << "(" << representableDiff << " * (" << totalComponentCount << ".0 * float(" << indexId << ") + " << componentIndex << ".0))) < " << threshold[rowNdx] << ")\n";
			}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants