diff --git a/Applications/PixelPaint/BucketTool.cpp b/Applications/PixelPaint/BucketTool.cpp index 92f3ab2aa8..978df6a5ef 100644 --- a/Applications/PixelPaint/BucketTool.cpp +++ b/Applications/PixelPaint/BucketTool.cpp @@ -58,10 +58,10 @@ static void flood_fill(Gfx::Bitmap& bitmap, const Gfx::IntPoint& start_position, while (!queue.is_empty()) { auto position = queue.dequeue(); - if (bitmap.get_pixel(position.x(), position.y()) != target_color) + if (bitmap.get_pixel(position.x(), position.y()) != target_color) continue; - bitmap.set_pixel(position.x(), position.y(), fill_color); + bitmap.set_pixel(position.x(), position.y(), fill_color); if (position.x() != 0) queue.enqueue(position.translated(-1, 0)); diff --git a/Applications/PixelPaint/SprayTool.cpp b/Applications/PixelPaint/SprayTool.cpp index 85a6200f97..f1f73c29aa 100644 --- a/Applications/PixelPaint/SprayTool.cpp +++ b/Applications/PixelPaint/SprayTool.cpp @@ -77,7 +77,7 @@ void SprayTool::paint_it() continue; if (ypos < 0 || ypos >= bitmap.height()) continue; - bitmap.set_pixel(xpos, ypos, m_color); + bitmap.set_pixel(xpos, ypos, m_color); } layer->did_modify_bitmap(*m_editor->image()); diff --git a/Libraries/LibGUI/Clipboard.cpp b/Libraries/LibGUI/Clipboard.cpp index a711fa2597..33c64c7157 100644 --- a/Libraries/LibGUI/Clipboard.cpp +++ b/Libraries/LibGUI/Clipboard.cpp @@ -141,7 +141,7 @@ RefPtr Clipboard::bitmap() const if (!format.has_value() || format.value() == 0) return nullptr; - auto clipping_bitmap = Gfx::Bitmap::create_wrapper((Gfx::BitmapFormat)format.value(), { (int)width.value(), (int)height.value() }, pitch.value(), (Gfx::RGBA32*)clipping.data.data()); + auto clipping_bitmap = Gfx::Bitmap::create_wrapper((Gfx::BitmapFormat)format.value(), { (int)width.value(), (int)height.value() }, pitch.value(), clipping.data.data()); auto bitmap = Gfx::Bitmap::create(Gfx::BitmapFormat::RGBA32, { (int)width.value(), (int)height.value() }); for (int y = 0; y < clipping_bitmap->height(); ++y) { diff --git a/Libraries/LibGfx/Bitmap.cpp b/Libraries/LibGfx/Bitmap.cpp index 484e6ff733..6b306f40f7 100644 --- a/Libraries/LibGfx/Bitmap.cpp +++ b/Libraries/LibGfx/Bitmap.cpp @@ -44,16 +44,34 @@ namespace Gfx { -static bool size_would_overflow(BitmapFormat, const IntSize& size) +size_t Bitmap::minimum_pitch(size_t width, BitmapFormat format) +{ + size_t element_size; + switch (determine_storage_format(format)) { + case StorageFormat::Indexed8: + element_size = 1; + break; + case StorageFormat::RGB32: + case StorageFormat::RGBA32: + element_size = 4; + break; + default: + ASSERT_NOT_REACHED(); + } + + return width * element_size; +} + +static bool size_would_overflow(BitmapFormat format, const IntSize& size) { if (size.width() < 0 || size.height() < 0) return true; // This check is a bit arbitrary, but should protect us from most shenanigans: if (size.width() >= 32768 || size.height() >= 32768) return true; - // This check is absolutely necessary. Note that Bitmap::Bitmap always stores - // data as RGBA32 internally, so currently we ignore the indicated format. - return Checked::multiplication_would_overflow(size.width(), size.height(), sizeof(RGBA32)); + // In contrast, this check is absolutely necessary: + size_t pitch = Bitmap::minimum_pitch(size.width(), format); + return Checked::multiplication_would_overflow(pitch, size.height()); } RefPtr Bitmap::create(BitmapFormat format, const IntSize& size) @@ -72,7 +90,7 @@ RefPtr Bitmap::create_purgeable(BitmapFormat format, const IntSize& size Bitmap::Bitmap(BitmapFormat format, const IntSize& size, Purgeable purgeable) : m_size(size) - , m_pitch(round_up_to_power_of_two(size.width() * sizeof(RGBA32), 16)) + , m_pitch(minimum_pitch(size.width(), format)) , m_format(format) , m_purgeable(purgeable == Purgeable::Yes) { @@ -81,10 +99,10 @@ Bitmap::Bitmap(BitmapFormat format, const IntSize& size, Purgeable purgeable) allocate_palette_from_format(format, {}); #ifdef __serenity__ int map_flags = purgeable == Purgeable::Yes ? (MAP_PURGEABLE | MAP_PRIVATE) : (MAP_ANONYMOUS | MAP_PRIVATE); - m_data = (RGBA32*)mmap_with_name(nullptr, size_in_bytes(), PROT_READ | PROT_WRITE, map_flags, 0, 0, String::format("GraphicsBitmap [%dx%d]", width(), height()).characters()); + m_data = mmap_with_name(nullptr, size_in_bytes(), PROT_READ | PROT_WRITE, map_flags, 0, 0, String::format("GraphicsBitmap [%dx%d]", width(), height()).characters()); #else int map_flags = (MAP_ANONYMOUS | MAP_PRIVATE); - m_data = (RGBA32*)mmap(nullptr, size_in_bytes(), PROT_READ | PROT_WRITE, map_flags, 0, 0); + m_data = mmap(nullptr, size_in_bytes(), PROT_READ | PROT_WRITE, map_flags, 0, 0); #endif if (m_data == MAP_FAILED) { perror("mmap"); @@ -94,7 +112,7 @@ Bitmap::Bitmap(BitmapFormat format, const IntSize& size, Purgeable purgeable) m_needs_munmap = true; } -RefPtr Bitmap::create_wrapper(BitmapFormat format, const IntSize& size, size_t pitch, RGBA32* data) +RefPtr Bitmap::create_wrapper(BitmapFormat format, const IntSize& size, size_t pitch, void* data) { if (size_would_overflow(format, size)) return nullptr; @@ -112,13 +130,16 @@ RefPtr Bitmap::load_from_file(const StringView& path) return nullptr; } -Bitmap::Bitmap(BitmapFormat format, const IntSize& size, size_t pitch, RGBA32* data) +Bitmap::Bitmap(BitmapFormat format, const IntSize& size, size_t pitch, void* data) : m_size(size) , m_data(data) , m_pitch(pitch) , m_format(format) { + ASSERT(pitch >= minimum_pitch(size.width(), format)); ASSERT(!size_would_overflow(format, size)); + // FIXME: assert that `data` is actually long enough! + allocate_palette_from_format(format, {}); } @@ -138,8 +159,8 @@ RefPtr Bitmap::create_with_shared_buffer(BitmapFormat format, NonnullRef Bitmap::Bitmap(BitmapFormat format, NonnullRefPtr&& shared_buffer, const IntSize& size, const Vector& palette) : m_size(size) - , m_data((RGBA32*)shared_buffer->data()) - , m_pitch(round_up_to_power_of_two(size.width() * sizeof(RGBA32), 16)) + , m_data(shared_buffer->data()) + , m_pitch(minimum_pitch(size.width(), format)) , m_format(format) , m_shared_buffer(move(shared_buffer)) { diff --git a/Libraries/LibGfx/Bitmap.h b/Libraries/LibGfx/Bitmap.h index f60ddfdd0a..022c841b80 100644 --- a/Libraries/LibGfx/Bitmap.h +++ b/Libraries/LibGfx/Bitmap.h @@ -56,6 +56,29 @@ enum class BitmapFormat { RGBA32, }; +enum class StorageFormat { + Indexed8, + RGB32, + RGBA32, +}; + +static StorageFormat determine_storage_format(BitmapFormat format) +{ + switch (format) { + case BitmapFormat::RGB32: + return StorageFormat::RGB32; + case BitmapFormat::RGBA32: + return StorageFormat::RGBA32; + case BitmapFormat::Indexed1: + case BitmapFormat::Indexed2: + case BitmapFormat::Indexed4: + case BitmapFormat::Indexed8: + return StorageFormat::Indexed8; + default: + ASSERT_NOT_REACHED(); + } +} + enum RotationDirection { Left, Right @@ -65,7 +88,7 @@ class Bitmap : public RefCounted { public: static RefPtr create(BitmapFormat, const IntSize&); static RefPtr create_purgeable(BitmapFormat, const IntSize&); - static RefPtr create_wrapper(BitmapFormat, const IntSize&, size_t pitch, RGBA32*); + static RefPtr create_wrapper(BitmapFormat, const IntSize&, size_t pitch, void*); static RefPtr load_from_file(const StringView& path); static RefPtr create_with_shared_buffer(BitmapFormat, NonnullRefPtr&&, const IntSize&); static RefPtr create_with_shared_buffer(BitmapFormat, NonnullRefPtr&&, const IntSize&, const Vector& palette); @@ -153,6 +176,8 @@ public: } } + static size_t minimum_pitch(size_t width, BitmapFormat); + unsigned bpp() const { return bpp_for_format(m_format); @@ -170,31 +195,17 @@ public: Color palette_color(u8 index) const { return Color::from_rgba(m_palette[index]); } void set_palette_color(u8 index, Color color) { m_palette[index] = color.value(); } - template - Color get_pixel(int x, int y) const - { - (void)x; - (void)y; - ASSERT_NOT_REACHED(); - } - + template + Color get_pixel(int x, int y) const; Color get_pixel(int x, int y) const; - Color get_pixel(const IntPoint& position) const { return get_pixel(position.x(), position.y()); } - template - void set_pixel(int x, int y, Color) - { - (void)x; - (void)y; - ASSERT_NOT_REACHED(); - } - + template + void set_pixel(int x, int y, Color); void set_pixel(int x, int y, Color); - void set_pixel(const IntPoint& position, Color color) { set_pixel(position.x(), position.y(), color); @@ -211,13 +222,13 @@ private: Yes }; Bitmap(BitmapFormat, const IntSize&, Purgeable); - Bitmap(BitmapFormat, const IntSize&, size_t pitch, RGBA32*); + Bitmap(BitmapFormat, const IntSize&, size_t pitch, void*); Bitmap(BitmapFormat, NonnullRefPtr&&, const IntSize&, const Vector& palette); void allocate_palette_from_format(BitmapFormat, const Vector& source_palette); IntSize m_size; - RGBA32* m_data { nullptr }; + void* m_data { nullptr }; RGBA32* m_palette { nullptr }; size_t m_pitch { 0 }; BitmapFormat m_format { BitmapFormat::Invalid }; @@ -229,12 +240,12 @@ private: inline u8* Bitmap::scanline_u8(int y) { - return (u8*)m_data + (y * m_pitch); + return reinterpret_cast(m_data) + (y * m_pitch); } inline const u8* Bitmap::scanline_u8(int y) const { - return (const u8*)m_data + (y * m_pitch); + return reinterpret_cast(m_data) + (y * m_pitch); } inline RGBA32* Bitmap::scanline(int y) @@ -248,86 +259,57 @@ inline const RGBA32* Bitmap::scanline(int y) const } template<> -inline Color Bitmap::get_pixel(int x, int y) const +inline Color Bitmap::get_pixel(int x, int y) const { return Color::from_rgb(scanline(y)[x]); } template<> -inline Color Bitmap::get_pixel(int x, int y) const +inline Color Bitmap::get_pixel(int x, int y) const { return Color::from_rgba(scanline(y)[x]); } template<> -inline Color Bitmap::get_pixel(int x, int y) const -{ - return Color::from_rgb(m_palette[scanline_u8(y)[x]]); -} - -template<> -inline Color Bitmap::get_pixel(int x, int y) const -{ - return Color::from_rgb(m_palette[scanline_u8(y)[x]]); -} - -template<> -inline Color Bitmap::get_pixel(int x, int y) const -{ - return Color::from_rgb(m_palette[scanline_u8(y)[x]]); -} - -template<> -inline Color Bitmap::get_pixel(int x, int y) const +inline Color Bitmap::get_pixel(int x, int y) const { return Color::from_rgb(m_palette[scanline_u8(y)[x]]); } inline Color Bitmap::get_pixel(int x, int y) const { - switch (m_format) { - case BitmapFormat::RGB32: - return get_pixel(x, y); - case BitmapFormat::RGBA32: - return get_pixel(x, y); - case BitmapFormat::Indexed1: - return get_pixel(x, y); - case BitmapFormat::Indexed2: - return get_pixel(x, y); - case BitmapFormat::Indexed4: - return get_pixel(x, y); - case BitmapFormat::Indexed8: - return get_pixel(x, y); + switch (determine_storage_format(m_format)) { + case StorageFormat::RGB32: + return get_pixel(x, y); + case StorageFormat::RGBA32: + return get_pixel(x, y); + case StorageFormat::Indexed8: + return get_pixel(x, y); default: ASSERT_NOT_REACHED(); } } template<> -inline void Bitmap::set_pixel(int x, int y, Color color) +inline void Bitmap::set_pixel(int x, int y, Color color) { scanline(y)[x] = color.value(); } - template<> -inline void Bitmap::set_pixel(int x, int y, Color color) +inline void Bitmap::set_pixel(int x, int y, Color color) { - scanline(y)[x] = color.value(); + scanline(y)[x] = color.value(); // drop alpha } - inline void Bitmap::set_pixel(int x, int y, Color color) { - switch (m_format) { - case BitmapFormat::RGB32: - set_pixel(x, y, color); + switch (determine_storage_format(m_format)) { + case StorageFormat::RGB32: + set_pixel(x, y, color); break; - case BitmapFormat::RGBA32: - set_pixel(x, y, color); + case StorageFormat::RGBA32: + set_pixel(x, y, color); break; - case BitmapFormat::Indexed1: - case BitmapFormat::Indexed2: - case BitmapFormat::Indexed4: - case BitmapFormat::Indexed8: + case StorageFormat::Indexed8: ASSERT_NOT_REACHED(); default: ASSERT_NOT_REACHED();