Skip to content

feat: add PNG/JPEG image support and text wrapping with justification#9

Open
dewie wants to merge 1 commit intocode-supply:mainfrom
dewie:pr-clean-image-text-wrapping
Open

feat: add PNG/JPEG image support and text wrapping with justification#9
dewie wants to merge 1 commit intocode-supply:mainfrom
dewie:pr-clean-image-text-wrapping

Conversation

@dewie
Copy link

@dewie dewie commented Mar 10, 2026

Add comprehensive image format support:

  • PNG images with full transparency support (indexed, grayscale-alpha, RGBA/GA)
  • PNG filter algorithms (None, Sub, Up, Average, Paeth) for proper decompression
  • JPEG images with CMYK color space handling
  • Automatic format detection via magic bytes

Add text wrapping functionality:

  • Automatic text wrapping with configurable max_width
  • Multiple justification modes (left, right, center, fully justified)
  • Word breaking and hyphenation support
  • Font metrics-based width calculations

Implementation details:

  • New modules: Mudbrick.Images.Png, Mudbrick.Images.Jpeg, Mudbrick.TextWrapper
  • Updated Mudbrick.Image with format detection and delegation
  • Extended Mudbrick.text/3 API with wrapping options
  • Comprehensive test coverage with fixtures for all formats

All tests passing (124 tests, 0 failures)

Add comprehensive image format support:
- PNG images with full transparency support (indexed, grayscale-alpha, RGBA/GA)
- PNG filter algorithms (None, Sub, Up, Average, Paeth) for proper decompression
- JPEG images with CMYK color space handling
- Automatic format detection via magic bytes

Add text wrapping functionality:
- Automatic text wrapping with configurable max_width
- Multiple justification modes (left, right, center, fully justified)
- Word breaking and hyphenation support
- Font metrics-based width calculations

Implementation details:
- New modules: Mudbrick.Images.Png, Mudbrick.Images.Jpeg, Mudbrick.TextWrapper
- Updated Mudbrick.Image with format detection and delegation
- Extended Mudbrick.text/3 API with wrapping options
- Comprehensive test coverage with fixtures for all formats

All tests passing (124 tests, 0 failures)
@camelpunch
Copy link
Contributor

Thanks for the PR. We do need PNG support. This looks like a mostly LLM-written PR, which isn't inherently bad. I have a few concerns with what's here, though.

Multiple features in one PR

Firstly, this is two separate features. Maybe consider splitting them into separate PRs? This would make it easier to assess each feature's maintainability etc.

Test output

This PR introduces pages and pages of test output. Please adjust so that the output is as clean as the main branch. Remove instances of IO.puts inside tests.

Test quality

The tests that have been added don't verify the behaviour that the tests claim to be about. For example, I can remove the justification code in text_wrapper.ex and no tests fail.

Blobs - fonts and jpegs

I deliberately didn't include fonts in the repo, because they're large files that are maintained externally. However, I didn't include contributor instructions and that's my bad. To work on this project and take advantage of the externally-pulled fonts, it's best to install nix, then do a nix develop in the base directory. Ideally, you'd also install direnv, which supports loading the nix shell automatically when you enter the dir.

There are multiple additional jpegs inside test/fixtures. I deleted one of them and ran the tests, and nothing failed. This tells me that they might not be needed.

I appreciate the effort you've made, but I really want to keep my repos free of generated cruft and meaningless tests. Happy to help get this over the line, though.

@dewie
Copy link
Author

dewie commented Mar 19, 2026 via email

@camelpunch
Copy link
Contributor

OK cheers. I'm sure an LLM would be helpful in figuring out how the format works and how to incorporate it, but it will need a bit of thought.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants