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

Mismatch between CSS and device pixels on Android #97

Closed
mickael-menu opened this issue Nov 6, 2020 · 9 comments
Closed

Mismatch between CSS and device pixels on Android #97

mickael-menu opened this issue Nov 6, 2020 · 9 comments

Comments

@mickael-menu
Copy link
Member

I'm submitting a bug report.

Short description of the issue/suggestion:

Pages are misaligned on large resources if the device has a fractional pixel density. See this related issue on Android: readium/kotlin-toolkit#166

Steps to reproduce the issue/enhancement:

  1. Go to the nearest Google store, not forgetting your mask.
  2. Buy an Android device with a fractional pixel density, such as a Pixel 3a (ratio 2.75).
  3. Open Children's Literature on R2 Reader.
  4. Swipe through the chapters until you see the pages offsetting.

What is the expected behaviour?

We should see only the current page on the screen.

What is the current behaviour?

90648045-58db4600-e239-11ea-8e33-ebad3ab4492c

Do you know which CSS modules (stylesheets) are impacted?

Probably the pagination one.

Please tell us about your environment:

  • Platform: Android
  • Browser / Rendering Engine: Android 11 Web View
  • Device: Pixel 3a

Other information (e.g. related issues, suggestions how to fix, links for us to have context)

I'm not 100% sure it is a Readium CSS issue but I'm opening a discussion on the subject, because I'm in a pickle to fix this Android issue.

First, here are some information about the Pixel 3a:

Property Value
Resolution 1080 x 2220 PX
Viewport Size 393 x 808 PX
Pixel Ratio 2.75
Pixel Density 441 PPI
Device-Independent Pixels 160 PPI

On the Kotlin EPUB navigator, scrolling through pages is done using Kotlin code and Android APIs. We're using the device's resolution width for all calculation, so in the case of the Pixel 3a, a page is 1080px large.

Unfortunately there's a mismatch with the page width we get in the web view, which has a viewport of 393px. But 1080 / 2.75 = 392.72. This small difference adds up to huge offsets in large resources.

What makes me think that the issue is more related to Readium CSS is some tests I did in the web inspector.

First, a weird thing is that although a CSS page should be 393px, document.scrollingElement.scrollWidth was 100631 which is not a multiple of 393 (nor 1080 / 1.75). So we can see pages getting shifted by doing either:

document.scrollingElement.scrollLeft = pageIndex * 393;
// or
document.scrollingElement.scrollLeft = pageIndex * (1080 / 1.75);

Then, I created an absolute-positioned div with width: 100% (I also tried width: 393px which gives the same result). While it appears to be the size of a page, I can see a really thin portion of it after turning the page. Whether I'm doing it from Kotlin, or by executing directly document.scrollingElement.scrollLeft = 393.

Screenshot_20201106-115157

Next page:

Screenshot_20201106-115144

@JayPanoz
Copy link
Collaborator

JayPanoz commented Nov 7, 2020

Hmmm what’s sure at first sight is that it will be very difficult to solve this one through ReadiumCSS since it is fed the computed (hence rounded) values by the rendering engine. We’re using percentage or viewport units in the pagination module so it sounds kinda hopeless/like a black hole to try to hack our way out of this there. I thought maybe it could be columns but at this size, column-count: 1 is applied anyway so all other column metrics are somewhat ignored.

Do you know what happens if you apply exactly 392.72px? My guess is that it will be rounded to 393px anyways since this is how browsers have been handling such fractional values for a very long time but maybe I missed something recently. Then if that works obviously the issue will be to get the exact value then enforce it on html via an inline style…

I’ll try to take a look at the Chromium issue tracker as I’m pretty sure this already popped up there.

@mickael-menu
Copy link
Member Author

Do you know what happens if you apply exactly 392.72px?

How would you do this? I tried injecting --RS__colWidth: 392.72px but no matter the value, it doesn't change anything in the rendering.

My guess is that it will be rounded to 393px anyways since this is how browsers have been handling such fractional values for a very long time but maybe I missed something recently.

I tried again the test with the red div, setting a width of 392.72px and it seems to be working. It fills the full width of the first page, but no part is visible on the second page.

@JayPanoz
Copy link
Collaborator

JayPanoz commented Nov 9, 2020

How would you do this? I tried injecting --RS__colWidth: 392.72px but no matter the value, it doesn't change anything in the rendering.

Given how columns work, the column-width is a mere indication in this case. What you have to do is to restrict the width of :root, hence html.

min-width: 100%;
width: 100%;
max-width: 100%;

It’s kinda clear I’ll have to create a custom property/variable for that as it is highly likely that it impacts Chrome (browser) too but in the interval, you can do something like:

<html style="…; min-width: 392.72px; width: 392.72px; max-width: 392.72px">

Getting a rounded value for 100% while the real width is fractional still feels like a bug to me so it should be probably be reduced to a minimal test case and raised in Chromium’s issue Tracker – if the issue doesn’t already exist – but we should at least have a CSS variable available in the API e.g. RS__viewportWidth (or something like that???) to get around this because it won’t be fixed anytime soon.

[edit] and obviously it has to be documented since you also have to keep track of orientation, resize, etc. changes.

@mickael-menu
Copy link
Member Author

but in the interval, you can do something like:

<html style="…; min-width: 392.72px; width: 392.72px; max-width: 392.72px">

That worked! I could reach the end of the chapter without any noticeable offset. After checking a screenshot the last page was 2 or 3 pixels off, probably because 392.72px is not precise enough (392.727272727...), I wonder if we could use calc() for this, to use (1080 / 1.75).

Getting a rounded value for 100% while the real width is fractional still feels like a bug to me so it should be probably be reduced to a minimal test case and raised in Chromium’s issue Tracker

Agreed, this smells like a rendering engine bug to me.

but we should at least have a CSS variable available in the API e.g. RS__viewportWidth (or something like that???) to get around this because it won’t be fixed anytime soon.

That would be super helpful. Do you think you would be able to implement this soonish since this issue is critical for Android? viewportWidth seems like a good name for that.

[edit] and obviously it has to be documented since you also have to keep track of orientation, resize, etc. changes.

Yeah, we probably need a JS snippet to handle this. I'll check if we can get the real viewport width from JS. I think that the pixel ratio is available with window.devicePixelRatio.

JayPanoz added a commit that referenced this issue Nov 9, 2020
Resolves #97 but doesn’t fix the overarching issue in Chromium. Also
needs docs before being pushed into main.
@JayPanoz
Copy link
Collaborator

JayPanoz commented Nov 9, 2020

@mickael-menu Just pushed the --RS__viewportWidth variable into develop (as a new beta version 3)

@mickael-menu
Copy link
Member Author

Great, thanks again!

@mickael-menu
Copy link
Member Author

It's pixel perfect with --RS__viewportWidth: calc(1080px / 2.75);, which can be set in JavaScript with:

setProperty("--RS__viewportWidth", "calc(" + nativeViewportWidth + "px / " + window.devicePixelRatio + ")");

I still have some other related issues to fix before sending a PR. I think they are caused by the existing code being tuned to the faulty web view behavior.

@JayPanoz
Copy link
Collaborator

Thanks for reporting the JS snippet back!

@mickael-menu
Copy link
Member Author

I opened a PR on the Kotlin navigator: readium/r2-navigator-kotlin#178

Considering that we can't really do much more at our level, feel free to close the issue when you want Jiminy. Thanks for your help!

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