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

Image slide show leaves artifacts from slide to slide #313

Open
rkwright opened this issue Apr 7, 2015 · 21 comments
Open

Image slide show leaves artifacts from slide to slide #313

rkwright opened this issue Apr 7, 2015 · 21 comments

Comments

@rkwright
Copy link
Contributor

rkwright commented Apr 7, 2015

An EPUB generated from InDesign has a slide show. As one navigates from page to page, it leaves behind slices of the previous page when the aspect rations of the images don't match. This bug is not occurring in SDKLauncher-OSX. Both apps are release/0.18
Sample screen capture:
idd-artifact
Test EPUB is here:
https://readiumfoundation.box.com/shared/static/7heji9osfyd1y2vghroeapb5lyrrb4ku.epub
IDD assets are here: (warning: 122 MB)
https://readiumfoundation.box.com/shared/static/mvfpfsbakfzxbbks34csa3kjx3wemfkt.zip

@danielweck
Copy link
Member

There's something wrong with this HTML page, irrespective of Readium. On Windows 8.1, I can click the image slideshow left/right arrows in IE and Firefox (the images change accordingly), but in Chrome nothing happens at all. That's when opening the HTML file on its own. Obviously in the Readium Chrome extension it doesn't work either.

@whmccoy
Copy link
Contributor

whmccoy commented Apr 8, 2015

Daniel, when you write "in the Readium Chrome extension it doesn't work
either" I presume by "doesn't work" that you mean that it leaves the
artifact as in Ric's original bug report, right? (rather than the images
not changing at all as you've found trying to open the HTML page itself in
Chrome? BTW might the Chrome behavior simply be an artifact of how Chrome
deals from a security perspective in defaulting not to opening local files
linked from local HTML files? It is rare that I find that something works
on IE and Firefox and not Chrome but this is one case where I have done
(there's a --allow-file-access-from-files flag that will enable it).

BTW I think Ric's original bug report was slightly ambiguous, where it
reads "As one navigates from page to page, it leaves behind slices..." it's
meant to say "from slide to slide on a particular page". It may also be
worth noting that the EPUB file generated from InDesign was fixed-layout
flavor not reflowable.

On Wed, Apr 8, 2015 at 1:34 PM, Daniel Weck [email protected]
wrote:

There's something wrong with this HTML page, irrespective of Readium. On
Windows 8.1, I can click the image slideshow left/right arrows in IE and
Firefox (the images change accordingly), but in Chrome nothing happens at
all. That's when opening the HTML file on its own. Obviously in the Readium
Chrome extension it doesn't work either.


Reply to this email directly or view it on GitHub
#313 (comment)
.

@danielweck
Copy link
Member

The EPUB image slideshow works fine in Readium cloud reader in IE and Firefox, but on Chrome (cloud reader and extension) the clicked arrows do not result in images switching around (nothing visible happens). Same issue / observations with the raw HTML file opened directly.
So I am effectively not even able to reproduce the reported bug (rendering artifacts) :)

@danielweck
Copy link
Member

@whmccoy same problem in Chrome when the HTML is served from HTTP, so the issue is not related to local filesystem access restrictions.

@whmccoy
Copy link
Contributor

whmccoy commented Apr 8, 2015

Reproduced reported bug (rendering artifacts on Mac OS/X 10.9.2 with
Readium for Chrome 2.15.2. but on Chrome OS 43.0 nothing visible happens
(just as Daniel reports with Chrome on Windows).

So the plot thickens (but note that this EPUB was apparently generated from
Adobe ID with default FXL settings so this could indicate an even more
serious issue). Might be worth investigating whether the slide-show widget
was really intended/supported as part of EPUB export (the file does
validate perfectly as EPUB 3.0 though).

--Bill

On Wed, Apr 8, 2015 at 1:58 PM, Daniel Weck [email protected]
wrote:

The EPUB image slideshow works fine in Readium cloud reader in IE and
Firefox, but on Chrome (cloud reader and extension) the clicked arrows do
not result in images switching around (nothing visible happens). Same issue
/ observations with the raw HTML file opened directly.
So I am effectively not even able to reproduce the reported bug (rendering
artifacts) :)


Reply to this email directly or view it on GitHub
#313 (comment)
.

@jccr
Copy link
Contributor

jccr commented Apr 10, 2015

Seems to only happen when the image is resized to something larger than its natural width/height:

My screen recording:
https://www.dropbox.com/s/v1bku9910zvhjo3/rjsv-313.mov?dl=0

Edit:
My environment: Mac OSX Yosemite. Chrome 42. Readium Chrome extension 2.15.2

@jccr
Copy link
Contributor

jccr commented Apr 10, 2015

This commit, when the "scaler" div for fxl was added, causes the issue for me.
readium/readium-shared-js@68d9ac9

It looks like this is a chrome bug when the iframe is scaled via transforms. (Reproduced outside of readium with a simple HTML test case)
Everything appears to be working fine If the iframe is not scaled from the outside, and instead the html element of the iframe's content document gets scaled (what we did before the "scaler" div was introduced)

Edit: I don't know the intent behind this change with the scaler div vs what came before it. The commit message talks about "bug 76" but this sounds like an id for a bug in bugzilla and not a github issue.

@danielweck
Copy link
Member

@jccr would this setting help?

https://github.com/readium/readium-shared-js/blob/develop/js/views/one_page_view.js#L565

reader.needsFixedLayoutScalerWorkAround()

(it's used by some native apps)

@jccr
Copy link
Contributor

jccr commented Apr 10, 2015

Yea that helps. If you set that to true, what happens on your end @danielweck? Do the buttons not work for you still?

@rkwright
Copy link
Contributor Author

Does the workaround that @danielweck proposed actually resolve the bug? If so, does it cause regressions elsewhere?

@danielweck : You say "some" apps use it. But not all? Because of regressions or they simply don't need it? Or both?

@danielweck
Copy link
Member

@rkwright currently only OSX uses this workaround, see:
readium/SDKLauncher-OSX#21 (comment)

Code references:
readium/SDKLauncher-OSX@e909269#diff-14a5cfdb45121ceecfd23a430ab5652cL58

readium/readium-shared-js@ca4e1d0#diff-7f14747656265ce3e39fee34e732d161R78

var reader = new ReadiumSDK.Views.ReaderView(
                 {
                    needsFixedLayoutScalerWorkAround: true,
                     el:"#viewport",
                     etc...
                 });
_needsFixedLayoutScalerWorkAround = options.needsFixedLayoutScalerWorkAround;
this.needsFixedLayoutScalerWorkAround = function() { return _needsFixedLayoutScalerWorkAround; }

@jccr
Copy link
Contributor

jccr commented Apr 13, 2015

@rkwright
Yes the use of the workaround resolves the bug. I'm sure this causes a regression for something, since this scaling technique was added in order to fix a bug. Do you have an archive of bugzilla bugs, is that still up somewhere? Knowing what "bug 76" was about would probably tell us what will be regressed.

@danielweck
Copy link
Member

needsFixedLayoutScalerWorkAround === true was introduced specifically to address a rendering bug in OSX WebView, and this technique currently breaks the cloud reader / Chrome extension "zoom" feature for any value greater than 100% (symptoms: cropped viewport). I am investigating a fix for that. There may also be other possible "regression" bugs, read below.

The "scaler div" was introduced via Bugzilla issue 76 (as Juan pointed out): https://app.devzing.com/Readium/bugzilla/show_bug.cgi?id=76
I'm guessing that the rationale for this change was related to some other rendering bug, but as this predates the utilization of GitHub's issue tracker, I'm not sure exactly why.

needsFixedLayoutScalerWorkAround === true may indeed address the rendering bug in Chrome OSX, but this does not fix the "arrow button clicks do not change slideshow picture" problem in Chrome-Windows. As I said before, this behaviour is unrelated to Readium as I am able to reproduce it by opening the HTML directly in Chrome (works fine in IE though ;) ).

@danielweck
Copy link
Member

Alright, follow-up to my comment above, here is a new issue, and its accompanying fix:
readium/readium-shared-js#177
readium/readium-shared-js@ac77726
(needs testing in OSX WebView)

@danielweck
Copy link
Member

Bugzilla 76 comments (thanks @rkwright !):

readium/readium-shared-js@68d9ac9

It seems that if scaling applied to the iframe it is picked by the rendering engine and applied to the background image. Scaling applied to the root element of the one page view solves the problem but brakes continues scrolling view because to stack pages properly there root element should be resized to the scaled size. To mitigate this introduced extra child div element (id="scaler") wrapped around the iframe and applied scale to it.

More debugging notes:
While setting the size of the iframe to the original height and width works for this case, "the iframe crops content when scale > 1 (tested with books that have a "background" image that fills the entire viewport precisely)." As reported by Daniel.

The problem seems to be that the background image is cropped to the size of the iframe. I tried using background-size attribute on the iframe, html element and body element (the background image is no the body) but could find no permutation that worked. I am wondering if this might have to do with the fact that the viewport size and the html size are different. The viewport size is defined in the head to be the size of the background image. The html height and width is specified in the style sheet for the epub.

if in the debugger I take the size and transform values off of the html and put them on the iframe everything works. Although I'm sure that is NOT the overall fix.

@danielweck
Copy link
Member

Extract from the above Bugzilla comment (and my follow-up below):
"Scaling applied to the root element of the one page view solves the problem but brakes continues scrolling view because to stack pages properly there root element should be resized to the scaled size. To mitigate this introduced extra child div element (id="scaler") wrapped around the iframe and applied scale to it."

I wonder if my trivial fix readium/readium-shared-js@ac77726 for issue readium/readium-shared-js#177 means that we can now apply the scale transform to the root HTML element whilst preserving (i.e. not breaking) the scroll-view iframe@height computations. To be tested.

@rkwright
Copy link
Contributor Author

The two posts above are the gist of the info from bug 76 in bugzilla. I can supply the entire blob if anyone is interested.

danielweck added a commit that referenced this issue May 13, 2015
…clipping when zoom > 100%) for needsFixedLayoutScalerWorkAround. Currently only affects OSX ReadiumSDK launcher, but the plan is to use the needsFixedLayoutScalerWorkAround scaling mode for other launchers too (this would address the rendering glitch in Chrome-Windows and Chrome-OS #313 )
@danielweck
Copy link
Member

Note that I merged the scaling bug fix into the develop branch:
readium/readium-shared-js#177
...but this only affects Readium apps that use needsFixedLayoutScalerWorkAround (currently, only ReadiumSDK OSX launcher).

This issue (#313) is still open, we need to test needsFixedLayoutScalerWorkAround in Chrome (Windows and Chrome-OS in particular) to check that there are no undesirable side effects / regression bugs (fixed-layout single and two-page-spread + document/continuous scroll view). We also need to decide whether to use needsFixedLayoutScalerWorkAround across the board (i.e. all Readium launchers), or to apply only in Chrome (baring in mind, Chrome-OSX does not seem to be affected by the "clipped rendering" bug).

@danielweck
Copy link
Member

According to the original comments in Bugzilla 76:

#313 (comment)

...the key potential problem when using needsFixedLayoutScalerWorkAround (i.e. CSS scale transform on HTML root element instead of parent div element of each iframe) seems to be related to the scroll view: height computations to stack spine items vertically. SDK Launcher OSX currently uses needsFixedLayoutScalerWorkAround and I am not able to reproduce layout bugs in the scroll view (document and continuous), and I tried both pre-paginated / fixed-layout documents as well as reflowable ones.

We could easily enforce the use of needsFixedLayoutScalerWorkAround for all ReadiumJS apps (i.e. Chrome extension and cross-browser cloud reader) by changing this line of code:

// is false by default, but just making this initialisation setting more explicit here.
readerOptions.needsFixedLayoutScalerWorkAround = false;

https://github.com/readium/readium-js/blob/master/epub-modules/Readium.js#L70

(this would not automatically affect iOS, Android, Windows etc. ReadiumSDK app launchers ... these would have to explicitly set needsFixedLayoutScalerWorkAround, as demonstrated here for OSX: #313 (comment) )

danielweck added a commit to readium/readium-js that referenced this issue May 13, 2015
@danielweck
Copy link
Member

Okay, as per my comment above, all ReadiumJS apps (i.e. Chrome extension and cross-browser cloud reader) now use the needsFixedLayoutScalerWorkAround fix:

readium/readium-js@e8e32da

The build from the develop branch is now deploying to DivShot http://readium-cloudreader.divshot.io
(next step: upload an InDesign MSO EPUB so users can test on their OS / browser of choice? CC @rkwright @ryanackley )

I tested the Readium scroll view in Chrome, everything looks fine. Fingers crossed.

@danielweck
Copy link
Member

PS: the previous/next slideshow arrows do not work in Chrome on Windows 8.1 (even when opening the HTML directly with the web browser, outside of Readium). Works fine in Firefox and IE.

Separate issue: #334

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

No branches or pull requests

4 participants