Skip to content

rename/deprecate block_size -> block_copy_size, improve docs#4647

Merged
Wumpf merged 8 commits intogfx-rs:trunkfrom
rerun-io:better-block-size
Nov 13, 2023
Merged

rename/deprecate block_size -> block_copy_size, improve docs#4647
Wumpf merged 8 commits intogfx-rs:trunkfrom
rerun-io:better-block-size

Conversation

@Wumpf
Copy link
Copy Markdown
Member

@Wumpf Wumpf commented Nov 7, 2023

Checklist

  • Run cargo clippy.
  • Run cargo clippy --target wasm32-unknown-unknown if applicable.
  • Add change to CHANGELOG.md. See simple instructions inside file.

Connections

Description
I keep seeing people getting confused about block_size, so I made its name more explicit and fixed the docs a bit (for instance the link in there was broken)

@Wumpf Wumpf requested a review from a team as a code owner November 7, 2023 09:17
@teoxoy
Copy link
Copy Markdown
Member

teoxoy commented Nov 7, 2023

texel_block_size has been renamed to texel_block_copy_footprint in the spec which is a lot clearer in what it means. Can we use that instead?

gpuweb/gpuweb#3922

@Wumpf
Copy link
Copy Markdown
Member Author

Wumpf commented Nov 7, 2023

Oh didn't notice, thank you! Let's go with that then and link to it ofc.
That's a pretty awkward name though imho. Also doesn't imply bytes -.-

@Wumpf
Copy link
Copy Markdown
Member Author

Wumpf commented Nov 7, 2023

actually.. linking it okay. But do we think anyone will ever find TextureFormat::block_copy_footprint if they're looking for a byte size of a texel?

@Wumpf
Copy link
Copy Markdown
Member Author

Wumpf commented Nov 7, 2023

Technically we are providing the informative value that is described in the spec as

The texel block memory cost of a GPUTextureFormat is the number of bytes needed to store one texel block. It is not fully defined for all formats. This value is informative and non-normative.

so from that pov we would need to call it block_memory_cost :/

@Wumpf
Copy link
Copy Markdown
Member Author

Wumpf commented Nov 7, 2023

moving (potential) discussion to Matrix

@teoxoy
Copy link
Copy Markdown
Member

teoxoy commented Nov 7, 2023

The texel block memory cost is only "informative and non-normative".
The texel block copy footprint is the useful value since it's needed to calculate some of the parameters of ImageDataLayout/ImageCopyBuffer.

@Wumpf Wumpf changed the title rename/deprecate block_size -> block_size_in_bytes, improve docs rename/deprecate block_size -> block_copy_size, improve docs Nov 11, 2023
teoxoy
teoxoy approved these changes Nov 13, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@teoxoy teoxoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Wumpf @teoxoy
Update CHANGELOG.md
b604a25 
Co-authored-by: Teodor Tanasoaia <28601907+teoxoy@users.noreply.github.com>
@Wumpf Wumpf enabled auto-merge (squash) November 13, 2023 18:56
@Wumpf Wumpf merged commit f742051 into gfx-rs:trunk Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@teoxoy teoxoy teoxoy approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Wumpf @teoxoy