mirror of
https://github.com/LadybirdBrowser/ladybird.git
synced 2025-08-01 05:39:11 +00:00
LibGfx/JPEG2000: Remove an incorrect VERIFY in TagTree construction
...and add a test case that shows why it's incorrect. If one dimension is 2^n + 1 and the other side is just 1, then the topmost node will have 2^n x 1 and 1 x 1 children. The first child will have n levels of children. The 1 x 1 child could end immediately, or it could require that it also has n levels of (all 1 x 1) children. The spec isn't clear on which of the two alternatives should happen. We currently have n levels of 1 x 1 blocks. This test case shows that a VERIFY we had was incorrect, so remove it. The alternative implementation is to keep the VERIFY and to add a if (x_count == 1 && y_count == 1) level = 0; to the top of TagTreeNode::create(). Then we don't have multiple levels of 1 x 1 nodes, and we need to read fewer bits. The images in the spec suggest that all nodes should have the same number of levels, so go with that interpretation for now. Once we can actually decode images, we'll hopefully see which of the two interpretations is correct. (The removed VERIFY() is hit when decoding Tests/LibGfx/test-inputs/jpeg2000/buggie-gray.jpf in a local branch that has some image decoding implemented. That file contains a packet with 1x3 code-blocks, which hits this case.)
This commit is contained in:
parent
342b358341
commit
f8362c8abf
Notes:
sideshowbarker
2024-07-17 00:27:16 +09:00
Author: https://github.com/nico
Commit: f8362c8abf
Pull-request: https://github.com/SerenityOS/serenity/pull/24137
Reviewed-by: https://github.com/timschumi ✅
2 changed files with 27 additions and 1 deletions
|
@ -682,6 +682,33 @@ TEST_CASE(test_jpeg2000_tag_tree)
|
|||
EXPECT_EQ(1u, MUST(tree.read_value(2, 1, read_bit, next_layer)));
|
||||
EXPECT_EQ(index, 7u); // Didn't change!
|
||||
}
|
||||
|
||||
{
|
||||
// This isn't in the spec. If one dimension is 2^n + 1 and the other side is just 1, then the topmost node will have
|
||||
// 2^n x 1 and 1 x 1 children. The first child will have n levels of children. The 1 x 1 child could end immediately,
|
||||
// or it could require that it also has n levels of (all 1 x 1) children. The spec isn't clear on which of
|
||||
// the two alternatives should happen. We currently have n levels of 1 x 1 blocks.
|
||||
constexpr auto n = 5;
|
||||
auto tree = TRY_OR_FAIL(Gfx::JPEG2000::TagTree::create((1 << n) + 1, 1));
|
||||
Vector<u8> bits;
|
||||
bits.append(1); // Finalize topmost node.
|
||||
bits.append(0); // Increment value in 1 x 1 child.
|
||||
bits.append(1); // Finalize 1 x 1 child.
|
||||
|
||||
// Finalize further 1 x 1 children, if present.
|
||||
for (size_t i = 0; i < n; ++i)
|
||||
bits.append(1);
|
||||
|
||||
size_t index = 0;
|
||||
Function<ErrorOr<bool>()> read_bit = [&]() -> bool {
|
||||
return bits[index++];
|
||||
};
|
||||
|
||||
EXPECT_EQ(1u, MUST(tree.read_value(1 << n, 0, read_bit)));
|
||||
|
||||
// This will read either 3 or 3 + n bits, depending on the interpretation.
|
||||
EXPECT_EQ(index, 3u + n);
|
||||
}
|
||||
}
|
||||
|
||||
TEST_CASE(test_pam_rgb)
|
||||
|
|
|
@ -942,7 +942,6 @@ struct TagTreeNode {
|
|||
return node;
|
||||
}
|
||||
|
||||
VERIFY(x_count > 1 || y_count > 1);
|
||||
u32 top_left_x_child_count = min(x_count, 1u << (max(level, 1) - 1));
|
||||
u32 top_left_y_child_count = min(y_count, 1u << (max(level, 1) - 1));
|
||||
for (u32 y = 0; y < 2; ++y) {
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue