Sitelet https://github.com/gpuweb/gpuweb/pull/2659
Skip to content

Make rg11b10ufloat RENDER_ATTACHMENT:yes and resolve:yes. - #2659

Closed
kdashg wants to merge 1 commit into
gpuweb:mainfrom
kdashg:rg11b10-renderable
Closed

kdashg wants to merge 1 commit into
gpuweb:mainfrom
kdashg:rg11b10-renderable

Conversation

@kdashg

@kdashg kdashg commented Mar 10, 2022 •

Copy link
Copy Markdown
Contributor

RENDER_ATTACHMENT:yes leaves out Adreno 5xx Vulkan, but it's probably
worth it to make it core, and work on polyfilling there after v1
release.

Satisfies #2648.


💥 Error: 500 Internal Server Error 💥

PR Preview failed to build. (Last tried on Mar 10, 2022, 12:34 AM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 CSS Spec Preprocessor - CSS Spec Preprocessor is the web service used to build Bikeshed specs.

🔗 Related URL

<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>500 Internal Server Error</title>
</head><body>
<h1>Internal Server Error</h1>
<p>The server encountered an internal error or
misconfiguration and was unable to complete
your request.</p>
<p>Please contact the server administrator at 
 [no address given] to inform them of the time this error occurred,
 and the actions you performed just before this error.</p>
<p>More information about this error may be available
in the server error log.</p>
<hr>
<address>Apache/2.4.10 (Debian) Server at api.csswg.org Port 443</address>
</body></html>

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

RENDER_ATTACHMENT:yes leaves out Adreno 5xx Vulkan, but it's probably
worth it to make it core, and work on polyfilling there after v1
release.

Satisfies gpuweb#2648.
@github-actions

Copy link
Copy Markdown
Contributor

Previews, as seen when this build job started (e96f021):
WebGPU | IDL
WGSL
Explainer

@kainino0x

Copy link
Copy Markdown
Contributor

If we're going the route of emulating/polyfilling this on Adreno 500 devices, we'll need to be confident that we can do so. I think it can be done in either of the ways we discussed, but there are a lot of details.

  • Actually back it with rgba16float, emulate copies with compute/fragment shaders
  • Create a temporary rgba16float when it's used as a render target, and resolve into the real texture at the end of a render pass

Given the complexity, still not sure we should try to land this in the spec this before 1.0, but I'm tentatively in favor of putting it in core if we can manage.

@kdashg

kdashg commented Mar 16, 2022

Copy link
Copy Markdown
Contributor Author
WebGPU meeting minutes 2022-03-16
  • CW: my understanding - some sort of emulation on Adreno 500 and below. Either emulating copies, or allocating temporary render targets to render into and then recompress.
  • So far, avoided doing this with textures. This one's special because:
  • (also we'll have feature levels in the future - e.g. float32 formats not filterable for some reason. we might have float32 blendable, this other thing renderable, etc.)
  • If we make these features, we don't need as much implementation for V1.
  • MM: general question of what should be polyfilled & not - if browser didn't polyfill it, what should web app do instead? If app using fatter textures, indicates we can do that for them. If they'd need to rewrite application…
  • MM: app can use small or big texture. Imagine both are possible. Which one to use, if their data fit into both of them? How to choose which to use? If they're under memory pressure, for example. We can do a better job for them than the web app.
  • KN: last bit I don't agree with. If data fits into both, you'll always want to choose the smaller one. App would say - if this is available I'll use it, otherwise this other one. Not a complex decision. Think it's OK for this to be a feature.
  • KN: this one specifically - what do we do when it's a really simple thing that we don't have because of e.g. Adreno 500? This one happens to be polyfillable, so some appeal to doing that.
  • KN: also possibility that we drop Adreno 500 from core. Not sure if we should, but it's getting old.
  • KG: one position - if we have to do this polyfill it's not exciting to do before V1. We're producing V1 spec / feature level - we don't have to have Adreno 500 necessarily as part of the first release. (can add support in browsers for adreno500 in subsequent releases)
  • MS: one example - only time we use wider float16 is if the packed format isn't available, or on debug builds.
  • MM: because debug tools work better with wider textures?
  • MS: that was a decision made before I joined. :) not sure why.
  • CW: thanks mrshannon, good data point. If we can support this feature in core, something in the back of our minds is still WebGPU-Compat. Subset, but not complete core WebGPU. Might not implement WebGPU core on Adreno 500 then, and remove this packed format from the features.
  • MM: sounds like, eventually, this polyfill could be written. And then it would allow Adreno 500 to support core. We have some precedent with depth24+ format to use wider textures when platform doesn't have support for the smaller version.
  • CW: w.r.t. not supporting core WebGPU - many things you can polyfill - how much effort do you want to go to? What performance do you want? This format might be too much. w.r.t. depth24+ - explicitly specified as 23+ bits of mantissa.
  • MM: if we change spec language would that alleviate concerns?
  • KG: are you suggesting RG11B10F+?
  • CW: no. :) depth24+ formats are wonky - lot of strange limitations. Just copies if we had this format, would have to be removed or explicitly emulated in the spec.
  • KN: think we could add that RG11B10+ format that isn't copyable. Not sure it satisfies needs. Reason depth24+ is way it is - some platforms have one, some the other - nothing every platform has. There's nothing forcing us to have the "+" option like we had to with depth.
  • KG: goal here - floating the idea of having this in core. If we can't do it - that's fine. Think we're close to being able to. Comes down to, how much do we want to ship V1 on Adreno 500?
  • MM: I think the next step is to have member companies figure out how important this addition to the standard vis-a-vis adreno500 and v1 releases.
  • CW: we'll look at that. (and others can, too, if they're interested.) We'll come up with compat thing later anyway, and will definitely include Adreno 500 there.
  • MS: if you do drop Adreno 500, then firstInstanceIndirect wouldn't need to be a feature, either.
  • KN: we can do that query against GPUInfo, see if it looks the same.
  • MM: can it be polyfilled later? Here, this narrow format can be polyfilled later.
  • CW: unlikely to ever do that.
  • KG: think we should take a week and discuss internally.
  • MM: also - idea of this compat spec isn't something this group has a resolution on.

@mrshannon

mrshannon commented Mar 23, 2022 •

Copy link
Copy Markdown
Contributor

Some thoughts from my group:

  • We are for dropping Adreno 5xx, especially if it has other benefits (but our use case does not fit low end devices anyways).
  • We would prefer not to have the implementation polyfilled as we can do it ourselves more efficiently.

The reason for using rg11b10ufloat is memory size/bandwidth. If the implementation does the polyfill it uses more memory and hurts memory bandwidth more than if rgba16float was used from the start. This is why we are against the implementation doing the polyfill as it removes all the benefits of using rg11b10ufloat in the first place and makes it worse than just using rgba16float directly.

EDIT: If the implementation does the polyfill, we will likely (as an optimization step) attempt to "fingerprint" the devices which this happens on and use rgba16float where we think it's likely it is being polyfilled.

@kainino0x

Copy link
Copy Markdown
Contributor

According to #2648 we decided to go with a feature flag instead (done in #3170).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants