Re: [zathura] [PATCH] Use signals to readjust_view_after_zooming()

2013-03-23 Thread Marwan Tanager
On Sat, Mar 23, 2013 at 10:52:09AM +0100, Benoît Knecht wrote:
 
> > > The only remaining issue I can see (but maybe you'll find others :) ) is
> > > that the page number is not updated if you (say) zoom out from page 1,
> > > and then zoom in (to whichever page is at the center of the screen). But
> > > that's not really an issue introduced by this patch, and there's a
> > > trivial fix for that.
> > 
> > Yeah, I see that too. However, I'm afraid to say that it's caused by your 
> > patch 
> > because I've tested it with the draft version and it doesn't happen there. 
> > But 
> > I think, like you said, that it's a minor issue somewhere.
> 
> I meant that it already happened on the current develop branch, with no
> patch applied. That's why I didn't think it was introduced by my patch,
> but perhaps yours was actually fixing this as well.

Oh, I didn't get it like that. Anyway, that's good to know :)



Marwan
___
zathura mailing list
zathura@lists.pwmt.org
http://lists.pwmt.org/mailman/listinfo/zathura


Re: [zathura] [PATCH] Use signals to readjust_view_after_zooming()

2013-03-23 Thread Benoît Knecht
Hi Marwan,

Marwan Tanager wrote:
> Sorry for being late on this.

No worries, and thanks again for taking the time to review this.

> On Fri, Mar 22, 2013 at 11:02:57AM +0100, Benoît Knecht wrote:
> > Hi Marwan,
> > 
> > Marwan Tanager wrote:
> > > This patch eliminates the aforementioned problems by setting the 
> > > adjustment 
> > > ratio dynamically from the 'value-changed' callback only if the vertical 
> > > adjustment upper bound hasn't changed. If it's found to be changed, this 
> > > means 
> > > that the document scale have changed (e.g due to zooming or resizing the 
> > > window) so the adjustment ratio shouldn't be updated. Otherwise, we 
> > > assume that 
> > > the adjustment value has been changed as a result of scrolling or jumping 
> > > to a 
> > > specific page.
> > > 
> > > Of course it's just a draft, but you can generalize it to account for the 
> > > horizontal adjustment as well as the adjustments lower bounds. IMO, the 
> > > adjustment_ratio struct could now be called something like 'struct 
> > > adjustments' 
> > > with possibly two substructures for the horizontal and vertical 
> > > adjustments 
> > > each containing the ratio and the upper/lower bounds, but of course it's 
> > > your 
> > > patch in the first place so you are free to arrange things as you see fit.
> > 
> > Great idea, checking the bounds in the "value-changed" callback! Your
> > comments and proof of concept were very helpful. I came up with a
> > slightly different implementation, but it should essentially be the same
> > concept as you just described. The only difference is, instead of
> > keeping track of the ratio and bounds inside the document struct, I
> > created two GtkAdjustments in zathura->ui, each keeping track of the
> > horizontal/vertical adjustments, respectively, through signals/handlers.
> > 
> > I'll send a patch for review right away, so you'll see what I mean.
> 
> I've just reviewed your patch and I should say thanks for the great work.  
> Actually, your approach is more intuitive and the documentation is very clear.

Thanks :)

> > The only remaining issue I can see (but maybe you'll find others :) ) is
> > that the page number is not updated if you (say) zoom out from page 1,
> > and then zoom in (to whichever page is at the center of the screen). But
> > that's not really an issue introduced by this patch, and there's a
> > trivial fix for that.
> 
> Yeah, I see that too. However, I'm afraid to say that it's caused by your 
> patch 
> because I've tested it with the draft version and it doesn't happen there. 
> But 
> I think, like you said, that it's a minor issue somewhere.

I meant that it already happened on the current develop branch, with no
patch applied. That's why I didn't think it was introduced by my patch,
but perhaps yours was actually fixing this as well.

> Also, there is another small issue which isn't specific to your 
> implementation, 
> as it happens with mine, too. When you try jumping to a page from the input 
> bar, it appears slightly scrolled up. You won't see this unless you have 
> page-padding set to some non-zero value. You will notice the padding showing 
> up 
> at the top of the view, which isn't used to happen before. If you pay 
> attention, you will see that the displacement happens as a result of the 
> input bar being hidden after you press Enter to jump to the page, because if 
> you activated it again after the jump you would see the page getting back to 
> it's correct position by displacing it by the same amount in the opposite 
> direction.

Yes you're right, I see that too. I think I have a fix for that, I'll
submit it in a different patch.

> Apart from this, I think the patch is now ready for getting merged.

I think so too, if no one else spotted major issues with it.

I'm going to submit it for inclusion now.

Cheers,

-- 
Benoît Knecht
___
zathura mailing list
zathura@lists.pwmt.org
http://lists.pwmt.org/mailman/listinfo/zathura


Re: [zathura] [PATCH] Use signals to readjust_view_after_zooming()

2013-03-22 Thread Marwan Tanager
Forget to mention that the version of the patch I've reviewd is "RFC v2"



Marwan
___
zathura mailing list
zathura@lists.pwmt.org
http://lists.pwmt.org/mailman/listinfo/zathura


Re: [zathura] [PATCH] Use signals to readjust_view_after_zooming()

2013-03-22 Thread Marwan Tanager
Hi Benoît,

Sorry for being late on this.

On Fri, Mar 22, 2013 at 11:02:57AM +0100, Benoît Knecht wrote:
> Hi Marwan,
> 
> Marwan Tanager wrote:
> > This patch eliminates the aforementioned problems by setting the adjustment 
> > ratio dynamically from the 'value-changed' callback only if the vertical 
> > adjustment upper bound hasn't changed. If it's found to be changed, this 
> > means 
> > that the document scale have changed (e.g due to zooming or resizing the 
> > window) so the adjustment ratio shouldn't be updated. Otherwise, we assume 
> > that 
> > the adjustment value has been changed as a result of scrolling or jumping 
> > to a 
> > specific page.
> > 
> > Of course it's just a draft, but you can generalize it to account for the 
> > horizontal adjustment as well as the adjustments lower bounds. IMO, the 
> > adjustment_ratio struct could now be called something like 'struct 
> > adjustments' 
> > with possibly two substructures for the horizontal and vertical adjustments 
> > each containing the ratio and the upper/lower bounds, but of course it's 
> > your 
> > patch in the first place so you are free to arrange things as you see fit.
> 
> Great idea, checking the bounds in the "value-changed" callback! Your
> comments and proof of concept were very helpful. I came up with a
> slightly different implementation, but it should essentially be the same
> concept as you just described. The only difference is, instead of
> keeping track of the ratio and bounds inside the document struct, I
> created two GtkAdjustments in zathura->ui, each keeping track of the
> horizontal/vertical adjustments, respectively, through signals/handlers.
> 
> I'll send a patch for review right away, so you'll see what I mean.

I've just reviewed your patch and I should say thanks for the great work.  
Actually, your approach is more intuitive and the documentation is very clear.

> The only remaining issue I can see (but maybe you'll find others :) ) is
> that the page number is not updated if you (say) zoom out from page 1,
> and then zoom in (to whichever page is at the center of the screen). But
> that's not really an issue introduced by this patch, and there's a
> trivial fix for that.

Yeah, I see that too. However, I'm afraid to say that it's caused by your patch 
because I've tested it with the draft version and it doesn't happen there. But 
I think, like you said, that it's a minor issue somewhere.

Also, there is another small issue which isn't specific to your implementation, 
as it happens with mine, too. When you try jumping to a page from the input 
bar, it appears slightly scrolled up. You won't see this unless you have 
page-padding set to some non-zero value. You will notice the padding showing up 
at the top of the view, which isn't used to happen before. If you pay 
attention, you will see that the displacement happens as a result of the 
input bar being hidden after you press Enter to jump to the page, because if 
you activated it again after the jump you would see the page getting back to 
it's correct position by displacing it by the same amount in the opposite 
direction.

Apart from this, I think the patch is now ready for getting merged.

> Anyway, thanks again for your precious feedback and advice, it was very
> much appreciated!

And thank you for your great work. Now we have zooming functionality that's 
worth using (It was very broken before).



Marwan
___
zathura mailing list
zathura@lists.pwmt.org
http://lists.pwmt.org/mailman/listinfo/zathura


Re: [zathura] [PATCH] Use signals to readjust_view_after_zooming()

2013-03-22 Thread Benoît Knecht
Hi Marwan,

Marwan Tanager wrote:
> This patch eliminates the aforementioned problems by setting the adjustment 
> ratio dynamically from the 'value-changed' callback only if the vertical 
> adjustment upper bound hasn't changed. If it's found to be changed, this 
> means 
> that the document scale have changed (e.g due to zooming or resizing the 
> window) so the adjustment ratio shouldn't be updated. Otherwise, we assume 
> that 
> the adjustment value has been changed as a result of scrolling or jumping to 
> a 
> specific page.
> 
> Of course it's just a draft, but you can generalize it to account for the 
> horizontal adjustment as well as the adjustments lower bounds. IMO, the 
> adjustment_ratio struct could now be called something like 'struct 
> adjustments' 
> with possibly two substructures for the horizontal and vertical adjustments 
> each containing the ratio and the upper/lower bounds, but of course it's your 
> patch in the first place so you are free to arrange things as you see fit.

Great idea, checking the bounds in the "value-changed" callback! Your
comments and proof of concept were very helpful. I came up with a
slightly different implementation, but it should essentially be the same
concept as you just described. The only difference is, instead of
keeping track of the ratio and bounds inside the document struct, I
created two GtkAdjustments in zathura->ui, each keeping track of the
horizontal/vertical adjustments, respectively, through signals/handlers.

I'll send a patch for review right away, so you'll see what I mean.

The only remaining issue I can see (but maybe you'll find others :) ) is
that the page number is not updated if you (say) zoom out from page 1,
and then zoom in (to whichever page is at the center of the screen). But
that's not really an issue introduced by this patch, and there's a
trivial fix for that.

Anyway, thanks again for your precious feedback and advice, it was very
much appreciated!

Cheers,

-- 
Benoît Knecht
___
zathura mailing list
zathura@lists.pwmt.org
http://lists.pwmt.org/mailman/listinfo/zathura


Re: [zathura] [PATCH] Use signals to readjust_view_after_zooming()

2013-03-21 Thread Marwan Tanager
This patch eliminates the aforementioned problems by setting the adjustment 
ratio dynamically from the 'value-changed' callback only if the vertical 
adjustment upper bound hasn't changed. If it's found to be changed, this means 
that the document scale have changed (e.g due to zooming or resizing the 
window) so the adjustment ratio shouldn't be updated. Otherwise, we assume that 
the adjustment value has been changed as a result of scrolling or jumping to a 
specific page.

Of course it's just a draft, but you can generalize it to account for the 
horizontal adjustment as well as the adjustments lower bounds. IMO, the 
adjustment_ratio struct could now be called something like 'struct adjustments' 
with possibly two substructures for the horizontal and vertical adjustments 
each containing the ratio and the upper/lower bounds, but of course it's your 
patch in the first place so you are free to arrange things as you see fit.

---
 callbacks.c |5 +
 document.c  |   13 +
 document.h  |3 +++
 utils.c |1 +
 zathura.c   |3 +++
 5 files changed, 25 insertions(+)

diff --git a/callbacks.c b/callbacks.c
index 59123f8..7dea733 100644
--- a/callbacks.c
+++ b/callbacks.c
@@ -105,6 +105,10 @@ cb_view_vadjustment_value_changed(GtkAdjustment* 
GIRARA_UNUSED(adjustment), gpoi
 zathura_page_widget_update_view_time(ZATHURA_PAGE(page_widget));
   }
 
+  if (zathura_document_get_adj_upper(zathura->document) == 
gtk_adjustment_get_upper(view_vadjustment)) {
+readjust_view_after_zooming(zathura);
+  }
+
   statusbar_page_number_update(zathura);
 }
 
@@ -149,6 +153,7 @@ cb_view_vadjustment_changed(GtkAdjustment* adjustment, 
gpointer data)
 
   double ratio = zathura_document_get_vadjustment_ratio(zathura->document);
   set_adjustment_from_ratio(adjustment, ratio);
+  zathura_document_set_adj_upper(zathura->document, 
gtk_adjustment_get_upper(adjustment));
 }
 
 void
diff --git a/document.c b/document.c
index b992010..49f6420 100644
--- a/document.c
+++ b/document.c
@@ -51,6 +51,7 @@ struct zathura_document_s {
   struct {
 double horizontal; /**< Horizontal adjustment ratio */
 double vertical; /**< Vertical adjustment ratio */
+double vertical_adj_upper;
   } adjustment_ratio;
 
   /**
@@ -423,6 +424,18 @@ zathura_document_set_adjustment_ratios(zathura_document_t* 
document,
   document->adjustment_ratio.vertical   = vadjustment_ratio;
 }
 
+void zathura_document_set_adj_upper(zathura_document_t* document, double upper)
+{
+  g_return_if_fail(document != NULL);
+  document->adjustment_ratio.vertical_adj_upper = upper;
+}
+
+double zathura_document_get_adj_upper(zathura_document_t* document)
+{
+  g_return_val_if_fail(document != NULL, 0.0);
+  return document->adjustment_ratio.vertical_adj_upper;
+}
+
 void
 zathura_document_get_cell_size(zathura_document_t* document,
unsigned int* height, unsigned int* width)
diff --git a/document.h b/document.h
index b8c3426..2f3bd90 100644
--- a/document.h
+++ b/document.h
@@ -191,6 +191,9 @@ void 
zathura_document_set_adjustment_ratios(zathura_document_t* document,
 double hadjustment_ratio,
 double vadjustment_ratio);
 
+double zathura_document_get_adj_upper(zathura_document_t* document);
+void zathura_document_set_adj_upper(zathura_document_t* document, double 
upper);
+
 /**
  * Returns the private data of the document
  *
diff --git a/utils.c b/utils.c
index c402512..47124b2 100644
--- a/utils.c
+++ b/utils.c
@@ -372,6 +372,7 @@ readjust_view_after_zooming(zathura_t *zathura)
   zathura_document_set_adjustment_ratios(zathura->document,
   compute_adjustment_ratio(hadjustment),
   compute_adjustment_ratio(vadjustment));
+  zathura_document_set_adj_upper(zathura->document, 
gtk_adjustment_get_upper(vadjustment));
 }
 
 void
diff --git a/zathura.c b/zathura.c
index 351ffed..d2815be 100644
--- a/zathura.c
+++ b/zathura.c
@@ -743,6 +743,9 @@ document_open(zathura_t* zathura, const char* path, const 
char* password,
 cb_view_vadjustment_value_changed(NULL, zathura);
   }
 
+  GtkAdjustment* view_vadjustment = 
gtk_scrolled_window_get_vadjustment(GTK_SCROLLED_WINDOW(zathura->ui.session->gtk.view));
+  zathura_document_set_adj_upper(zathura->document, 
gtk_adjustment_get_upper(view_vadjustment));
+
   return true;
 
 error_free:
-- 
1.7.10.4

___
zathura mailing list
zathura@lists.pwmt.org
http://lists.pwmt.org/mailman/listinfo/zathura


Re: [zathura] [PATCH] Use signals to readjust_view_after_zooming()

2013-03-21 Thread Marwan Tanager
On Thu, Mar 21, 2013 at 05:18:15PM +0100, Benoît Knecht wrote:
> Hi Marwan,
> 
> Thanks a lot for your review and feedback.
> 
> Marwan Tanager wrote:
> > On Wed, Mar 20, 2013 at 10:17:36PM +, Benoît Knecht wrote:
> > > Instead of guesstimating the values of the scrollbars adjustments after
> > > a change in zoom level, connect callbacks to the "changed" GtkAdjustment
> > > event (which is emitted when the bounds or page_size of the adjustment
> > > change, e.g. when the zoom level changes), and compute the new values
> > > from there.
> > > 
> > > cb_view_hadjustment_changed() centers the page horizontally if a
> > > "best-fit" or "width" zoom is being performed, or if "zoom-center" is
> > > true; otherwise, it keeps the view horizontally centered around the same
> > > area of the page.
> > > 
> > > cb_view_vadjustment_changed() always keeps the view vertically centered
> > > around the same area of the page.
> > > ---
> > > This is in preparation for addressing . A 
> > > patch
> > > will follow shortly.
> > 
> > Just a couple of notes based on my experimentation with the patch:
> > 
> > - Jumping to a page using a numeric command doesn't work anymore. The 
> >   page number is updated on the status bar, but the view is not 
> >   adjusted to the target page. This will not happen unless you zoom at 
> >   least one time.
> 
> I wasn't able to reproduce this one, unfortunately.

That's ironic because it's fairly predictable on my system. Here is the steps I 
follow:

- Open some document.

- Jump to a page by typing a number in the input bar. This works 
  normally.

- Zoom in/out, or press 'a' or 's'.

- Jump again to an arbitrary page. This time it doesn't work and the 
  view stays in it's current position.

Note that you must change the scale of the document at least one time before 
you would be able to reproduce it.

> 
> > - Activating the input bar using ':' will cause the view to be adjusted 
> >   to the same position it was in during the most recent zoom. Again, 
> >   this happens only after at least one zoom.
> 
> This one I was able to reproduce, though.
> 
> > I think this happens because in both cases the act of showing the input bar 
> > causes the upper bound of the vertical adjustment to be changed which 
> > triggers 
> > cb_view_vadjustment_changed which in turn will call 
> > set_adjustment_from_ratio.  
> > Since you don't update the adjustment ratios during the above operations, 
> > set_adjustment_from_ratio will get us back to where we were during the last 
> > zoom.
> 
> Yes, I think you're right. I hadn't thought of that.
> 
> > Also, your patch makes it necessary to update the adjustment ratios in the 
> > adjustment_ratio struct during scrolling in the document to reflect the 
> > changed 
> > adjustments.
> > 
> > One could argue that we should then call 
> > zathura_readjust_view_after_zooming[1]
> > from cb_view_vadjstment_value_changed in order to update the ratio whenever 
> > the 
> > vertical adjustment changes, and indeed when I did that, the above problems 
> > disappeared, but now zooming doesn't work as it should, because when we 
> > zoom,
> > "both" the adjustment upper bound and value change, and apparently, the 
> > 'value-changed' callback is called "before" the 'changed' callback. So, the 
> > 'value-changed' callback will update the ratio based on the new upper bound 
> > before the 'changed' callback has a chance to update the adjustment value 
> > from 
> > the ratio. This sort of chicken-and-egg problem will cause zooming to 
> > effectively scroll the document backward, because the 'changed' callback 
> > will 
> > then set the adjustment value using the wrong ratio computed from the 
> > 'value-changed' callback.
> 
> You're absolutely right, nice analysis!
> 
> > I hope I haven't lost you somewhere!
> 
> Not at all, it was very clear, thanks.
> 
> > Similar arguments could be made for the horizontal adjustment.
> > 
> > [1] I think this function should now be called something more generic 
> > because 
> > all that it does now is updating the adjustment ratios, which isn't 
> > specific to 
> > zooming and should be done whenever the adjustments change (e.g. scrolling).
> 
> Indeed, but that's the easy part ;)
> 
> I think one way to solve the issue you brought up without having to
> remember saving the ratio every time we move around in the document,
> would be to do it in set_adjustment() from utils.c. But that would mean
> accessing zathura->document from there, so it isnt' as straightforward
> as I'd like it to be.
> 
> What do you think? Do you see possible issues there? Do you have a
> better idea?

Well, changing the ratios in set_adjustment will only be useful in cases where 
we explicitly change the adjustments like when we jump to a specific page, but 
we would still need a way to update the ratios for implicit cases like 
scrolling t

Re: [zathura] [PATCH] Use signals to readjust_view_after_zooming()

2013-03-21 Thread Benoît Knecht
Hi Marwan,

Thanks a lot for your review and feedback.

Marwan Tanager wrote:
> On Wed, Mar 20, 2013 at 10:17:36PM +, Benoît Knecht wrote:
> > Instead of guesstimating the values of the scrollbars adjustments after
> > a change in zoom level, connect callbacks to the "changed" GtkAdjustment
> > event (which is emitted when the bounds or page_size of the adjustment
> > change, e.g. when the zoom level changes), and compute the new values
> > from there.
> > 
> > cb_view_hadjustment_changed() centers the page horizontally if a
> > "best-fit" or "width" zoom is being performed, or if "zoom-center" is
> > true; otherwise, it keeps the view horizontally centered around the same
> > area of the page.
> > 
> > cb_view_vadjustment_changed() always keeps the view vertically centered
> > around the same area of the page.
> > ---
> > This is in preparation for addressing . A 
> > patch
> > will follow shortly.
> 
> Just a couple of notes based on my experimentation with the patch:
> 
>   - Jumping to a page using a numeric command doesn't work anymore. The 
> page number is updated on the status bar, but the view is not 
> adjusted to the target page. This will not happen unless you zoom at 
> least one time.

I wasn't able to reproduce this one, unfortunately.

>   - Activating the input bar using ':' will cause the view to be adjusted 
> to the same position it was in during the most recent zoom. Again, 
> this happens only after at least one zoom.

This one I was able to reproduce, though.

> I think this happens because in both cases the act of showing the input bar 
> causes the upper bound of the vertical adjustment to be changed which 
> triggers 
> cb_view_vadjustment_changed which in turn will call 
> set_adjustment_from_ratio.  
> Since you don't update the adjustment ratios during the above operations, 
> set_adjustment_from_ratio will get us back to where we were during the last 
> zoom.

Yes, I think you're right. I hadn't thought of that.

> Also, your patch makes it necessary to update the adjustment ratios in the 
> adjustment_ratio struct during scrolling in the document to reflect the 
> changed 
> adjustments.
> 
> One could argue that we should then call 
> zathura_readjust_view_after_zooming[1]
> from cb_view_vadjstment_value_changed in order to update the ratio whenever 
> the 
> vertical adjustment changes, and indeed when I did that, the above problems 
> disappeared, but now zooming doesn't work as it should, because when we zoom,
> "both" the adjustment upper bound and value change, and apparently, the 
> 'value-changed' callback is called "before" the 'changed' callback. So, the 
> 'value-changed' callback will update the ratio based on the new upper bound 
> before the 'changed' callback has a chance to update the adjustment value 
> from 
> the ratio. This sort of chicken-and-egg problem will cause zooming to 
> effectively scroll the document backward, because the 'changed' callback will 
> then set the adjustment value using the wrong ratio computed from the 
> 'value-changed' callback.

You're absolutely right, nice analysis!

> I hope I haven't lost you somewhere!

Not at all, it was very clear, thanks.

> Similar arguments could be made for the horizontal adjustment.
> 
> [1] I think this function should now be called something more generic because 
> all that it does now is updating the adjustment ratios, which isn't specific 
> to 
> zooming and should be done whenever the adjustments change (e.g. scrolling).

Indeed, but that's the easy part ;)

I think one way to solve the issue you brought up without having to
remember saving the ratio every time we move around in the document,
would be to do it in set_adjustment() from utils.c. But that would mean
accessing zathura->document from there, so it isnt' as straightforward
as I'd like it to be.

What do you think? Do you see possible issues there? Do you have a
better idea?

In any case, thanks again for your insight.

Cheers,

-- 
Benoît Knecht
___
zathura mailing list
zathura@lists.pwmt.org
http://lists.pwmt.org/mailman/listinfo/zathura


Re: [zathura] [PATCH] Use signals to readjust_view_after_zooming()

2013-03-21 Thread Marwan Tanager
On Wed, Mar 20, 2013 at 10:17:36PM +, Benoît Knecht wrote:
> Instead of guesstimating the values of the scrollbars adjustments after
> a change in zoom level, connect callbacks to the "changed" GtkAdjustment
> event (which is emitted when the bounds or page_size of the adjustment
> change, e.g. when the zoom level changes), and compute the new values
> from there.
> 
> cb_view_hadjustment_changed() centers the page horizontally if a
> "best-fit" or "width" zoom is being performed, or if "zoom-center" is
> true; otherwise, it keeps the view horizontally centered around the same
> area of the page.
> 
> cb_view_vadjustment_changed() always keeps the view vertically centered
> around the same area of the page.
> ---
> This is in preparation for addressing . A patch
> will follow shortly.

Just a couple of notes based on my experimentation with the patch:

- Jumping to a page using a numeric command doesn't work anymore. The 
  page number is updated on the status bar, but the view is not 
  adjusted to the target page. This will not happen unless you zoom at 
  least one time.

- Activating the input bar using ':' will cause the view to be adjusted 
  to the same position it was in during the most recent zoom. Again, 
  this happens only after at least one zoom.

I think this happens because in both cases the act of showing the input bar 
causes the upper bound of the vertical adjustment to be changed which triggers 
cb_view_vadjustment_changed which in turn will call set_adjustment_from_ratio.  
Since you don't update the adjustment ratios during the above operations, 
set_adjustment_from_ratio will get us back to where we were during the last 
zoom.

Also, your patch makes it necessary to update the adjustment ratios in the 
adjustment_ratio struct during scrolling in the document to reflect the changed 
adjustments.

One could argue that we should then call zathura_readjust_view_after_zooming[1]
from cb_view_vadjstment_value_changed in order to update the ratio whenever the 
vertical adjustment changes, and indeed when I did that, the above problems 
disappeared, but now zooming doesn't work as it should, because when we zoom,
"both" the adjustment upper bound and value change, and apparently, the 
'value-changed' callback is called "before" the 'changed' callback. So, the 
'value-changed' callback will update the ratio based on the new upper bound 
before the 'changed' callback has a chance to update the adjustment value from 
the ratio. This sort of chicken-and-egg problem will cause zooming to 
effectively scroll the document backward, because the 'changed' callback will 
then set the adjustment value using the wrong ratio computed from the 
'value-changed' callback.

I hope I haven't lost you somewhere!

Similar arguments could be made for the horizontal adjustment.

[1] I think this function should now be called something more generic because 
all that it does now is updating the adjustment ratios, which isn't specific to 
zooming and should be done whenever the adjustments change (e.g. scrolling).



Marwan
___
zathura mailing list
zathura@lists.pwmt.org
http://lists.pwmt.org/mailman/listinfo/zathura