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

Multi-column page layout #4184

Open
kjk opened this issue Apr 13, 2024 · 7 comments
Open

Multi-column page layout #4184

kjk opened this issue Apr 13, 2024 · 7 comments

Comments

@kjk
Copy link
Member

kjk commented Apr 13, 2024

This is to record work on showing multiple columns. It subsumes #246, #776, #1171

The work is done on multi-column branch. It's ok to rebase multi-column branch against master branch and force push, to keep it in sync with master.

Desired features:

  • a way to explicitly set number of columns (with a reasonable limit). It's a generalization of "facing" view where we allow more than 2 columns
  • a "liquid" layout where we show as many columns as fit given a fixed zoom level

It requires re-thinking of our View options, both in UI and how it's persisted in settings.

How do other programs handle this in their UI?

@kjk kjk changed the title Multi-column view Multi-column page layout Apr 13, 2024
@kjk kjk mentioned this issue Apr 13, 2024
@kjk
Copy link
Member Author

kjk commented Apr 13, 2024

To set number of columns we could have menu items with explicit numbers or add a modal dialog similar to IDD_DIALOG_GOTO_PAGE where user can enter number of columns. We would clip it to valid range 1 - N.

1 maps to single page, 2 to facing and the rest needs to be remembered in settings.

@kjk
Copy link
Member Author

kjk commented Apr 13, 2024

The combinations of view options and zoom levels is confusing already and will be even more. It would be good to design a modal dialog for setting those options in a way to provides a UI hint on how things will look. I assume there are programs that already do something like that and we can steal their design.

@kjk
Copy link
Member Author

kjk commented Apr 13, 2024

We also need to improve our render cache. At the very least significantly bump the number of cached pages because it was designed with assumption it only needs to keep few pages around. With 4 columns and 16 pages visible at a time, we re-render the pages on scroll.

Cache eviction logic should be changed. We should unconditionally evict pages rendered at no-longer-valid zoom. But eviction of rendered pages at currently valid zoom should be based on memory used by rendered bitmaps vs. amount of free memory in the system.

We should also have a bool evict_emergency() function that we can call if we fail to make a significant allocation. It would delete all non-visible pages (aggressive) and return true if evicted something, in which case we would retry an allocation. We don't want to retry every failed allocation, just big ones (> 1MB?) and only in specific contexts. If we fail to allocate 1 KB then we're likely not going to recover anyway. So we would probably add a retry_malloc or add a global flag similar to gAllowAllocFailure (gRetryAlloc) that we would strategically set around allocations that should be retried.

Also, we should increase number of render threads. Our system was designed long time ago and only uses a single thread that picks items to render from a queue.

Today it makes more sense to have up to N threads (where N is number of cores).

Instead of keeping a rendering thread we could launch them on demand and throttle number of actively working threads via semaphore.

Which brings another point: apparently they way I did multi-threading is not how mupdf wants it. Each mupdf-based document has a single fz_context which contains locks. Turns out that each thread should use fz_context from fz_clone_context().

All this is big job in itself and probably deserves its own bug / branch but without at least increasing cache size multi-column view will be bad experience for users.

@kjk
Copy link
Member Author

kjk commented Apr 13, 2024

Another thing: it seems in what is now non-continuous view it also makes sense to provide multiple rows, if the space is there.

We also need to decide how to use remaining space: have it as left/right margin, distribute between pages. In HTML there's display: flex and it has different justify-content settings. We need to decide which one of those we want to implement (maybe multiple?)

@changlichun
Copy link
Collaborator

changlichun commented Apr 13, 2024

I was mostly inspired by office word, which allows the number of columns to change while zooming. They have such a setting dialog(I dont have a English version).
image
When set to multi page, the column number will auto change while zooming.

They do not support non-continuousely mode.

Maybe we can have a option like show pages continuously to triggle fix or float columns. And a dialog like custom zoom to set the column number.

@changlichun
Copy link
Collaborator

One known issue now. When columns>2, setting to fit page or fit width will decrease the column number unexceptly.

@kjk
Copy link
Member Author

kjk commented Apr 13, 2024

Yes, something like that word dialog but adapted.

I would say we need liquid (exact name to be determined) in addition to fit width, fit page etc. It shows as many columns as will fit based on zoom level.

In non-continuous mode it also fits as many rows as will fit in a window.

Then instead of multi-page I would do columns text box where user can enter a fixed number of columns. We either auto-size the column in fit window mode or have scrolling if fixed zoom level.

I don't want to do grid. That's just a bit too much and makes no sense in continuous mode. Ideally in non-continuous mode we fit as many rows as fits in window or we just stick with current single row.

Layout code is tricky and with those additions will get even more tricky. A good time to add tests to verify all permutations. The algo is well defined: it gets width of viewport, layout mode (zoom, fit, continuous or not), number and size of pages at 100%. It calculates the size of the viewport and positions of all pages in it.

Currently the code is tied to all other info about pages but could be transformed to only use the layout info / bounding boxes of rectangles (pages) and spit out their position. We could then write unit tests for specific configurations and for ad-hoc testing we could save position of rectangles to text file and then post-process it in Go to generate HTML that visualizes the layout of the page within viewport.

There's also an outstanding layout issue that we should address when touching this code.

When I wrote layout I assumed that all pages have the same size and therefore for layouts where we auto-fit pages to fit within viewport (window), we need to calculate a single zoom value for all pages.

This breaks for non-uniform pages so a single big page in fit width mode makes smaller pages render too small. We want all pages have the same width in a column which means we need to calculate / remember custom zoom for each page.

And finally, we have single layout logic for all possible variations, which might be more complex than necessary. Maybe it would be simpler to split the code into trivial layouts (fixed zoom and number of columns) and complex layouts (where we need to calculate zoom for pages to fit them into viewport).

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

No branches or pull requests

2 participants