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

Asset v2: Asset path serialization fix #9756

Merged

Conversation

Neo-Zhixing
Copy link
Contributor

@Neo-Zhixing Neo-Zhixing commented Sep 11, 2023

Objective

  • Silence Failed to send DropEvent for StrongHandle "SendError(..)" errors when StrongHandle were dropped during application shutdown.
  • Re-export BoxedFuture considering that it's used everywhere in bevy_asset
  • Fixed an issue introduced by Copy on Write AssetPaths #9729.
    Asset 'final_gather.rgen' encountered an error in dust_render::shader::spirv::SpirvLoader: Failed to deserialize asset meta: SpannedError { code: InvalidValueForType { expected: "string AssetPath", found: "a sequence" }, position: Position { line: 9, col: 24 } }
    
    Basically, for processed assets with dependencies, bevy will serialize the metafile as follows:
    (
      meta_format_version: "1.0",
      processed_info: Some((
          hash: (203, 239, 108, 156, 180, 23, 157, 217, 159, 36, 158, 193, 185, 253, 242, 156),
          full_hash: (77, 58, 30, 200, 21, 180, 221, 133, 151, 83, 247, 47, 193, 70, 228, 97),
          process_dependencies: [
              (
                  full_hash: (56, 46, 55, 118, 3, 6, 213, 250, 124, 26, 153, 87, 15, 85, 4, 89),
                  path: ("standard.glsl"), # <<---------- See here
              ),
          ],
      )),
      asset: Load(
          loader: "dust_render::shader::spirv::SpirvLoader",
          settings: (),
      ),
    )
    
    AssetPath gets serialized as ("standard.glsl") which was then deserialized as a sequence instead of our AssetPath.

Solution

  • Serialize AssetPath directly as a string instead. The above metafile would be serialized as follows:
 (
   meta_format_version: "1.0",
   processed_info: Some((
       hash: (203, 239, 108, 156, 180, 23, 157, 217, 159, 36, 158, 193, 185, 253, 242, 156),
       full_hash: (77, 58, 30, 200, 21, 180, 221, 133, 151, 83, 247, 47, 193, 70, 228, 97),
       process_dependencies: [
           (
               full_hash: (56, 46, 55, 118, 3, 6, 213, 250, 124, 26, 153, 87, 15, 85, 4, 89),
               path: "standard.glsl", # <<------- No longer a tuple struct
           ),
       ],
   )),
   asset: Load(
       loader: "dust_render::shader::spirv::SpirvLoader",
       settings: (),
   ),
 )

@Neo-Zhixing Neo-Zhixing changed the title Asset v: Asset path serialization fix Asset v2: Asset path serialization fix Sep 11, 2023
@Neo-Zhixing
Copy link
Contributor Author

Side note: We might wanna serialize the hash into something more compact, like a hex string.

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Assets Load files from disk to use for things like images, models, and sounds labels Sep 11, 2023
@alice-i-cecile alice-i-cecile added this to the 0.12 milestone Sep 11, 2023
@cart cart added this pull request to the merge queue Sep 13, 2023
@cart
Copy link
Member

cart commented Sep 13, 2023

Good calls!

We might wanna serialize the hash into something more compact, like a hex string.

Agreed on this too!

Merged via the queue into bevyengine:main with commit 8fa500d Sep 13, 2023
21 checks passed
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
# Objective

- Silence `Failed to send DropEvent for StrongHandle "SendError(..)"`
errors when `StrongHandle` were dropped during application shutdown.
- Re-export `BoxedFuture` considering that it's used everywhere in
bevy_asset
- Fixed an issue introduced by bevyengine#9729. 
  ```
Asset 'final_gather.rgen' encountered an error in
dust_render::shader::spirv::SpirvLoader: Failed to deserialize asset
meta: SpannedError { code: InvalidValueForType { expected: "string
AssetPath", found: "a sequence" }, position: Position { line: 9, col: 24
} }
  ```
Basically, for processed assets with dependencies, bevy will serialize
the metafile as follows:
  ```
  (
    meta_format_version: "1.0",
    processed_info: Some((
hash: (203, 239, 108, 156, 180, 23, 157, 217, 159, 36, 158, 193, 185,
253, 242, 156),
full_hash: (77, 58, 30, 200, 21, 180, 221, 133, 151, 83, 247, 47, 193,
70, 228, 97),
        process_dependencies: [
            (
full_hash: (56, 46, 55, 118, 3, 6, 213, 250, 124, 26, 153, 87, 15, 85,
4, 89),
                path: ("standard.glsl"), # <<---------- See here
            ),
        ],
    )),
    asset: Load(
        loader: "dust_render::shader::spirv::SpirvLoader",
        settings: (),
    ),
  )
  ```
`AssetPath` gets serialized as `("standard.glsl")` which was then
deserialized as a sequence instead of our `AssetPath`.

## Solution

- Serialize `AssetPath` directly as a string instead. The above metafile
would be serialized as follows:
 ```
  (
    meta_format_version: "1.0",
    processed_info: Some((
hash: (203, 239, 108, 156, 180, 23, 157, 217, 159, 36, 158, 193, 185,
253, 242, 156),
full_hash: (77, 58, 30, 200, 21, 180, 221, 133, 151, 83, 247, 47, 193,
70, 228, 97),
        process_dependencies: [
            (
full_hash: (56, 46, 55, 118, 3, 6, 213, 250, 124, 26, 153, 87, 15, 85,
4, 89),
path: "standard.glsl", # <<------- No longer a tuple struct
            ),
        ],
    )),
    asset: Load(
        loader: "dust_render::shader::spirv::SpirvLoader",
        settings: (),
    ),
  )
  ```

---
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants