From 508d56318931c0d6a3fd077ac2dc9f83bbfe612b Mon Sep 17 00:00:00 2001 From: Tobias Christiansen Date: Fri, 3 Sep 2021 18:46:11 +0200 Subject: [PATCH] PixelPaint: Add ProjectLoader to abstract away opening of files This new class will open and parse files (either images directly or .pp project files) and one can get the parsed Image as well as other information from it. This patch removes a bunch of 'try_create_from..." methods from Image in favor of using the ProjectLoader. The only json_metadata that is available are Guides for now. --- .../Applications/PixelPaint/CMakeLists.txt | 1 + Userland/Applications/PixelPaint/Image.cpp | 118 +++--------------- Userland/Applications/PixelPaint/Image.h | 10 +- .../Applications/PixelPaint/ProjectLoader.cpp | 75 +++++++++++ .../Applications/PixelPaint/ProjectLoader.h | 35 ++++++ Userland/Applications/PixelPaint/main.cpp | 18 +-- 6 files changed, 142 insertions(+), 115 deletions(-) create mode 100644 Userland/Applications/PixelPaint/ProjectLoader.cpp create mode 100644 Userland/Applications/PixelPaint/ProjectLoader.h diff --git a/Userland/Applications/PixelPaint/CMakeLists.txt b/Userland/Applications/PixelPaint/CMakeLists.txt index fa7cf8e41d..3fe320a349 100644 --- a/Userland/Applications/PixelPaint/CMakeLists.txt +++ b/Userland/Applications/PixelPaint/CMakeLists.txt @@ -30,6 +30,7 @@ set(SOURCES PenTool.cpp PickerTool.cpp PixelPaintWindowGML.h + ProjectLoader.cpp RectangleTool.cpp RectangleSelectTool.cpp Mask.cpp diff --git a/Userland/Applications/PixelPaint/Image.cpp b/Userland/Applications/PixelPaint/Image.cpp index 524e03f3e9..5d32cf2dc8 100644 --- a/Userland/Applications/PixelPaint/Image.cpp +++ b/Userland/Applications/PixelPaint/Image.cpp @@ -24,23 +24,6 @@ namespace PixelPaint { -static RefPtr try_decode_bitmap(ByteBuffer const& bitmap_data) -{ - // Spawn a new ImageDecoder service process and connect to it. - auto client = ImageDecoderClient::Client::construct(); - - // FIXME: Find a way to avoid the memory copying here. - auto decoded_image_or_error = client->decode_image(bitmap_data); - if (!decoded_image_or_error.has_value()) - return nullptr; - - // FIXME: Support multi-frame images? - auto decoded_image = decoded_image_or_error.release_value(); - if (decoded_image.frames.is_empty()) - return nullptr; - return move(decoded_image.frames[0].bitmap); -} - RefPtr Image::try_create_with_size(Gfx::IntSize const& size) { if (size.is_empty()) @@ -72,6 +55,23 @@ void Image::paint_into(GUI::Painter& painter, Gfx::IntRect const& dest_rect) con } } +RefPtr Image::try_decode_bitmap(ByteBuffer const& bitmap_data) +{ + // Spawn a new ImageDecoder service process and connect to it. + auto client = ImageDecoderClient::Client::construct(); + + // FIXME: Find a way to avoid the memory copying here. + auto decoded_image_or_error = client->decode_image(bitmap_data); + if (!decoded_image_or_error.has_value()) + return nullptr; + + // FIXME: Support multi-frame images? + auto decoded_image = decoded_image_or_error.release_value(); + if (decoded_image.frames.is_empty()) + return nullptr; + return move(decoded_image.frames[0].bitmap); +} + RefPtr Image::try_create_from_bitmap(NonnullRefPtr bitmap) { auto image = try_create_with_size({ bitmap->width(), bitmap->height() }); @@ -86,44 +86,6 @@ RefPtr Image::try_create_from_bitmap(NonnullRefPtr bitmap) return image; } -Result, String> Image::try_create_from_pixel_paint_fd(int fd, String const& file_path) -{ - auto file = Core::File::construct(); - file->open(fd, Core::OpenMode::ReadOnly, Core::File::ShouldCloseFileDescriptor::No); - if (file->has_error()) - return String { file->error_string() }; - - return try_create_from_pixel_paint_file(file, file_path); -} - -Result, String> Image::try_create_from_pixel_paint_path(String const& file_path) -{ - auto file_or_error = Core::File::open(file_path, Core::OpenMode::ReadOnly); - if (file_or_error.is_error()) - return String { file_or_error.error().string() }; - - return try_create_from_pixel_paint_file(*file_or_error.value(), file_path); -} - -Result, String> Image::try_create_from_pixel_paint_file(Core::File& file, String const& file_path) -{ - auto contents = file.read_all(); - - auto json_or_error = JsonValue::from_string(contents); - if (!json_or_error.has_value()) - return String { "Not a valid PP file"sv }; - - auto& json = json_or_error.value().as_object(); - auto image_or_error = try_create_from_pixel_paint_json(json); - - if (image_or_error.is_error()) - return image_or_error.release_error(); - - auto image = image_or_error.release_value(); - image->set_path(file_path); - return image; -} - Result, String> Image::try_create_from_pixel_paint_json(JsonObject const& json) { auto image = try_create_with_size({ json.get("width").to_i32(), json.get("height").to_i32() }); @@ -163,52 +125,6 @@ Result, String> Image::try_create_from_pixel_paint_json(Jso return image.release_nonnull(); } -Result, String> Image::try_create_from_fd_and_close(int fd, String const& file_path) -{ - auto image_or_error = try_create_from_pixel_paint_fd(fd, file_path); - if (!image_or_error.is_error()) { - close(fd); - return image_or_error.release_value(); - } - - auto file_or_error = MappedFile::map_from_fd_and_close(fd, file_path); - if (file_or_error.is_error()) - return String::formatted("Unable to mmap file {}", file_or_error.error().string()); - - auto& mapped_file = *file_or_error.value(); - // FIXME: Find a way to avoid the memory copy here. - auto bitmap = try_decode_bitmap(ByteBuffer::copy(mapped_file.bytes())); - if (!bitmap) - return String { "Unable to decode image"sv }; - auto image = Image::try_create_from_bitmap(bitmap.release_nonnull()); - if (!image) - return String { "Unable to allocate Image"sv }; - image->set_path(file_path); - return image.release_nonnull(); -} - -Result, String> Image::try_create_from_path(String const& file_path) -{ - auto image_or_error = try_create_from_pixel_paint_path(file_path); - if (!image_or_error.is_error()) - return image_or_error.release_value(); - - auto file_or_error = MappedFile::map(file_path); - if (file_or_error.is_error()) - return String { "Unable to mmap file"sv }; - - auto& mapped_file = *file_or_error.value(); - // FIXME: Find a way to avoid the memory copy here. - auto bitmap = try_decode_bitmap(ByteBuffer::copy(mapped_file.bytes())); - if (!bitmap) - return String { "Unable to decode image"sv }; - auto image = Image::try_create_from_bitmap(bitmap.release_nonnull()); - if (!image) - return String { "Unable to allocate Image"sv }; - image->set_path(file_path); - return image.release_nonnull(); -} - void Image::serialize_as_json(JsonObjectSerializer& json) const { json.add("width", m_size.width()); diff --git a/Userland/Applications/PixelPaint/Image.h b/Userland/Applications/PixelPaint/Image.h index 13430df761..1ffa9d382e 100644 --- a/Userland/Applications/PixelPaint/Image.h +++ b/Userland/Applications/PixelPaint/Image.h @@ -46,10 +46,10 @@ protected: class Image : public RefCounted { public: static RefPtr try_create_with_size(Gfx::IntSize const&); - static Result, String> try_create_from_fd_and_close(int fd, String const& file_path); - static Result, String> try_create_from_path(String const& file_path); - static RefPtr try_create_from_bitmap(NonnullRefPtr); static Result, String> try_create_from_pixel_paint_json(JsonObject const&); + static RefPtr try_create_from_bitmap(NonnullRefPtr); + + static RefPtr try_decode_bitmap(const ByteBuffer& bitmap_data); // This generates a new Bitmap with the final image (all layers composed according to their attributes.) RefPtr try_compose_bitmap(Gfx::BitmapFormat format) const; @@ -103,10 +103,6 @@ public: private: explicit Image(Gfx::IntSize const&); - static Result, String> try_create_from_pixel_paint_fd(int fd, String const& file_path); - static Result, String> try_create_from_pixel_paint_path(String const& file_path); - static Result, String> try_create_from_pixel_paint_file(Core::File& file, String const& file_path); - void did_change(Gfx::IntRect const& modified_rect = {}); void did_change_rect(); void did_modify_layer_stack(); diff --git a/Userland/Applications/PixelPaint/ProjectLoader.cpp b/Userland/Applications/PixelPaint/ProjectLoader.cpp new file mode 100644 index 0000000000..0dfab4bdbf --- /dev/null +++ b/Userland/Applications/PixelPaint/ProjectLoader.cpp @@ -0,0 +1,75 @@ +/* + * Copyright (c) 2021, Tobias Christiansen + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include "ProjectLoader.h" +#include "Image.h" +#include "Layer.h" +#include +#include +#include +#include +#include +#include + +namespace PixelPaint { + +Result ProjectLoader::try_load_from_fd_and_close(int fd, StringView path) +{ + auto file = Core::File::construct(); + file->open(fd, Core::OpenMode::ReadOnly, Core::File::ShouldCloseFileDescriptor::No); + if (file->has_error()) + return String { file->error_string() }; + + auto contents = file->read_all(); + + auto json_or_error = JsonValue::from_string(contents); + if (!json_or_error.has_value()) { + m_is_raw_image = true; + + auto file_or_error = MappedFile::map_from_fd_and_close(fd, path); + if (file_or_error.is_error()) + return String::formatted("Unable to mmap file {}", file_or_error.error().string()); + + auto& mapped_file = *file_or_error.value(); + // FIXME: Find a way to avoid the memory copy here. + auto bitmap = Image::try_decode_bitmap(ByteBuffer::copy(mapped_file.bytes())); + if (!bitmap) + return String { "Unable to decode image"sv }; + auto image = Image::try_create_from_bitmap(bitmap.release_nonnull()); + if (!image) + return String { "Unable to allocate Image"sv }; + + image->set_path(path); + m_image = image; + return {}; + } + + close(fd); + auto& json = json_or_error.value().as_object(); + auto image_or_error = Image::try_create_from_pixel_paint_json(json); + + if (image_or_error.is_error()) + return image_or_error.release_error(); + + auto image = image_or_error.release_value(); + image->set_path(path); + + if (json.has("guides")) + m_json_metadata = json.get("guides").as_array(); + + m_image = image; + return {}; +} +Result ProjectLoader::try_load_from_path(StringView path) +{ + auto file_or_error = Core::File::open(path, Core::OpenMode::ReadOnly); + if (file_or_error.is_error()) + return String::formatted("Unable to open file because: {}", file_or_error.release_error()); + + return try_load_from_fd_and_close(file_or_error.release_value()->fd(), path); +} + +} diff --git a/Userland/Applications/PixelPaint/ProjectLoader.h b/Userland/Applications/PixelPaint/ProjectLoader.h new file mode 100644 index 0000000000..eaed4eabbe --- /dev/null +++ b/Userland/Applications/PixelPaint/ProjectLoader.h @@ -0,0 +1,35 @@ +/* + * Copyright (c) 2021, Tobias Christiansen + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include "Image.h" +#include +#include +#include + +namespace PixelPaint { + +class ProjectLoader { +public: + ProjectLoader() = default; + ~ProjectLoader() = default; + + Result try_load_from_fd_and_close(int fd, StringView path); + Result try_load_from_path(StringView path); + + bool is_raw_image() const { return m_is_raw_image; } + bool has_image() const { return !m_image.is_null(); } + RefPtr release_image() const { return move(m_image); } + JsonArray const& json_metadata() const { return m_json_metadata; } + +private: + RefPtr m_image { nullptr }; + bool m_is_raw_image { false }; + JsonArray m_json_metadata {}; +}; + +} diff --git a/Userland/Applications/PixelPaint/main.cpp b/Userland/Applications/PixelPaint/main.cpp index 26dbc1e81e..5f88543da9 100644 --- a/Userland/Applications/PixelPaint/main.cpp +++ b/Userland/Applications/PixelPaint/main.cpp @@ -16,6 +16,7 @@ #include "LayerListWidget.h" #include "LayerPropertiesWidget.h" #include "PaletteWidget.h" +#include "ProjectLoader.h" #include "Tool.h" #include "ToolPropertiesWidget.h" #include "ToolboxWidget.h" @@ -114,6 +115,7 @@ int main(int argc, char** argv) }; Function)> create_new_editor; + PixelPaint::ProjectLoader loader; auto& layer_list_widget = *main_widget.find_descendant_of_type_named("layer_list_widget"); layer_list_widget.on_layer_select = [&](auto* layer) { @@ -151,23 +153,25 @@ int main(int argc, char** argv) window); auto open_image_file = [&](auto& path) { - auto image_or_error = PixelPaint::Image::try_create_from_path(path); - if (image_or_error.is_error()) { + auto try_load = loader.try_load_from_path(path); + if (try_load.is_error()) { GUI::MessageBox::show_error(window, String::formatted("Unable to open file: {}", path)); return; } - auto& image = *image_or_error.value(); + auto& image = *loader.release_image(); create_new_editor(image); layer_list_widget.set_image(&image); }; auto open_image_fd = [&](int fd, auto& path) { - auto image_or_error = PixelPaint::Image::try_create_from_fd_and_close(fd, path); - if (image_or_error.is_error()) { - GUI::MessageBox::show_error(window, String::formatted("Unable to open file: {}, {}", path, image_or_error.error())); + auto try_load = loader.try_load_from_fd_and_close(fd, path); + + if (try_load.is_error()) { + GUI::MessageBox::show_error(window, String::formatted("Unable to open file: {}, {}", path, try_load.error())); return; } - auto& image = *image_or_error.value(); + + auto& image = *loader.release_image(); create_new_editor(image); layer_list_widget.set_image(&image); };