diff options
author | Caoimhe <caoimhebyrne06@gmail.com> | 2023-03-23 20:51:42 +0000 |
---|---|---|
committer | Linus Groh <mail@linusgroh.de> | 2023-03-24 09:38:46 +0000 |
commit | 208e3f197846ce34ffb8153b71162a5b4583ef6e (patch) | |
tree | 1e1119f59c5bb6b604d81a09b683ca87a352cd9f /Userland/Applications/ImageViewer | |
parent | c943ab823d74040f026454e329824cbe4223ec8d (diff) | |
download | serenity-208e3f197846ce34ffb8153b71162a5b4583ef6e.zip |
ImageViewer: Use `LibFileSystemAccessClient`
This commit also starts the adoption of ErrorOr<T> and the String class
in ImageViewer. However, there is still a few more changes that could
be made.
Since the actions of using LibFSAC and using String in more places are
tightly coupled, I decided to put them in one commit.
Diffstat (limited to 'Userland/Applications/ImageViewer')
-rw-r--r-- | Userland/Applications/ImageViewer/CMakeLists.txt | 2 | ||||
-rw-r--r-- | Userland/Applications/ImageViewer/ViewWidget.cpp | 67 | ||||
-rw-r--r-- | Userland/Applications/ImageViewer/ViewWidget.h | 10 | ||||
-rw-r--r-- | Userland/Applications/ImageViewer/main.cpp | 50 |
4 files changed, 83 insertions, 46 deletions
diff --git a/Userland/Applications/ImageViewer/CMakeLists.txt b/Userland/Applications/ImageViewer/CMakeLists.txt index 0826d53a56..717a88c95e 100644 --- a/Userland/Applications/ImageViewer/CMakeLists.txt +++ b/Userland/Applications/ImageViewer/CMakeLists.txt @@ -12,4 +12,4 @@ set(SOURCES ) serenity_app(ImageViewer ICON filetype-image) -target_link_libraries(ImageViewer PRIVATE LibCore LibDesktop LibGUI LibGfx LibConfig LibImageDecoderClient LibMain) +target_link_libraries(ImageViewer PRIVATE LibCore LibDesktop LibFileSystemAccessClient LibGUI LibGfx LibConfig LibImageDecoderClient LibMain) diff --git a/Userland/Applications/ImageViewer/ViewWidget.cpp b/Userland/Applications/ImageViewer/ViewWidget.cpp index 61818e6d22..ee31d37025 100644 --- a/Userland/Applications/ImageViewer/ViewWidget.cpp +++ b/Userland/Applications/ImageViewer/ViewWidget.cpp @@ -4,6 +4,7 @@ * Copyright (c) 2021, Mohsan Ali <mohsan0073@gmail.com> * Copyright (c) 2022, Mustafa Quraish <mustafa@serenityos.org> * Copyright (c) 2022, the SerenityOS developers. + * Copyright (c) 2023, Caoimhe Byrne <caoimhebyrne06@gmail.com> * * SPDX-License-Identifier: BSD-2-Clause */ @@ -16,6 +17,7 @@ #include <LibCore/MappedFile.h> #include <LibCore/MimeData.h> #include <LibCore/Timer.h> +#include <LibFileSystemAccessClient/Client.h> #include <LibGUI/Application.h> #include <LibGUI/MessageBox.h> #include <LibGfx/Bitmap.h> @@ -71,6 +73,13 @@ bool ViewWidget::is_previous_available() const return false; } +// FIXME: Convert to `String` & use LibFileSystemAccessClient + `Core::System::unveil(nullptr, nullptr)` +// - Converting to String is not super-trivial due to the LexicalPath usage, while we can do a bunch of +// String::from_deprecated_string() and String.to_deprecated_string(), it is quite ugly to read and +// probably not the best approach. +// +// - If we go full-unveil (`Core::System::unveil(nullptr, nullptr)`) this functionality does not work, +// we can not access the list of contents of a directory through LibFileSystemAccessClient at the moment. Vector<DeprecatedString> ViewWidget::load_files_from_directory(DeprecatedString const& path) const { Vector<DeprecatedString> files_in_directory; @@ -86,11 +95,11 @@ Vector<DeprecatedString> ViewWidget::load_files_from_directory(DeprecatedString return files_in_directory; } -void ViewWidget::set_path(DeprecatedString const& path) +void ViewWidget::set_path(String const& path) { m_path = path; - m_files_in_same_dir = load_files_from_directory(path); - m_current_index = m_files_in_same_dir.find_first_index(path); + m_files_in_same_dir = load_files_from_directory(path.to_deprecated_string()); + m_current_index = m_files_in_same_dir.find_first_index(path.to_deprecated_string()); } void ViewWidget::navigate(Directions direction) @@ -110,8 +119,14 @@ void ViewWidget::navigate(Directions direction) index = m_files_in_same_dir.size() - 1; } + auto result = FileSystemAccessClient::Client::the().request_file_read_only_approved(window(), m_files_in_same_dir.at(index)); + if (result.is_error()) + return; + m_current_index = index; - this->load_from_file(m_files_in_same_dir.at(index)); + + auto value = result.release_value(); + open_file(value.filename(), value.stream()); } void ViewWidget::doubleclick_event(GUI::MouseEvent&) @@ -147,39 +162,34 @@ void ViewWidget::mouseup_event(GUI::MouseEvent& event) GUI::AbstractZoomPanWidget::mouseup_event(event); } -void ViewWidget::load_from_file(DeprecatedString const& path) +void ViewWidget::open_file(String const& path, Core::File& file) { - auto show_error = [&] { - GUI::MessageBox::show(window(), DeprecatedString::formatted("Failed to open {}", path), "Cannot open image"sv, GUI::MessageBox::Type::Error); - }; + auto open_result = try_open_file(path, file); + if (open_result.is_error()) { + auto error = open_result.release_error(); + auto user_error_message = String::formatted("Failed to open the image: {}.", error).release_value_but_fixme_should_propagate_errors(); - auto file_or_error = Core::MappedFile::map(path); - if (file_or_error.is_error()) { - show_error(); - return; + GUI::MessageBox::show_error(nullptr, user_error_message); } +} - auto& mapped_file = *file_or_error.value(); - +ErrorOr<void> ViewWidget::try_open_file(String const& path, Core::File& file) +{ // Spawn a new ImageDecoder service process and connect to it. - auto client = ImageDecoderClient::Client::try_create().release_value_but_fixme_should_propagate_errors(); + auto client = TRY(ImageDecoderClient::Client::try_create()); auto mime_type = Core::guess_mime_type_based_on_filename(path); - auto decoded_image_or_error = client->decode_image(mapped_file.bytes(), mime_type); - if (!decoded_image_or_error.has_value()) { - show_error(); - return; + auto decoded_image_or_none = client->decode_image(TRY(file.read_until_eof()), mime_type); + if (!decoded_image_or_none.has_value()) { + return Error::from_string_literal("Failed to decode image"); } - m_decoded_image = decoded_image_or_error.release_value(); + m_decoded_image = decoded_image_or_none.release_value(); m_bitmap = m_decoded_image->frames[0].bitmap; if (m_bitmap.is_null()) { - show_error(); - return; + return Error::from_string_literal("Image didn't contain a bitmap"); } set_original_rect(m_bitmap->rect()); - if (on_image_change) - on_image_change(m_bitmap); if (m_decoded_image->is_animated && m_decoded_image->frames.size() > 1) { auto const& first_frame = m_decoded_image->frames[0]; @@ -190,13 +200,18 @@ void ViewWidget::load_from_file(DeprecatedString const& path) m_timer->stop(); } - m_path = Core::DeprecatedFile::real_path_for(path); - GUI::Application::the()->set_most_recently_open_file(String::from_utf8(path).release_value_but_fixme_should_propagate_errors()); + set_path(path); + GUI::Application::the()->set_most_recently_open_file(path); + + if (on_image_change) + on_image_change(m_bitmap); if (scaled_for_first_image()) scale_image_for_window(); else reset_view(); + + return {}; } void ViewWidget::drag_enter_event(GUI::DragEvent& event) diff --git a/Userland/Applications/ImageViewer/ViewWidget.h b/Userland/Applications/ImageViewer/ViewWidget.h index 4d6d7043e6..96ff096c17 100644 --- a/Userland/Applications/ImageViewer/ViewWidget.h +++ b/Userland/Applications/ImageViewer/ViewWidget.h @@ -4,6 +4,7 @@ * Copyright (c) 2021, Mohsan Ali <mohsan0073@gmail.com> * Copyright (c) 2022, Mustafa Quraish <mustafa@serenityos.org> * Copyright (c) 2022, the SerenityOS developers. + * Copyright (c) 2023, Caoimhe Byrne <caoimhebyrne06@gmail.com> * * SPDX-License-Identifier: BSD-2-Clause */ @@ -30,12 +31,12 @@ public: virtual ~ViewWidget() override = default; Gfx::Bitmap const* bitmap() const { return m_bitmap.ptr(); } - DeprecatedString const& path() const { return m_path; } + String const& path() const { return m_path; } void set_toolbar_height(int height) { m_toolbar_height = height; } int toolbar_height() { return m_toolbar_height; } bool scaled_for_first_image() { return m_scaled_for_first_image; } void set_scaled_for_first_image(bool val) { m_scaled_for_first_image = val; } - void set_path(DeprecatedString const& path); + void set_path(String const& path); void resize_window(); void scale_image_for_window(); void set_scaling_mode(Gfx::Painter::ScalingMode); @@ -47,7 +48,7 @@ public: void flip(Gfx::Orientation); void rotate(Gfx::RotationDirection); void navigate(Directions); - void load_from_file(DeprecatedString const&); + void open_file(String const&, Core::File&); Function<void()> on_doubleclick; Function<void(const GUI::DropEvent&)> on_drop; @@ -66,8 +67,9 @@ private: void set_bitmap(Gfx::Bitmap const* bitmap); void animate(); Vector<DeprecatedString> load_files_from_directory(DeprecatedString const& path) const; + ErrorOr<void> try_open_file(String const&, Core::File&); - DeprecatedString m_path; + String m_path; RefPtr<Gfx::Bitmap const> m_bitmap; Optional<ImageDecoderClient::DecodedImage> m_decoded_image; diff --git a/Userland/Applications/ImageViewer/main.cpp b/Userland/Applications/ImageViewer/main.cpp index 49d4f8470e..ab8bf6c57f 100644 --- a/Userland/Applications/ImageViewer/main.cpp +++ b/Userland/Applications/ImageViewer/main.cpp @@ -1,6 +1,7 @@ /* * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org> * Copyright (c) 2021, Mohsan Ali <mohsan0073@gmail.com> + * Copyright (c) 2023, Caoimhe Byrne <caoimhebyrne06@gmail.com> * * SPDX-License-Identifier: BSD-2-Clause */ @@ -12,6 +13,7 @@ #include <LibCore/ArgsParser.h> #include <LibCore/System.h> #include <LibDesktop/Launcher.h> +#include <LibFileSystemAccessClient/Client.h> #include <LibGUI/Action.h> #include <LibGUI/ActionGroup.h> #include <LibGUI/Application.h> @@ -49,6 +51,12 @@ ErrorOr<int> serenity_main(Main::Arguments arguments) TRY(Desktop::Launcher::add_allowed_handler_with_only_specific_urls("/bin/Help", { URL::create_with_file_scheme("/usr/share/man/man1/ImageViewer.md") })); TRY(Desktop::Launcher::seal_allowlist()); + // FIXME: Use unveil when we solve the issue with ViewWidget::load_files_from_directory, an explanation is given in ViewWidget.cpp + // TRY(Core::System::unveil("/tmp/session/%sid/portal/filesystemaccess", "rw")); + // TRY(Core::System::unveil("/tmp/session/%sid/portal/image", "rw")); + // TRY(Core::System::unveil("/res", "r")); + // TRY(Core::System::unveil(nullptr, nullptr)); + auto app_icon = GUI::Icon::default_icon("filetype-image"sv); StringView path; @@ -68,9 +76,6 @@ ErrorOr<int> serenity_main(Main::Arguments arguments) auto main_toolbar = TRY(toolbar_container->try_add<GUI::Toolbar>()); auto widget = TRY(root_widget->try_add<ViewWidget>()); - if (!path.is_empty()) { - widget->set_path(path); - } widget->on_scale_change = [&](float scale) { if (!widget->bitmap()) { window->set_title("Image Viewer"); @@ -96,8 +101,12 @@ ErrorOr<int> serenity_main(Main::Arguments arguments) window->move_to_front(); auto path = urls.first().path(); - widget->set_path(path); - widget->load_from_file(path); + auto result = FileSystemAccessClient::Client::the().request_file_read_only_approved(window, path); + if (result.is_error()) + return; + + auto value = result.release_value(); + widget->open_file(value.filename(), value.stream()); for (size_t i = 1; i < urls.size(); ++i) { Desktop::Launcher::open(URL::create_with_file_scheme(urls[i].path().characters()), "/bin/ImageViewer"); @@ -112,11 +121,12 @@ ErrorOr<int> serenity_main(Main::Arguments arguments) // Actions auto open_action = GUI::CommonActions::make_open_action( [&](auto&) { - auto path = GUI::FilePicker::get_open_filepath(window, "Open Image"); - if (path.has_value()) { - widget->set_path(path.value()); - widget->load_from_file(path.value()); - } + auto result = FileSystemAccessClient::Client::the().open_file(window, "Open Image"); + if (result.is_error()) + return; + + auto value = result.release_value(); + widget->open_file(value.filename(), value.stream()); }); auto delete_action = GUI::CommonActions::make_delete_action( @@ -294,8 +304,12 @@ ErrorOr<int> serenity_main(Main::Arguments arguments) TRY(file_menu->add_recent_files_list([&](auto& action) { auto path = action.text(); - widget->set_path(path); - widget->load_from_file(path); + auto result = FileSystemAccessClient::Client::the().request_file_read_only_approved(window, path); + if (result.is_error()) + return; + + auto value = result.release_value(); + widget->open_file(value.filename(), value.stream()); })); TRY(file_menu->try_add_action(quit_action)); @@ -346,13 +360,19 @@ ErrorOr<int> serenity_main(Main::Arguments arguments) }))); TRY(help_menu->try_add_action(GUI::CommonActions::make_about_action("Image Viewer", app_icon, window))); + window->show(); + + // We must do this here and not any earlier, as we need a visible window to call FileSystemAccessClient::Client::request_file_read_only_approved(); if (path != nullptr) { - widget->load_from_file(path); + auto result = FileSystemAccessClient::Client::the().request_file_read_only_approved(window, path); + if (result.is_error()) + return 1; + + auto value = result.release_value(); + widget->open_file(value.filename(), value.stream()); } else { widget->clear(); } - window->show(); - return app->exec(); } |