Add additional dstformat in copyExternalImageToTexture with Tier1 - #5299
Add additional dstformat in copyExternalImageToTexture with Tier1#5299mei1127 wants to merge 2 commits into
Conversation
Sorry, something went wrong.
|
Is it defined what the conversion is to snorm formats? |
Sorry, something went wrong.
Why not? It should have the same arithmetic results as copy to unorm or float - just different encodings happen at the end to get it into the texture. I am noticing that the definition currently links to equivalent texel representation though which is not appropriate (oops) - that's only worded to apply to copies that don't involve format conversions. In principle this is supposed to effectively just be a rendering operation, so I guess we should probably actually word the spec like it performs a render pass where the shader reads the value and writes it out from the fragment shader. |
Sorry, something went wrong.
| - {{GPUTextureFormat/"rgb10a2unorm"}} | ||
| - {{GPUTextureFormat/"rgba16float"}} | ||
| - {{GPUTextureFormat/"rgba32float"}} | ||
| or the below formats if {{GPUFeatureName/"texture-formats-tier1"}} is enabled: |
There was a problem hiding this comment.
This line is not needed, because it's implicitly necessary to have created the destination texture (with RENDER_ATTACHMENT usage) in the first place.
Sorry, something went wrong.
| - {{GPUTextureFormat/"r8snorm"}} | ||
| - {{GPUTextureFormat/"rg8snorm"}} | ||
| - {{GPUTextureFormat/"rgba8snorm"}} | ||
| - {{GPUTextureFormat/"rg11b10ufloat"}} |
There was a problem hiding this comment.
Both (1) whether we do this and (2) whether we replace this list with a simple "must be a renderable color format supporting "unfilterable-float" sample type" are both TBD. I am marking this PR as draft until the working group has discussed it.
Sorry, something went wrong.
|
PRs should link to the relevant issue. I have added a link to #5289 in the PR summary. |
Sorry, something went wrong.
Given the current spec, I would expect loading normal (non-gainmap) jpg or png into a rgba8snorm texture throws away half the precision and all you get is values from 0 to 127 (the binary data in the texture). Values from 0.0 to 1.0. I suspect no one wants that. Rather, they'd like to load a normal map into rgba8snorm and get values from -1.0 to 1.0. If we're sticking to the "it only loads values from 0 to 1" current spec, then I think we should not add the snorm formats at this time. |
Sorry, something went wrong.
|
Fair point. Seems if we wanted to support that we would need some extra metadata, like a "color space" that maps the source 0..1 to destination -1..1. Not sure if that's something we would want to do or just make apps deal with this themselves. |
Sorry, something went wrong.
mwyrzykowski
left a comment
There was a problem hiding this comment.
I share similar concerns as @greggman - it would seem this operation is perhaps not well defined for sNorm formats and unless someone has been asking for support, it would be preferable to exclude snorm formats.
Sorry, something went wrong.
|
If we just add it, it will be well defined per the existing spec, and IMO users wouldn't assume any other behavior. For example I would definitely be against defaulting to mapping 0..1 to -1..1 for snorm, that would be very surprising. My concern is more with it being useless and thus something we shouldn't spend time testing and implementing. |
Sorry, something went wrong.
Rather than have a list of allowed formats, the new spec allows all regular texture formats that are renderable and not compressed, not snorm, not integer. gpuweb/gpuweb#5289 gpuweb/gpuweb#5299 gpuweb/gpuweb#5323
|
Based on the changed WG resolution I have opened #5289 which incorporates this PR. |
Sorry, something went wrong.
…extures (#5333) Per [today's WG discussion](https://github.com/gpuweb/gpuweb/wiki/GPU-Web-2025%E2%80%9010-01#allow-copyexternalimagetotexture-for-all-renderable-formats-5323). Fixes #5289 Closes #5323 Closes #5299 by superseding it. (This PR is based on that one.) --------- Co-authored-by: shanxing <shanxing.mei@intel.com>
Rather than have a list of allowed formats, the new spec allows all regular texture formats that are renderable and not compressed, not snorm, not integer. gpuweb/gpuweb#5289 gpuweb/gpuweb#5299 gpuweb/gpuweb#5323
Rather than have a list of allowed formats, the new spec allows all regular texture formats that are renderable and not compressed, not snorm, not integer. gpuweb/gpuweb#5289 gpuweb/gpuweb#5299 gpuweb/gpuweb#5323
This commit adds additional destination.texture.format in copyExternalImageToTexture function with texture-formats-tier1 enabled. These additional formats are renderable with the feature.
Issue: #5289