Rusty imports

[robtfm]

reworks the import mechanism to support more syntax to make it more rusty, and test for item use before importing to determine which imports are modules and which are items, which allows:

• use rust-style imports

#import bevy_pbr::{pbr_functions::{alpha_discard as discard, pbr}, mesh_bindings}

• import partial paths:

#import part::of::path
...
path::remainder::function();

which will call to part::of::path::remainder::function

• use fully qualified paths without importing:

// #import bevy_pbr::pbr_functions
bevy_pbr::pbr_functions::pbr()

• use imported items without qualifying

#import bevy_pbr::pbr_functions::pbr
// or, for backwards compatibility (i think this style should be deprecated)
// #import bevy_pbr::pbr_functions pbr
...
pbr()

• existing code should work without changes (unless it uses #imported overrides), see below

• allows most imported items to end with _ and numbers (Naga invariant error is confusing #30). still doesn't allow struct members to end with _ or numbers but it's progress.

• includes (a reworking of) include ifops in effective defs #39

• supercedes don't require full module path in qualifiers #24 and Auto import fully qualified identifiers #25

• partly breaks overrides - now overrides will only be applied if the overriding module is added as an additional import in the arguments to Composer::make_naga_module or Composer::add_composable_module. this is necessary to support determining whether imports are modules or items. i think it's a worthwhile trade-off (not sure anybody is using overrides yet)

i've spot-checked bevy, will test more thoroughly after some feedback. would very much appreciate tests from people using the crate outside of bevy core - @DGriffin91, @NiseVoid, @Aaron1011

[robtfm]

fixed

• "quoted" imports were not parsed correctly
• an issue where an imported name was also used as a variable/parameter and declared with a trailing colon.
• source offset was wrong when reporting naga-level errors in top-level shaders

added some further tests to catch regressions.

thanks @NiseVoid for testing and providing an import test case.

[cart]

I'm on board for this direction! This is going to make a lot of people very happy.

[DGriffin91]

This looks awesome! I'm hoping to check it out this weekend.

[robtfm]

i've tested properly against the bevy examples as well now (see draft pr linked above), and fixed a couple of minor issues (mainly around reporting errors in import declarations).
edit to add: the bevy examples worked without the wgsl changes in that pr as well.

[DGriffin91]

@robtfm One odd thing I found is that when using as modules now have to have names that never overlap with variable names. Previously this would work:

#import "shaders/foo.wgsl" as foo
fn func() {
     let foo = vec3(0.0);
}

Now I get this error:

error: expected identifier, found '"'
   ┌─ shaders/glowy.wgsl:47:9
   │
47 │     let "foo.wgsl" = vec3(0.0);
   │         ^ expected identifier
   │
   = expected identifier, found '"'

But

This still works fine:

#import "shaders/foo.wgsl"::{somefunc}
fn func() {
     let somefunc = vec3(0.0);
}

I'm not sure if the as feature is expected to still be available in the future? I like being able to sometimes import files and use them without picking each thing individually. Ex:

#import "shaders/foo.wgsl" as foo
fn func() {
     let a = foo::somefunc();
}

This does still work currently, just wanted to check that you plan to continue to support this, and if so if it would be possible to avoid the var name issue?

[robtfm]

One odd thing I found is that when using as modules now have to have names that never overlap with variable names.

fixed, thanks. this happens because we (with very limited context) substitute imported identifiers directly. for imported names without quotes, the replacement is harmless - your local variable gets renamed in all the places it is used. with quoted imports the quoted name is not a valid variable name, so it failed. i've special-cased this.

there will still be potentially misleading errors if your local variables shadow imported names (either raw module/item names or as names), but it won't fail if the shader code is valid.

just wanted to check that you plan to continue to support this

yes, definitely.

[cart]

Very happy with these changes!

[cart]

I agree that this style should be deprecated. Maybe we should print a warning when we identify these?

[robtfm]

i added this. i guess that also means we should do a major minor version bump. this pr is not technically breaking but existing users of bevy 0.11 with naga_oil 0.9 would get deprecated warning spam from the console if the warning is on by default (which i think it should be).

[cart]

Yeah I think this should land in naga_oil 0.10. I agree the warning should be on by default.