Add definitions about copy commands with textures by Jiawei-Shao · Pull Request #623 · gpuweb/gpuweb · GitHub

This patch adds several definitions that are required in the
validation rules of copy commands with textures.

the internal slots of a GPUTexture
texel block
texel block size
texel block width
texel block height
the physical size of a texture subresource
GPUTextureCopyView

This patch also adds a sub-title for GPUImageBitmapCopyView to
keep it in the same style as GPUBufferCopyView.

💥 Error: 500 Internal Server Error 💥

PR Preview failed to build. (Last tried on Mar 23, 2020, 9:25 PM UTC).

More

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

🚨 HTML Diff Service - The HTML Diff Service is used to create HTML diffs of the spec changes suggested in a pull request.

🔗 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 
 sysreq@w3.org 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>
</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.

This patch adds several definitions that are required in the validation rules of copy commands with textures. - the internal slots of a GPUTexture - texel block - texel block size - texel block width - texel block height - the physical size of a texture subresource

The definitions look great, I have some comments about nomenclature.

this is unclear. One "image" is an array layer here

I've made some changes here. PTAL, thanks!

In a way I think we should define what a texel block is, with a definition that explains that regular format have a texel block size of 1. Then we can define pixel-based formats (1x1 blocks), and block-based formats (other block sizes). The discussion of compressed formats would only be in examples.

Then a lot of the discussion can only talk about block sizes in general, and have notes for what happens for pixel-based formats (i.e. no extra padding).

Basically the spec would be written as if all formats are block-based, but would explain what happens in the usual case when the block size is 1x1.

The definition of "texel block" is in the Chapter "Texture Formats", which I think is a better place for it. What do you think?

It's probably okay if the definition of texel block is in Texture Formats but we reference it from here. The important comment here, though, is about rewording everything so that there's no "pixel-based" vs "block-based" format distinction. It allows us to write a lot of spec text without mentioning the difference at all, which simplifies it and makes it more readable.

The error message of the build failure is not related to this patch ("bikeshed requires Python '>=3.7' but the running Python is 2.7.15"). PTAL, thanks!

nit: texel block byte size, otherwise it sounds like (width, height)

LGTM, I don't want to block this further and would be happy to make edits afterwards.

Agreed, we can follow-up, no need to block further.

"compressed block of textures"?

"texel", "element", "pixel" - are all referring to the same thing...

Editing nits

Suggested change

      
            [=subresource=] in texels that includes the possible extra paddings to form complete [=texel blocks=] in the
          
            [=subresource=] in texels that may include padding to form complete [=texel blocks=] in the

Suggested change

      
                used in the sampling hardwares.
          
                used in sampling hardware.

Suggested change

      
                have paddings.
          
                have padding.

Suggested change

      
            The <dfn dfn>texel block</dfn> is a single addressable element of the textures in pixel-based {{GPUTextureFormat}}s,
          
            The <dfn dfn>texel block</dfn> is a single addressable element of textures with pixel-based {{GPUTextureFormat}}s,

Suggested change

      
            and a single compressed block of the textures in block-based compressed {{GPUTextureFormat}}s.
          
            and a single compressed block of textures with block-based compressed {{GPUTextureFormat}}s.

editors meeting resolution: we'll merge this and do revisions in follow-up.

Please rebase after #633

Pushed a merge so we can get this landed.

* Define multiple concepts about copy commands with textures This patch adds several definitions that are required in the validation rules of copy commands with textures. - the internal slots of a GPUTexture - texel block - texel block size - texel block width - texel block height - the physical size of a texture subresource * Address reviewer's feedbacks * Format the example in the "physical size" section * Address more comments from reviewers

…#623) When we create a view for a particular view dimension type from a texture, the texture's dimension type should be compatible with the view's dimension type.

Jiawei-Shao changed the title Define multiple concepts about copy commands with textures Add definitions about copy commands with textures Mar 18, 2020

Kangz reviewed Mar 18, 2020

View reviewed changes

Comment thread spec/index.bs Outdated

Kangz reviewed Mar 18, 2020

View reviewed changes

Comment thread spec/index.bs Outdated

Kangz reviewed Mar 18, 2020

View reviewed changes

Comment thread spec/index.bs Outdated

Kangz reviewed Mar 18, 2020

View reviewed changes

Comment thread spec/index.bs Outdated

Kangz reviewed Mar 18, 2020

View reviewed changes

kvark requested changes Mar 18, 2020

View reviewed changes

Jiawei-Shao added 2 commits March 20, 2020 16:51

Address reviewer's feedbacks

2aec456

Format the example in the "physical size" section

f31f11b

Kangz reviewed Mar 20, 2020

View reviewed changes

Comment thread spec/index.bs Outdated

Kangz reviewed Mar 20, 2020

View reviewed changes

Comment thread spec/index.bs Outdated

Kangz reviewed Mar 20, 2020

View reviewed changes

kvark reviewed Mar 20, 2020

View reviewed changes

Comment thread spec/index.bs Outdated

Address more comments from reviewers

ac1ad30

Kangz reviewed Mar 23, 2020

View reviewed changes

kvark approved these changes Mar 23, 2020

View reviewed changes

kvark reviewed Mar 23, 2020

View reviewed changes

JusSn reviewed Mar 23, 2020

View reviewed changes

Merge branch 'master' into add-definition-for-copy

ddcecac

kainino0x merged commit 29138ee into gpuweb:master Mar 23, 2020

Jiawei-Shao deleted the add-definition-for-copy branch March 24, 2020 00:07

magcius mentioned this pull request Aug 1, 2021

Why do block-compressed textures have to be aligned to a multiple of the texel block size? #2006

Open

+              A {{GPUTextureCopyView}} is a view of a sub-region of a [=texture=] [=subresource=] with the initial offset {{GPUOrigin3D} in
+              texels, used when copying data between two [=texture=]s, from a [=buffer=] to a [=texture=] and from a [=texture=] to a [=buffer=].
+                - For {{GPUTextureDimension/2d}} textures, data is copied between one [=image=] and one [=array layer=].

               are considered to have {{GPUTextureUsage/OUTPUT_ATTACHMENT}}
               for the [=usage scope=] of this render pass.
+              The <dfn dfn>physical size</dfn> of a {{GPUTexture}} [=subresource=] is the dimension of the {{GPUTexture}}

+                - For block-based compressed {{GPUTextureFormat}}s, the [=texel block width=] is the number of texels in each row of one [=texel block=],
+                  and the [=texel block height=] is the number of texel rows in one [=texel block=].
+              The <dfn dfn>texel block size</dfn> of a {{GPUTextureFormat}} is the number of bytes to store one [=texel block=].

               should follow the convention here, with the texture name as a prefix. e.g.
               `etc2-rgba8unorm`.
+              The <dfn dfn>texel block</dfn> is a single addressable element of the textures in pixel-based {{GPUTextureFormat}}s,

	[=subresource=] in texels that includes the possible extra paddings to form complete [=texel blocks=] in the
	[=subresource=] in texels that may include padding to form complete [=texel blocks=] in the

+              [=subresource=].
+                - For pixel-based {{GPUTextureFormat}}s, the [=physical size=] is always equal to the size of the [=subresource=]
+                  used in the sampling hardwares.

	The <dfn dfn>texel block</dfn> is a single addressable element of the textures in pixel-based {{GPUTextureFormat}}s,
	The <dfn dfn>texel block</dfn> is a single addressable element of textures with pixel-based {{GPUTextureFormat}}s,

	and a single compressed block of the textures in block-based compressed {{GPUTextureFormat}}s.
	and a single compressed block of textures with block-based compressed {{GPUTextureFormat}}s.

Add definitions about copy commands with textures by Jiawei-Shao · Pull Request #623 · gpuweb/gpuweb · GitHub

Uh oh!

Add definitions about copy commands with textures - #623

Add definitions about copy commands with textures#623kainino0x merged 5 commits intogpuweb:mastergpuweb/gpuweb:masterfrom Jiawei-Shao:add-definition-for-copyJiawei-Shao/gpuweb:add-definition-for-copyCopy head branch name to clipboard

Conversation

Jiawei-Shao commented Mar 18, 2020 • edited by pr-preview Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💥 Error: 500 Internal Server Error 💥

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Kangz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Jiawei-Shao commented Mar 23, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kangz left a comment

Choose a reason for hiding this comment

Uh oh!

kvark left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JusSn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kainino0x commented Mar 23, 2020

Uh oh!

kdashg commented Mar 23, 2020

Uh oh!

kainino0x commented Mar 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Add definitions about copy commands with textures#623
kainino0x merged 5 commits into
gpuweb:masterfrom
Jiawei-Shao:add-definition-for-copy

Jiawei-Shao commented Mar 18, 2020 •
edited by pr-preview Bot
Loading