Page MenuHomePhabricator

tga loader: implement handling of palette

Authored by nyir on Nov 7 2019, 11:22 AM.



implemented 8 bit color mapped images with 24/32bpp palette.

This is coming from an issue on feh to support indexed TGA images. Written by rofl0r, I'm forwarding it because I already have the tools for submission set up.

N.b. I'm not terribly familiar with the particular code style, if there's anything to format differently I'm happy to update.

Test Plan
  • Compared example file with known good in GIMP, looks okay.
  • Tested that said example file doesn't load without this patch (in feh, which uses Imlib2).

Diff Detail

No Linters Available
No Unit Test Coverage
Build Status
Buildable 14495
Build 9970: arc lint + arc unit
nyir requested review of this revision.Nov 7 2019, 11:22 AM
nyir created this revision.
nyir edited the summary of this revision. (Show Details)Nov 7 2019, 11:25 AM
nyir edited the test plan for this revision. (Show Details)
nyir added a reviewer: kwo.
nyir edited the summary of this revision. (Show Details)
kwo added a comment.Nov 8 2019, 10:49 AM

Looks good to me except that all of the error exits need proper cleanup (munmap(), __imlib_FreeData()).
I have made some cleanups in the tga loader so now you should just goto quit on error.
I have also eliminated the somewhat nasty WRITE_RGBA() macro - please use PIXEL_ARGB instead.
As for indentation - I use indent (version 2.2.12 - matters!), there is an in the top level directory, so you can just do like
$ indent src/modules/loaders/loader_tga.c
But no problem - I'm fine with amending the indent.

Could you please redo your changes (with proper exit handling etc.) on top of mine?

Thanks! :)

nyir updated this revision to Diff 26874.Nov 13 2019, 2:37 PM
  • implement loading of 8bit palette TGAs
  • Update indentation.
nyir added a comment.Nov 13 2019, 2:41 PM
In D10618#198592, @kwo wrote:

Could you please redo your changes (with proper exit handling etc.) on top of mine?

Cool, thanks; rofl0r did do so, I'm just forwarding their changes again; I ran indent as well, but committed that as a separate commit because of the IMO questionable change on the line break. Hope this looks good then.

kwo accepted this revision.Nov 15 2019, 8:17 AM

Pushed (with minor formatting adjustments).
Thanks :)

This revision is now accepted and ready to land.Nov 15 2019, 8:17 AM
kwo closed this revision.Nov 15 2019, 8:17 AM