From 2c98eff558f43afb031d23b3ccbf9360ccee0149 Mon Sep 17 00:00:00 2001
From: Lucas CHOLLET <lucas.chollet@free.fr>
Date: Sat, 25 Feb 2023 17:15:54 -0500
Subject: [PATCH] LibGfx: Consider component interleaving when reading a scan

Scan with only one component are by definition not interleaved, meaning
that each value is linearly ordered in the stream. Grayscale images
were supported thanks to a hack, by forcing the subsampling to 1.

Now we properly support grayscale image with other subsampling (even if
it doesn't make sense) and more generally scans with only one component
and any sampling factors.

While this solution is more general than the last one it also feels a
bit hackish. We should probably refactor the way we iterate over
components and macroblocks. But that's work for latter, especially when
we will add support for other subsampling than 4-2-2.
---
 Userland/Libraries/LibGfx/JPEGLoader.cpp | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/Userland/Libraries/LibGfx/JPEGLoader.cpp b/Userland/Libraries/LibGfx/JPEGLoader.cpp
index 5145ac6b0f..f34b837f6c 100644
--- a/Userland/Libraries/LibGfx/JPEGLoader.cpp
+++ b/Userland/Libraries/LibGfx/JPEGLoader.cpp
@@ -202,6 +202,12 @@ struct Scan {
     u8 successive_approximation {};
 
     HuffmanStreamState huffman_stream;
+
+    // See the note on Figure B.4 - Scan header syntax
+    bool are_components_interleaved() const
+    {
+        return components.size() != 1;
+    }
 };
 
 struct JPEGLoadingContext {
@@ -399,7 +405,11 @@ static ErrorOr<void> build_macroblocks(JPEGLoadingContext& context, Vector<Macro
 
         for (u8 vfactor_i = 0; vfactor_i < scan_component.component.vsample_factor; vfactor_i++) {
             for (u8 hfactor_i = 0; hfactor_i < scan_component.component.hsample_factor; hfactor_i++) {
+                // A.2.3 - Interleaved order
                 u32 mb_index = (vcursor + vfactor_i) * context.mblock_meta.hpadded_count + (hfactor_i + hcursor);
+                if (!context.current_scan.are_components_interleaved())
+                    mb_index = vcursor * context.mblock_meta.hpadded_count + (hfactor_i + (hcursor * scan_component.component.vsample_factor) + (vfactor_i * scan_component.component.hsample_factor));
+
                 Macroblock& block = macroblocks[mb_index];
 
                 if (context.current_scan.spectral_selection_start == 0)
@@ -844,12 +854,6 @@ static ErrorOr<void> read_start_of_frame(AK::SeekableStream& stream, JPEGLoading
         component.vsample_factor = subsample_factors & 0x0F;
 
         if (i == 0) {
-            // If there is only a single component, i.e. grayscale, the macroblocks will not be interleaved, even if
-            // the horizontal or vertical sample factor is larger than 1.
-            if (component_count == 1) {
-                component.hsample_factor = 1;
-                component.vsample_factor = 1;
-            }
             // By convention, downsampling is applied only on chroma components. So we should
             //  hope to see the maximum sampling factor in the luma component.
             if (!validate_luma_and_modify_context(component, context)) {