Repository: flxzt/rnote (language: rust)

Issue:
Add import option to improve PDF annotation use-case
**Is your feature request related to a problem? Please describe.**  
I was looking at RNote as a way to take notes for some of my university classes (annotating slide decks, handouts, etc.), but I noticed that importing PDFs into the software "played with" the PDF page layout.

For example, here's a screenshot from Okular showing a slide deck in a psychology class I'm taking:
![image](https://github.com/flxzt/rnote/assets/111894108/12f4d98f-75b4-4dae-81e1-d01b010b5616)

Then, I import that PDF into RNote, which, I'll say, is a bit unintuitive with the current import options that show up (I'm not sure what the options mean):
![image](https://github.com/flxzt/rnote/assets/111894108/636e0194-5708-48aa-95fe-ea1585d7d183)

Already, this is *not* what I expect when I import a PDF into an annotation tool. Ideally, all the pages would just show up ready to be focused on and annotating. The continuous option seems to help:
![image](https://github.com/flxzt/rnote/assets/111894108/b4511d24-0bf3-4380-9aa8-3778de7cfdaf)

but still, it moreso "opens" a file to be annotated on the current RNote file, rather than creating a new area to annotate a specific PDF file, which is what my current use case is.

Then, trying to *export* this file with whatever annotations I might have will result in a PDF largely unusable, when compared to the design and purposes of the original PDF. Either I can export  the pages of the document I've manually selected (which is infeasible with PDFs like the 100-odd page slide decks some professors use), which looks like this:
![image](https://github.com/flxzt/rnote/assets/111894108/2e406924-ffea-4785-bd4f-53a23a56afe2)

Or I can export the entire document, which breaks the pages based on how the import function worked:
![image](https://github.com/flxzt/rnote/assets/111894108/2c60607e-8e87-4c9e-b4a6-ba8c91e8dee2)

Or I can export the individual document's pages, which seemingly does the same thing here:
![image](https://github.com/flxzt/rnote/assets/111894108/cc741c51-8112-4edb-b762-d67f751f12ab)

None of which, unfortunately, give me the easy export-back-to-original-format behavior that I want.


**Describe the solution you'd like**  
Ideally, when importing a PDF, perhaps an option to open the PDF separately (rather than adding it to the current workspace) could be presented, and the workspace *is* that PDF, and nothing more. That way, exporting it back into a PDF will retain all of the original aspects of the PDF, while adding all of the annotations I might have created during my not-taking.

**Describe alternatives you've considered**  
Perhaps something else could be a more fleshed-out way to select only the pages of the PDF I've imported in the current workspace, and a better recognition of those pages' layout. Honestly, as long as there's an easy way to import a PDF, annotate it, and export it in a way that makes it look largely the same save my new annotations, I think that'll work.

**Additional context**  
Again, this is all for the primary use case of downloading PDF handouts from instructors to annotate them with notes, where the structure should be largely unchanged, and it can be used to look back on for exam review (notes with personal context)



Discussion / hints:
some opinions from another user:
1. in my understanding rnote is _not_ a pdf annotation tool, its a note-taking app which _always_ operates on its own canvas. Like similar tools (ie oneNote) imports will inherently result in a loss of data (text for example) because they're only added ontop of to the existing canvas
2. you import your pdf into an existing rnote document. This has existing Page and Document format settings in place. If you want every one of your pdf pages to 1:1 cover a rnote page then you just have to configure your rnote document to do that
3. if you do that then the export will work pretty well. The imported and exported pages have a 1:1 mapping. The only thing that's lost is any sort of invisible structure the pdf had (most importantly, text) which is because the pdf is being converted to an image when importing and then converted back when exporting
I do see the benefit of an pdf import option that adjusts the document format to the size of the pdf, aligns it to the pages and sets the page layout to fixed size.

> 1. in my understanding rnote is _not_ a pdf annotation tool, its a note-taking app which _always_ operates on its own canvas. Like similar tools (ie oneNote) imports will inherently result in a loss of data (text for example) because they're only added ontop of to the existing canvas

I was not aware of this! I was under the impression that, on top of being a note-taking tool, it also was intended to have PDF annotation functionalities. That being said, as mentioned by @flxzt, I think supporting this use case would greatly improve the user experience of the app.

> 2. you import your pdf into an existing rnote document. This has existing Page and Document format settings in place. If you want every one of your pdf pages to 1:1 cover a rnote page then you just have to configure your rnote document to do that

I'll have to look into that. A lot of the RNote UI doesn't feel super intuitive to me (could very well be a skill issue), so it'll take some effort, but that's effort I'm certainly willing to expend if it means I can take better class notes!

> 3. if you do that then the export will work pretty well. The imported and exported pages have a 1:1 mapping. The only thing that's lost is any sort of invisible structure the pdf had (most importantly, text) which is because the pdf is being converted to an image when importing and then converted back when exporting

This seems pretty promising, though yes, the internal data of how the PDF is structured being lost would be unfortunate. Also, how seamless would this document setup be for the case where each document is anywhere between 100-150 pages long? If there's some way I can configure RNote to automatically detect the PDF page structure, that's ideal, but otherwise, my original issue request stands.

This being said, thank you for pointing me in the direction of alternatives! I'll have to explore Rnote's other functionalities and see what else it can do for me.
seamless in what way?
Without the proposed new feature you'd maybe have to readjust page sizing.
Longer documents might also take considerable time to load (though I've also had ~150 page docs load almost instantly) #1024
the double conversion might also significantly increase file sizes https://github.com/flxzt/rnote/issues/668#issuecomment-1646027192

also: #153

Fix patch:
diff --git a/crates/rnote-engine/src/engine/import.rs b/crates/rnote-engine/src/engine/import.rs
index 7f50a5aba9..6cec525db3 100644
--- a/crates/rnote-engine/src/engine/import.rs
+++ b/crates/rnote-engine/src/engine/import.rs
@@ -1,5 +1,6 @@
 // Imports
 use super::{EngineConfig, EngineViewMut, StrokeContent};
+use crate::document::Layout;
 use crate::pens::Pen;
 use crate::pens::PenStyle;
 use crate::store::chrono_comp::StrokeLayer;
@@ -7,6 +8,8 @@ use crate::store::StrokeKey;
 use crate::strokes::{BitmapImage, Stroke, VectorImage};
 use crate::{CloneConfig, Engine, WidgetFlags};
 use futures::channel::oneshot;
+use rnote_compose::ext::Vector2Ext;
+use rnote_compose::shapes::Shapeable;
 use serde::{Deserialize, Serialize};
 use std::ops::Range;
 use std::path::PathBuf;
@@ -91,6 +94,9 @@ pub struct PdfImportPrefs {
     /// Whether the imported Pdf pages have drawn borders
     #[serde(rename = "page_borders")]
     pub page_borders: bool,
+    /// Whether the document layout should be adjusted to the Pdf
+    #[serde(rename = "adjust_document")]
+    pub adjust_document: bool,
 }
 
 impl Default for PdfImportPrefs {
@@ -101,6 +107,7 @@ impl Default for PdfImportPrefs {
             page_spacing: PdfImportPageSpacing::default(),
             bitmap_scalefactor: 1.8,
             page_borders: true,
+            adjust_document: false,
         }
     }
 }
@@ -271,6 +278,8 @@ impl Engine {
     /// Generate image strokes for each page for the bytes.
     ///
     /// The bytes are expected to be from a valid Pdf.
+    ///
+    /// Note: `insert_pos` does not have an effect when the `adjust_document` import pref is set true.
     #[allow(clippy::type_complexity)]
     pub fn generate_pdf_pages_from_bytes(
         &self,
@@ -282,6 +291,11 @@ impl Engine {
             oneshot::channel::<anyhow::Result<Vec<(Stroke, Option<StrokeLayer>)>>>();
         let pdf_import_prefs = self.import_prefs.pdf_import_prefs;
         let format = self.document.format;
+        let insert_pos = if self.import_prefs.pdf_import_prefs.adjust_document {
+            na::Vector2::<f64>::zeros()
+        } else {
+            insert_pos
+        };
 
         rayon::spawn(move || {
             let result = || -> anyhow::Result<Vec<(Stroke, Option<StrokeLayer>)>> {
@@ -329,13 +343,30 @@ impl Engine {
         strokes: Vec<(Stroke, Option<StrokeLayer>)>,
     ) -> WidgetFlags {
         let mut widget_flags = WidgetFlags::default();
+        if strokes.is_empty() {
+            return widget_flags;
+        }
+        let adjust_document = self.import_prefs.pdf_import_prefs.adjust_document;
+        let select = !self.import_prefs.pdf_import_prefs.adjust_document;
 
-        // we need to always deselect all strokes -
-        // even tough changing the pen style deselects too, it does only when the pen is actually different.
+        // we need to always deselect all strokes. Even tough changing the pen style deselects too, it does only when
+        // the pen is actually different.
         let all_strokes = self.store.stroke_keys_as_rendered();
         self.store.set_selected_keys(&all_strokes, false);
 
-        widget_flags |= self.change_pen_style(PenStyle::Selector);
+        if select {
+            widget_flags |= self.change_pen_style(PenStyle::Selector);
+        }
+
+        if adjust_document {
+            let max_size = strokes
+                .iter()
+                .map(|(stroke, _)| stroke.bounds().extents())
+                .fold(na::Vector2::<f64>::zeros(), |acc, x| acc.maxs(&x));
+            self.document.format.set_width(max_size[0]);
+            self.document.format.set_height(max_size[1]);
+            widget_flags |= self.set_doc_layout(Layout::FixedSize) | self.doc_resize_autoexpand()
+        }
 
         let inserted = strokes
             .into_iter()
@@ -344,7 +375,9 @@ impl Engine {
 
         // resize after the strokes are inserted, but before they are set selected
         widget_flags |= self.doc_resize_to_fit_content();
-        self.store.set_selected_keys(&inserted, true);
+        if select {
+            self.store.set_selected_keys(&inserted, true);
+        }
         widget_flags |= self.current_pen_update_state();
         widget_flags |= self.store.record(Instant::now());
         widget_flags.resize = true;
diff --git a/crates/rnote-engine/src/strokes/bitmapimage.rs b/crates/rnote-engine/src/strokes/bitmapimage.rs
index deb324a5ed..f1ccb4baf7 100644
--- a/crates/rnote-engine/src/strokes/bitmapimage.rs
+++ b/crates/rnote-engine/src/strokes/bitmapimage.rs
@@ -122,7 +122,11 @@ impl BitmapImage {
     ) -> Result<Vec<Self>, anyhow::Error> {
         let doc = poppler::Document::from_bytes(&glib::Bytes::from(to_be_read), None)?;
         let page_range = page_range.unwrap_or(0..doc.n_pages() as u32);
-        let page_width = format.width() * (pdf_import_prefs.page_width_perc / 100.0);
+        let page_width = if pdf_import_prefs.adjust_document {
+            format.width()
+        } else {
+            format.width() * (pdf_import_prefs.page_width_perc / 100.0)
+        };
         // calculate the page zoom based on the width of the first page.
         let page_zoom = if let Some(first_page) = doc.page(0) {
             page_width / first_page.size().0
@@ -195,12 +199,16 @@ impl BitmapImage {
                 let image_pos = na::vector![x, y];
                 let image_size = na::vector![width, height];
 
-                y += match pdf_import_prefs.page_spacing {
-                    PdfImportPageSpacing::Continuous => {
-                        height + Stroke::IMPORT_OFFSET_DEFAULT[1] * 0.5
-                    }
-                    PdfImportPageSpacing::OnePerDocumentPage => format.height(),
-                };
+                if pdf_import_prefs.adjust_document {
+                    y += height
+                } else {
+                    y += match pdf_import_prefs.page_spacing {
+                        PdfImportPageSpacing::Continuous => {
+                            height + Stroke::IMPORT_OFFSET_DEFAULT[1] * 0.5
+                        }
+                        PdfImportPageSpacing::OnePerDocumentPage => format.height(),
+                    };
+                }
 
                 Ok((png_data, image_pos, image_size))
             })
diff --git a/crates/rnote-engine/src/strokes/vectorimage.rs b/crates/rnote-engine/src/strokes/vectorimage.rs
index 5d4c2f83ee..096c251e34 100644
--- a/crates/rnote-engine/src/strokes/vectorimage.rs
+++ b/crates/rnote-engine/src/strokes/vectorimage.rs
@@ -191,7 +191,11 @@ impl VectorImage {
         let doc = poppler::Document::from_bytes(&glib::Bytes::from(bytes), None)?;
         let page_range = page_range.unwrap_or(0..doc.n_pages() as u32);
 
-        let page_width = format.width() * (pdf_import_prefs.page_width_perc / 100.0);
+        let page_width = if pdf_import_prefs.adjust_document {
+            format.width()
+        } else {
+            format.width() * (pdf_import_prefs.page_width_perc / 100.0)
+        };
         // calculate the page zoom based on the width of the first page.
         let page_zoom = if let Some(first_page) = doc.page(0) {
             page_width / first_page.size().0
@@ -281,12 +285,16 @@ impl VectorImage {
 
                 let bounds = Aabb::new(na::point![x, y], na::point![x + width, y + height]);
 
-                y += match pdf_import_prefs.page_spacing {
-                    PdfImportPageSpacing::Continuous => {
-                        height + Stroke::IMPORT_OFFSET_DEFAULT[1] * 0.5
-                    }
-                    PdfImportPageSpacing::OnePerDocumentPage => format.height(),
-                };
+                if pdf_import_prefs.adjust_document {
+                    y += height
+                } else {
+                    y += match pdf_import_prefs.page_spacing {
+                        PdfImportPageSpacing::Continuous => {
+                            height + Stroke::IMPORT_OFFSET_DEFAULT[1] * 0.5
+                        }
+                        PdfImportPageSpacing::OnePerDocumentPage => format.height(),
+                    };
+                }
 
                 match res() {
                     Ok(svg_data) => Some(render::Svg { svg_data, bounds }),
diff --git a/crates/rnote-ui/data/ui/dialogs/import.ui b/crates/rnote-ui/data/ui/dialogs/import.ui
index 369cc6cf33..3016a9384f 100644
--- a/crates/rnote-ui/data/ui/dialogs/import.ui
+++ b/crates/rnote-ui/data/ui/dialogs/import.ui
@@ -96,6 +96,12 @@
                     <property name="climb-rate">1</property>
                   </object>
                 </child>
+                <child>
+                  <object class="AdwSwitchRow" id="pdf_import_adjust_document_row">
+                    <property name="title" translatable="yes">Adjust Document</property>
+                    <property name="subtitle" translatable="yes">Whether the document layout should be adjusted to the Pdf</property>
+                  </object>
+                </child>
                 <child>
                   <object class="AdwSpinRow" id="pdf_import_width_row">
                     <property name="title" translatable="yes">Page Width (%)</property>
diff --git a/crates/rnote-ui/src/dialogs/import.rs b/crates/rnote-ui/src/dialogs/import.rs
index c21e60301b..971061cbd6 100644
--- a/crates/rnote-ui/src/dialogs/import.rs
+++ b/crates/rnote-ui/src/dialogs/import.rs
@@ -112,9 +112,22 @@ pub(crate) async fn dialog_import_pdf_w_prefs(
         builder.object("pdf_import_bitmap_scalefactor_row").unwrap();
     let pdf_import_page_borders_row: adw::SwitchRow =
         builder.object("pdf_import_page_borders_row").unwrap();
+    let pdf_import_adjust_document_row: adw::SwitchRow =
+        builder.object("pdf_import_adjust_document_row").unwrap();
 
     dialog.set_transient_for(Some(appwindow));
 
+    pdf_import_adjust_document_row
+        .bind_property("active", &pdf_import_width_row, "sensitive")
+        .invert_boolean()
+        .sync_create()
+        .build();
+    pdf_import_adjust_document_row
+        .bind_property("active", &pdf_import_page_spacing_row, "sensitive")
+        .invert_boolean()
+        .sync_create()
+        .build();
+
     let pdf_import_prefs = canvas.engine_ref().import_prefs.pdf_import_prefs;
 
     // Set the widget state from the pdf import prefs
@@ -132,6 +145,7 @@ pub(crate) async fn dialog_import_pdf_w_prefs(
     pdf_import_page_spacing_row.set_selected(pdf_import_prefs.page_spacing.to_u32().unwrap());
     pdf_import_bitmap_scalefactor_row.set_value(pdf_import_prefs.bitmap_scalefactor);
     pdf_import_page_borders_row.set_active(pdf_import_prefs.page_borders);
+    pdf_import_adjust_document_row.set_active(pdf_import_prefs.adjust_document);
 
     pdf_page_start_row
         .bind_property("value", &pdf_page_end_row.adjustment(), "lower")
@@ -185,6 +199,12 @@ pub(crate) async fn dialog_import_pdf_w_prefs(
         }),
     );
 
+    pdf_import_adjust_document_row.connect_active_notify(
+        clone!(@weak canvas, @weak appwindow => move |row| {
+            canvas.engine_mut().import_prefs.pdf_import_prefs.adjust_document = row.is_active();
+        }),
+    );
+
     if let Ok(poppler_doc) =
         poppler::Document::from_gfile(&input_file, None, None::<&gio::Cancellable>)
     {


Test patch: