cloudhead / rx

👾 Modern and minimalist pixel editor
https://discord.gg/xHggPjfsS9
GNU General Public License v3.0
3.1k stars 109 forks source link

Adding the option to save and load .bmp images #27

Closed rbisewski closed 4 years ago

rbisewski commented 4 years ago

I saw your project on HN and was inspired to try writing in Rust and making some contributions to your nifty pixel editor.

Specifically, this PR adds the option to load and save bitmap image files using the image crate library. The way this is handled is that this checks if the file suffix is .bmp during save and load.

I have ran some quick tests on my system and it seems to work, but let me know what you think.

Thanks again :)

cloudhead commented 4 years ago

Thanks! I'm a little unsure about adding .bmp support unless it has the same capabilities as .png - the reason being that if there are features that one format supports and not the other, I'll have to maintain two code paths, or restrict the capabilities to the less capable format (in this case probably .bmp). This requires further research :smile:

rbisewski commented 4 years ago

Sounds fine by me, and thanks again for writing this project and open sourcing it :smile:

Out of curiosity, were there very specific PNG features you needed? Bitmaps tend to have good color ranges and transparency support. Or maybe your program depends on features from the png library, such as filters or blending?

Just a quick gander at the image library, I see a number of filters and transforms and even animation decoders. I think it could potentially allow your program to support many image types and it seems to be a good and feature-rich library; though perhaps I could be mistaken?

In any event, merely my humble thoughts above.

cloudhead commented 4 years ago

Indeed, I wasn't sure anymore if BMP supported a full 8 bit alpha channel, but it looks like it does.

The other requirement which is perhaps odd is that I want to embed custom meta-data in the image file, and I'd like all supported image formats to support this. So I just need to ensure this is possible with both image formats.

rbisewski commented 4 years ago

Yeah, modern bitmaps support a full range of colors and various popular image features.

With regards to metadata, did you mean something like perhaps a "Created with rx" sort of descriptor field? Or something else like an IPTC entity?

A quick Google check seems to turn up the rexiv2 which supports editing the metadata of the various image formats.

If you give me an idea of what sort of metadata, I can look into it, if you want?

cloudhead commented 4 years ago

For now, one of the things I need to store in there is the number of frames in a sprite - this will avoid having to call :slice <n> whenever you reload. The other thing is the animation delay. So basically some arbitrary data. It's quite common that file formats support an arbitrary binary section at the end of the file - so that's what I was thinking.

rbisewski commented 4 years ago

So you want the following two fields of metadata:

Seems like a good idea to implement, so I'll look into how feasible it is on this weekend using the rexiv2 library. That said, do you currently save any metadata to images right now?

cloudhead commented 4 years ago

Yeah those are basically the two things I'd like to store at the moment.

Since this isn't Exif related, I don't think using rexiv2 is a good idea. It also happens to pull in some heavy dependencies, and I'd like to keep compile times and non-rust dependencies to a minimum.

From a quick look at the PNG spec: https://www.w3.org/TR/REC-png.pdf - section 4.2.7, it looks like it's possible to store arbitrary key/value pairs, which is exactly what is needed.

I'm not sure the png create exposes this though - but I'm happy for you to look into it if you have time!

rbisewski commented 4 years ago

You're probably right, the various common crates (the png crate included) that manipulate images don't seem to really allow directly adjusting the metadata.

I looked at a number of crates and most didn't seem to offer functionality to edit that section of the metadata (though perhaps I am mistaken?), so writing out the metadata like this could be more of a mini-project; e.g. wrapping C libraries or writing a rust crate.

cloudhead commented 4 years ago

Support for additional formats needs more thought.