Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[naga] Remove uses of Handle::index for identifier generation from backends #5845

Merged
merged 4 commits into from
Jun 21, 2024

Conversation

jimblandy
Copy link
Member

@jimblandy jimblandy commented Jun 20, 2024

Remove all uses of Handle::index for generating identifiers from Naga backends.

This is a step towards making Handle::index less prominent in the code base. Eventually, I'd like to use well-typed collections for everything, and have Handle::index be private to arena.rs.

  • [naga msl-out] Use Handle::write_prefixed instead of index.

    This replaces all uses of Handle::index for generating identifiers
    in naga::back::msl with uses of Handle::write_prefixed.

    There are still some uses of Handle::index remaining.

  • [naga dot-out] Use Handle::write_prefixed instead of index.

    This replaces all uses of Handle::index in naga::back::dot with
    uses of Handle::write_prefixed.

  • [naga hlsl-out] Use Baked for baked expressions.

    Make the HLSL backend more like other backends by using back::Baked
    to generate names for baked expression identifiers. This removes the
    final uses of Handle::index from the HLSL backend.

    This is separated out from the previous commit because it changes lots
    of snapshot tests, whereas the previous commit has no effect on Naga's
    output.

  • [naga] Introduce Baked newtype for writing baked expression names.

    Introduce a newtype naga::back::Baked that wraps a
    Handle<Expression> and formats with std::fmt::Display as a baked
    expression identifier. Use this in all backends for generating baked
    identifiers.

    Delete BAKE_PREFIX, as it's no longer used outside of Baked's
    Display implementation.

Introduce a newtype `naga::back::Baked` that wraps a
`Handle<Expression>` and formats with `std::fmt::Display` as a baked
expression identifier. Use this in all backends for generating baked
identifiers.

Delete `BAKE_PREFIX`, as it's no longer used outside of `Baked`'s
`Display` implementation.

This is a step towards making `Handle::index` less prominent in the
code base.
Make the HLSL backend more like other backends by using `back::Baked`
to generate names for baked expression identifiers. This removes the
final uses of `Handle::index` from the HLSL backend.

This is separated out from the previous commit because it changes lots
of snapshot tests, whereas the previous commit has no effect on Naga's
output.
This replaces all uses of `Handle::index` in `naga::back::dot` with
uses of `Handle::write_prefixed`.
@jimblandy jimblandy requested a review from a team as a code owner June 20, 2024 01:40
This replaces all uses of `Handle::index` for generating identifiers
in `naga::back::msl` with uses of `Handle::write_prefixed`.

There are still some uses of `Handle::index` remaining.
@jimblandy jimblandy added area: naga back-end Outputs of naga shader conversion naga Shader Translator kind: refactor Making existing function faster or nicer labels Jun 20, 2024
Copy link
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.

Nice!

@teoxoy teoxoy merged commit beb89f7 into gfx-rs:trunk Jun 21, 2024
25 checks passed
@jimblandy jimblandy deleted the handle-index-private branch June 21, 2024 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: naga back-end Outputs of naga shader conversion kind: refactor Making existing function faster or nicer naga Shader Translator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants