831269 - Print Preview turns blank after toggling large image between Landscape & Portrait with E10S

Status: RESOLVED WORKSFORME

(Core :: Print Preview, defect, P2)

Description

[Paul Silaghi, QA [:pauly]]

STR:
1. Open a large image (http://www.bmwblog.ro/wp-content/uploads/2012/07/BMW-X6-M50d-0311.jpg)
2. Print preview
3. Switch between portrait/landscape couple of times

AR: Blank print preview

Nightly 2013-01-11 Ubuntu 12.04

Comment 1

[Daniel Holbert [:dholbert]]

Confirmed. In my case, the print-previewed page is blank, until I move my cursor off of the [Portrait][Landscape] button.

After my cursor has moved off the button, the image shows up.  Does that match what you see?

Comment 2

[Paul Silaghi, QA [:pauly]]

The image doesn't show up when I move the mouse, but it does when I scroll the page.
Also, fortunately, I noticed that the page is printed successfully even the preview is blank.

Comment 3

[Daniel Holbert [:dholbert]]

I get this regression range:
Last good nightly: 2012-11-30
First bad nightly: 2012-12-01

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=abb39d1df815&tochange=d2fbc67f69f5

At first glance, Bug 816603 looks most related -- might be a regression from that.

Comment 4

[Daniel Holbert [:dholbert]]

Yup -- if I back out bug 816603's changeset locally (i.e. remove the chunk of code that it added), then this becomes WORKSFORME.
 --> Marking as blocking that bug
 --> Also, removing connection to bug 521204, since (per comment 2) no page content ends up
     missing when you actually print. (and the page content missing in print-preview does
     show if you scroll or do other sorts of interactions)

Comment 5

[u279076]

Nominating for tracking since this is a regression.

Comment 6

[Alex Keybl [:akeybl]]

Uncommon user scenario, would accept a low risk uplift if found.

Comment 7

[:Hb]

There is a similar issue in bug 909098.  That error happens only when Hardware Acceleration is ON, i.e. if layers.acceleration.disabled = false.  Someone with a hardware accelerated Linux system might want to test this, please.

Comment 8

[Liviu Cirdei]

User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:48.0) Gecko/20100101 Firefox/48.0
Build ID 	20160320030409

This issue is still reproducible on latest Nightly 48.0a1 with e10s enabled. If I disable e10s the issue is no more reproducible. I have also tested on Windows 7 and I had the same results.

Comment 9

[Milan Sreckovic [:milan] (needinfo for best results)]

Given comment 8, calling this a regression in 48.  Peter, there was a print bug you recently fixed, could you take a look at this one as well?

Comment 10

[Peter Chang[:pchang]]

Sure, I will take a look at this issue.

Comment 11

[Peter Chang[:pchang]]

I can reproduce this with FF50 on Ubuntu. Checking...

Comment 12

[Peter Chang[:pchang]]

I tried to reproduce this on Mac with the following printpreview addon but I wasn't able to see the preview on Mac.

https://addons.mozilla.org/en-US/firefox/addon/printprint-preview/

Comment 13

[Peter Chang[:pchang]]

Looks like the synchronization between content rendering and image decode, I couldn't reproduce this if attached with gdb

Comment 14

[Gerry Chang [:gchang]]

Hi Peter,
Is there any updates here or you can have someone to help?

Comment 15

[Peter Chang[:pchang]]

I will take a look from next week.

Comment 16

[Peter Chang[:pchang]]

If I modified the decoding from async to sync for this big image in [1], I could always see this image.
So I confirmed it was the timing between image decoding and content update.

[1]https://dxr.mozilla.org/mozilla-central/source/image/RasterImage.cpp#1061

Comment 17

[Peter Chang[:pchang]]

I assume we should receive the frame callback for this imageframe(big image) in [1] to invalidate itself. But I didn't see this callback called.

[1]https://dxr.mozilla.org/mozilla-central/source/layout/generic/nsImageFrame.cpp#506

Comment 18

[Peter Chang[:pchang]]

Attachment: Bug 831269 - Enable sync decoding when image is not ready to paint,

Review commit: https://reviewboard.mozilla.org/r/76690/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/76690/

Comment 19

[Peter Chang[:pchang]]

The nsImageFrame of the big image[1] will be re-created everytime when changes between portrait and landscape mode. Therefore, there is no previous image cached and it takes time to decode big images.

I made a patch in attachment 8788101 [details] to force sync decoding to fix above problem.

[1]http://www.bmwblog.ro/wp-content/uploads/2012/07/BMW-X6-M50d-0311.jpg

Comment 20

[Peter Chang[:pchang]]

Markus, could you help to review this?

Comment 21

[Markus Stange [:mstange]]

Sure, but is this the right fix? Shouldn't we rather find out why we don't get the decode notification, fix that, and keep it async?

Comment 22

[Peter Chang[:pchang]]

(In reply to Markus Stange [:mstange] from comment #21)
> Sure, but is this the right fix? Shouldn't we rather find out why we don't
> get the decode notification, fix that, and keep it async?
I understood your point, but the big image didn't get updated when the decode notification was received[1].
I'm afraid the image had to wait the next content update(next compisition cycle).

[1]https://dxr.mozilla.org/mozilla-central/source/layout/generic/nsImageFrame.cpp?q=nsImageFrame.cpp&redirect_type=direct#506

Comment 23

[Markus Stange [:mstange]]

Well, we need to make sure to trigger a paint when the image has been decoded. That means that we'll still see a brief flash while the image is decoding. I think that's fine.
The next step would be to not throw away the decoded image, but somehow hand it off to the new image frame. I don't know how this should be done.

Comment 24

[Peter Chang[:pchang]]

I just checked the latest nightly and it was fixed. Not sure which bug fixes this.

Comment 25

[Jonathan Watt [:jwatt]]

Unfortunately the link from comment 0 in 404. Let's assume comment 24 is correct and this was working the last time this was checked.