Hi,
That looks quite good to me :) I attach a couple of patches changing some
things, mainly simplifying the initial range setup. In detail:
* added / fixed some comments
* Simplified initial range setup. As we don't use the jumplist for state, we
only need to look back one jump. I mean, when we start a new bisect, we do
it between current page (which may or may not be current jump) and previous
jump.
* Save jump before page_set as Marwan suggests. This is relevant for the
initial step, in the case current page != current jump.
We don't have to check jumps -1 and -3 or anything like that though. We only
need the previous jump, and only when beginning a bisect to set the initial
range. In this case the previous jump is set by the user, and not the bisect
algorithm, so we are good.
* get rid of the cb_view_hadjustment_changed. This was there to recenter the
page when zoom-center is enabled. Now, since the jumplist stores
adjustments, this should be done right after every page_set, otherwise
duplicte jumps may appear. I think it is better to call
cb_view_hadjustment_changed from page_set. I attach a separate patch doing
that.
The patches:
page-set-fix.patch: call cb_view_hadkustment_changed from page_set
bisect-on-master.diff: simplified bisect. diff on top of master
bisect-on-sebastian.diff: diff on top of Sebastian code, so it's easier to
see
what I changed.
The order to apply them is first the page-set-fix and then the bisect-on-master.
Thank you both Sebastian and Marwan for taking the time to improve this!
Abdó Roig.
>From 5ed40a98551cb0492d801cfb2ec086156e62538f Mon Sep 17 00:00:00 2001
From: Abdo Roig-Maranges <[email protected]>
Date: Sat, 6 Jul 2013 21:55:35 +0200
Subject: [PATCH] refresh horizontal position in page_set
After page_set cb_view_hadjustment_changed must be called so that when
zoom-center is enabled, the page is recentered.
---
shortcuts.c | 7 -------
zathura.c | 3 +++
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/shortcuts.c b/shortcuts.c
index bfb4cf3..7ec9fa1 100644
--- a/shortcuts.c
+++ b/shortcuts.c
@@ -325,9 +325,6 @@ sc_goto(girara_session_t* session, girara_argument_t* argument, girara_event_t*
page_set(zathura, zathura_document_get_number_of_pages(zathura->document) - 1);
}
- /* adjust horizontal position */
- GtkAdjustment* hadjustment = gtk_scrolled_window_get_hadjustment(GTK_SCROLLED_WINDOW(session->gtk.view));
- cb_view_hadjustment_changed(hadjustment, zathura);
zathura_jumplist_add(zathura);
return false;
@@ -464,10 +461,6 @@ sc_navigate(girara_session_t* session, girara_argument_t* argument,
page_set(zathura, new_page);
- /* adjust horizontal position */
- GtkAdjustment* hadjustment = gtk_scrolled_window_get_hadjustment(GTK_SCROLLED_WINDOW(session->gtk.view));
- cb_view_hadjustment_changed(hadjustment, zathura);
-
return false;
}
diff --git a/zathura.c b/zathura.c
index 0165e40..d4d638d 100644
--- a/zathura.c
+++ b/zathura.c
@@ -1031,6 +1031,9 @@ page_set(zathura_t* zathura, unsigned int page_id)
zathura_adjustment_set_value(view_hadjustment, offset.x);
zathura_adjustment_set_value(view_vadjustment, offset.y);
+ /* refresh horizontal adjustment, to honor zoom-center */
+ cb_view_hadjustment_changed(view_hadjustment, zathura);
+
statusbar_page_number_update(zathura);
return true;
--
1.8.3.2
diff --git a/shortcuts.c b/shortcuts.c
index 7ec9fa1..244ada1 100644
--- a/shortcuts.c
+++ b/shortcuts.c
@@ -19,6 +19,14 @@
#include "page-widget.h"
#include "adjustment.h"
+#ifndef MIN
+#define MIN(a,b) (((a)<(b))?(a):(b))
+#endif
+
+#ifndef MAX
+#define MAX(a,b) (((a)>(b))?(a):(b))
+#endif
+
/* Helper function; see sc_display_link and sc_follow. */
static bool
draw_links(zathura_t* zathura)
@@ -779,126 +787,115 @@ sc_bisect(girara_session_t* session, girara_argument_t* argument,
g_return_val_if_fail(argument != NULL, false);
g_return_val_if_fail(zathura->document != NULL, false);
- unsigned int number_of_pages, cur_page, prev_page, prev2_page;
- bool have_prev, have_prev2;
-
- zathura_jump_t* prev_jump = NULL;
- zathura_jump_t* prev2_jump = NULL;
-
- number_of_pages= zathura_document_get_number_of_pages(zathura->document);
- cur_page = zathura_document_get_current_page_number(zathura->document);
-
- prev_page = prev2_page = 0;
- have_prev = have_prev2 = false;
+ const unsigned int num_pages = zathura_document_get_number_of_pages(zathura->document);
+ const unsigned int cur_page = zathura_document_get_current_page_number(zathura->document);
/* process arguments */
int direction;
- if (t > 0 && t <= number_of_pages) {
- /* jump to page t, and bisect between cur_page and t */
- zathura_jumplist_add(zathura);
- page_set(zathura, t-1);
- zathura_jumplist_add(zathura);
- if (t-1 > cur_page) {
+ if (t > 0 && t <= num_pages) {
+ /* bisect between cur_page and t */
+ t -= 1;
+ if (t == cur_page) {
+ /* nothing to do */
+ return false;
+ }
+ else if (t > cur_page) {
+ zathura->bisect.start = cur_page;
+ zathura->bisect.end = t;
direction = BACKWARD;
} else {
+ zathura->bisect.start = t;
+ zathura->bisect.end = cur_page;
direction = FORWARD;
}
-
} else if (argument != NULL) {
direction = argument->n;
- } else {
- return false;
- }
+ /* setup initial bisect range */
+ zathura_jump_t* jump = zathura_jumplist_current(zathura);
+ if (jump == NULL) {
+ girara_debug("bisecting between first and last page because there are no jumps");
+ zathura->bisect.start = 0;
+ zathura->bisect.end = num_pages - 1;
+
+ } if (jump->page != cur_page || jump->page != zathura->bisect.last_jump) {
+ girara_debug("last jump doesn't match up, starting new bisecting");
+ zathura->bisect.start = 0;
+ zathura->bisect.end = num_pages - 1;
+
+ unsigned int prev_page;
+ if (direction == FORWARD) {
+ prev_page = num_pages - 1;
+ } else {
+ prev_page = 0;
+ }
- cur_page = zathura_document_get_current_page_number(zathura->document);
+ /* check if we have previous jumps */
+ if (zathura_jumplist_has_previous(zathura) == true) {
+ zathura_jumplist_backward(zathura);
+ jump = zathura_jumplist_current(zathura);
+ if (jump != NULL) {
+ prev_page = jump->page;
+ }
+ zathura_jumplist_forward(zathura);
+ }
- if (zathura_jumplist_has_previous(zathura)) {
- /* If there is a previous jump, get its page */
- zathura_jumplist_backward(zathura);
- prev_jump = zathura_jumplist_current(zathura);
- if (prev_jump) {
- prev_page = prev_jump->page;
- have_prev = true;
+ zathura->bisect.start = MIN(prev_page, cur_page);
+ zathura->bisect.end = MAX(prev_page, cur_page);
+ zathura->bisect.last_jump = cur_page;
}
+ } else {
+ return false;
+ }
- if (zathura_jumplist_has_previous(zathura)) {
- /* If there is a second previous jump, get its page. */
- zathura_jumplist_backward(zathura);
- prev2_jump = zathura_jumplist_current(zathura);
- if (prev2_jump) {
- prev2_page = prev2_jump->page;
- have_prev2 = true;
- }
- zathura_jumplist_forward(zathura);
- }
- zathura_jumplist_forward(zathura);
+ girara_debug("bisecting between %d and %d, at %d", zathura->bisect.start, zathura->bisect.end, cur_page);
+ if (zathura->bisect.start == zathura->bisect.end) {
+ /* nothing to do */
+ return false;
}
- /* now, we are back at the initial jump. prev_page and prev2_page contain
- the pages for previous and second previous jump if they exist. */
+ unsigned int next_page = cur_page;
+ unsigned int next_start = zathura->bisect.start;
+ unsigned int next_end = zathura->bisect.end;
+
+ /* here we have next_start <= next_page <= next_end */
- /* bisect */
+ /* bisect step */
switch(direction) {
case FORWARD:
- if (have_prev && cur_page <= prev_page) {
- /* add a new jump point */
- if (cur_page < prev_page) {
- zathura_jumplist_add(zathura);
- page_set(zathura, (cur_page + prev_page)/2);
- zathura_jumplist_add(zathura);
+ if (cur_page != zathura->bisect.end) {
+ next_page = (cur_page + zathura->bisect.end) / 2;
+ if (next_page == cur_page) {
+ ++next_page;
}
-
- } else if (have_prev2 && cur_page <= prev2_page) {
- /* save current position at previous jump point */
- if (cur_page < prev2_page) {
- zathura_jumplist_backward(zathura);
- zathura_jumplist_add(zathura);
- zathura_jumplist_forward(zathura);
-
- page_set(zathura, (cur_page + prev2_page)/2);
- zathura_jumplist_add(zathura);
- }
- } else {
- /* none of prev_page or prev2_page comes after cur_page */
- zathura_jumplist_add(zathura);
- page_set(zathura, (cur_page + number_of_pages - 1)/2);
- zathura_jumplist_add(zathura);
+ next_start = cur_page;
}
break;
case BACKWARD:
- if (have_prev && prev_page <= cur_page) {
- /* add a new jump point */
- if (prev_page < cur_page) {
- zathura_jumplist_add(zathura);
- page_set(zathura, (cur_page + prev_page)/2);
- zathura_jumplist_add(zathura);
+ if (cur_page != zathura->bisect.start) {
+ next_page = (cur_page + zathura->bisect.start) / 2;
+ if (next_page == cur_page) {
+ --next_page;
}
-
- } else if (have_prev2 && prev2_page <= cur_page) {
- /* save current position at previous jump point */
- if (prev2_page < cur_page) {
- zathura_jumplist_backward(zathura);
- zathura_jumplist_add(zathura);
- zathura_jumplist_forward(zathura);
-
- page_set(zathura, (cur_page + prev2_page)/2);
- zathura_jumplist_add(zathura);
- }
-
- } else {
- /* none of prev_page or prev2_page comes before cur_page */
- zathura_jumplist_add(zathura);
- page_set(zathura, cur_page/2);
- zathura_jumplist_add(zathura);
+ next_end = cur_page;
}
break;
}
- /* adjust horizontal position */
- GtkAdjustment* hadjustment = gtk_scrolled_window_get_hadjustment(GTK_SCROLLED_WINDOW(session->gtk.view));
- cb_view_hadjustment_changed(hadjustment, zathura);
+ if (next_page == cur_page) {
+ /* nothing to do */
+ return false;
+ }
+
+ girara_debug("bisecting between %d and %d, jumping to %d", zathura->bisect.start, zathura->bisect.end, next_page);
+ zathura->bisect.last_jump = next_page;
+ zathura->bisect.start = next_start;
+ zathura->bisect.end = next_end;
+
+ zathura_jumplist_add(zathura);
+ page_set(zathura, next_page);
+ zathura_jumplist_add(zathura);
return false;
}
diff --git a/zathura.h b/zathura.h
index ac22636..18d3132 100644
--- a/zathura.h
+++ b/zathura.h
@@ -163,6 +163,15 @@ struct zathura_s
unsigned int size;
unsigned int num_cached_pages;
} page_cache;
+
+ /**
+ * Bisect stage
+ */
+ struct {
+ unsigned int last_jump; /**< */
+ unsigned int start; /**< Bisection range - start */
+ unsigned int end; /**< Bisection range - end */
+ } bisect;
};
/**
diff --git a/shortcuts.c b/shortcuts.c
index bca7621..244ada1 100644
--- a/shortcuts.c
+++ b/shortcuts.c
@@ -811,68 +811,38 @@ sc_bisect(girara_session_t* session, girara_argument_t* argument,
} else if (argument != NULL) {
direction = argument->n;
+ /* setup initial bisect range */
zathura_jump_t* jump = zathura_jumplist_current(zathura);
if (jump == NULL) {
girara_debug("bisecting between first and last page because there are no jumps");
zathura->bisect.start = 0;
zathura->bisect.end = num_pages - 1;
- } else {
- if (jump->page != cur_page || jump->page != zathura->bisect.last_jump) {
- girara_debug("last jump doesn't match up, starting new bisecting");
- zathura->bisect.start = 0;
- zathura->bisect.end = num_pages - 1;
-
- /* check if we have previous jumps */
- if (zathura_jumplist_has_previous(zathura) == true) {
- zathura_jumplist_backward(zathura);
- jump = zathura_jumplist_current(zathura);
-
- unsigned int prev_page = 0;
- unsigned int prev_page2 = num_pages - 1;
- if (jump != NULL) {
- prev_page = jump->page;
- }
- prev_page2 = prev_page;
- if (zathura_jumplist_has_previous(zathura) == true) {
- zathura_jumplist_backward(zathura);
- jump = zathura_jumplist_current(zathura);
- if (jump != NULL) {
- prev_page2 = jump->page;
- }
- zathura_jumplist_forward(zathura);
- }
- zathura_jumplist_forward(zathura);
-
- if (prev_page == prev_page2) {
- if (prev_page > cur_page) {
- zathura->bisect.end = prev_page;
- } else {
- zathura->bisect.start = prev_page;
- }
- } else {
- zathura->bisect.start = MIN(prev_page, prev_page2);
- zathura->bisect.end = MAX(prev_page, prev_page2);
- }
- }
+ } if (jump->page != cur_page || jump->page != zathura->bisect.last_jump) {
+ girara_debug("last jump doesn't match up, starting new bisecting");
+ zathura->bisect.start = 0;
+ zathura->bisect.end = num_pages - 1;
- /* some sanity checks on new bounds */
- if (cur_page < zathura->bisect.start) {
- if (direction == FORWARD) {
- zathura->bisect.start = cur_page;
- } else {
- zathura->bisect.start = cur_page;
- zathura->bisect.end = 0;
- }
- } else if (cur_page > zathura->bisect.end) {
- if (direction == BACKWARD) {
- zathura->bisect.end = cur_page;
- } else {
- zathura->bisect.start = cur_page;
- zathura->bisect.end = num_pages - 1;
- }
+ unsigned int prev_page;
+ if (direction == FORWARD) {
+ prev_page = num_pages - 1;
+ } else {
+ prev_page = 0;
+ }
+
+ /* check if we have previous jumps */
+ if (zathura_jumplist_has_previous(zathura) == true) {
+ zathura_jumplist_backward(zathura);
+ jump = zathura_jumplist_current(zathura);
+ if (jump != NULL) {
+ prev_page = jump->page;
}
+ zathura_jumplist_forward(zathura);
}
+
+ zathura->bisect.start = MIN(prev_page, cur_page);
+ zathura->bisect.end = MAX(prev_page, cur_page);
+ zathura->bisect.last_jump = cur_page;
}
} else {
return false;
@@ -884,14 +854,13 @@ sc_bisect(girara_session_t* session, girara_argument_t* argument,
return false;
}
- /* now, we are back at the initial jump. prev_page and prev2_page contain
- the pages for previous and second previous jump if they exist. */
-
unsigned int next_page = cur_page;
unsigned int next_start = zathura->bisect.start;
unsigned int next_end = zathura->bisect.end;
- /* bisect */
+ /* here we have next_start <= next_page <= next_end */
+
+ /* bisect step */
switch(direction) {
case FORWARD:
if (cur_page != zathura->bisect.end) {
@@ -924,13 +893,10 @@ sc_bisect(girara_session_t* session, girara_argument_t* argument,
zathura->bisect.start = next_start;
zathura->bisect.end = next_end;
+ zathura_jumplist_add(zathura);
page_set(zathura, next_page);
zathura_jumplist_add(zathura);
- /* adjust horizontal position */
- GtkAdjustment* hadjustment = gtk_scrolled_window_get_hadjustment(GTK_SCROLLED_WINDOW(session->gtk.view));
- cb_view_hadjustment_changed(hadjustment, zathura);
-
return false;
}
_______________________________________________
zathura mailing list
[email protected]
http://lists.pwmt.org/mailman/listinfo/zathura
