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

Generics: Bevy use case #63

Open
mighdoll opened this issue Jan 7, 2025 · 3 comments
Open

Generics: Bevy use case #63

mighdoll opened this issue Jan 7, 2025 · 3 comments

Comments

@mighdoll
Copy link
Contributor

mighdoll commented Jan 7, 2025

from @pcwalton on the bevy discord:

The biggest thing I want is some way to simplify this
whether that's generics, macros, or some combination of those two

        pbr_input.material.base_color *=
#ifdef MESHLET_MESH_MATERIAL_PASS
            textureSampleGrad(
#else   // MESHLET_MESH_MATERIAL_PASS
            textureSampleBias(
#endif  // MESHLET_MESH_MATERIAL_PASS
#ifdef BINDLESS
                pbr_bindings::base_color_texture[slot],
                pbr_bindings::base_color_sampler[slot],
#else   // BINDLESS
                pbr_bindings::base_color_texture,
                pbr_bindings::base_color_sampler,
#endif  // BINDLESS
#ifdef STANDARD_MATERIAL_BASE_COLOR_UV_B
                uv_b,
#else
                uv,
#endif
#ifdef MESHLET_MESH_MATERIAL_PASS
                bias.ddx_uv,
                bias.ddy_uv,
#else   // MESHLET_MESH_MATERIAL_PASS
                bias.mip_bias,
#endif  // MESHLET_MESH_MATERIAL_PASS
        );
@k2d222
Copy link
Contributor

k2d222 commented Jan 22, 2025

With the current conditional translation spec only, the code cannot be fundamentally improved. But I think even with bevy preprocessor the code would be more readable by moving arguments into declarations.

Bevy:

#ifdef BINDLESS
    let texture = pbr_bindings::base_color_texture[slot];
    let sampler = pbr_bindings::base_color_sampler[slot];
#else
    let texture = pbr_bindings::base_color_texture;
    let sampler = pbr_bindings::base_color_sampler;
#endif

#ifdef STANDARD_MATERIAL_BASE_COLOR_UV_B
    let coords = uv_b;
#else
    let coords = uv;
#endif

#ifdef MESHLET_MESH_MATERIAL_PASS
    let color = textureSampleGrad(texture, sampler, coords, bias.ddx_uv, bias.ddy_uv);
#else
    let color = textureSampleBias(texture, sampler, coords, bias.mip_bias);
#endif

pbr_input.material.base_color *= color;

WESL:

@if(BINDLESS)
    let texture = pbr_bindings::base_color_texture[slot];
@else 
    let texture = pbr_bindings::base_color_texture;

@if(BINDLESS)
    let sampler = pbr_bindings::base_color_sampler[slot];
@else
    let sampler = pbr_bindings::base_color_sampler;

@if(STANDARD_MATERIAL_BASE_COLOR_UV_B)
    let coords = uv_b;
@else
    let coords = uv;

@if(MESHLET_MESH_MATERIAL_PASS)
    let color = textureSampleGrad(texture, sampler, coords, bias.ddx_uv, bias.ddy_uv);
@else
    let color = textureSampleBias(texture, sampler, coords, bias.mip_bias);

pbr_input.material.base_color *= color;

WESL, if we allow @if (with mandatory @else) on expressions (which is a bit nicer):

let texture = @if(BINDLESS) pbr_bindings::base_color_texture[slot] @else pbr_bindings::base_color_texture;
let sampler = @if(BINDLESS) pbr_bindings::base_color_sampler[slot] @else pbr_bindings::base_color_sampler;
let coords = @if(STANDARD_MATERIAL_BASE_COLOR_UV_B) uv_b @else uv;
let color = 
    @if(MESHLET_MESH_MATERIAL_PASS) textureSampleGrad(texture, sampler, coords, bias.ddx_uv, bias.ddy_uv)
    @else textureSampleBias(texture, sampler, coords, bias.mip_bias);

pbr_input.material.base_color *= color;

I think tho a better approach would be something similar to what Use.GPU does, i.e. getter functions

// pbr_bindings.wesl
@if(BINDLESS) @group() @binding() var base_color_texture: ...;
@else         @group() @binding() var base_color_texture: ...;

fn get_base_color_texture() -> texture_2d<f32> {
    @if(BINDLESS) {
        return base_color_texture[slot];
    } @else {
        return base_color_texture;
    }
}

// main.wesl
import pbr_bindings::{ get_base_color_texture, get_base_color_sampler, get_coords };

pbr_input.material.base_color *=
    @if(MESHLET_MESH_MATERIAL_PASS)
        textureSampleGrad(get_base_color_texture(), get_base_color_sampler(), get_coords(), bias.ddx_uv, bias.ddy_uv)
    @else 
        textureSampleBias(tget_base_color_texture(), get_base_color_sampler(), get_coords(), bias.mip_bias);

This is a good exercise to find the flaws in our design. In this particular example, the attributes on expressions with mandatory else would be very useful. I also found that the current syntax does not allow declaration grouping:

#if SOME_CONDITION
    let foo = ...
    let bar = ...
    let baz = ...
#else
    let foo = ...
    let bar = ...
    let baz = ...
#endif
@if (SOME_CONDITION)
    let foo = ...
@else
    let foo = ...
@if (SOME_CONDITION)
    let bar = ...
@else
    let bar = ...
@if (SOME_CONDITION)
    let baz = ...
@else
    let baz = ...

@mighdoll
Copy link
Contributor Author

... getter functions

So, perhaps the getter function ought be written manually by the programmer for now? Seems like it adds clarity to this example..

a better approach

The wesl version you have here looks a lot cleaner!

// pbr_bindings.wesl
@if(BINDLESS) @group() @binding() var base_color_texture: ...;
@else         @group() @binding() var base_color_texture: ...;

fn get_base_color_texture() -> texture_2d<f32> {
   @if(BINDLESS) {
       return base_color_texture[slot];
   } @else {
       return base_color_texture;
   }
}

// main.wesl
import pbr_bindings::{ get_base_color_texture, get_base_color_sampler, get_coords };

pbr_input.material.base_color *=
   @if(MESHLET_MESH_MATERIAL_PASS)
       textureSampleGrad(get_base_color_texture(), get_base_color_sampler(), get_coords(), bias.ddx_uv, bias.ddy_uv)
   @else 
       textureSampleBias(tget_base_color_texture(), get_base_color_sampler(), get_coords(), bias.mip_bias);

If deferring @else is desirable for now, I suppose it could be written without @else, by using @if(!BINDLESS), at some loss of clarity.

Do you think we should continue to defer @else? It seems likely @else will come in eventually. I suppose you'll have a better sense of the complexity cost for the grammar and conditional evaluation..

@k2d222
Copy link
Contributor

k2d222 commented Jan 22, 2025

If deferring @else is desirable for now, I suppose it could be written without @else, by using @if(!BINDLESS), at some loss of clarity.

Yes and no. Having the @else would allow decorating expressions with attributes. In the current spec (roughly) only statements and declarations can be decorated with @if. The idea is to only decorate nodes that can be removed without breaking syntax. If you remove an expression then the program is syntactically incorrect. But with the @else, we can guarantee that there will always be syntax node.

So TL;DR @else allows this: let foo = @if(A) something @else something_else; instead of @if(A) let foo = something; @if(!A) something_else;.

Do you think we should continue to defer @else? It seems likely @else will come in eventually. I suppose you'll have a better sense of the complexity cost for the grammar and conditional evaluation..

I would not suggest having the else be part of MVP, we already have much on our plate :) Let's keep it that way for now.

Besides, I have not yet submitted the grammar change proposal I mentionned on discord to the gpuweb folks, but if they agree with that it would make the grammar significantly more friendly to those attributes :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

No branches or pull requests

2 participants