pdfrx icon indicating copy to clipboard operation
pdfrx copied to clipboard

ScrollPhysics + page fit + maintain position on layout and size changes

Open enhancient opened this issue 7 months ago • 17 comments

This PR adds support for ScrollPhysics (via changes to the forked InteractiveViewer in pdfrx), and somewhat related changes that came up when looking into how to manage boundaryMargin which is required for ScrollPhysics to work properly against the document boundaries.

These changes include:

  • _adjustBoundaryMargins() function which auto-adjusts boundaryMargins for content smaller than the viewPort and centers it.
  • _calcPanAxis() function which restricts panning in InteractiveViewer in a dimension where the content doesn't overflow (this is expected behaviour for BouncingScrollPhysics() where content snaps to a boundary and provides a better user experience)
  • Additional of calculations including boundaryMargin for various built in functions such as _calcMatrixForPage, _calcCoverFitScale
  • boundary clamping (via _calcMatrixForClampedToNearestBoundary) for _goToPage and for layout / size changes (see below), effectively meaning that when scrollPhysics is defined, we don't need the operations in _normalizeMatrix and there is a check which returns from this function.
  • in _updateLayout, for layout changes, there is a new _goToPosition() function that calculates the same position (pinned to the top-left of the screen) in a new layout so that for things like device rotation, the same position is maintained, providing a better user experience. For cases where there is no layout change, but a size change (e.g. device rotation), the content is clamped to the nearest boundary to avoid overshooting.
  • pageFit - a new PdfPageFit option providing options of: fit (equivalent of alternativeFitScale), fill (equivalent of coverScale), auto (providing alternative fit options depending on device orientation), and none (previous behaviour). Coupled with this, _layoutPages has been expanded to provide different page layouts based on PdfPageFit to better utilise the screen and provide for fairly standard layouts that users would expect for a pdf reader. Horizontal in addition to the original vertical support is built in.

I've tried to make everything backwards compatible so existing implementations will behave as before.

enhancient avatar May 20 '25 04:05 enhancient

For panAxis, I think PdfViewer should honor the programmer's intension; I want to remove _calcPanAxis function:

https://github.com/enhancient/pdfrx/blob/enhancient/lib/src/widgets/pdf_viewer.dart#L441

espresso3389 avatar May 20 '25 16:05 espresso3389

For panAxis, I think PdfViewer should honor the programmer's intension; I want to remove _calcPanAxis function:

https://github.com/enhancient/pdfrx/blob/enhancient/lib/src/widgets/pdf_viewer.dart#L441

Yes, this makes sense - I've actually found a much better way to handle this within InteractiveViewer using ScrollPhysics.shouldAcceptUserOffset() - this then behaves as for other Scrollables when the content is at or less than the viewport size, scroll is not permitted. But it can be overridden in the usual way - e.g. scrollPhysics: AlwaysScrollableScrollPhysics(parent: BouncingScrollPhysics()).

enhancient avatar May 22 '25 04:05 enhancient

I think PdfPageAnchor and PdfPageFit conflicts each other. In theory, their roles seem very similar and could be integrated into one.

I'm gradually deciding that we must make breaking changes to introduce the new behavior; it's almost the time to release a new major version.

espresso3389 avatar May 22 '25 08:05 espresso3389

And, another topic here is what's the fit zoom ratio. For facing pages view, the fit ratio should be double-page width and the current logic could not allow such dynamic calculation. We need to add some callback function to customize them. At least, maxPageWidth/maxPageHeight are not enough.

espresso3389 avatar May 22 '25 08:05 espresso3389

I think PdfPageAnchor and PdfPageFit conflicts each other. In theory, their roles seem very similar and could be integrated into one.

I'm gradually deciding that we must make breaking changes to introduce the new behavior; it's almost the time to release a new major version.

I admit I'm not quite sure what PdfPageAnchor is intended for. I thought it was more about where the layout should be anchored on screen, although with _normalizeMatrix, everything seems to be centered in any case? Note that with some changes to _adjustBoundaryMargins() it would be possible to add different layout positions of the document based on PdfPageAnchor (.e.g bottomRight), but right now it centers like _normalizeMatrix does.

The intention for PdfPageFit is decide how to scale the pages according to preference (which could very well be end-user choice). It seems to me that it's a different focus than PdfPageAnchor.

The fit options proposed are: .fit (scale so the whole page fits in view) .fill (scale so the page covers the entire view, cropping one axis if necessary) .none (no uniform scaling; pages keep their native size) .auto (choose between .fit/.fill based on orientation - and we could extend that to consider viewPort h/w ratios to better support difference devices)

It can make a big difference in usability to be able to choose such fit options. I also think they're fairly well understood (perhaps not auto because we're abstracting some intelligent selection), and from a developer perspective easy to set according to needs.

enhancient avatar May 22 '25 09:05 enhancient

And, another topic here is what's the fit zoom ratio. For facing pages view, the fit ratio should be double-page width and the current logic could not allow such dynamic calculation. We need to add some callback function to customize them. At least, maxPageWidth/maxPageHeight are not enough.

I think maxPageWidth could still be used, and in a side-by-side layout, the fit would be maxPageWidth/2? One popular pdf reader lays side-by-side like in the attached. Screenshot 2025-05-22 at 7 24 02 pm

enhancient avatar May 22 '25 09:05 enhancient

Looking closer at this, I agree additional info was required for the PdfPageLayoutFunction - I've added pageFitWidths and pageFitHeights that are calculated based on the page fit and I think this makes it a lot more flexible. I've updated the examples in main for horizontal and facing pages layout examples to show its use. It seems to be working well. I think for a facing pages layout PdfPageFit of .fill makes most sense.

I thought about whether the pageFitWidth and pageFitHeight could be added to PdfPage instead to save changing PdfPageLayoutFunction - what do you think about this, and the solution in general?

enhancient avatar May 23 '25 04:05 enhancient

Just a note:

On non-iOS platforms, as you said, scrollPhysics is not BouncingScrollPhysics and the UI normally don't have bouncing back effect. But some platforms such as Android support certain kind of overscroll animation like stretching effect or glow effect.

At the current implementation, our InteractiveViewer does not have such effect anyway. It seems we'd better support emitting ScrollNotification events on overscroll case. But I don't know whether StretchingOverscrollIndicator easily handle the events from InteractiveViewer or not.

espresso3389 avatar May 23 '25 16:05 espresso3389

Looking at the most popular PDF readers on Android, none of them implement an overscroll stretch or flow effect. It seems that they all vary slightly with some allowing overscroll and some allowing zoom below/above min/max to somewhat mimmic iOS without the friction.

Here is a summary:

< min zoom > max zoom start end sides
acrobat Y^ Y^ Y^^ Y^^ Y^^
pspdf/nutrient Y^ N N N N
foxit N N Y Y Y^^
mupdf N N Y Y Y^^

^ only allows overshoot to a point then locks ^^ only if content overflows boundary

On this basis, I think it makes sense to create a custom ScrollPhysics to be used on Android to provide users with an option to enable overscroll - the way I've implemented ScrollPhysics in InteractiveViewer though doesn't allow for a distinction between zoom and pan overscroll, so would need to consider how important that is.

enhancient avatar May 24 '25 22:05 enhancient

Basically, in my design, PdfPageAnchor is used to detemine which edge or vertex should be algined inside the view if the area to be shown into the view is larger than the view size. It's not for fit or layouting pages.

Your PR contains codes that mis-use PdfPageAnchor and they should be removed at least. I'm checking the code consistency carefully to remove such mis-uses.

espresso3389 avatar May 26 '25 06:05 espresso3389

At least, _calcPageSizes and _calcCoverFitScale contains such code and hotestly (I don't intend to blame you), I could not understand your intention of these uses so far...

espresso3389 avatar May 26 '25 06:05 espresso3389

At least, _calcPageSizes and _calcCoverFitScale contains such code and hotestly (I don't intend to blame you), I could not understand your intention of these uses so far...

I am using PdfPageAnchor here to determine page layout orientation (horizontal or vertical) as per use in main:

pageAnchor: isHorizontalLayout ? PdfPageAnchor.left : PdfPageAnchor.top, pageAnchorEnd: isHorizontalLayout ? PdfPageAnchor.right : PdfPageAnchor.bottom,

Effectively I am reversing this logic to determine which pageAnchor results in a horizontal layout. Perhaps it would be better to specific a layout orientation option rather than deriving from the PdfPageAnchor, which doesn't really seem very robust, because what if the PdfPageAnchor is topLeft for example - how do you determine the layout orientation.

enhancient avatar May 26 '25 07:05 enhancient

I prefer to introduce fitHorz, fitVert, fit,... to determine the behavior. But anyway, it's deeply related to layout logic and we should introduce one-stop customization point to do layout and fit behavior.

I'm now agressively changing the existing code and don't hesitate to change anything anyway :)

espresso3389 avatar May 26 '25 07:05 espresso3389

Mmm, even if we introduce something like fitHorz, we should also handle facing-page case in some custom way. It should be more configurable...

espresso3389 avatar May 26 '25 07:05 espresso3389

Mmm, even if we introduce something like fitHorz, we should also handle facing-page case in some custom way. It should be more configurable...

I think a facing-page layout could be either vertical or horizontal? At least one PDF reader I looked at supports this.

enhancient avatar May 26 '25 07:05 enhancient

I think a facing-page layout could be either vertical or horizontal?

It is just the layout you mentioned here. The layout is called "facing sequential view" on Acrobat.

Anyway, we should determine the basic things:

  • Reduce confusing terms (i.e, zoom and scale, which do you think better?)
  • Remove alternativeFitScale (as you know, it's confusing)
  • At least, we need fitH, fitV (and larger one will be same to fill)
  • What is auto
  • ~~null~~ or none (null could not get well with copyWith semantics)

espresso3389 avatar May 26 '25 08:05 espresso3389

Anyway, we should determine the basic things:

  • Reduce confusing terms (i.e, zoom and scale, which do you think better?)

I think 'scale' makes sense for variable/method names. It's okay to talk about zooming as an action though.

  • Remove alternativeFitScale (as you know, it's confusing)

Agreed.

  • At least, we need fitH, fitV (and larger one will be same to fill)

Fitting horizontally and vertically are not the same as the .fit and .fill approaches I've included.

.fit - .fit is neither purely horizontal nor purely vertical - it computes both, and takes the smaller of the two such that both width and height of the page stay fully visible.

  • What is auto

You can see the logic in _calcPageFit()- it is making a decision about which fit (.fit of .fill) makes sense based on device orientation and scroll direction.

If you can, have a look at Pdf Reader from Nutrient on iOS - it has the most advanced page layout options I've seen and provides a good reference point.

I believe that we should introduce a scrollDirection parameter (.vertical and .horizontal) so that we can avoid inappropriate checks of PdfPageAnchor. Maybe that's actually a field in PdfPageLayout.

This also raises the question of whether to provide an option other than 'continuous scroll' - most readers when they are in horizontal mode, provide a page by page advance - one option for how it might work is that each individual page appears in a separate InteractiveViewer wrapped in a PageView, but I haven't explored this further. The feel is of moving between pages is like in a PageView where there is some resistance when scrolling and it snaps to the next page. Zooming in and out operates on just the page and not the neighbouring pages.

enhancient avatar May 27 '25 01:05 enhancient

Please check again when I use uri. The page got cut off in a very strange way

chiphan-quickcare avatar Jun 10 '25 06:06 chiphan-quickcare

@espresso3389 What's your thinking on including this pull request into master?

enhancient avatar Aug 14 '25 06:08 enhancient

I really want to merge it. But the changes is too much.

espresso3389 avatar Aug 14 '25 06:08 espresso3389

I really want to merge it. But the changes is too much.

So shall we break it down - just include the ScrollPhysics and look at the page fit separately?

enhancient avatar Aug 14 '25 06:08 enhancient

So shall we break it down - just include the ScrollPhysics and look at the page fit separately?

Yes, breaking the things down to primitive level will work.

espresso3389 avatar Aug 15 '25 07:08 espresso3389