mozilla :: #gfx

12 Sep 2017
01:59pulsebotCheck-in: - Ethan Lin - Bug 1398666 - Reset nsDisplayTransForm's bound flag when updating bounds. r=mattwoodrow
09:28padenotjnicol, ping
09:28padenotjnicol, paint flashing is not broken on osx, but the devtools checkbox that enables it is broken
09:29jnicolpadenot: cool
09:29jnicolpadenot: so can you see what is being repainted?
09:29padenotjnicol, getting you a screenshot right now, it's repainting the world as fast as it can (i.e. about once a second on this recent macbook pro)
09:29padenotnot screenshot, screencast
09:29mstangeethlin_: why does the border width need to be scaled but not, for example, the border radius?
09:30padenotjnicol, they must be using a ton of SVG because they're modern and trendy people
09:32ethlin_mstange: oh...right, border radius should be scaled too. I forget it.
09:33mstangeah ok
09:33ethlin_mstange: it's so annoying. a little bit easy to forget.
09:33mstangeethlin_: other things that come to mind are text decoration thickness, box shadow metrics, and once the blur filter is supported, the blur radius there
09:33mstangemstange: I agree, this solution seems to be a bit brittle
09:33mstangeethlin_: ^
09:33ethlin_mstange: originally i think we set the scale to the draw target.
09:34mstangeethlin_: why doesn't the scaling happen on the webrender side? is it because we need to snap pixels on the gecko side?
09:34mstangeit seems like giving webrender a scale transform somehow would be easier
09:37ethlin_mstange: before bug 1391499, we set the scale to the webrender's stacking context
09:37firebot FIXED, Apply correct scale transform with nested transform divs
09:39mstangeethlin_: I see
09:40ethlin_mstange: i think it's because we want to reorder the transform. I'm not sure if there is a better way.
09:40mstangeethlin_: kats's comment sounds like this deserves some discussion with webrender people
09:41ethlin_mstange: maybe we should move the all reorder trasnform things to webrender.
09:43ethlin_mstange: You are right. I should also apply the scale to box shadow and text if we want to do this in gecko. Let me cancel the review request first.
10:05padenotthis video has been scaled by 0.5, it's too big otherwise
13:26padenotjnicol, JAMIE
13:26padenotjnicol, I found the cause
13:27padenotjnicol, I had my whatsapp page zoomed to 110%
13:27padenotif I set it back to 100% it's fast as expected
13:27jnicolpadenot: awesome!
13:28padenotjnicol, yeah paint flashing confirms this
13:29padenotjnicol, crazy though
13:29jnicolpadenot: I was about to send a sad face because 110% didn't reproduce for me
13:29jnicolpadenot: but 133% did!
13:30padenotin other news,
13:30firebotBug 1399108 NEW, Paint flashing activation using the dev tools is broken in Windows and OSX
13:30jnicolpadenot: okay cool, I can hopefully figure this out now I can reproduce. thanks!
13:30padenotmad props to whoever put the little bubble that indicates the zoom level in the address bar
13:37padenotjnicol, I can repro on all platforms, of course the zoom level is different each time, I have different monitor with different density/resolution on each machine
13:37jnicolmakes sense
13:37padenotyeah there was nothing platform specific in the profile
13:37padenotI was really confused
13:38jnicoljust the zoom was saved in the profile? ha
13:38padenotnot that I know a lot about gfx
13:38padenotjnicol, yeah, it's persisted accross refresh and all
13:38padenotjnicol, I tried trashing all caches, service worker, everything
13:51mconleymstange: ping
13:51mstangemconley: pong
13:52mconleymstange: heya! I'm trying to make some sense of (from bug 1397092). There are some _crazy_ long composites in this profile. In the hundreds of milliseconds sometimes. Sometimes over a second. And a bunch of missing samples. Is this likely a case of thread starvation?
13:52firebot ASSIGNED, High Cpu/GPU usage due to "Ping Pong" loading indicators in Firefox 57 affecting browser performance
13:53mstangemconley: it sure looks like thread starvation to me
13:53mstangeor maybe memory bandwidth depletion
13:54mstangemconley: I've followed that bug but don't have any ideas
13:54mstangeit seems like composites on that machine / windows installation are just wayyy to expensive in general
13:54* mconley nods
13:55mstangemaybe the screen(s?) is gigantic? maybe there are thousands of windows stacked on top of each other so that the window server has to do a ton of work?
13:56mstangemaybe the desktop background is an expensive 3d animation? I'm running out of ideas here
13:57mconleymstange: Hm, okay. Nothing jump out when reading the about:support in ?
13:59mstangeit seems to be a discrete GPU... so if memory bandwidth is the issue, I'm even more confused
13:59mstangemconley: I forget; did the reporter try with layers acceleration disabled?
14:00mstangemconley: disabling advanced layers might also be an interesting data point
14:00mconleymstange: that's disabling "Use HWA when available" in about:prefs, right?
14:00mstangethe former is that checkbox, yes; the latter is layers.mlgpu.enabled
14:03vladmconley: why are there 1000ms RefreshDriverTicks? and a 20s GCMajor?
14:15jimmmstange: what is 'advanced layers'?
14:16mstangejimm: also called "Mid-level GPU" - a different Windows compositor backend that does more batching
14:16firebotBug 1385051 FIXED, Let Advanced Layers ride the trains
14:18jimmmstange: thx
14:19mconleyvlad: I ... I don't know. My best guess right now is thread starvation; we're not doing a good job of prioritizing threads that are doing the important stuff
14:19mconleybut that's just theory - still trying to make sense of the profile
14:21mconleyvlad: my only evidence to support that is all of the missing samples during the really slow bits
14:22vladyeah, could just be "computer is really busy"? might be interesting to periodically query overall system CPU usage and report that with profiles every second or so
14:22vlad(don't mind me, I'm just over here trying to see if I can borrow the profiler code for something else ;)
14:22mstangeoh, interesting
14:23mstangethe code has become rather readable recently, it should be easy to extract what you need
14:24mstangevlad: and if you could donate a kernel driver, I'd be eternally grateful :)
14:24firebotBug 1379286 NEW, Use a kernel driver for profiling on Windows
14:24vladmstange: hm, they exist.. I think CodeXL is open source now?
14:25mstangethe kernel driver is not, but the people behind it have offered to put in the hooks we need
14:25mstangejrmuizel has talked to them
14:25vladahh, lame. I mean good, but lame.
15:25Gankrohey kats, so what's the current situation with layers-free reftests? Seems like you've got it working, and `qr-` will touch them? But it's opt-in?
15:25katsGankro: so at the moment the linux64-qr reftest jobs will run the reftests in layers-full mode by default. however if you set the pref(gfx.webrender.layers-free,true) annotation on a particular reftest, it will run in layers-free mode
15:25katsso in effect, yes, it is opt in for now
15:26Gankrokats: ok and you're looking to soon make it so there's only layers-free, and it's implied by webrender?
15:27katsGankro: yeah. there's still a lot of test failures in that configuration though so i'm not sure if we want to try and iron out some more of those before we switch over
15:28Gankrokats: you're in charge of the webrender try builds, so I'll leave that judgement to you :)
15:28katsi'd like to do it sooner rather than later. maybe by the end of this week
15:29Gankrohey nical, I hear you're the person to ask about how the webrender api handles the lifetime of resources and the display lists that reference them?
15:30GankroTrying to figure out precisely when it's safe for a client to delete an image used by a display list
15:35nicalGankro yep
15:35Gankronical: is set_display_list the safety barrier? If call that, and then delete all images, that's fine?
15:37nicalset_display_list contains a ResourceUpdates which contains things like add/remove/update images/fonts. You need to make sure that if you delete an image in this resource updates list, the display list does not refer to it
15:39nicalif you call set_display_list with a display list that does not refer to a particular image you are indeed free to call DeleteImage (that's how I am interpreting your last sentence)
15:40Gankronical: oh are all resource updates tied to display_lists?
15:42nicalyou can send resource updates with the display list or standalone. We have to be able to associate some resource updates to the display list because of that annoying thing we call frame consistency, where some image updates have to be applied in the same frame as some layout changes
15:42nicalbut in the case of deleting images it shouldn't matter
15:44nicalso to update resources you can either do it via api.set_display_list when you need some changes to be tied to the display list or api.update_resources if you don't have that requirement
15:45nicalit would be best to not create and delete an image in the same ResourceUpdates list (right now it works but I'd like to do things that would break it)
15:47Gankronical: what sort of deletions might we do without a display list?
15:49nicalyou can delete images and fonts without a display list. all you need is to be sure that the current display list (the last one that was set) does not refer to images or fonts you want to delete
15:49Gankronical: ok so you *can't* do set_display_list(); update_resources(delete_image); ?
15:50nicalyou can't if that list you just set refers to the image
15:51nicalbut you can do set_displaylist(some_list_that_refers_to_the_image); set_displaylist(some_list_that_do_not_refer_to_the_image); update_resources(delete_the_image);
15:51Gankrointeresting, ok
15:53Gankronical: if I accidentally weave the update_resources between those two set_display_lists, is it deterministically going to fail or is it a race condition?
15:56nicalTypically, in gecko most of the resource updates happen either during a transaction when (a) painting/building display items, in which case you should have access to the resource update queue of the transaction and just push updates into that and it will go with the display list, or (b) when deleting things like layers, image containers or whatnot,
15:56nical that hold on to image keys, in which case if you can probably assume that we are not going to render whatever is referring to the image anymore just send the deletion in an update_resources messages, or in doubt you can queue up some resource updates for the next transaction
15:56nicalif you accidentally weave the update_resources, it may crash
15:57nicalbecause you can get some APZ scrolling that triggers the generation of a frame using the current display list that still refers to the image
15:58nicalbut it may not fail if you are lucky and nothing caused webrender to generate a new frame until you send the next display list
15:59nicalGankro: do you have a specific scenario in mind?
16:01nicalI am in the middle of some (fairly heavy) refactoring that may or may not be relevant in bug 1393031
16:01firebot ASSIGNED, Batch resource updates in transactions.
16:04Gankronical: this is why we're crashing right now:
16:07nicalGankro: ok my gut feeling is that instead of callding DiscardImage just before SetDisplayList in WebRenderLayerManager::EndTransaction, we should push the image keys to delete in the resource updates of the transaction
16:09nicalright now what happens is the scenario you mentioned earlier, we send the dele image before applying the new display list and that can crash if anything triggers a new frame to be rendered between the two
16:11mstange...such as the tab hover animation
16:12Gankronical: so summarizing, concat what we'd pass to up_resources to the set_dl input?
16:13nicalGankro: yeah that would be great
16:13nicalmstange: yes definitely
16:33Gankronical: oh boy this code is more complicated and ipdl-y than I expected. Got any pointers for where I should be trying to weave in the DiscardImage values?
16:35nicalGankro: I can sneak a fix in the middle of the work I am currently doing if you don't mind me stealing the bug from you (that would be convenient for me because I have already touched that code a lot and rebasing is no fun)
16:36Gankronical: oh man hell yeah
16:36nicalin particular this code is changing substancially in a patch that is waiting for a review
16:36Gankronical: landing Soon(tm) ?
16:36nicaldepends on jrmuizel (wink wink)
16:37* Gankro starts kicking in doors
16:45Gankronical: assigned, good luck!
17:22nicaljrmuizel: let me know if/when we havse stuff to post in the webrender newsletter
17:34Gankromilan: I think I'm being tracked as "external" in the WebRender tracking sheet?
18:01gw280jrmuizel: apple designed the GPU for the new iphone
18:01gw280RIP imagination
18:01jrmuizelgw280: citation needed?
18:01gw280jrmuizel: they just said it in the keynote
18:04jrmuizelgw280: what's the best place to see the coverage these days?
18:04gw280jrmuizel: I'm watching it on my apple tv
18:04milanGankro: possible. May be OK, depending on how much Rust work you have, but we can adjust that.
18:04gw280jrmuizel: but apple is streaming it on I assume
18:14katshow do we draw svg with webrender now?
18:14kats(in layers-free)
18:15mstangekats: it goes through the fallback path and is recorded to a blob image through FrameLayerBuilder, I think
18:15mstangeor maybe not FrameLayerBuilder, since it doesn't need container items
18:16katsmstange: so if there's a clip path inside an svg, it would get totally handled on the gecko side?
18:16mstangehmm, I'm not sure how clip path is handled
18:16mstangethat would be an nsDisplayMask
18:17katshm that would probably get pushed into a clip item in webrender then
18:17katsi'm just surprised at what layout/reftests/w3c-css/submitted/masking/clip-path-viewBox-1b.html looks like in layers-free WR
18:17katscompared to -1a.html
18:18mstange(maybe it's broken)
18:29Gankrokats: yeah we create a blob image mask, and feed it into a clip. It's not *totally* broken; the fadeout effect for long text in tabs uses it
18:30Gankro(source: this is the dang image that keeps getting removed and crashing)
18:30katsok. my query was mostly idle curiousity anyway. looking through the layers-free reftest failures to see if there are any patterns
18:31katsit looks like it's going to be a lot of manual triaging of fuzzy vs fails
18:34Gankronical: getting confused by patch review interface; do I need to publish comments or something? I left a comment about your patch not compiling.
18:34GankroNot sure if it's showing up
18:35mstangeGankro: yeah, comments need to be published
18:35mstangewith the Publish button at the top
18:36Gankromstange: ok I think that worked
18:52pulsebotCheck-in: - Matt Woodrow - Only record RealDisplayItemData when we are retaining
18:54katsmrobinson: are you around?
19:50mconleymilan: ping
19:50milanmconley: 'ello
19:51mconleymilan: g'day! Has your team recently heard about perf bugs involving the Windows 10 Insider Preview stuff?
19:51mconleyI'm working with a contributor trying to reproduce a perf bug, and I'm starting to tear my hair out. Turns out the user is on Insider Preview, so I'm getting a copy, but want to know if there's anything already known there
19:52milanmconley: bas runs insider preview, if there is anything like this, he'd know
19:53mconleymilan: thanks!
19:53mconleyBas: anything worth calling out?
19:53Basmconley milan:I use Firefox on insider preview at home, but the last time was a couple of weeks ago. I didn't notice anything in particular though.
19:54mconleyBas: okay, cool, thanks
20:13jrmuizellsalzman: ping
20:14jrmuizellsalzman: do you know how skia's fRuns, fAA blitter infrastructure works?
20:15lsalzmanhaven't looked at it in ages
20:17jrmuizelwe're trying to figure out how it works
20:18jrmuizellsalzman: like for what are alpha and runs parameters?
20:21lsalzmanonly thing i can really do is read the code for you
20:23lsalzmanbut presumably runs is the length of the run, and alpha is the value of the run
21:02lsalzmandon't really know more than that offhand
22:40pulsebotCheck-in: - Miko Mynttinen - Do not create a temporary variable in NewPaintedLayerData()
22:40pulsebotCheck-in: - Miko Mynttinen - backface change
22:40pulsebotCheck-in: - Miko Mynttinen - Add missing negation r=matt
13 Sep 2017
No messages
Last message: 8 days and 5 hours ago