Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix pdf-viewer's page display in non continuous mode #3952

Merged
merged 3 commits into from
Jan 19, 2025

Conversation

octaeder
Copy link
Contributor

@octaeder octaeder commented Jan 15, 2025

The pdf-viewer has a (numeric) page display. It displays a text of the kind pages x to y of z. Under certain circumstances it happens that y in the page display is wrong. The reason for this is a calculation error, because the page offset of the first page is not taken into account. Even so this error also happens in continuous mode, it can't be noticed. We will clarify, why this is the case.

The calculations in mind follows:

visiblePageCount = qMin(gridx * gridy, realNumPages() - realPageIndex)

This should return the number of pages visible in the grid in use. To be more precise, it is the number of grid faces filled with a page. Due to zooming it may be that you can't see all the pages in the grid, so the word visible is a little bit misleading. To see how this works, we scroll down such that the first page gets out of sight. Hence pages are filled from the upper left corner of the grid from left to right row by row. The number of pages in the grid is atmost the area of the grid, i.e. gridx * gridy. This happens for sure, if the last page of the document follows the page in the lower right corner.

But if the last page is within the grid, then empty grid faces may appear. Thus the number of pages visible can be less than the grid area. The page index (this is counting pages from 0) of the first page in the grid is held in realPageIndex. It follows that the number of pages given is realNumPages() - realPageIndex. Since realNumPages() is 1 greater than the page index of the last page, we don't need to add an extra 1. The result needed is the minimum of both. So far so good.

When we scroll to the first page, it can be different. This shows the following image of two running txs apps side by side:

grafik
click for full size

The case on the left is totally the same as before (page display is Pages 1 to 6 of 10). But on the right we find an offset greater 0 for the first page. Even so the index of the first page is realPageIndex=0. So above calculation would yield the same result. To fix this we reduce the grid area by subtracting pageOffset (counting the empty faces at the beginning). Again we must be aware of the place of the last page. Now, all cases can be put together as follows:

availableGridFaces = gridx * gridy - (realPageIndex>0 ? 0 : getPageOffset());
visiblePageCount = qMin(availableGridFaces, realNumPages() - realPageIndex);

Now we undestand why the two cases in the following image show the same page display Pages 1 to 6 of 10 as above on the left. All three show txs build from current master. On the right of the first image you see my fixed version. The page display there shows Pages 1 to 5 of 10, which is correct (and if page offset were 2 we would see Pages 1 to 4 of 10 ).

grafik
click for full size

Obviously the formula given at the beginning is correct when grid offset is 0. Otherwise the result may be too large. This fact was first stated in a comment added in 2018. But the value is not directly used for the page display. The algorithm is as follows: The highest page index of the pages calculated to be visible is used as a starting point to search the highest page index of the page which has a non empty intersection with the canvas (view port) used to present the pdf document. If the intersection is empty, then the page index is reduced by one (not really optimal) and this page is checked, and so on. Now, in continuous mode the pages always fill the canvas from top to bottom (not necessarly from left to right). Hence the page index found by this algorithm is indeed suitable as y in the page display. But in non continuous mode the algorithm works the same but the page now lies below the grid. Thus the page display is wrong. It is therefore crucial to calculate the visible pages correctly.

@octaeder octaeder marked this pull request as draft January 15, 2025 21:05
@sunderme
Copy link
Member

this pr is in draft mode.
are any more changes coming ?

@octaeder
Copy link
Contributor Author

I want to do some more testing. I assume that I can remove draft latest on sunday.

@sunderme
Copy link
Member

okay, i was just wondering

@octaeder octaeder marked this pull request as ready for review January 19, 2025 09:37
@octaeder
Copy link
Contributor Author

I have improved my initial comment of this PR. @sunderme please start review.

@sunderme sunderme merged commit ed2ce22 into texstudio-org:master Jan 19, 2025
7 checks passed
@sunderme
Copy link
Member

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants