Skip to content

fix: incorrect RGB channel ordering in palette mapping#52

Closed
sogladev wants to merge 1 commit into
wowemulation-dev:mainfrom
sogladev:fix-color-swap
Closed

fix: incorrect RGB channel ordering in palette mapping#52
sogladev wants to merge 1 commit into
wowemulation-dev:mainfrom
sogladev:fix-color-swap

Conversation

@sogladev

@sogladev sogladev commented Mar 3, 2026

Copy link
Copy Markdown

Pull Request

Summary

Uncompressed / RAW BLP files are stored in BGRA format. The lib is currently reading these in the wrong order.

[[AI SUMMARY of the changes]]

Corrects the order in which RGB channels are packed and unpacked when converting palette data, ensuring accurate color rendering. Previously, red and blue channels were swapped, causing colors to display incorrectly.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing
    functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test improvements
  • Build system/dependency changes
  • Security fix

Changes Made

Related Issues

Fixes #(issue number)
Closes #(issue number)
Related to #(issue number)

Testing

Tested with uncompressed BLP files in ITEM/TEXTURECOMPONENTS/

Test Cases Added/Modified

  • Unit tests
  • Integration tests
  • Compliance tests (StormLib compatibility)
  • Performance benchmarks
  • Manual testing

Test Results

# Example test commands and results
cargo test
cargo test -p wow-mpq
cargo bench

Tested On

  • Linux
  • macOS
  • Windows
  • Cross-compilation targets

WoW Versions Tested

  • 1.12.1 (Vanilla)
  • 2.4.3 (TBC)
  • 3.3.5a (WotLK)
  • 4.3.4 (Cataclysm)
  • 5.4.8 (MoP)
  • Other: _______________

Quality Assurance

Code Quality

  • Code follows project style guidelines
  • Self-review of code completed
  • Code is properly documented
  • No obvious performance regressions
  • Error handling is appropriate

Required Checks

  • cargo fmt --all - Code is formatted
  • cargo clippy --all-targets --all-features - No clippy warnings
  • cargo test --all-features - All tests pass
  • cargo test --no-default-features - Tests pass without features
  • cargo deny check - No security/license issues
  • Documentation builds successfully

Compatibility

  • No breaking changes to public API (or properly documented)
  • Backward compatibility maintained where possible
  • StormLib compatibility preserved (if applicable)
  • Cross-platform compatibility verified

Documentation

  • Updated relevant documentation in docs/
  • Updated CHANGELOG.md
  • Updated README.md (if applicable)
  • Added/updated code examples
  • Added/updated CLI help text
  • API documentation updated (rustdoc)

Benchmarks

Performance Impact

  • No performance impact
  • Performance improvement (include metrics)
  • Performance regression (justified and documented)

Benchmark Results

# Before:
test bench_parse_archive ... bench: 1,234 ns/iter (+/- 56)

# After:
test bench_parse_archive ... bench: 987 ns/iter (+/- 43)

Breaking Changes

API Changes

Migration Guide

Security Considerations

  • No security implications
  • Security improvement
  • Potential security impact (reviewed and justified)

Additional Context

https://wowwiki-archive.fandom.com/wiki/BLP_file#Compression

Dependencies

Known Limitations

Screenshots/Examples


Reviewer Notes

Areas of Focus

Questions for Reviewers


By submitting this PR, I confirm that:

  • I have read and agree to the Contributing Guidelines
  • This PR is ready for review (not a draft)
  • I am willing to address feedback and make necessary changes
  • I understand this may take time to review and merge

Corrects the order in which RGB channels are packed and unpacked
when converting palette data, ensuring accurate color rendering.
Previously, red and blue channels were swapped, causing colors
to display incorrectly.
@codecov

codecov Bot commented Mar 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
file-formats/graphics/wow-blp/src/convert/raw1.rs 0.00% 8 Missing ⚠️
...le-formats/graphics/wow-blp/src/convert/palette.rs 0.00% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@danielsreichenbach

Copy link
Copy Markdown
Member

I merged this together with an update to the actual comments referencing this to make it a bit more complete. Thanks very much for the submission.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants