Closed
Bug 1098958
Opened 10 years ago
Closed 10 years ago
Images cut off
Categories
(Core :: Graphics: ImageLib, defect)
Tracking
()
RESOLVED
FIXED
mozilla37
People
(Reporter: alice0775, Assigned: seth)
References
()
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
2.25 KB,
patch
|
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
[Tracking Requested - why for this release]: regressed in Aurora35.0a2 and Nightly36.0a1
Steps To reproduce:
1. Clear cache
2. Put mouse pointer to the bottom of the browser screen
3. Open http://ie.microsoft.com/testdrive/Performance/FlyingImages/Default.html
--- observe images
4. Change Image size (Default IE Logo, Large IE Logo,Mix Photo, My Browser, All Browsers)
--- observe images
Actual Results:
Images cut off at bottom
Expected Results:
Image should draw all its height
Regression window(m-c)
Good:
https://hg.mozilla.org/mozilla-central/rev/43ab820c7b68
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0 ID:20140926183438
Bad:
https://hg.mozilla.org/mozilla-central/rev/818f353b7aa6
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0 ID:20140926185339
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=43ab820c7b68&tochange=818f353b7aa6
Regressed by:
818f353b7aa6 Seth Fowler — Bug 1069369 - Remove all manual discarding during frame lookup. r=tn a=kwierso
Note:
there is other Bug 1098895 in Aurora35.0a2.
![]() |
Reporter | |
Updated•10 years ago
|
Flags: needinfo?(seth)
![]() |
Reporter | |
Comment 2•10 years ago
|
||
[Tracking Requested - why for this release]:
status-firefox37:
--- → affected
tracking-firefox37:
--- → ?
Assignee | ||
Comment 3•10 years ago
|
||
Since debugging showed that the images involved here are getting decoded, notified about, etc. successfully, my next theory was that the layers::ImageContainer was falling out of sync. This proved to be correct, and the fix is extremely straightforward.
I do feel like we're starting to get too much logic about invalidations in FinishedSomeDecoding; a future refactoring should try to clean this stuff up a little. At least it's the *only* place in RasterImage we have logic about invalidations due to decoding now.
Attachment #8534554 -
Flags: review?(tnikkel)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → seth
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•10 years ago
|
||
Before that patch, I could reproduce the problem 75% of the time on a shift-reload. After the patch, I cannot reproduce it at all. That said, this bug is timing dependent, so I think we should ask QA for a verification of the fix.
This also needs an uplift to Aurora. (And a try job, but try is closed right now.)
Updated•10 years ago
|
Attachment #8534554 -
Flags: review?(tnikkel) → review+
Assignee | ||
Comment 5•10 years ago
|
||
Thanks for the review! I made a minor tweak; now we only call UpdateImageContainer if the default decode flags are used. Doing it otherwise is pointless, since we use the default decode flags version of the image in UpdateImageContainer.
Assignee | ||
Updated•10 years ago
|
Attachment #8534554 -
Attachment is obsolete: true
Assignee | ||
Comment 6•10 years ago
|
||
Here's a try job:
https://tbpl.mozilla.org/?tree=Try&rev=bdbe1d8d2310
Assignee | ||
Comment 7•10 years ago
|
||
Try looks good. Pushed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a5fcba591258
Flags: needinfo?(seth)
Updated•10 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Assignee | ||
Comment 9•10 years ago
|
||
Comment on attachment 8534720 [details] [diff] [review]
Call UpdateImageContainer whenever we send invalidations in RasterImage
This needs an uplift to Aurora. I am very sorry to say that because the dependencies here are *massive*, we cannot uplift to 35, even though it's affected. =( I'll look into the possibility of making a different version of the patch that could land on 35.
Approval Request Comment
[Feature/regressing bug #]: 1069369
[User impact if declined]: Sometimes part of an image will be cut off.
[Describe test coverage new/current, TBPL]: On m-c. Current tests should cover this; it's not clear to me right now why we're not getting intermittent oranges from this issue. (OTOH, there are several unexplained intermittent orange bugs in ImageLib right now, so maybe we are...)
[Risks and why]: Low risk. We just update the version of the image displayed onscreen more often.
[String/UUID change made/needed]: None.
Attachment #8534720 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
Attachment #8534720 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 10•10 years ago
|
||
Comment 11•10 years ago
|
||
wontfix for 35 based on comment 9
You need to log in
before you can comment on or make changes to this bug.
Description
•