Skip to content

Commit

Permalink
LibGfx: Respect order of image data in TGAImageDecoderPlugin
Browse files Browse the repository at this point in the history
Previously, the order of data was incorrectly based on x_origin and
y_origin, which are meant for where on the screen the image should be
placed (TarGA is an image format meant for graphic cards), instead of
Image descriptor bits 4 and 5, which specify the direction.

This fixes a problem where tga files generated with magick would render
up-side-down.

The test images where created in GIMP, 4 simple 2x2 images
exported as .tga, no compression, origin: Bottom Left

square-bottom-left.tga:
Red, Green
Blue, Magenta

square-bottom-right.tga:
Green, Red
Magenta, Blue

square-top-left.tga:
Blue, Magenta
Red, Green

square-top-right.tga:
Magenta, Blue
Green, Red

then this script was ran:
```python
def update_tga_descriptor(file_path, top_to_bottom, left_to_right):
    with open(file_path, 'r+b') as f:
        header = f.read(18)
        descriptor = header[17]

        descriptor &= ~(1 << 4);
        descriptor &= ~(1 << 5);

        if left_to_right:
            descriptor |= 1 << 4;

        if top_to_bottom:
            descriptor |= 1 << 5;

        header = header[:17] + bytes([descriptor])
        f.seek(0)
        f.write(header)

update_tga_descriptor('square-bottom-left.tga', False, False)
update_tga_descriptor('square-bottom-right.tga', False, True)
update_tga_descriptor('square-top-left.tga', True, False)
update_tga_descriptor('square-top-right.tga', True, True)
```
  • Loading branch information
mkanilsson authored and nico committed Sep 18, 2024
1 parent 76301a6 commit 66bdb3b
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 18 deletions.
23 changes: 23 additions & 0 deletions Tests/LibGfx/TestImageDecoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -850,6 +850,29 @@ TEST_CASE(test_targa_black_and_white_uncompressed)
TRY_OR_FAIL(expect_single_frame(*plugin_decoder));
}

TEST_CASE(test_targa_image_descriptor)
{
Array test_inputs = {
TEST_INPUT("tga/square-bottom-left.tga"sv),
TEST_INPUT("tga/square-bottom-right.tga"sv),
TEST_INPUT("tga/square-top-left.tga"sv),
TEST_INPUT("tga/square-top-right.tga"sv),
};

for (auto test_input : test_inputs) {
auto file = TRY_OR_FAIL(Core::MappedFile::map(test_input));
EXPECT(Gfx::TGAImageDecoderPlugin::validate_before_create(file->bytes()));
auto plugin_decoder = TRY_OR_FAIL(Gfx::TGAImageDecoderPlugin::create(file->bytes()));

auto frame = TRY_OR_FAIL(expect_single_frame_of_size(*plugin_decoder, { 2, 2 }));

EXPECT_EQ(frame.image->get_pixel(0, 0), Gfx::Color::NamedColor::Red);
EXPECT_EQ(frame.image->get_pixel(1, 0), Gfx::Color::NamedColor::Green);
EXPECT_EQ(frame.image->get_pixel(0, 1), Gfx::Color::NamedColor::Blue);
EXPECT_EQ(frame.image->get_pixel(1, 1), Gfx::Color::NamedColor::Magenta);
}
}

TEST_CASE(test_tiff_uncompressed)
{
auto file = TRY_OR_FAIL(Core::MappedFile::map(TEST_INPUT("tiff/uncompressed.tiff"sv)));
Expand Down
Binary file added Tests/LibGfx/test-inputs/tga/square-bottom-left.tga
Binary file not shown.
Binary file not shown.
Binary file added Tests/LibGfx/test-inputs/tga/square-top-left.tga
Binary file not shown.
Binary file added Tests/LibGfx/test-inputs/tga/square-top-right.tga
Binary file not shown.
25 changes: 7 additions & 18 deletions Userland/Libraries/LibGfx/ImageFormats/TGALoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,7 @@ ErrorOr<ImageFrameDescriptor> TGAImageDecoderPlugin::frame(size_t index, Optiona
auto data_type = m_context->header.data_type_code;
auto width = m_context->header.width;
auto height = m_context->header.height;
auto x_origin = m_context->header.x_origin;
auto y_origin = m_context->header.y_origin;
auto image_descriptior = m_context->header.image_descriptor;

if (index != 0)
return Error::from_string_literal("TGAImageDecoderPlugin: frame index must be 0");
Expand All @@ -178,18 +177,8 @@ ErrorOr<ImageFrameDescriptor> TGAImageDecoderPlugin::frame(size_t index, Optiona
return Error::from_string_literal("TGAImageDecoderPlugin: Can only handle 8, 24 and 32 bits per pixel");
}

// FIXME: Try to understand the Image origin (instead of X and Y origin coordinates)
// based on the Image descriptor, Field 5.6, bits 4 and 5.

// NOTE: If Y origin is set to a negative number, just assume the generating software
// meant that we start with Y origin at the top height of the picture.
// At least this is the observed behavior when generating some pictures in GIMP.
if (y_origin < 0)
y_origin = height;
if (y_origin != 0 && y_origin != height)
return Error::from_string_literal("TGAImageDecoderPlugin: Can only handle Y origin which is 0 or the entire height");
if (x_origin != 0 && x_origin != width)
return Error::from_string_literal("TGAImageDecoderPlugin: Can only handle X origin which is 0 or the entire width");
auto is_top_to_bottom = (image_descriptior & 1 << 5) == 0;
auto is_left_to_right = (image_descriptior & 1 << 4) == 0;

VERIFY((bits_per_pixel % 8) == 0);
auto bytes_per_pixel = bits_per_pixel / 8;
Expand All @@ -200,10 +189,10 @@ ErrorOr<ImageFrameDescriptor> TGAImageDecoderPlugin::frame(size_t index, Optiona
for (int row = 0; row < height; ++row) {
for (int col = 0; col < width; ++col) {
auto actual_row = row;
if (y_origin < height)
if (is_top_to_bottom)
actual_row = height - 1 - row;
auto actual_col = col;
if (x_origin > width)
if (!is_left_to_right)
actual_col = width - 1 - col;
bitmap->scanline(actual_row)[actual_col] = TRY(read_pixel_from_stream(m_context->stream, bytes_per_pixel));
}
Expand All @@ -224,10 +213,10 @@ ErrorOr<ImageFrameDescriptor> TGAImageDecoderPlugin::frame(size_t index, Optiona
int row = current_pixel_index / width;
int col = current_pixel_index % width;
auto actual_row = row;
if (y_origin < height)
if (is_top_to_bottom)
actual_row = height - 1 - row;
auto actual_col = col;
if (x_origin > width)
if (!is_left_to_right)
actual_col = width - 1 - col;
bitmap->scanline(actual_row)[actual_col] = pixel;
if (pixel_packet_header.raw && (current_pixel_index + 1) < max_pixel_index)
Expand Down

0 comments on commit 66bdb3b

Please sign in to comment.