Skip to content

add native zstd support#19793

Merged
alice-i-cecile merged 5 commits intobevyengine:mainfrom
atlv24:zstd
Jun 26, 2025
Merged

add native zstd support#19793
alice-i-cecile merged 5 commits intobevyengine:mainfrom
atlv24:zstd

Conversation

@atlv24
Copy link
Copy Markdown
Contributor

@atlv24 atlv24 commented Jun 24, 2025

Objective

  • add support for alternate zstd backend through zstd for faster decompression

Solution

  • make existing zstd feature only specify that support is required, disambiguate which backend to use via two other features zstd_native and zstd_rust.
  • Similar to the approach taken by KTX2 Updates: ETC1s/BasisLZ, ASTC HDR, and faster Zstd #18411, but we keep current behavior by defaulting to the rust implementation because its safer, and isolate this change.

NOTE: the default feature-set may seem to not currently require zstd, but it does, it is enabled transitively by the tonemapping_luts feature, which is a default feature. Thus this does not add default features.

Testing

  • Cargo clippy on both feature combinations

@atlv24 atlv24 added A-Rendering Drawing game state to the screen D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 24, 2025
@Victoronz Victoronz added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 24, 2025
@mnmaita
Copy link
Copy Markdown
Member

mnmaita commented Jun 24, 2025

Should this have a small migration guide entry to state that an additional feature is required when using zstd?

@atlv24
Copy link
Copy Markdown
Contributor Author

atlv24 commented Jun 24, 2025

its a default feature, so unless you're using default-features=false there's no migration

@atlv24 atlv24 force-pushed the zstd branch 2 times, most recently from b48013b to 7c2fb6d Compare June 24, 2025 10:22
@janhohenheim
Copy link
Copy Markdown
Member

janhohenheim commented Jun 24, 2025

@atlv24 doing default-features = false is the recommended way of creating third-party plugins, so lots of people use that.
As a plugin author, I'd be happy if we considered things like this in our migration guide. I've many times upgraded a plugin to a new Bevy version only to get cryptic error messages because some feature is now needed, but not documented anywhere ;)

@mockersf
Copy link
Copy Markdown
Member

it's mentioned in the migration guide 👍

@janhohenheim
Copy link
Copy Markdown
Member

Oh thanks, then I'm happy :D

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 26, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jun 26, 2025
# Objective

- add support for alternate zstd backend through `zstd` for faster
decompression

## Solution

- make existing `zstd` feature only specify that support is required,
disambiguate which backend to use via two other features `zstd_native`
and `zstd_rust`.
- Similar to the approach taken by #18411, but we keep current behavior
by defaulting to the rust implementation because its safer, and isolate
this change.

NOTE: the default feature-set may seem to not currently require `zstd`,
but it does, it is enabled transitively by the `tonemapping_luts`
feature, which is a default feature. Thus this does not add default
features.

## Testing

- Cargo clippy on both feature combinations
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 26, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 26, 2025
Merged via the queue into bevyengine:main with commit 3080040 Jun 26, 2025
36 checks passed
@atlv24 atlv24 deleted the zstd branch July 23, 2025 23:47
github-merge-queue bot pushed a commit that referenced this pull request Sep 22, 2025
# Objective

Fixes #21089 

The migration guide for the `zstd` feature changes didn't explain how to
migrate from the old `zstd` feature to the new split features.

## Solution

Added migration instructions showing that the old `zstd` feature should
be replaced with either `zstd_rust` (safe default) or `zstd_c` (faster
but unsafe).

## Testing

- Documentation change only, no code to test
- Verified the feature change in git history (PR #19793)

##
Link to the issue:
[https://github.com/bevyengine/bevy/issues/21089](https://github.com/bevyengine/bevy/issues/21089)

---------

Co-authored-by: Alice Cecile <[email protected]>
mockersf pushed a commit that referenced this pull request Sep 22, 2025
# Objective

Fixes #21089 

The migration guide for the `zstd` feature changes didn't explain how to
migrate from the old `zstd` feature to the new split features.

## Solution

Added migration instructions showing that the old `zstd` feature should
be replaced with either `zstd_rust` (safe default) or `zstd_c` (faster
but unsafe).

## Testing

- Documentation change only, no code to test
- Verified the feature change in git history (PR #19793)

##
Link to the issue:
[https://github.com/bevyengine/bevy/issues/21089](https://github.com/bevyengine/bevy/issues/21089)

---------

Co-authored-by: Alice Cecile <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Rendering Drawing game state to the screen D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

9 participants