From b6419f2cf24a37f99823acdc3ec095c48c3cdd3e Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sat, 20 Nov 2021 13:04:38 +0100 Subject: [PATCH] LibGUI: Make clipboard-as-bitmap parsing less data-race-y This encourages the caller to first fetch data and type atomically, and then parse that, instead of potentially making multiple requests. --- .../Applications/PixelPaint/MainWidget.cpp | 2 +- Userland/Libraries/LibGUI/Clipboard.cpp | 20 +++++++++---------- Userland/Libraries/LibGUI/Clipboard.h | 3 ++- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/Userland/Applications/PixelPaint/MainWidget.cpp b/Userland/Applications/PixelPaint/MainWidget.cpp index bc14a3f2f1..4e3613a457 100644 --- a/Userland/Applications/PixelPaint/MainWidget.cpp +++ b/Userland/Applications/PixelPaint/MainWidget.cpp @@ -214,7 +214,7 @@ void MainWidget::initialize_menubar(GUI::Window& window) auto* editor = current_image_editor(); if (!editor) return; - auto bitmap = GUI::Clipboard::the().bitmap(); + auto bitmap = GUI::Clipboard::the().data_and_type().as_bitmap(); if (!bitmap) return; diff --git a/Userland/Libraries/LibGUI/Clipboard.cpp b/Userland/Libraries/LibGUI/Clipboard.cpp index 389d99c90b..19b0281cf8 100644 --- a/Userland/Libraries/LibGUI/Clipboard.cpp +++ b/Userland/Libraries/LibGUI/Clipboard.cpp @@ -64,34 +64,34 @@ Clipboard::DataAndType Clipboard::data_and_type() const return { data.release_value(), type, metadata }; } -RefPtr Clipboard::bitmap() const +RefPtr Clipboard::DataAndType::as_bitmap() const { - auto clipping = data_and_type(); - - if (clipping.mime_type != "image/x-serenityos") + if (mime_type != "image/x-serenityos") return nullptr; - auto width = clipping.metadata.get("width").value_or("0").to_uint(); + auto width = metadata.get("width").value_or("0").to_uint(); if (!width.has_value() || width.value() == 0) return nullptr; - auto height = clipping.metadata.get("height").value_or("0").to_uint(); + auto height = metadata.get("height").value_or("0").to_uint(); if (!height.has_value() || height.value() == 0) return nullptr; - auto scale = clipping.metadata.get("scale").value_or("0").to_uint(); + auto scale = metadata.get("scale").value_or("0").to_uint(); if (!scale.has_value() || scale.value() == 0) return nullptr; - auto pitch = clipping.metadata.get("pitch").value_or("0").to_uint(); + auto pitch = metadata.get("pitch").value_or("0").to_uint(); if (!pitch.has_value() || pitch.value() == 0) return nullptr; - auto format = clipping.metadata.get("format").value_or("0").to_uint(); + auto format = metadata.get("format").value_or("0").to_uint(); if (!format.has_value() || format.value() == 0) return nullptr; - auto clipping_bitmap_or_error = Gfx::Bitmap::try_create_wrapper((Gfx::BitmapFormat)format.value(), { (int)width.value(), (int)height.value() }, scale.value(), pitch.value(), clipping.data.data()); + // We won't actually write to the clipping_bitmap, so casting away the const is okay. + auto clipping_data = const_cast(data.data()); + auto clipping_bitmap_or_error = Gfx::Bitmap::try_create_wrapper((Gfx::BitmapFormat)format.value(), { (int)width.value(), (int)height.value() }, scale.value(), pitch.value(), clipping_data); if (clipping_bitmap_or_error.is_error()) return nullptr; auto clipping_bitmap = clipping_bitmap_or_error.release_value_but_fixme_should_propagate_errors(); diff --git a/Userland/Libraries/LibGUI/Clipboard.h b/Userland/Libraries/LibGUI/Clipboard.h index 5b0637007d..4034555b75 100644 --- a/Userland/Libraries/LibGUI/Clipboard.h +++ b/Userland/Libraries/LibGUI/Clipboard.h @@ -32,6 +32,8 @@ public: ByteBuffer data; String mime_type; HashMap metadata; + + RefPtr as_bitmap() const; }; static void initialize(Badge); @@ -40,7 +42,6 @@ public: DataAndType data_and_type() const; ByteBuffer data() const { return data_and_type().data; } String mime_type() const { return data_and_type().mime_type; } - RefPtr bitmap() const; void set_data(ReadonlyBytes data, String const& mime_type = "text/plain", HashMap const& metadata = {}); void set_plain_text(String const& text) { set_data(text.bytes()); }