mozilla :: #gfx

10 Aug 2017
14:17katsmilan: i think bug 1371101 should probably not be a blocker for shipping wr, it's something we don't do in gecko now and would be a nice win down the road but it's unclear how much work it is
14:17firebothttps://bugzil.la/1371101 NEW, nobody@mozilla.org Run color, border-color, and background-color animations off the main thread
14:18milankats: got it
14:36Gankrokats what's the plan for running tests in layers-free mode?
14:37katsGankro: at some point hopefully in the near future we'll turn layers-free on by default so the linux64-qr test suites will be testing layers-free
14:37katsGankro: right now doing so results in crashes but ethan traced that down to a canvas-related crash which he's fixing
14:37katswe'll have to see what the results are like once that's fixed
14:38Gankrokats: cool, thanks!
14:47GankroSo do I need to force cbindgen to use nightly somehow?
14:48katsGankro: yeah
14:48GankroJust override set nightly in the root dir?
14:49katsrustup run nightly <cbindgen command line>
14:49Gankrooh neat
14:49katsi sent an email about that on friday?
14:49Gankrokats: yes, it was faster to just ask in here :)
14:49katsfair
15:24jrmuizelkats: is it easy for you to make a short list of some of the webrender changes that have broken gecko?
15:25katsjrmuizel: about this easy: https://pastebin.mozilla.org/9029395
15:26jrmuizelkats: those look like bindings changes, how about correctness problems?
15:27katsthat&#39;s a little harder. i don&#39;t think i&#39;ve handled those as consistently
15:27katsi can look through the last few WR update bugs
15:31katsjrmuizel: in the last update (the one i just landed) there were two: one was fixed by https://github.com/servo/webrender/pull/1557 even before i ran into it, and the other was fixed by https://github.com/servo/webrender/pull/1559
15:32katsjrmuizel: i&#39;m not sure how you want to count changes in WR that exposed badness in our bindings
15:32katswe had one of those too
15:32katstechnically not a WR bug but it would have been caught by testing on gecko
15:33jrmuizelkats: ok that gives me some ideas
15:36katsjrmuizel: the update in bug 1374730 links to a bunch of WR issues, i think that one ran into a few upstream bugs
15:36firebothttps://bugzil.la/1374730 FIXED, bugmail@mozilla.staktrace.com Update webrender to 519e51986308fc11d6ba6771f1c11ea6a3133921
15:59mstangekats++ for the process update email title
15:59kats:)
16:15katsmchang: does https://bugzilla.mozilla.org/show_bug.cgi?id=1343727 need landing, or is it obsolete?
16:15firebotBug 1343727 NEW, mchang@mozilla.com Properly clip outset box shadows with border-radius
16:15katshasn&#39;t been touched in many months
16:49mstangekats: it&#39;s funny how both jeff and I missed that missing return
16:49katsmstange: yeah. and i missed the reams of warnings that the compiler was spewing out too
16:49mstangeI was about to ask for a PodZero instead of the memset but then dropped it, and I still didn&#39;t see it
16:49katsheh
16:50katssince i&#39;m gonna reland, want me to change that?
16:50mstangeup to you
16:51katsi might as well. the docs on PodOperations.h say it&#39;s preferred to raw memset
16:51mstangeok
17:15mchangkats: probably obselete
17:15kats-lunchmchang: the bug or the patch?
17:15mchangkats-lunch: probably both, im not sure the status of box shadows on WR atm
17:16kats-lunchhm ok
17:19Basmstange: Can you reply to Kats on &#39;Part 1&#39; of bug 1363922 as well?
17:19firebothttps://bugzil.la/1363922 NEW, nobody@mozilla.org spend too much time snapping scroll areas to layer pixels
17:20mstangeBas: oh, yes, I will
17:20BasThanks! :)
17:20mstangeBas: there&#39;s no more review request to me on the first part, so I thought everything was taken care of
17:20mstangebut I didn&#39;t actually read what was on the bug
17:22Basmstange: Yeah, sorry, reviewboard and all.
17:24mstangeBas: which question in particular should I reply to?
17:25Basmstange: I think he was mainly interested in knowing if there was a better way to top-down pass the reference frame, but I&#39;m under the impression that&#39;s what the DLbuilder already does.
17:25mstangeah
17:25mstangeyeah let me try to understand this again
17:25Basmstange: I mainly think a quick review of the patch (it isn&#39;t complicated), is probably the most useful thing you can do.
17:25mstangeok
17:27mstangeBas: can you explain to me how this patch helps?
17:27mstangedoes it hit the aFrame == mCurrentFrame condition in nsDisplayListBuilder::FindReferenceFrameFor?
17:27Basmstange: It does. Always, afaict.
17:28mstangethat makes me think of an alternative proposal
17:28mstangeI&#39;ll write it in a review comment
17:31Basmstange: Let&#39;s make sure it&#39;s not too much bigger a change, I want to get this into 57 so I want low risk patches.
17:31mstangeBas: it&#39;s going to be low risk
17:32BasPerfect.
17:34mstangeBas: there you go, what do you think?
17:34mstangeignore the spurious diff at the end; reviewboard was doing stuped things
17:34mstange*stupid
17:37Basmstange: I was thinking storing DisplayListBuilder might help with optimizing some other things in the future as it can essentially be used as a per-paint cache, and it would guarantee things worked correct if mOuter -wasn&#39;t- the current frame in case there&#39;s edge cases where that&#39;s not true. Those were my reasons :)
17:38mstangeI see
17:38Bas(But you know this code a lot better than I do, so my assumptions may very well have been wrong)
17:38mstangeI hope we won&#39;t need more per-paint caches for methods that are called without a display list builder
17:40Basmstange: Perhaps, it looks like I&#39;ve mostly got the stuff from ScrollFrameHelper under control with this patch, it&#39;s now only 3% of displaylistbuilding.
17:40mstangethat&#39;s great
17:43Basmstange: I&#39;ll implement your suggestion though if you think that&#39;s best :-)
17:43mstangeBas: yes, I believe it is
17:44mstangeBas: mOuter will always be the current frame on the display list builder
17:44BasPerfect!
17:44mstangeBas: ScrollFrameHelper::BuildDisplayList is called from two places, HTMLScrollFrame and XULScrollFrame, and both initialize mHelper with &quot;this&quot; as mOuter
18:00mstangekats: thanks for pointing me to that bug, I wasn&#39;t aware of it
18:00katsmstange: yeah ryan just filed it the other day
19:16rhuntkats: how frequently are the bustages from wr updates caused by reftest failures without an api change?
19:17katsrhunt: i don&#39;t have actual data but it&#39;s much less frequent than API changes
19:18katsrhunt: in the last 5 WR updates for example i would guess that maybe 15% of bustages were reftest failures without an API change
19:18katsrhunt: total guess though
19:19rhuntkats: okay, I was curious if it&#39;d be worth it to expand the wrench reftest coverage by modifying the reftest harness to take &quot;display list snapshots&quot; instead of actual snapshots, and then exporting them to yaml. we could update that test suite periodically as how gecko creates display lists changes
19:20rhuntbut that looks like it&#39;d be a bit of work, so it&#39;d be good to know if it&#39;d be worth it
19:21rhuntI suppose it&#39;d also be useful for debugging reftest failures
19:21katsrhunt: i think probably not worth it right now. if the api changes were taken care of i think that would help a lot more
19:21rhuntkats: yeah, I&#39;d agree
20:46Basmstange: Not saying we should do this now, but couldn&#39;t we have a cheap way of getting the scale for the current layer just by tracking it through the display list builder?
20:48mattwoodrowYes
20:49BasOkay, just making sure I wasn&#39;t misreading anything. Obviously this doesn&#39;t seem to be important.
21:02gwmilan: kats: do you have admin rights for the dev-tech-gfx mailing list?
22:16Aryxhi, these crash test failures on autoland/assertions, fallout from https://hg.mozilla.org/integration/autoland/rev/6a174665b5bc34d6d2d40917b3dac5f1d0bfa237 ? e.g. https://treeherder.mozilla.org/logviewer.html#?job_id=122391008&repo=autoland
11 Aug 2017
No messages
   
Last message: 12 days and 23 hours ago