From ee4f9ef82cef7f2e64a2087717fd4fd54d66a28b Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Tue, 5 May 2026 11:28:42 +0200 Subject: [PATCH] LibGfx: Preserve native YUV samples for CPU conversion This is a small prerequisite for moving decoded YUV frames from ImmutableBitmap into Media::VideoFrame. Once the frame owns the planes directly, CPU consumers and the Skia upload path will share the same YUVData object. Those consumers need different representations. Skia expects high bit depth YUVA pixmaps to be full-range 16-bit samples, while CPU conversion paths must see the decoder-native samples and the original bit depth. Keep YUVData in decoder-native form, and make the Skia expansion a temporary copy owned by SkYUVAPixmaps. Add a regression test that checks 10-bit pixmaps are expanded for Skia without mutating the source planes or changing YUVData::to_bitmap() output. --- Libraries/LibGfx/ImmutableBitmap.cpp | 3 - Libraries/LibGfx/YUVData.cpp | 55 ++++++++++++------ Libraries/LibGfx/YUVData.h | 2 - Tests/LibGfx/CMakeLists.txt | 4 ++ Tests/LibGfx/TestYUVData.cpp | 85 ++++++++++++++++++++++++++++ 5 files changed, 126 insertions(+), 23 deletions(-) create mode 100644 Tests/LibGfx/TestYUVData.cpp diff --git a/Libraries/LibGfx/ImmutableBitmap.cpp b/Libraries/LibGfx/ImmutableBitmap.cpp index aceb0a1629..dac6ce9ad8 100644 --- a/Libraries/LibGfx/ImmutableBitmap.cpp +++ b/Libraries/LibGfx/ImmutableBitmap.cpp @@ -211,9 +211,6 @@ ErrorOr> ImmutableBitmap::create_from_yuv(Nonnull { auto color_space = TRY(ColorSpace::from_cicp(yuv_data->cicp())); - if (yuv_data->bit_depth() > 8) - yuv_data->expand_samples_to_full_16_bit_range(); - ImmutableBitmapImpl impl { .bitmap = nullptr, .yuv_data = move(yuv_data), diff --git a/Libraries/LibGfx/YUVData.cpp b/Libraries/LibGfx/YUVData.cpp index 0f0b7c4932..0ca90b4673 100644 --- a/Libraries/LibGfx/YUVData.cpp +++ b/Libraries/LibGfx/YUVData.cpp @@ -225,21 +225,6 @@ ErrorOr> YUVData::to_bitmap() const return bitmap; } -void YUVData::expand_samples_to_full_16_bit_range() -{ - auto const shift = 16 - m_impl->bit_depth; - auto const inverse_shift = m_impl->bit_depth - shift; - - for (auto buffer : { m_impl->y_buffer.span(), m_impl->u_buffer.span(), m_impl->v_buffer.span() }) { - auto* samples = reinterpret_cast(buffer.data()); - auto sample_count = buffer.size() / sizeof(u16); - for (size_t i = 0; i < sample_count; i++) - samples[i] = static_cast((samples[i] << shift) | (samples[i] >> inverse_shift)); - } - - m_impl->bit_depth = 16; -} - static SkYUVColorSpace skia_yuv_color_space(Media::CodingIndependentCodePoints cicp) { bool full_range = cicp.video_full_range_flag() == Media::VideoFullRangeFlag::Full; @@ -280,6 +265,31 @@ static SkYUVAInfo::Subsampling skia_subsampling(Media::Subsampling subsampling) return SkYUVAInfo::Subsampling::k420; } +static u16 expand_sample_to_full_16_bit_range(u16 sample, u8 bit_depth) +{ + if (bit_depth >= 16) + return sample; + + auto const shift = 16 - bit_depth; + auto const inverse_shift = bit_depth - shift; + return static_cast((sample << shift) | (sample >> inverse_shift)); +} + +static void copy_plane_expanded_to_full_16_bit_range(FixedArray const& source_buffer, SkPixmap const& destination, IntSize plane_size, u8 bit_depth) +{ + VERIFY(bit_depth > 8); + + auto const* source = reinterpret_cast(source_buffer.data()); + auto source_stride = static_cast(plane_size.width()); + + for (int row = 0; row < plane_size.height(); row++) { + auto const* source_row = source + (static_cast(row) * source_stride); + auto* destination_row = destination.writable_addr16(0, row); + for (int column = 0; column < plane_size.width(); column++) + destination_row[column] = expand_sample_to_full_16_bit_range(source_row[column], bit_depth); + } +} + SkYUVAPixmaps YUVData::make_pixmaps() const { auto skia_size = SkISize::Make(m_impl->size.width(), m_impl->size.height()); @@ -298,9 +308,18 @@ SkYUVAPixmaps YUVData::make_pixmaps() const data_type = SkYUVAPixmapInfo::DataType::kUnorm8; component_size = 1; } else { - color_type = kA16_unorm_SkColorType; - data_type = SkYUVAPixmapInfo::DataType::kUnorm16; - component_size = 2; + SkYUVAPixmapInfo pixmap_info(yuva_info, SkYUVAPixmapInfo::DataType::kUnorm16, nullptr); + auto pixmaps = SkYUVAPixmaps::Allocate(pixmap_info); + if (!pixmaps.isValid()) + return pixmaps; + + copy_plane_expanded_to_full_16_bit_range(m_impl->y_buffer, pixmaps.plane(0), m_impl->size, m_impl->bit_depth); + + auto uv_size = m_impl->subsampling.subsampled_size(m_impl->size); + copy_plane_expanded_to_full_16_bit_range(m_impl->u_buffer, pixmaps.plane(1), uv_size, m_impl->bit_depth); + copy_plane_expanded_to_full_16_bit_range(m_impl->v_buffer, pixmaps.plane(2), uv_size, m_impl->bit_depth); + + return pixmaps; } auto y_row_bytes = static_cast(m_impl->size.width()) * component_size; diff --git a/Libraries/LibGfx/YUVData.h b/Libraries/LibGfx/YUVData.h index 1d51cee73e..f7c7ee5f5f 100644 --- a/Libraries/LibGfx/YUVData.h +++ b/Libraries/LibGfx/YUVData.h @@ -48,8 +48,6 @@ public: SkYUVAPixmaps make_pixmaps() const; - void expand_samples_to_full_16_bit_range(); - private: explicit YUVData(NonnullOwnPtr); diff --git a/Tests/LibGfx/CMakeLists.txt b/Tests/LibGfx/CMakeLists.txt index 0a2cf80711..0088496c24 100644 --- a/Tests/LibGfx/CMakeLists.txt +++ b/Tests/LibGfx/CMakeLists.txt @@ -1,3 +1,5 @@ +include(skia) + set(TEST_SOURCES BenchmarkJPEGLoader.cpp TestColor.cpp @@ -13,3 +15,5 @@ set(TEST_SOURCES foreach(source IN LISTS TEST_SOURCES) ladybird_test("${source}" LibGfx LIBS LibGfx) endforeach() + +ladybird_test(TestYUVData.cpp LibGfx LIBS LibGfx skia) diff --git a/Tests/LibGfx/TestYUVData.cpp b/Tests/LibGfx/TestYUVData.cpp new file mode 100644 index 0000000000..fec8a5c3a0 --- /dev/null +++ b/Tests/LibGfx/TestYUVData.cpp @@ -0,0 +1,85 @@ +/* + * Copyright (c) 2026, Aliaksandr Kalenik + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include +#include +#include + +#include + +static u16 expand_10_bit_sample_to_full_16_bit_range(u16 sample) +{ + return static_cast((sample << 6) | (sample >> 4)); +} + +template +static void write_samples(Bytes plane, Array const& samples) +{ + auto* destination = reinterpret_cast(plane.data()); + for (size_t i = 0; i < Size; i++) + destination[i] = samples[i]; +} + +template +static void expect_samples(Bytes plane, Array const& expected) +{ + auto const* samples = reinterpret_cast(plane.data()); + for (size_t i = 0; i < Size; i++) + EXPECT_EQ(samples[i], expected[i]); +} + +template +static void expect_expanded_pixmap_samples(SkPixmap const& plane, Array const& native_samples, int width) +{ + for (size_t i = 0; i < Size; i++) { + auto x = static_cast(i % static_cast(width)); + auto y = static_cast(i / static_cast(width)); + EXPECT_EQ(*plane.addr16(x, y), expand_10_bit_sample_to_full_16_bit_range(native_samples[i])); + } +} + +TEST_CASE(high_bit_depth_pixmaps_expand_without_mutating_native_samples) +{ + auto const cicp = Media::CodingIndependentCodePoints { + Media::ColorPrimaries::BT709, + Media::TransferCharacteristics::BT709, + Media::MatrixCoefficients::BT709, + Media::VideoFullRangeFlag::Full, + }; + auto yuv_data = TRY_OR_FAIL(Gfx::YUVData::create({ 2, 2 }, 10, Media::Subsampling { false, false }, cicp)); + + Array const y_samples { 0, 341, 682, 1023 }; + Array const u_samples { 512, 513, 514, 515 }; + Array const v_samples { 508, 509, 510, 511 }; + + write_samples(yuv_data->y_data(), y_samples); + write_samples(yuv_data->u_data(), u_samples); + write_samples(yuv_data->v_data(), v_samples); + + auto bitmap_before = TRY_OR_FAIL(yuv_data->to_bitmap()); + + auto pixmaps = yuv_data->make_pixmaps(); + EXPECT(pixmaps.isValid()); + EXPECT(pixmaps.ownsStorage()); + EXPECT(pixmaps.dataType() == SkYUVAPixmapInfo::DataType::kUnorm16); + EXPECT_EQ(pixmaps.numPlanes(), 3); + + expect_expanded_pixmap_samples(pixmaps.plane(0), y_samples, 2); + expect_expanded_pixmap_samples(pixmaps.plane(1), u_samples, 2); + expect_expanded_pixmap_samples(pixmaps.plane(2), v_samples, 2); + + EXPECT_EQ(yuv_data->bit_depth(), 10); + expect_samples(yuv_data->y_data(), y_samples); + expect_samples(yuv_data->u_data(), u_samples); + expect_samples(yuv_data->v_data(), v_samples); + + auto bitmap_after = TRY_OR_FAIL(yuv_data->to_bitmap()); + for (int y = 0; y < yuv_data->size().height(); y++) { + for (int x = 0; x < yuv_data->size().width(); x++) + EXPECT_EQ(bitmap_after->get_pixel(x, y), bitmap_before->get_pixel(x, y)); + } +}