mozilla :: #gfx

19 Apr 2017
00:29pulsebotCheck-in: https://hg.mozilla.org/projects/graphics/rev/33a756ee984d - sotaro - Bug 1357338 - Deallocate external image id in WebRenderDisplayItemLayer r=nical
01:02daoshengmujgilbert, will we have stand-up meeting today? We are here at Graphics channel.
01:03jgilbertoh, be right there
01:13jrmuizelmattwoodrow: ping
01:14mattwoodrowjrmuizel: pong
01:14jrmuizelmattwoodrow: so what needs to be done to get rid of that clip?
01:14mattwoodrowjrmuizel: basically just get patch 5 reviewed and landed
01:14jrmuizelmattwoodrow: ok
01:15mattwoodrowI just cant remember why that didnt happen at the time
01:15jrmuizeljfkthame asked for a potential change
01:15jrmuizelmattwoodrow: https://bugzilla.mozilla.org/show_bug.cgi?id=1317862#c14
01:15firebotBug 1317862 FIXED, matt.woodrow@gmail.com Add a TextLayer
01:15mattwoodrowsure, but it didnt look like a particularly hard one
01:16jrmuizelmattwoodrow: want to do it and rerequest?
01:17mattwoodrowjrmuizel: ok
01:17jrmuizelmattwoodrow: thanks
01:55mattwoodrowmstange: ping
01:55mstangemattwoodrow: pong
01:55mattwoodrowmstange: I have an ASR question, using this display list dump - https://pastebin.mozilla.org/9019394
01:56mstangewe need display list dump syntax highlighting
01:56mattwoodrowThe blend container (line 4) has the same AGR as its children (the HTML canvas frame), but a different ASR (it has the root asr, children have the scrollframe)
01:56mattwoodrowI *think* that this means we can hit this code: http://searchfox.org/mozilla-central/source/layout/painting/FrameLayerBuilder.cpp#3843
01:57mattwoodrowmarking the ContainerLayer as opaque, even though it doesnt have FrameMetrics and its children do
01:57mattwoodrowand when we go into checkerboarding, its not really opaque at all
01:58mattwoodrowmy question is: bug?
02:00mstangeprobably, give me a sec
02:06mstangeso the ASR being null is correct, since it has a fixed child and container items need to have an ancestor ASR of their children
02:07mstangeand yes, we can probably hit that code, and that would be wrong
02:07mstangemattwoodrow: can we rewrite that code to use ASRs instead of AGRs?
02:08mattwoodrowmstange: probably? Though I think this means the AGR is just wrong too
02:08mstangearguably
02:08mattwoodrowWell, it doesnt move with the scrollframe
02:08pulsebotCheck-in: https://hg.mozilla.org/projects/graphics/rev/9c846a716941 - sotaro - Bug 1355702 - Reduce WrImageKey allocation for ExternalImages on sync ImageContainer
02:09mattwoodrowmstange: Does using ASRs introduce an equivalent bug with async transforms?
02:09mstangehmm, I suppose its bounds don't move, that's true
02:09mstangehmm
02:10mstangemattwoodrow: you're right, I think
02:10mstange... let's use both?
02:10mstangeI'm not proud of that suggestion
02:10mattwoodrowmstange: So for retained display lists I think we need a 3rd concept
02:11mstangeuhhhh
02:11mattwoodrowin between AGR and ASR
02:11mattwoodrowAGR = things that move, NEWTHING = things that move asynchronously, ASR = things that scroll asynchronously
02:11mattwoodroweach being a subset of the previous one
02:12mstangehuh
02:12mstangeoh
02:12mstangeok, yes, I agree
02:12mstangecan we have a single thing that has some attributes?
02:13mattwoodrowI was thinking subclasses, but yeah, that works for me too
02:13mstangensIScrollableFrame* mScrollFrame that can be null, bool mCanMoveAsynchronously
02:13mattwoodrowwe definitely want to merge the concepts together
02:13mstangemy intention with ASRs was to add these fields at some point and slowly overtake AGRs
02:13mstangeand then rename ASR to AGR
02:14mattwoodrowdo you have a timeline for working on that?
02:14mstangeno
02:14mattwoodrowok
02:14mstangemy original timeline was "some time last year"
02:14mstangebut then APZ shipped and the profiler became more important :)
02:14mattwoodrowfair enough, profiler is definitely important
03:15pchangstandups: will check try failures for WebRenderOMTA
03:15standupsOk, submitted #44936 for https://www.standu.ps/user/pchang/
03:50Lenzakstandups: bug1355430 got backed out because of unexpected-pass on Windows7, I will fix the mochitest-manifest and reland.
03:50standupsOk, submitted #44937 for https://www.standu.ps/user/Lenzak/
03:50firebothttps://bugzil.la/1355430 NEW, cleu@mozilla.com [WebGL Mochitest] Add Mochitest to test Video Fast-path uploading
03:53Jerry_IRCCloudstandups: NV12 supports in WR https://github.com/servo/webrender/issues/1131
03:53standupsI don't trust you, Jerry_IRCCloud, are you identified with nickserv?
03:54Jerry_IRCCloudstandups: NV12 supports in WR https://github.com/servo/webrender/issues/1131
03:54standupsI don't trust you, Jerry_IRCCloud, are you identified with nickserv?
03:59Jerry_IRCCloudstandups: NV12 supports in WR https://github.com/servo/webrender/issues/1131
03:59standupsOk, submitted #44940 for https://www.standu.ps/user/Jerry_IRCCloud/
05:11daoshengmulenzak, https://treeherder.mozilla.org/#/jobs?repo=try&revision=37a28db37cad3378c5530c38fa58bcc1e4d8b17c
09:41pchangstandups: !talkback quiet
09:41standupsI will only PM users now.
10:00pulsebotCheck-in: https://hg.mozilla.org/projects/graphics/rev/765939ca757d - sotaro - Bug 1357644 - Use wr::ExternalImageId instead of uint64_t for external image id r=nical
10:59pulsebotCheck-in: https://hg.mozilla.org/projects/graphics/rev/a7152802939c - Kartikaya Gupta - Bug 1357541 - Use the full 64 bit value of the PipelineId when converting to a layers ID. r=nical
11:45pulsebotCheck-in: https://hg.mozilla.org/projects/graphics/pushloghtml?startID=495&endID=496 - 219 changesets - Merge m-c to graphics
14:37mrobinsonmstange: So I suppose the tricky thing about CSS clip is that any number of them can be applied at once, all in different scroll frames.
14:38mrobinsonmstange: Ah, only the innermost one matters.
14:40mstangemrobinson: well, the thing that limits the impact of the "clip" property is that it can only be applied to position:absolute elements
14:41mrobinsonmstange: Ah, I didn't realize that.
14:41mstangewhich are containing blocks for position:absolute descendants
14:41mstangeso you can only really get the "scrolled clip" effect by using position:fixed inside it
14:41mstangebecause that's the only way you can break out of such an element
14:42mrobinsonmstange: They do combine though, if you put them in a hierarchy?
14:42mstangemrobinson: yes, they do
14:42mrobinsonAs in: the clipped area is the intersection.
14:43Gankromilan: working from home today, where do I join the daily meeting?
14:44milanGankro: send me your skype id
14:45Gankromilan: gankro1
14:57milanGankro: I didn't expect it in the clear :), but either way, you should have an invite from the gfx team skype account
14:57mrobinsonmstange: I suppose in Gecko this is possible because each clip has an owning scroll root and also a parent clip or is there some other way that the intersection is encoded?
15:00Gankromilan: if a spam bot scrapes that from irc and correlates it to skype, they earned it.
15:08rhuntGankro: are you running with the layers.advanced.* prefs? they should enable more WR display items to make gecko more like servo
15:10Gankrorhunt: jrmuizel turned on some stuff, I'm not sure if it was all stuff? Seems to be sending still an order of magnitude less stuff
15:11jrmuizelGankro: you can turn on webrender.highlight-painted-layers to see what's not using webrender
15:11jrmuizelGankro: pink stuff is not using webrender
15:11rhuntGankro: okay that's probably it, I'm sure we're not sending as much as servo right now
15:11GankroNotably the aux lists are empty in Firefox
15:12Gankrojrmuizel: that doesn't seem to be doing anything; what's the exact flag?
15:13rhuntGankro: so no gradient stops? do you have http://searchfox.org/mozilla-central/source/gfx/thebes/gfxPrefs.h#491 on?
15:14Gankrorhunt: no I didn't; seems to be sending a couple gradients now
15:14jrmuizelGankro: webrender.highlight-painted-layers is the exact name
15:14Gankrojrmuizel: bool?
15:15jrmuizelGankro: perhaps your build doesn't include it
15:15GankroSeems so
15:15jrmuizelGankro: it was added yesterday morning
15:15GankroAh
15:15jrmuizelGankro: it should be in the list
15:15GankroYeah I haven't pulled from master in a few days
15:37katsnical: want to rubberstamp the destructor-safe change to wr_api_delete? i can land it
15:38kats.. directly
15:38katsor rhunt ^
15:41katsfine. r=me it is!
15:50rhuntkats: sorry, i'm a little behind on this, why are we getting these errors now? was bindings changes caused this?
15:51rhunts/was/what
15:54katsrhunt: beta is built with webrender disabled entirely, so WR_FUNC evaluates to http://searchfox.org/mozilla-central/source/gfx/webrender_bindings/webrender_ffi.h#96
15:54katsrhunt: which and that macro expands to something that never returns, which produces compiler errors on windows when used in class destructors
15:55katsrhunt: the comment a few lines up explains
15:55katsrhunt: it doesn't happen on nightly because MOZ_BUILD_WEBRENDER is true there
15:55katsrhunt: although if you built with --disable-webrender in your mozconfig (on windows) you'd probably see the same thing
15:56rhuntkats: ah, that's right central is building webrender now
15:56katsyeah
15:57rhunti was concerned it got dropped in the cbindgen and other changes
15:57katsah, no, we just added more wr_* calls from destructors over time and missed the fallout
16:05nicalkats: I landed it on inbound already, you mean land it on central ?
16:05katsnical: no, there was a second function that needed the same fix
16:05nicalah
16:05katsi landed it on inbound, see bug
18:18katsmchang: is bug 1346487 waiting on anything?
18:18firebothttps://bugzil.la/1346487 NEW, mchang@mozilla.com Support border clip for webrender borders
18:19mstangeBenWa++, thanks for moving forward with that spec
18:20BenWamstange: No problem, please chime in to these issues if you have time: https://github.com/WICG/scroll-boundary-behavior/issues
18:20mstangeBenWa: time is the problem at the moment :)
18:20BenWaNo worries then
18:21mstangeI think the repo would benefit from a few real world examples, so that you can find out what behavior makes sense in those examples
18:22mchangkats: hmm, I wanted to check for something but forgot what. IIRC i was waiting on ethin to land something
18:22mchangill take a look at it again
18:23katsmchang: thx. if you find out what it was please add a dependency in the bug
19:41GankroIs there someone in particular I can ask about multiprocess threat modeling?
19:53botondGankro: you could start by asking in #security
19:55Gankrobotond: will do, thx
19:57botondalso billm ^
19:57billmGankro: you can ask jimm or bobowen or haik in #e10s
19:58billmthey work on sandboxing
20:26GankroWill I see the cost of pushing elements into the DisplayListBuilder in the current profiling?
20:40botondmstange: ^
20:41mstangeGankro: is this question about webrender's built in profiling overlay?
20:41mstangebecause I know nothing about that one :)
20:42Gankromstange: yeah that one; if there's other profilers in our project I should be looking at, let me know
20:42mstangeGankro: nothing webrender-specific, just the Gecko profiler
20:44mstangebut you already have Instruments, and the gecko profiler doesn't really give you much benefit when profiling webrender
20:44mstangeexcept that it's easier to see at what times we were painting, by looking at the paint markers in the timeline
21:07jrmuizelmattwoodrow: ping
21:07mattwoodrowjrmuizel: pong
21:07jrmuizelmattwoodrow: did you ever figure out what chrome was doing to handle the bad incremental display list building problems?
21:09mattwoodrowjrmuizel: Like the apz one? Nope!
21:10jrmuizelI think I have
21:10jrmuizelso when they had the original algorithm they were doing layerization in blink
21:11jrmuizelso display lists were built post layerization
21:11mattwoodrowright
21:11jrmuizelso that avoids the problem
21:11jrmuizelbut they wanted to switch to post display list layerization
21:11jrmuizelso they switched the algorithm
21:11jrmuizelhttps://codereview.chromium.org/892293002
21:13jrmuizelthe key part that I think (not certain) enables resolving this problem is having BeginSubtreeDisplayItem and EndSubtreeDisplayItems
21:13mattwoodrowI think that works
21:14mattwoodrowthough I want to know if they are emitting that for every node in the frame tree
21:14jrmuizelI believe this gives you enough information to preserve the parent-child ordering post merge
21:14mattwoodrowor if they are only doing it for more interesting nodes
21:14jrmuizelI believe only for more interesting nodes
21:14mattwoodrowThe former seems like a lot of extra items, the latter means your partial build still has to descend to those interesting nodes
21:15jrmuizelmattwoodrow: https://src.chromium.org/viewvc/blink/trunk/Source/core/paint/BlockPainter.cpp?r1=189610&r2=189609&pathrev=189610
21:16jrmuizelmattwoodrow: I don't think you need to visit all of the interesting nodes for it to work
21:16jrmuizelbut I'm not sure
21:18mattwoodrowjrmuizel: I think you would, but only the nearest interesting nodes (you dont need to descend through interesting nodes to visit ones under them)
21:18jrmuizelyeah, that matches what I see
21:20jrmuizelmattwoodrow: is this a better approach than what you proposed last time we chatted?
21:22mattwoodrowjrmuizel: That part Im not sure about :)
21:22jrmuizelmattwoodrow: well think about it and let me know what you end up with
21:23mattwoodrowjrmuizel: Hrm, so I dont see how the interesting nodes could be anthing other than stacking (or pseudo-stacking) contexts
21:23mattwoodrowotherwise z-index sorting can move things around such that you cant have a sane start/end
21:24jrmuizelmattwoodrow: hmmm
21:24mattwoodrowSo if their algorithm only lets the partial DL build skip entire stacking contexts that are outside of the changed rectangle, then I suspect the current plan will do better (since itll be able to do that too)
21:26jrmuizelthe fact that they put subtree display items for every block painter suggests that it's not just for stacking contexts
21:26jrmuizelbut I'll need to think about the z-index thing
21:38jrmuizelmattwoodrow: can you give a more concrete example of the z-index problem that you were talking about?
21:39mattwoodrowjrmuizel: https://pastebin.mozilla.org/9019479
21:40mattwoodrowSomething like that, emitting a container (or start/end pair) for 1 that contains 2 and 3 doesnt work
21:41jrmuizelmattwoodrow: right ok
21:41jrmuizelmattwoodrow: where in our code do we sort by z-index?
21:42mattwoodrowjrmuizel: And a change to 4 doesnt mean we can skip building at 1, since 4 would need to be sorted in the middle of 2/3 and we need those items to do that
21:42mattwoodrowjrmuizel: http://searchfox.org/mozilla-central/source/layout/generic/nsFrame.cpp#2587
21:42mattwoodrowat each stacking context
21:43mattwoodrowI guess one question is, can we sort after merging and not before
21:43mattwoodrowseems hard without making a copy
21:47jrmuizelmattwoodrow: somewhat related I just found https://chromium.googlesource.com/chromium/src/+show/master/third_party/WebKit/Source/core/paint/README.md
21:56mattwoodrowjrmuizel: Interesting, though a lot of the slimming v2 stuff is TODO
21:57mattwoodrowHmm, I wonder if we could detect z-indexes and emit the wrapping objects if there werent any
21:57jrmuizelmattwoodrow: yeah, I don't know if that document is up to date
21:59jrmuizelmattwoodrow: https://chromium.googlesource.com/chromium/src/+/HEAD/third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp#162
21:59jrmuizelsubtree's got renamed to subsequences
22:01jrmuizelolder versions of that function are more interesting https://chromium.googlesource.com/chromium/src/+/70b5b81d191f2cf8057f5609ed1e53ee392f7ea1/third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp#117
22:02mattwoodrowthe stacking context bit there is particularly interesting
22:03mattwoodrowoh, i think thats in SupportsSubsequenceCaching now
22:08jrmuizelmattwoodrow: it sort of looks like they add the subsequence items after z sorting
22:09mattwoodrowjrmuizel: How do you use that to determine which branches of the frame tree you can skip over when building though?
22:10jrmuizelmattwoodrow: my wild guess is that they can iterate over the frame tree in z-order
22:10mattwoodrowjrmuizel: Oh, I guess thats possible. They definitely do a separate pass for each phase, and only one of those is sorted
22:11mattwoodrowso they could iterate differently for positioned-descendants
22:11mattwoodrowWe explicitly went away from doing that when we switched to display lists (I believe, before my time)
22:12jrmuizelmattwoodrow: it sort of looks like they keep/update lists of frames in z-order
22:12jrmuizelmattwoodrow: that way they only need to sort when things change
22:17mattwoodrowjrmuizel: Its hard to see how we could do that without massive changes
22:18mattwoodrowthough I think we could record frame state bits on frames that have a descendant with z-index
22:18mattwoodrowand create sub-sequence style things for frames that dont have the bit
22:18jrmuizelmattwoodrow: yeah, I can imagine it could be a difficult change
22:18jrmuizelmattwoodrow: well, I need to head out now
22:18mattwoodrowok
22:19mattwoodrowI have things to think about :)
22:40pulsebotCheck-in: https://hg.mozilla.org/projects/graphics/rev/8a3a9bb42322 - Mason Chang - Bug 1337761 - Implement WebRenderLayerManager::EndEmptyTransaction. r=mattwoodrow
22:40pulsebotCheck-in: https://hg.mozilla.org/projects/graphics/rev/735695011771 - Mason Chang - Bug 1337761 - Part 2: Don't send external images to the parent side if a transaction is incomplete. r=nical
22:43pulsebotCheck-in: https://hg.mozilla.org/projects/graphics/rev/8b33bdca334e - Mason Chang - Bug 1337761. Mark tests as random-if for intermittent reftest failure. r=kats
20 Apr 2017
No messages
   
Last message: 119 days and 11 hours ago