Add the "discard-to-zero" store op.#358
Conversation
spec/index.bs
Outdated
| enum GPUStoreOp { | ||
| "store" | ||
| "store", | ||
| "discard-to-zero" |
There was a problem hiding this comment.
I would rather have either "discard" or "zero", but we definitely need this for efficient multisampling.
There was a problem hiding this comment.
I remember some other options too, "invalidate", "mark-cleared"/"mark-zeroed", and then if we disallowed using uninitialized resources, "discard" or "invalidate"
There was a problem hiding this comment.
+1 for preferring "discard"
What's the difference between "discard" and "discard-to-zero"?
There was a problem hiding this comment.
There is no true "discard" as it implies that it is zero-cost and the resource is uninitialized afterward, when in fact it is zeroed (possibly lazily) (like it would be immediately after being created).
There was a problem hiding this comment.
Seeing this begs the question of ""where's discard". What's the reason we don't want to allow a true Discard operation? Shouldn't the developers be in control over exactly when the Clear happens (and to what value)?
There was a problem hiding this comment.
Good point, @Kangz !
To provide some more context to @damyanp - the problem with (2) was that it would limit the ability of the backend to handle this internally, since the API would have to be conservatively defined. For example, if we copy 2 texel rects into a texture, one for the bottom half and one for the top half, it's tricky to write specification in such a way that this is explicitly allowed.
With this in mind, (1) was considered the only viable option.
There was a problem hiding this comment.
Option 2 still seems the better option to me. Is the issue with the conservative spec that you'd like to be able to partially initialize a resource? I'd be fine with prohibiting this and stating that initialization is either a clear or a full copy. I'll concede on this though, I've had my say :)
I will add thought that forcing users to read "discard-to-zero" when then mental model should really just be "discard" seems like unnecessary overhead. A casual developer who is used to using other APIs will need to read the docs to understand what all the "to-zero" stuff is about. Whereas someone who has thought deeply enough to be concerned about the possible implications of a raw "discard" operation would need to read the docs. I'd expect that the former would outnumber the latter.
There was a problem hiding this comment.
Keep in mind if we ship the conservative spec today, we can always add a "discard" with looser semantics tomorrow, but if we decide that "discard" was too loose, we won't really add anything with a "discard-to-zero" since we still have to support "discard".
There was a problem hiding this comment.
My favorite plan for this is that "discard" is meant to yield undefined data, even if initial implementations may clear it.
What I don't want, even today, is to entice apps into using "discard-to-zero" to clear things. We should encourage explicit clearing, not reliance on side-effects of today's restricted behavior.
There was a problem hiding this comment.
We already discussed this as part of issue #97. The consensus at the time was to call the StoreOp discard-to-zero.
@jdashg , if we want prevent people from forming the bad habit of using discard-to-zero to clear things, I think the best we can do is spec things such that the user is required to fill every byte of a discarded texture/buffer with data before they can use it in the pipeline. Doing this will require more work from implementations that wish to cut corners and eagerly clear discarded (or newly created) resources that the developer fills in multiple steps.
|
Rebased over #365 |
damyanp
left a comment
There was a problem hiding this comment.
I think that this should be "discard".
|
I won't hold this up, but I think discard-to-zero is the wrong direction. |
|
Discussed in the 2019-07-22 meeting. Resolution: rename to "clear", add a non-normative "Note:" that it's expected to be faster than a real clear. |
|
Landing this with "clear" as was resolved in the call. |
Preview | Diff