1323320 - Intermittent image/image-srcset-basic-selection-width-10x-ref.html | assertion count 2 is more than expected 0 assertions from ASSERTION: Displayport must be a valid texture size: 'result.height <= GetMaxDisplayPortSize(aContent)'

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Treeherder Bug Filer]

Filed by: philringnalda [at] gmail.com

https://treeherder.mozilla.org/logviewer.html#?job_id=7856961&repo=autoland

https://queue.taskcluster.net/v1/task/dsFTVqWASXeRmYLQ7ALY5w/runs/0/artifacts/public/logs/live_backing.log

https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://queue.taskcluster.net/v1/task/dsFTVqWASXeRmYLQ7ALY5w/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1

Comment 1

[Intermittent Failures Robot]

9 failures in 892 pushes (0.01 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* autoland: 4
* mozilla-central: 3
* try: 1
* mozilla-inbound: 1

Platform breakdown:
* linux64-qr: 9

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1323320&startday=2017-06-19&endday=2017-06-25&tree=all

Comment 2

[Intermittent Failures Robot]

12 failures in 718 pushes (0.017 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* mozilla-central: 4
* mozilla-inbound: 3
* autoland: 3
* try: 2

Platform breakdown:
* linux64-qr: 12

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1323320&startday=2017-06-26&endday=2017-07-02&tree=all

Comment 3

[Intermittent Failures Robot]

14 failures in 656 pushes (0.021 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* mozilla-inbound: 5
* autoland: 4
* mozilla-central: 3
* try: 2

Platform breakdown:
* linux64-qr: 14

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1323320&startday=2017-07-03&endday=2017-07-09&tree=all

Comment 4

[Intermittent Failures Robot]

26 failures in 720 pushes (0.036 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* autoland: 8
* mozilla-inbound: 7
* try: 6
* mozilla-central: 5

Platform breakdown:
* linux64-qr: 26

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1323320&startday=2017-07-10&endday=2017-07-16&tree=all

Comment 5

[Intermittent Failures Robot]

27 failures in 822 pushes (0.033 failures/push) were associated with this bug in the last 7 days.   

Repository breakdown:
* autoland: 11
* try: 10
* mozilla-inbound: 4
* pine: 1
* mozilla-central: 1

Platform breakdown:
* linux64-qr: 27

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1323320&startday=2017-07-17&endday=2017-07-23&tree=all

Comment 6

[Intermittent Failures Robot]

43 failures in 1008 pushes (0.043 failures/push) were associated with this bug in the last 7 days.   

** This failure happened more than 30 times this week! Resolving this bug is a high priority. **

** Try to resolve this bug as soon as possible. If unresolved for 2 weeks, the affected test(s) may be disabled. ** 

Repository breakdown:
* mozilla-inbound: 14
* try: 13
* autoland: 12
* mozilla-central: 4

Platform breakdown:
* linux64-qr: 43

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1323320&startday=2017-07-24&endday=2017-07-30&tree=all

Comment 7

[Kartikaya Gupta (email:[e-mail])]

I'll investigate this.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

So the test in question is setting a reftest-zoom level of 10x, which drops the auPerDevPixel value to 6. And sometimes we run into the scenario where the displayport base rect is set to 48000 x 60000 AUs which ends up being 8000 x 10000 dev pixels which exceeds the max texture size of 8192. Note that the inflation/alignment code is working fine, but the code doesn't account for the possibility of a base rect that already exceeds the max texture size. I suspect this only happens in this case because of a race condition where the zoom level is changed and we immediately recompute the displayport, while it takes a round-trip to the APZ thread before the base rect is updated to account for the new zoom level.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Timothy, does comment 8 make sense? If so, does it seem reasonable to recompute the displayport base rect somewhere inside nsPresContext::AppUnitsPerDevPixelChanged()? I guess we'd only want to do it for the root scrollframe of the presShell.

Also FWIW I verified in [1] that the displayport base rect is *not* getting set at [2] for this bug.

[1] https://treeherder.mozilla.org/logviewer.html#?job_id=120791855&repo=try&lineNumber=23791
[2] http://searchfox.org/mozilla-central/rev/f0e4ae5f8c40ba742214e89aba3f554da0b89a33/layout/base/PresShell.cpp#5973

Comment 10

[Timothy Nikkel (:tnikkel)]

Comment 8 sounds reasonable. I think that this image visibility code is the only place (on the layout side) where we could use an outdated base rect that hasn't been adjusted for a zoom level change. Any other displayport getter should be setting the base rect beforehand.

So we could just ignore the displayport in MarkFramesInSubtreeApproximatelyVisible if it's absurdly large (like we already do if there is no baserect set). Or just ignore the assertion and use the too-big displayport if we are getting the displayport via GetDisplayPortForVisibilityTesting. Changing the zoom by a factor of 10 is only a thing we do in tests, users will be changing the zoom in small increments: even if they do multiple small increments there is likely a delay because each one requires a key press.

Comment 11

[Intermittent Failures Robot]

38 failures in 888 pushes (0.043 failures/push) were associated with this bug in the last 7 days. 

This is the #48 most frequent failure this week.  

** This failure happened more than 30 times this week! Resolving this bug is a high priority. **

** Try to resolve this bug as soon as possible. If unresolved for 2 weeks, the affected test(s) may be disabled. ** 

Repository breakdown:
* autoland: 14
* mozilla-inbound: 10
* try: 9
* mozilla-central: 5

Platform breakdown:
* linux64-qr: 38

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1323320&startday=2017-07-31&endday=2017-08-06&tree=all

Comment 12

[Kartikaya Gupta (email:[e-mail])]

(In reply to Timothy Nikkel (:tnikkel) from comment #10)
> Or just ignore the assertion and
> use the too-big displayport if we are getting the displayport via
> GetDisplayPortForVisibilityTesting.

I chose to go with this approach, as it seemed straightforward.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dfc070f5227e225a359b398b58b3aab3c38ccda2

Comment 13

[Kartikaya Gupta (email:[e-mail])]

Sadly there are still failures. I guess I only investigated/patched one of the multiple codepaths that triggers this assertion. I didn't look closely to see if the remaining assertions are similar or what, will do that tomorrow.

Comment 14

[Intermittent Failures Robot]

29 failures in 166 pushes (0.175 failures/push) were associated with this bug yesterday.   

Repository breakdown:
* autoland: 16
* mozilla-central: 6
* mozilla-inbound: 5
* try: 2

Platform breakdown:
* linux64-stylo: 14
* linux64-qr: 6
* linux32: 5
* linux64-ccov: 2
* linux64: 2

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1323320&startday=2017-08-11&endday=2017-08-11&tree=all

Comment 15

[Intermittent Failures Robot]

104 failures in 901 pushes (0.115 failures/push) were associated with this bug in the last 7 days. 

This is the #7 most frequent failure this week. 

** This failure happened more than 75 times this week! Resolving this bug is a very high priority. **

** Try to resolve this bug as soon as possible. If unresolved for 1 week, the affected test(s) may be disabled. **  

Repository breakdown:
* autoland: 63
* try: 16
* mozilla-central: 14
* mozilla-inbound: 11

Platform breakdown:
* linux64-stylo: 42
* linux64-qr: 41
* linux32: 14
* linux64: 5
* linux64-ccov: 2

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1323320&startday=2017-08-07&endday=2017-08-13&tree=all

Comment 16

[Kartikaya Gupta (email:[e-mail])]

New try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6ffba14478f2d2fe1ca25ab1e1ff39a51c063a20

Comment 17

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1323320 - Skip displayport size assertions when getting the displayport for visibility testing.

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

Comment 18

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8897075 [details]
Bug 1323320 - Skip displayport size assertions when getting the displayport for visibility testing.

https://reviewboard.mozilla.org/r/168362/#review173676

Looks great, thanks!

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4b52f8da62f7
Skip displayport size assertions when getting the displayport for visibility testing. r=tnikkel

Comment 20

[Intermittent Failures Robot]

33 failures in 149 pushes (0.221 failures/push) were associated with this bug yesterday.   

Repository breakdown:
* autoland: 24
* try: 3
* mozilla-inbound: 3
* mozilla-central: 3

Platform breakdown:
* linux64-stylo: 21
* linux32: 6
* linux64-qr: 5
* linux64: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1323320&startday=2017-08-14&endday=2017-08-14&tree=all

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4b52f8da62f7

Comment 22

[Intermittent Failures Robot]

44 failures in 949 pushes (0.046 failures/push) were associated with this bug in the last 7 days. 

This is the #34 most frequent failure this week.  

** This failure happened more than 30 times this week! Resolving this bug is a high priority. **

** Try to resolve this bug as soon as possible. If unresolved for 2 weeks, the affected test(s) may be disabled. ** 

Repository breakdown:
* autoland: 24
* try: 10
* mozilla-inbound: 5
* mozilla-central: 5

Platform breakdown:
* linux64-stylo: 26
* linux64-qr: 8
* linux32: 7
* linux64: 2
* linux64-ccov: 1

For more details, see:
https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=1323320&startday=2017-08-14&endday=2017-08-20&tree=all

Comment 23

[Kartikaya Gupta (email:[e-mail])]

The patch that landed seems to have fixed this, according to the OF graph.