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.

Sign up for GitHub

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

Jump to bottom

Invalid shader import after #13121 #13207

Closed
arcashka opened this issue May 3, 2024 · 0 comments · Fixed by #13209
Closed

Invalid shader import after #13121 #13207

arcashka opened this issue May 3, 2024 · 0 comments · Fixed by #13209
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior
Milestone
0.14

Comments

@arcashka
Copy link
Contributor

arcashka commented May 3, 2024

Bevy version

main branch, regression after #13121

Issue

Tonemapping doesn't work if you don't use bevy_pbr feature

❯ cargo run
   Compiling bevy_render v0.14.0-dev (/home/arcashka/Documents/projects/other/bevy/crates/bevy_render)
   Compiling bevy_core_pipeline v0.14.0-dev (/home/arcashka/Documents/projects/other/bevy/crates/bevy_core_pipeline)
   Compiling bevy_internal v0.14.0-dev (/home/arcashka/Documents/projects/other/bevy/crates/bevy_internal)
   Compiling bevy v0.14.0-dev (/home/arcashka/Documents/projects/other/bevy)
   Compiling issue2 v0.1.0 (/home/arcashka/Documents/projects/other/issue2)
    Finished dev [unoptimized + debuginfo] target(s) in 7.50s
     Running `target/debug/issue2`
2024-05-03T11:22:14.962803Z  INFO bevy_winit::system: Creating new window "App" (Entity { index: 0, generation: 1 })
2024-05-03T11:22:14.963476Z  INFO winit::platform_impl::platform::x11::window: Guessed window scale factor: 1
2024-05-03T11:22:15.023778Z  INFO bevy_render::renderer: AdapterInfo { name: "Intel(R) Graphics (ADL GT2)", vendor: 32902, device: 17958, device_type: IntegratedGpu, driver: "Intel open-source Mesa driver", driver_info: "Mesa 23.2.1-1ubuntu3.1", backend: Vulkan }
2024-05-03T11:22:15.230009Z ERROR bevy_render::render_resource::pipeline_cache: failed to process shader:
error: required import 'bevy_pbr::utils' not found
    ┌─ /home/arcashka/Documents/projects/other/bevy/crates/bevy_core_pipeline/src/tonemapping/tonemapping_shared.wgsl:355:15
    │
355 │     var hsv = rgb_to_hsv(color);
    │               ^
    │
    = missing import 'bevy_pbr::utils'


2024-05-03T11:22:20.791119Z  INFO bevy_window::system: No windows are open, exiting

Minimal example

[package]
name = "issue2"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
bevy = { path = "../bevy", default-features = false, features = [
    "bevy_core_pipeline",
    "tonemapping_luts",
    "x11",
] }
use bevy::{core_pipeline::tonemapping::Tonemapping, prelude::*};

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, setup)
        .run();
}

fn setup(mut commands: Commands) {
    commands.spawn(Camera2dBundle {
        camera: Camera {
            hdr: false,
            ..default()
        },
        tonemapping: Tonemapping::BlenderFilmic,
        ..default()
    });
}

What you did

I was investigating #13118 and found 2 other issues, so right now there are 3 issues related to the same scenario
@arcashka arcashka added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels May 3, 2024
@arcashka arcashka mentioned this issue May 3, 2024
Merged
@IceSentry IceSentry added A-Rendering Drawing game state to the screen and removed S-Needs-Triage This issue needs to be labelled labels May 3, 2024
@IceSentry IceSentry added this to the 0.14 milestone May 3, 2024
github-merge-queue bot pushed a commit that referenced this issue May 4, 2024
@arcashka
move wgsl color operations from bevy_pbr to bevy_render (#13209)
6027890 
# Objective

`bevy_pbr/utils.wgsl` shader file contains mathematical constants and
color conversion functions. Both of those should be accessible without
enabling `bevy_pbr` feature. For example, tonemapping can be done in non
pbr scenario, and it uses color conversion functions.

Fixes #13207

## Solution

* Move mathematical constants (such as PI, E) from
`bevy_pbr/src/render/utils.wgsl` into `bevy_render/src/maths.wgsl`
* Move color conversion functions from `bevy_pbr/src/render/utils.wgsl`
into new file `bevy_render/src/color_operations.wgsl`

## Testing
Ran multiple examples, checked they are working:
* tonemapping
* color_grading
* 3d_scene
* animated_material
* deferred_rendering
* 3d_shapes
* fog
* irradiance_volumes
* meshlet
* parallax_mapping
* pbr
* reflection_probes
* shadow_biases
* 2d_gizmos
* light_gizmos
---

## Changelog
* Moved mathematical constants (such as PI, E) from
`bevy_pbr/src/render/utils.wgsl` into `bevy_render/src/maths.wgsl`
* Moved color conversion functions from `bevy_pbr/src/render/utils.wgsl`
into new file `bevy_render/src/color_operations.wgsl`

## Migration Guide
In user's shader code replace usage of mathematical constants from
`bevy_pbr::utils` to the usage of the same constants from
`bevy_render::maths`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior
Projects
None yet
Milestone
0.14
Development

Successfully merging a pull request may close this issue.

move wgsl color operations from bevy_pbr to bevy_render arcashka/bevy
2 participants
@IceSentry @arcashka