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] Exercise GLSL double-precision builtin functions, and fix the fallout. #4684

Merged
merged 5 commits into from
Nov 15, 2023

Conversation

jimblandy
Copy link
Member

@jimblandy jimblandy commented Nov 14, 2023

Exercise GLSL double-precision builtin functions, and fix the fallout.

  • [naga] New tests for GLSL double-precision builtin functions.

  • [naga wgsl-out] Correctly include width in matrix constructions.

    When generating WGSL for an Expression::Compose constructing a
    matrix, consult TypeInner::Matrix::width when writing the
    type name in the construction expression, rather than just always
    writing matNxM<f32>.

    Fixes WGSL backend writes matNxM<f64> construction expressions incorrectly. #4681.

  • [naga glsl-in] Fix type of double ldexp's second arg.

    The second argument of the GLSL ldexp builtin is always a 32-bit
    integer or a vector of such, never a 64-bit integer. In
    inject_common_builtin, that argument's type should not be influenced
    by float_width.

    Fixes [naga glsl-in] double-precision ldexp overload expects a 64-bit integer argument #4680.

  • [naga glsl-in] Fix double overload for dot/reflect/distance/ldexp.

    The first argument of the dot, reflect, distance, and ldexp
    GLSL builtin functions may be either a float or a double, and thus the
    argument type registered by inject_common_builtin must depend on the
    float_width argument; it cannot simply be Scalar::F32.

    Introduced by [naga] Introduce Scalar type to IR. #4673.

Checklist

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

@jimblandy jimblandy requested a review from a team as a code owner November 14, 2023 18:03
@jimblandy jimblandy force-pushed the naga-glsl-fix-double-builtins branch from ee70980 to d956464 Compare November 14, 2023 18:04
Copy link
Member

@ErichDonGubler ErichDonGubler left a comment

Choose a reason for hiding this comment

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

LGTM, minus nits/questions.

naga/src/back/wgsl/writer.rs Show resolved Hide resolved
naga/tests/in/glsl/double-math-functions.frag Outdated Show resolved Hide resolved
naga/tests/in/glsl/double-math-functions.frag Show resolved Hide resolved
@jimblandy jimblandy force-pushed the naga-glsl-fix-double-builtins branch from 6cf836d to 37d0553 Compare November 15, 2023 03:58
The first argument of the `dot`, `reflect`, `distance`, and `ldexp`
GLSL builtin functions may be either a float or a double, and thus the
argument type registered by `inject_common_builtin` must depend on the
`float_width` argument; it cannot simply be `Scalar::F32`.

Introduced by gfx-rs#4673.
The second argument of the GLSL `ldexp` builtin is always a 32-bit
integer or a vector of such, never a 64-bit integer. In
`inject_common_builtin`, that argument's type should not be influenced
by `float_width`.

Fixes gfx-rs#4680.
When generating WGSL for an `Expression::Compose` constructing a
matrix, consult `TypeInner::Matrix::width` when writing the
type name in the construction expression, rather than just always
writing `matNxM<f32>`.

Fixes gfx-rs#4681.
@jimblandy jimblandy force-pushed the naga-glsl-fix-double-builtins branch from 37d0553 to 9081501 Compare November 15, 2023 04:00
@jimblandy jimblandy enabled auto-merge (rebase) November 15, 2023 04:00
@jimblandy jimblandy merged commit 611471c into gfx-rs:trunk Nov 15, 2023
27 checks passed
@jimblandy jimblandy deleted the naga-glsl-fix-double-builtins branch December 13, 2023 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants