[zathura] [PATCH] Center pages

[akbjker]

Quick patch to center-align pages that have different sizes.
Also fixes a bug that forces all the ZathuraPages to the size of the
largest one, therefore causing GTK's default background color to be
drawn instead of the one set in zathurarc.

This feels kind of hacky, though. In the long term, it might be better
to manually draw the whole content area, rather than relying on Gtk's
layout containers.
-------------- next part --------------
diff --git a/zathura.c b/zathura.c
index 1f5f4fd..5335def 100644
--- a/zathura.c
+++ b/zathura.c
@@ -943,10 +943,6 @@ document_close(zathura_t* zathura, bool keep_monitor)

/* remove widgets */
gtk_container_foreach(GTK_CONTAINER(zathura->ui.page_widget), remove_page_from_table, (gpointer) 1);
- for (unsigned int i = 0; i < zathura_document_get_number_of_pages(zathura->document); i++) {
- g_object_unref(zathura->pages[i]);
- g_object_unref(zathura->pages[i]); // FIXME
- }
free(zathura->pages);
zathura->pages = NULL;

@@ -1062,9 +1058,15 @@ page_widget_set_mode(zathura_t* zathura, unsigned int page_padding,
int x = (i + first_page_column - 1) % pages_per_row;
int y = (i + first_page_column - 1) / pages_per_row;

- zathura_page_t* page = zathura_document_get_page(zathura->document, i);
- GtkWidget* page_widget = zathura_page_get_widget(zathura, page);
- gtk_grid_attach(GTK_GRID(zathura->ui.page_widget), page_widget, x, y, 1, 1);
+ GtkWidget* page_widget = zathura->pages[i];
+
+ GtkWidget* align = gtk_alignment_new(0.5, 0.5, 0, 0);
+ if (gtk_widget_get_parent(page_widget))
+ gtk_widget_reparent(page_widget, align);
+ else
+ gtk_container_add(GTK_CONTAINER(align), page_widget);
+
+ gtk_grid_attach(GTK_GRID(zathura->ui.page_widget), align, x, y, 1, 1);
}

gtk_widget_show_all(zathura->ui.page_widget);

[Sebastian Ramacher]

Post by akbjker
Quick patch to center-align pages that have different sizes.
Also fixes a bug that forces all the ZathuraPages to the size of the
largest one, therefore causing GTK's default background color to be
drawn instead of the one set in zathurarc.
This feels kind of hacky, though. In the long term, it might be better
to manually draw the whole content area, rather than relying on Gtk's
layout containers.

hmm, the background color still does not seem to be correct. I get the
same background color as before. Maybe there is some color setting
missing for a widget?

Regards

[akbjker]

Basically, all of the ZathuraPages get resized to the largest one, and
GtkDrawingArea's default background color gets drawn in the leftover
space of the smaller pages.
It's more noticeable when pages are grossly different sizes, like in
this attached PDF.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: out.pdf
Type: application/pdf
Size: 313126 bytes
Desc: not available
URL: <http://lists.pwmt.org/archive/zathura/attachments/20140119/98361b43/attachment-0001.pdf>

[Sebastian Ramacher]

Post by akbjker
Basically, all of the ZathuraPages get resized to the largest one, and
GtkDrawingArea's default background color gets drawn in the leftover
space of the smaller pages.
It's more noticeable when pages are grossly different sizes, like in
this attached PDF.

Oh, I see. I had a PDF open where I thought the first page had a
different size, but in fact that wasn't true.

Regards

[Sebastian Ramacher]

Post by akbjker
diff --git a/zathura.c b/zathura.c
index 1f5f4fd..5335def 100644
--- a/zathura.c
+++ b/zathura.c
@@ -943,10 +943,6 @@ document_close(zathura_t* zathura, bool keep_monitor)
/* remove widgets */
gtk_container_foreach(GTK_CONTAINER(zathura->ui.page_widget), remove_page_from_table, (gpointer) 1);
- for (unsigned int i = 0; i < zathura_document_get_number_of_pages(zathura->document); i++) {
- g_object_unref(zathura->pages[i]);
- g_object_unref(zathura->pages[i]); // FIXME
- }
free(zathura->pages);
zathura->pages = NULL;
@@ -1062,9 +1058,15 @@ page_widget_set_mode(zathura_t* zathura, unsigned int page_padding,
int x = (i + first_page_column - 1) % pages_per_row;
int y = (i + first_page_column - 1) / pages_per_row;
- zathura_page_t* page = zathura_document_get_page(zathura->document, i);
- GtkWidget* page_widget = zathura_page_get_widget(zathura, page);
- gtk_grid_attach(GTK_GRID(zathura->ui.page_widget), page_widget, x, y, 1, 1);
+ GtkWidget* page_widget = zathura->pages[i];
+
+ GtkWidget* align = gtk_alignment_new(0.5, 0.5, 0, 0);
+ if (gtk_widget_get_parent(page_widget))
+ gtk_widget_reparent(page_widget, align);
+ else
+ gtk_container_add(GTK_CONTAINER(align), page_widget);
+
+ gtk_grid_attach(GTK_GRID(zathura->ui.page_widget), align, x, y, 1, 1);
From a first look at it, this leaks all old GtkAlignment instances on

every page_widget_set_mode call.

Regards

[akbjker]

From a first look at it, this leaks all old GtkAlignment instances on every page_widget_set_mode call.

Whoops, you're right. I misunderstood the purpose of gtk_widget_reparent.

Here's the patch v2.
-------------- next part --------------
diff --git a/zathura.c b/zathura.c
index 75aabc5..49a78b0 100644
--- a/zathura.c
+++ b/zathura.c
@@ -717,6 +717,7 @@ document_open(zathura_t* zathura, const char* path, const char* password,
goto error_free;
}

+ g_object_ref(page_widget);
zathura->pages[page_id] = page_widget;

g_signal_connect(G_OBJECT(page_widget), "text-selected",
@@ -951,7 +952,6 @@ document_close(zathura_t* zathura, bool keep_monitor)
gtk_container_foreach(GTK_CONTAINER(zathura->ui.page_widget), remove_page_from_table, (gpointer) 1);
for (unsigned int i = 0; i < zathura_document_get_number_of_pages(zathura->document); i++) {
g_object_unref(zathura->pages[i]);
- g_object_unref(zathura->pages[i]); // FIXME
}
free(zathura->pages);
zathura->pages = NULL;
@@ -1068,9 +1068,18 @@ page_widget_set_mode(zathura_t* zathura, unsigned int page_padding,
int x = (i + first_page_column - 1) % pages_per_row;
int y = (i + first_page_column - 1) / pages_per_row;

- zathura_page_t* page = zathura_document_get_page(zathura->document, i);
- GtkWidget* page_widget = zathura_page_get_widget(zathura, page);
- gtk_grid_attach(GTK_GRID(zathura->ui.page_widget), page_widget, x, y, 1, 1);
+ GtkWidget* page_widget = zathura->pages[i];
+
+ GtkWidget* align = gtk_alignment_new(0.5, 0.5, 0, 0);
+ GtkWidget* parent = gtk_widget_get_parent(page_widget);
+ if (parent)
+ {
+ gtk_container_remove(GTK_CONTAINER(parent), page_widget);
+ g_object_unref(parent);
+ }
+
+ gtk_container_add(GTK_CONTAINER(align), page_widget);
+ gtk_grid_attach(GTK_GRID(zathura->ui.page_widget), align, x, y, 1, 1);
}

gtk_widget_show_all(zathura->ui.page_widget);

[Sebastian Ramacher]

Post by akbjker

From a first look at it, this leaks all old GtkAlignment instances on every page_widget_set_mode call.

Whoops, you're right. I misunderstood the purpose of gtk_widget_reparent.
Here's the patch v2.

Thank you. I've pushed it for now. We probably need to check if the
layouting code in adjustment.c, etc. is okay with this change. But
pretty much everything gets centered, if I remember the correctly.

Regards