On Thu, Mar 21, 2013 at 09:04:57AM +0100, Benoît Knecht wrote:
> Marwan Tanager wrote:
> > On Wed, Mar 20, 2013 at 11:03:54PM +0000, Benoît Knecht wrote:
> > > This wasn't correct. Padding is already accounted for:
> > >
> > > scale = (width - (pages_per_row - 1) * padding) /
> > > (pages_per_row * cell_width)
> > >
> > > If you add padding on the denominator, you end up with black margins on
> > > the sides of the window, which isn't what padding is for (i.e. insert a
> > > gap between rendered pages), and defeats the purpose of a "best-fit".
> >
> > Not accounting for the padding in the denominator is what will actually
> > defeat
> > the purpose of best-fit.
> >
> > To get a feel for this, try setting pages-per-row to e.g. 4, and
> > page-padding
> > to e.g. 7, while the show-scrollbars setting is set to true, then press 's'
> > or
> > 'a' to fit the collective width of the pages into the view. You will notice
> > that the horizontal scrollbar is still showing up which shouldn't happen
> > because the pages now should be fitted entirely inside the view.
>
> Indeed, I see that too.
>
> > This happens because not adding the paddings to the width of the cells will
> > end
> > you up with a scaling factor that makes the pages fit into the view but
> > "only"
> > if there is no paddings. To verify this, set page-padding to 0, and repeat
> > the
> > same procedure. Now, things will work as expected.
>
> But your fix was in the wrong place, it should go within the
> "if (show_scrollbars)" section, line 153, as everything works well when
> show-scrollbars is false. And from what I can tell, the extra width
> isn't proportional to padding (try setting it to something big, then
> double it for instance); it seems to be just one pixel too wide, or
> something like that.
>
> > Since the paddings aren't part of the cells, consume some space of the
> > view,
> > and aren't subject to scaling, we should add them here to the total width
> > of
> > the cells in order to "best-fit" them together with the cells themselves.
>
> No, padding is already accounted for in the numerator (it is substracted
> from the window's width). So essentially, the scale is computed by
> saying that the width of all pages on a row (the denominator) has to fit
> in the width of the window minus the padding (the numerator). If you
> scale it like that, and then add padding between the pages, they will
> fit exactly in the window. The scrollbar issue is something completely
> different.
>
> > And regarding the borders on the edges, this only happens with multiple
> > pages
> > per row, which isn't the typical use case. And IMHO, as long as the pages
> > are
> > fitted entirely inside the view, the user wouldn't care. But having the
> > scrollbar showing after best-fit is what would more likely be considered an
> > issue.
>
> Sure, the scrollbar issue needs to be fixed, but as I said, this isn't
> the correct way to fix it, and it affects best-fit's behavior when
> show-scrollbars is false, even though it was working fine before.
It turns out you are right indeed. That was a misunderstanding of the formula
on my side.
Also, I've just noticed something that confirms that the problem with the
scrollbar is entirely unrelated: try experimenting with multiple subsequent
values of page-padding and you will notice that the scrollbar sometimes appears
and sometimes doesn't.
Thanks for spotting this.
Marwan
_______________________________________________
zathura mailing list
[email protected]
http://lists.pwmt.org/mailman/listinfo/zathura
