Closed
Bug 1132371
Opened 9 years ago
Closed 9 years ago
/layout/reftests/bugs/1119117-1b.html fails on Windows/mac when event-regions are enabled
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
FIXED
mozilla38
People
(Reporter: tnikkel, Assigned: tnikkel)
References
Details
Attachments
(2 files, 1 obsolete file)
9.92 KB,
patch
|
mattwoodrow
:
review+
bajaj
:
approval-mozilla-b2g37+
|
Details | Diff | Splinter Review |
7.46 KB,
patch
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•9 years ago
|
||
Without event regions this test had a background color item inside an opacity item, and the opacity item flattened away and passed it's opacity to the background color item and the background color item got a color layer. With event regions we get an event region items inside the opacity and the optimization no longer workers. And we get an-inactive opacity layer and painted layer. We get a 128 vs 127 difference in the result so I'm assuming that we round 127.5 ( = 255 * the opacity in the test of 0.5) differently depending of if it gets a color layer. I extended the optimization to three child items because I thought I needed it for this testcase. But afterwards I checked and we only need two children for this particular case.
Attachment #8563277 -
Flags: review?(matt.woodrow)
Comment 2•9 years ago
|
||
Comment on attachment 8563277 [details] [diff] [review] patch Review of attachment 8563277 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/base/nsDisplayList.cpp @@ +3865,5 @@ > + } > + } > + > + for (uint32_t i = 0; i < numChildren; i++) { > + if (children[i].item) { How could this ever be false? ::: layout/base/nsDisplayList.h @@ +1389,5 @@ > */ > virtual bool ApplyOpacity(nsDisplayListBuilder* aBuilder, > float aOpacity, > const DisplayItemClip* aClip) { > return false; How about drop the bool retval and just assert that we don't run this function?
Attachment #8563277 -
Flags: review?(matt.woodrow) → review-
Assignee | ||
Comment 3•9 years ago
|
||
Good ideas, thank you.
Attachment #8563277 -
Attachment is obsolete: true
Attachment #8566222 -
Flags: review?(matt.woodrow)
Assignee | ||
Comment 4•9 years ago
|
||
Updated•9 years ago
|
Attachment #8566222 -
Flags: review?(matt.woodrow) → review+
Assignee | ||
Comment 5•9 years ago
|
||
So this patch actually causes the reftest for bug 1119117 to fail on b2g. I checked and the reftest fails with event regions turned off. The reftest landed after event regions were turned on so that explains that. I guess the rounding issue goes the other way on b2g from mac/windows. Changing the opacity from 0.5 to 0.50196078431 fixes the rounding issue, so I'm just going to change the test.
Assignee | ||
Comment 6•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/5ef8e549a1d9
Comment 7•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/5ef8e549a1d9
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox38:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Comment 8•9 years ago
|
||
tn, can you request uplift to b2g37 for this bug? It is needed in order to fix bug 1139541 on 2.2. Thanks!
Updated•9 years ago
|
Flags: needinfo?(tnikkel)
Assignee | ||
Comment 9•9 years ago
|
||
Comment on attachment 8566222 [details] [diff] [review] patch v2 NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): need this to fix bug 1139541 User impact if declined: bug 1139541 Testing completed: has been landed for quite a while now, no issues Risk to taking this patch (and alternatives if risky): should be pretty safe, just extends an optimization to more cases String or UUID changes made by this patch: none
Flags: needinfo?(tnikkel)
Attachment #8566222 -
Flags: approval-mozilla-b2g37?
Updated•9 years ago
|
Attachment #8566222 -
Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
You need to log in
before you can comment on or make changes to this bug.
Description
•