mozilla :: #gfx

20 Apr 2017
01:17jrmuizelrhunt: ping
01:18rhuntjrmuizel: pong
01:18jrmuizelrhunt: why do we need WrGlyphInstance?
01:19jrmuizelrhunt: is it because euclid types are not repr(C)?
01:20rhuntjrmuizel: yes
01:20jrmuizelrhunt: it's weird that rustc let's repr(C) types contain non repr(C) types
01:20rhuntjrmuizel: is there an advantage to euclid types not being repr(c)?
01:21jrmuizelrhunt: I guess it can have some use
01:21jrmuizelrhunt: probably not
01:21gwit sounds like euclid types should be marked repr(c)
01:21jrmuizelgw: I think so
01:22rhuntgw: I agree, I think it would help the ffi story
01:22jrmuizelgw: do you have deciding powers for euclid?
01:22jrmuizelhttps://github.com/servo/euclid/commit/70ae01ed4381b4aa23cef5c4957accae6476c41b
01:24gwjrmuizel: I do :)
01:24rhuntjrmuizel: I think mixing and matching repr's makes the most sense with repr(packed) and things like it
01:24pulsebotCheck-in: https://hg.mozilla.org/projects/graphics/rev/e1e65f91a190 - sotaro - Bug 1356944 - Fix external image id allocation of WebRenderTextureHost r=nical
01:24rhuntcan't think of a good use case for repr(rust) in repr(c) besides freedom
01:26jrmuizelrhunt: so it looks like euclid should already be repr(C)
01:27jrmuizelrhunt: so we should just be able to alias WrGlyphInstance
01:27rhuntjrmuizel: ah
01:28jrmuizelrhunt: does the binding generator check that structs are repr(C)?
01:29rhuntjrmuizel: yes, but that doesn't work here because it's not smart enough to find euclid yet
01:29rhuntit can only find dep crates in the same dir
01:29jrmuizelrhunt: ah
01:29jrmuizelrhunt: so that's what we'd need to fix before aliasing GlyphInstance?
01:31rhuntjrmuizel: yes, I have a plan to fix that, but it might be faster to do a workaround if this is blocking something
01:32rhuntlike mem transmute or something
01:32jrmuizelrhunt: it's not blocking anything
01:32jrmuizelrhunt: what's the plan?
01:34rhuntjrmuizel: use cargo metadata or cargo.lock to find all the packages
01:34rhunti haven't verified all the information we need is there yet
01:35jrmuizelrhunt: it seems like it will be a bit painful to get
01:37rhuntjrmuizel: why do you say that?
01:37rhuntI'm concerned about custom registries like third_party/rust
01:38jrmuizelrhunt: I guess one question is whether you'd try to reuse code from cargo or just reimplement it
01:38jrmuizelrhunt: adding a dependency on cargo is probably not great
01:38jrmuizelrhunt: because it brings in a lot of code
01:38jrmuizelrhunt: I wonder what racer does
01:39jrmuizelrhunt: racer does not depend on cargo
01:39jrmuizelrhunt: maybe we can just copy it?
01:39rhuntjrmuizel: ah I was thinking shelling out to cargo
01:39jrmuizelrhunt: oh that might be even easier
01:39rhuntthere's a cargo command called metadata
01:39jrmuizelrhunt: that's probably best
01:39rhuntjrmuizel: I think that's what racer does
01:43jrmuizelrhunt: https://github.com/phildawes/racer/blob/master/src/racer/cargo.rs looks like it just does it by hand without using 'cargo metadata'
01:46rhuntjrmuizel: huh, that might work
01:46rhuntracer might do it for speed
01:46rhuntwhich isn't the biggest priority here
01:46jrmuizelrhunt: yeah, if cargo metadata gives us the info we want, it's probably better to just use that
01:52rhuntjrmuizel: one other problem with euclid structs is that they're generics, and generics only work so far behind type aliases, which isn't the case in GlyphInstance
01:53rhuntso if offset has a unit, it would help to tag it with it in a type alias
01:53jrmuizelrhunt: I think we can probably give it a unit
01:57jrmuizelrhunt: I think LayerPixel is probably the one we want
02:01rhuntjrmuizel: not LayoutPixel? I haven't touched fonts much
02:01jrmuizelrhunt: yes LayoutPixel is it
02:01jrmuizelrhunt: right now they're the same
02:02rhuntjrmuizel: ah they are, ok
02:16jrmuizelrhunt: https://github.com/servo/webrender/pull/1143
02:50Jerry_IRCCloudstandups: NV12 supports. Try to use shader preprocessor to handle the multiple texture targets configuration.
03:15daoshengmustandups: Bug 1353523 - Adjust threshold for Gamepad button `pressed` state and introduce pref to handle slightly sticky controller buttons; r?
03:15standupsI don't trust you, daoshengmu, are you identified with nickserv?
03:15firebothttps://bugzil.la/1353523 NEW, dmu@mozilla.com Adjust threshold for Gamepad button `pressed` state and introduce pref to handle slightly sticky con
03:16daoshengmustandups: Bug 1353523 - Adjust threshold for Gamepad button `pressed` state and introduce pref to handle slightly sticky controller buttons; r?
03:16standupsI don't trust you, daoshengmu, are you identified with nickserv?
03:17daoshengmunickserv
03:20Lenzakstandups: I will review daosheng's patch on bug1366542 and investigate bug related to bug1207710 to find where can we improve our performance.
03:21daoshengmustandups: Bug 1353523 - Adjust threshold for Gamepad button `pressed` state and introduce pref to handle slightly sticky controller buttons; r?
03:30daoshengmustandups: Refactor GamepadManager fire event functions in Bug 1356452
03:30firebothttps://bugzil.la/1356452 NEW, dmu@mozilla.com For Vive controllers the hand attribute is not always available
04:44Caspy7I'm seeing a question asking if the OOP compositor will come to Linux
04:44Caspy7anyone have input there?
04:45Caspy7it's this comment if anyone (who's on reddit) would like to drop in https://www.reddit.com/r/firefox/comments/669zpl/firefox_530_release_find_out_what_is_new/dghv9o7/
05:02rhuntCaspy7: your comment on reddit seems spot on from my understanding of it
05:02Caspy7rhunt: thanks
05:03rhuntwe don't really accelerate on linux, and the compositor process shouldn't have any performance benefits
05:03rhuntnp!
05:03Caspy7rhunt: true, but there do seem to be some plans to do more HWA in the future on linux. Particularly for video I think
05:03Caspy7VAPI or some such thing??
05:07rhuntCaspy7: definitely, I don't know the plans very well but I do know there was a push to enable opengl compositing on linux that got backed out because of issues
05:07rhuntbug 594876
05:07firebothttps://bugzil.la/594876 REOPENED, andrew@comminos.com [Tracking bug] [Linux] Turn on OpenGL accelerated layers by default for at least some subset of hard
05:08Caspy7hrm, my search merely found 894372
05:08Caspy7bug 894372
05:08firebothttps://bugzil.la/894372 WONTFIX, nobody@mozilla.org Gstreamer backend don't use hardware accelerate (VAAPI)
05:09Caspy7well, guess I was focused on VAAPI atm
10:03daoshengmustandups: Working on Bug 1358010 to figure out if it should skip
10:03firebothttps://bugzil.la/1358010 NEW, nobody@mozilla.org Permaorange assertion failure and crash in test_vrDisplay_getFrameData.html when Gecko 55 merges to
10:13daoshengmustandups: Bug 1356452 - For Vive controllers the hand attribute is not always available; done
10:13firebothttps://bugzil.la/1356452 NEW, dmu@mozilla.com For Vive controllers the hand attribute is not always available
13:37pulsebotCheck-in: https://hg.mozilla.org/projects/graphics/pushloghtml?startID=499&endID=500 - 228 changesets - Merge m-c to graphics
13:47katsrhunt: keep in mind euclid does some funky macro expansion stuff too, it might be nontrivial to parse that and get useful results
13:50nicaleuclid types have the same representation as moz2d ones (and are repr(C))
13:51nicalperhaps we could just have an annotation for cbindgen that says "this_rust_thing = that_cpp_doodad"?
14:11milanGankro: https://public.etherpad-mozilla.org/p/gfx-daily
14:40pulsebotCheck-in: https://hg.mozilla.org/projects/graphics/rev/9801b8ad5dc2 - Kartikaya Gupta - Bug 1357754 - Add a mechanism to send scroll data to APZ over PWebRenderBridge. r=jrmuizel
14:40pulsebotCheck-in: https://hg.mozilla.org/projects/graphics/rev/022304bf1ad7 - Kartikaya Gupta - Bug 1357754 - Extract a template function from UpdateHitTestingTree. r=botond
14:40pulsebotCheck-in: https://hg.mozilla.org/projects/graphics/rev/9ce54d2f9c4f - Kartikaya Gupta - Bug 1357754 - Replace LayerMetricsWrapper::AsRefLayer with GetReferentId. r=botond
14:40pulsebotCheck-in: https://hg.mozilla.org/projects/graphics/rev/2a256d666481 - Kartikaya Gupta - Bug 1357754 - Add WebRenderScrollDataWrapper. r=botond
14:40pulsebotCheck-in: https://hg.mozilla.org/projects/graphics/rev/1f032699c9c5 - Kartikaya Gupta - Bug 1357754 - Implement the traversal functions of WebRenderScrollDataWrapper. r=botond
14:41pulsebotCheck-in: https://hg.mozilla.org/projects/graphics/rev/e8979e2cb9b7 - Kartikaya Gupta - Bug 1357754 - Add ScrollDirection IPC serialization code. r=botond
14:41pulsebotCheck-in: https://hg.mozilla.org/projects/graphics/rev/ea612503b63f - Kartikaya Gupta - Bug 1357754 - Add more data to the WebRenderScrollData object. r=botond,jrmuizel
14:41pulsebotCheck-in: https://hg.mozilla.org/projects/graphics/rev/e891376e9f56 - Kartikaya Gupta - Bug 1357754 - Hook up chaining layer trees. r=botond,jrmuizel
14:41pulsebotCheck-in: https://hg.mozilla.org/projects/graphics/rev/9a89384b0d2c - Kartikaya Gupta - Bug 1357754 - Record the isFirstPaint flag in the WebRenderScrollData. r=jrmuizel
14:41pulsebotCheck-in: https://hg.mozilla.org/projects/graphics/rev/c003db4fe3a4 - Kartikaya Gupta - Bug 1357754 - Trigger the APZ hit-testing tree rebuild. r=jrmuizel
14:46rhuntkats: ah you're right
14:47rhuntnical, kats: yeah I think some kind of workaround like that will be simplest
14:47rhuntit might be possible to parse out the results of the crate with the macros expanded by rustc
14:47rhuntbut i don't think it's worth it to fiddle with that
15:10Gankrowho knows stuff about scene "pipelines"? Bas/kvark?
15:11kvarkWR pipelines?
15:11Gankroyes
15:11Gankro"path to the screen" is just like "yes there are pipelines" without explaining why or how they're actually used
15:11kvarkI have a basic understanding, ask away
15:11mrobinsonrhunt: Did you get a chance to check out that diff I commented about on the wrench crash?
15:12Gankrokvark: what leads to there being more than one (e.g. non-root) pipeline?
15:12rhuntmrobinson: yes, I just ran it and am still getting a crash
15:12kvarkAFAIK, multiple documents and iframes
15:13kvarkGankro: what does it have to do with "path to the screen"?
15:13mrobinsonrhunt: Okay. Thanks! I'll take another look at your PR soon.
15:13Gankrokvark: that's just the biggest piece of updated docs we have :)
15:13Gankrokvark: multiple documents meaning multiple tabs?
15:13kvarkyes
15:14Gankrokvark: so what is the root? The document in the current visible tab?
15:15rhuntGankro: the root is the chrome of the browser, it has an iframe for the current tab referencing the pipeline ID of the current visible docuemtn
15:15rhuntit's needed because painting happens asynchronously and in different processes
15:16GankroWhy would the renderer be storing data about how to draw tabs that aren't visible? Presumably that data can be regenerated on tab switch, considering it's regenerated every frame anyway?
15:18rhuntGankro: We don't send display lists for tabs/pipelines that aren't visible, not sure if WebRender retains this information
15:19Gankrorhunt: do multiple windows share rendering backends?
15:20rhuntmrobinson: np, like I said in the PR I don't fully understand the api yet so I don't know if it's a bandaid
15:20rhuntmrobinsons: also if you want to repro the problem with the bin, you'll need to cherry-pick https://github.com/servo/webrender/pull/1129/commits/50f47da7ff3b5626d8c6ae0704ffc02579df3a60 onto the cset I mention
15:21kvarkGankro: I'd expect them to have a single instance of WR with different RenderApi front-ends
15:21kvarkso I think we retain the resources of invisible pipelines
15:22rhuntkvark: I believe we create a separate (api, render_backend, renderer) for each window right now
15:22Gankrokvark: which means by the time we get to deserializing display_lists, we're totally unified?
15:22rhuntnot sure, and that probably isn't optimal
15:22kvarkrhunt: oh
15:22rhuntfor tabs, they should share a render_backend
15:23rhuntotherwise it wouldn't work
15:23GankroIs there any reason to not unify windows?
15:24rhuntI think it's just because each window has it's own gl context, so it needs it's own renderer
15:24rhuntthey could probably share render_backends
15:24Gankrorhunt: should I operate as if unification is expected?
15:25rhuntGankro: why do you need different windows to be in the same render_backend? all the tabs and chrome of a window are in the same render_backend
15:25Gankrorhunt: I don't need anything in particular, I'm just trying to understand the system's design :)
15:27rhuntGankro: ah okay, yeah it's complicated because the system needs to support many different things
15:27nicalright now each window comes with its own render_backend (and render backend thread)
15:28GankroNext up: does anyone recall the motivation of aux lists? Is it literally just to save space in DisplayLists, or does decoupling them do a useful thing architecturally?
15:29nicalI think that aux lists predate my and kvark's involvement with the project, maybe mrobinson knows
15:30nicalotherwise you'll have to ask glenn
15:33mrobinsonGankro: gw is the person to ask about aux lists, I think.
15:33GankroAh ok
15:33Gankrohe's usually online in the late afternoon?
15:36GankroIt&#39;s looking possible that we can have DisplayItems permanently serialized, and prevent ever having a Vec<DisplayItem> in memory. This would save us a lot of space without having to maintain hand-crafted layout optimizations like outlining to aux lists. You can even combine two serialized display lists by concatenating their bytes. However needing to update
15:36Gankroaux_list indices breaks that
15:37Gankro(also this would obviously save some mallocs/copies)
15:40nicalI think that we can&#39;t do this with the serialzation stuff we currently use but I&#39;m not sure
15:41nicali mean iterate on the serialized data and &quot;decompress&quot; each element locally as we go
15:41Gankronical: yes push_built_display_list also does these fixups
15:42Gankronical: from what I&#39;ve seen every user of the display lists on the backend just iterates them sequentially
15:42nicalyeah that&#39;s my impression
15:43Gankronical: currently we just transmute the Vec<u8> to a Vec<DisplayItem>
15:43mrobinsonGankro: Another person to ask about the history of display list serialization is pcwalton, I think.
15:43Gankromrobinson: ah good point
15:44nicalI guess we could manually iterate on the blob and deserialize DisplayItem objects instead of deserializing the Vec<DisplayItem> itself
15:44Gankronical: yes exactly
15:44Gankrobincode supports this https://docs.rs/bincode/1.0.0-alpha7/bincode/fn.deserialize_from.html
15:45nicalnice
15:45GankroIt might hurt hot loops but it doesn&#39;t look like any are really that hot
15:46Gankroe.g. each iter needs to do interesting work
15:46Gankroskip_current_stacking_context is like the main loser
15:47GankroBut we would have paid the cost of deserializing those elements elsewhere
17:18mstangejrmuizel: https://bugzilla.mozilla.org/show_bug.cgi?id=1299715#c35
17:18firebotBug 1299715 FIXED, cku@mozilla.com Replace ContainerItemType::eSVGEffects with eMask and eFilter.
18:24mstangejrmuizel: http://smfr.org/css/complex-rounded-corners.html
18:29jgilbertmstange, ew
18:37mstangejrmuizel: https://bugzilla.mozilla.org/show_bug.cgi?id=1007702#c36
18:37firebotBug 1007702 FIXED, mchang@mozilla.com Enable skia content on Windows by default when not using D2D
18:39mchangmstange: a positive comment, its kinda amazing
18:39mstangemchang: exactly, I was really surprised and happy to read it
18:40mchangmstange: what a great day, should print out the comment and frame it
18:40mstangelol
18:54snorpjgilbert: are you getting review requests from me or am I fucking up mozreview somehow
18:54snorpfor https://reviewboard.mozilla.org/r/129010/diff/3#index_header
18:54jgilbertsnorp, oh, sorry, I&#39;m getting them
18:54snorpjgilbert: no worries, just wanted to make sure
19:22gwGankro: regarding the pipelines discussion above - it can be any embedded document. could be tabs, but more likely, a single tab that contains 1+ iframes embedded within that document.
19:22gwGankro: in servo, iframes are handled by a different thread / process, thus they produce a different display list that is linked via the iframe/pipeline display item.
19:22GankroOh hey gw; thanks for the clarification!
19:23Gankrogw: wait, ff does iframes as the same dl?
19:23gwGankro: I&#39;m not sure how/if gecko handles iframes with WR yet
19:29Gankrogw: ideally firefox would do the same as servo here?
19:31gwGankro: I think so, yes - in an ideal world, I&#39;d like to expand the concept of an iframe/pipeline to be a general framework for embedding/linking DLs - and use them for the overall chrome UI to contain tab DLs etc
19:31gwGankro: but that needs more discussion with gecko team
19:32gwGankro: this could also potentially be a gateway to building large display lists in &quot;chunks&quot; for performance reasons
19:32Gankrogw: does &quot;hardcoding&quot; them for only iframes actually do something from the renderers perspective?
19:33gwGankro: no, it&#39;s mostly a conceptual change for WR, it shouldn&#39;t be much / if any different code-wise
19:34Gankrogw: so just to verify -- auxlists only exist to make display list sending a series of POD Vecs?
19:34GankroThere&#39;s no other architectural benefit?
19:35gwGankro: yup - it&#39;s purely for performance reasons - the original implementation did not have/use aux lists
19:36Gankrogw: is there some interesting reason why they&#39;re all concatenated into a single Vec<u8> before being sent over IPC? (why not send each individually?)
19:36gwGankro: that&#39;s a pcwalton question - he did all that work :)
19:36pcwaltonGankro: saves syscalls
19:37Gankropcwalton: but DisplayList and AuxList are still sent seperately? I guess AuxList was often small enough to justify this micro opt?
19:37pcwaltonyeah
19:38pcwaltonI mean, it might not make much of a difference
19:38pcwaltonI dont recall if I measured that micro-optimization specifically
19:42Gankrogw: do you know what the cached display lists in the content process correspond to in gecko?
19:42Gankro(afaict Servo doesn&#39;t do this)
19:43gwGankro: That&#39;s something in gecko you mean? I know very little about the gecko internals, sorry.
19:45Gankrogw: ah no worries; it&#39;s whatever&#39;s consuming push_built_display_list.
19:45jrmuizelpcwalton: where/how does servo do hit testing?
19:46pcwaltonjrmuizel: it builds a special display list for hit testing with backlinks back to the DOM nodes and processes that
19:46jrmuizelpcwalton: does it build the display list for entire page?
19:46pcwaltonfor a large area near the cursor
19:46pcwaltonnot the whole page
19:47jrmuizelpcwalton: why a large area?
19:47pcwaltonno good reason
19:48pcwaltonits just that the same code does DL building for display and for hit testing
19:48pcwaltonso I didnt bother to add a special case for hit testing
19:48pcwaltoncould be a nice fix
19:49jrmuizelpcwalton: is it a lot cheaper to build a display list for a smaller area?
19:49pcwaltonits significantly cheaper
19:50pcwaltonI dont know how perf critical this code is though
19:50jrmuizelpcwalton: i.e. will the process avoid traversing into most frames?
19:50pcwaltonyeah, if you build for a smaller area then fewer fragments are visited
19:50GankroWait the content process uses display lists in a way other than just immediately sending them to the render process?
19:50jrmuizelpcwalton: my questions are more targeted around incremental display list construction
19:51pcwaltonGankro: yes, but theyre a different kind of display list
19:51jrmuizelpcwalton: i.e. is it easy and a lot cheaper to build a display list for a given set of dirty rects
19:51Gankropcwalton: not a Vec<DisplayItem>?
19:51pcwaltonits certainly easier to build DLs for dirty rects than to go full on incremental DL construction this is what roc wanted to do at one point
19:52pcwaltonGankro: I have to go to the code to see how its changed :) give me a sec
19:52pcwaltonjrmuizel: the flow tree effectively gives us an AABB tree
19:52jrmuizelpcwalton: Gecko is looking a building DLs for dirty rects and then merging those into previously built display list
19:52pcwaltonyeah, something similar should work for Servo
19:53jrmuizelpcwalton: so servo actually has an AABB tree?
19:53pcwaltonthe layout tree should be an AABB tree, yes
19:53pcwaltonisnt Geckos frame tree the same way?
19:53jrmuizelpcwalton: no
19:53Gankrojrmuizel: &quot;merging&quot; here hopefully meaning a series of well-timed `append` calls?
19:53pcwaltonI mean it may be a poorly balanced AABB tree
19:53pcwaltonbut its an AABB tree
19:53jrmuizelpcwalton: parents don&#39;t know the bounds of their descendents
19:53pcwaltonjrmuizel: we have an overflow field on Flow which is the AABB
19:54pcwaltonoh, wow
19:54pcwaltonI thought that was, like, necessary for anything to work :)
19:54pcwalton(what does FinishAndStoreOverflow do then? I thought that was what it did)
19:54jrmuizelpcwalton: perhaps I&#39;m misunderstanding the gecko code
19:54pcwalton(that is I thought it computed the overflow)
19:54pcwaltonI mean, how does stuff like overflow:scroll work
19:54pcwaltonthe outer frame has to know the overflow region of its descendants for overflow:scroll to work in Gecko
19:55jrmuizelpcwalton: what you&#39;re saying sounds reasonable
19:55jrmuizelpcwalton: I don&#39;t understand enough to answer
19:55pcwaltonah, ok
19:55pcwaltondbaron would know for sure
19:57pcwaltonGankro: check out https://github.com/servo/servo/blob/master/components/gfx/display_list/mod.rs#L118
19:57pcwaltonGankro: in particular look at https://github.com/servo/servo/blob/master/components/gfx/display_list/mod.rs#L792
19:57pcwaltonthis is where the stuff needed for hit testing is stored
19:58GankroHoly shit DisplayList is a real type
19:58GankroThe legends are true
19:59jrmuizelpcwalton: mstange reminded me of things
19:59pcwaltonok
19:59jrmuizelpcwalton: https://people-mozilla.org/~jmuizelaar/tmp/overflow.html
20:00jrmuizelpcwalton: in servo does the red frame know about the bounds of the green frame?
20:01* pcwalton goes to the code
20:01Gankropcwalton: servo is drunk on the name &quot;display list&quot; and needs to stop. there&#39;s too many kinds.
20:01pcwaltonfeel free to rename
20:03pcwaltonjrmuizel: the answer to your question is: yes, because servo renders it wrong
20:03Gankropcwalton: but anyway, if I mess with webrender&#39;s DisplayItem/list, it shouldn&#39;t at all interact with this stuff, eh?
20:03pcwaltonlet me look at the spec...
20:04pcwalton It affects the clipping of all of the element&#39;s content except any descendant elements (and their respective content and descendants) whose containing block is the viewport or an ancestor of the element.
20:04pcwaltonok, yeah, servo is wrong here
20:04* pcwalton goes to file a bug
20:05pcwaltonthe important thing here is: Servo presently relies on the flow trees overflow property being an AABB tree
20:05pcwaltonbecause of partial DL building
20:05pcwaltonso when we go to fix this bug I think we will need another notion of overflow
20:05pcwalton(we already have two notions of overflow: the visible area and the scrollable region)
20:06jrmuizelpcwalton: so the AABB tree property will be preserved
20:06jrmuizel?
20:06pcwaltonright now, it needs to be
20:06pcwaltonbecause of partial DL building
20:06pcwaltonwe could give that up, but Im not sure that would be particularly forward thinking
20:06jrmuizelit seems wise to preserve it
20:06pcwaltonbecause incremental DL construction is nice
20:06pcwaltonagreed
20:06jrmuizelbut I don&#39;t know the reason Gecko doesn&#39;t have an AABB tree
20:07jrmuizeland maybe it&#39;s a good one
20:07pcwaltonI would assume just because it doesnt need one to render CSS right
20:07pcwaltonkeeping an AABB tree up to date requires more work and memory
20:07jrmuizelyeah, maybe
20:08jrmuizelthe work that we do to handle it not being an AABB tree isn&#39;t particular efficient though either
20:08pcwaltonyeah, you pay either way
20:08pcwaltonand the overflow regions are *almost* an AABB tree
20:09pcwaltonyoure computing something very similar to make overflow:scroll work
20:26jrmuizelacomminos: ping
20:27mstangejrmuizel: https://developer.mozilla.org/en/docs/Web/CSS/border-collapse
20:28rhuntgw: ping
20:37gwrhunt: pong
20:37rhuntgw: do you know the reason why render targets are cleared with [1,1,1,0]?
20:37rhunthttps://github.com/servo/webrender/blob/master/webrender/src/renderer.rs#L1851
20:37Basjrmuizel: Ugh, putting the property table entries on the Frame itself is rather... messy.. codewise.
20:38gwrhunt: the alpha is so that when we composite an intermediate target, it&#39;s transparent where it wasn&#39;t written to - if that&#39;s what you mean?
20:38jrmuizelBas: I believe it&#39;s probably not the most fun :)
20:39Basjrmuizel: Well, asides from not being fun it&#39;s going to be ugly, probably.
20:39rhuntgw: is there a reason for rgb = 1? or is it okay to change it to transparent black?
20:40jrmuizelBas: well once we have numbers we can make a more informed decision about it&#39;s worth
20:40jrmuizelin uglyness
20:40gwrhunt: I did try that recently - I thought we could remove the isolate clears altogether as a quick optimization. And it made several tests fail. But I didn&#39;t look into why.
20:40gwrhunt: So it could be worth investigating to determine what the test failures are, it may be something simple
20:42gwrhunt: As in, I made the overal target clear color 0,0,0,0 - and removed the isolate clears.
20:42Basjrmuizel: I&#39;m probably first going to do a patch that keeps the FramePropertyTable around and just caches stuff on there, that should give us a decent idea of what perf will be like without investing the several days worth of work probably involved in doing this properly.
20:42rhuntgw: okay, I seem to remember a comment about the colors being chosen to help out clipping, but that&#39;s probably changed
20:42rhuntgw: yeah, that would be nice, i&#39;ll look into it
20:42gwrhunt: yes - that&#39;s no longer relevant, since the clips are done in a separate A8 target now
20:43gwrhunt: so if you can make it pass the tests with 0,0,0,0 (or even find out what the issue is) that&#39;d be great! :)
20:43jrmuizelBas: sounds ok
20:44rhuntgw: okay, ps_composite assumes its inputs are premultiplied and 1,1,1,0 causes problems with that and semi-transparent surfaces
20:44rhuntso i&#39;ll try and fix that
20:44rhuntmeaning remove isolate clears
20:45rhuntgw: was it wrench test failures or servo ones?
20:47gwrhunt: ah, interesting. I think it was wrench failures - I don&#39;t think I bothered checking the servo ones yet
20:48gwrhunt: that&#39;d be a nice optimization and simplify the code if we could remove those clears though! you might like to mention to kvark too - as he&#39;s planning to use some of the isolate_clear infrastructure as part of the preserve-3d support.
20:49gwrhunt: not the clears themselves, just the code that marks a stacking context as being isolated
20:50rhuntgw: yeah, I didn&#39;t really like passing specific rects for clearing so it&#39;d be nice to remove it
20:50kvarkgw: thanks for the heads up. I think it will be the least of my problems at the moment ;)
20:50gwkvark: :P
20:51rhuntgw: so, just ran the wrench reftests with clear_color = [0,0,0,0] and I didn&#39;t get any failures
20:51rhuntcan&#39;t believe we&#39;re up to 77 reftests
20:53GankroDisplayListBuilder::new to DisplayListBuilder::finalize is fairly one-shot, right? No one sits around and goes to do some other work in the middle?
20:53gwrhunt: huh, maybe try the servo mozilla/ wpt subdir - those might be the ones I tried - they are a subset of ~500 servo specific tests that cover a lot - they only have a couple mins to run (once you&#39;ve built servo!)
20:57rhuntgw: will do
20:57rhuntkvark: if this works, do you have an issue with me removing all of isolate_clear, or would you rather I leave the stacking_context marking?
20:58kvarkrhunt: I don&#39;t need isolate_clear in particular right now, I&#39;m fine as long as the whole thing is cleared
20:59rhuntkvark: ok
21:20Basjrmuizel: Fwiw, I&#39;m running a second test to verify, but for FLB times on Matt&#39;s DL-stresstest, on this machine: Without any pointer patch: 30ms, with DID direct pointer patch: 26ms, and with FramePropertyTable Pointer patch: 28ms.
21:21jrmuizelBas: interesting
21:23Basjrmuizel: Fwiw, possibly this could be improved somewhat by actually changing how PropertyValues work a little and putting them in an nsAutoTArray on the frames. But that&#39;d be a little bit more work to experiment with. I&#39;m not 100% certain that work will have a positive return on investment on the short term.
21:24jrmuizelBas: I think an nsAutoArray is probably too big to put on the Frame
21:24Basjrmuizel: That&#39;s what I do for DisplayItemData I think.
21:25jrmuizelBas: oh you&#39;re not just adding a single pointer?
21:26Basjrmuizel: Nope, a size 2 AutoTArray is what I add.
21:26jrmuizelBas: oh
21:26jrmuizelBas: thats adding a lot then isn&#39;t it?
21:26jrmuizelBas: how big is it?
21:27BasOff the top of my head on 32-bit either 8 or 12 bytes.
21:27Basjrmuizel: Anyway, we have the total memory increase numbers from Talos :)
21:27BasSo we don&#39;t need to speculate too much about individual sizes.
21:28Basjrmuizel: It&#39;s a 1-2% regression in memory usage.
21:29jrmuizelBas: well, a regression in Talos needs to be understood in an appropriate context
21:29jrmuizelBas: i.e Talos doesn&#39;t test the worst case here
21:30jrmuizelBas: it&#39;s worth figuring out the number of bytes exactly
21:30Basjrmuizel: There is no doubt 12 bytes per frame is &#39;significant&#39; on the current size of a frame the question is. But as for 5 million frames that&#39;s still only 50MB, the size of a large canvas, do we care? I personally don&#39;t think we do.
21:30Basjrmuizel: That&#39;s not exactly difficult either :p
21:30jrmuizelBas: we&#39;ve cared a lot in the past about the size of a Frame
21:31Basjrmuizel: We&#39;ve cared wrongly about a lot of things in the past. There might be good reasons :) I haven&#39;t seen them yet.
21:32jrmuizelBas: it&#39;s true
21:34Basjrmuizel: I don&#39;t even think 5 million frames is a realistic scenario? But maybe it is when you have tons of tabs open or something? But even if your average tab has in the order of 10 thousand frames that normally wouldn&#39;t get DisplayItemData, you&#39;re still talking in the order of 100s KB regression per tab, which doesn&#39;t seem like that much to me.
21:35jrmuizelBas: I&#39;m not a person who&#39;s tried to keep the size of Frames smalls so you don&#39;t need to convince me
21:36jrmuizelBas: just make sure you talk to the people who have tried to keep it small
21:36Basjrmuizel: Yeah, in any case, I seem to have made a mistake in my earlier measurement, so the numbers may change even.
21:36Bas(The time measurement for the PropertyValue pointer case)
21:38Basjrmuizel: Or rather, the data seems to be noisy on a &#39;per run&#39; basis, which is rather mysterious, may have do to with coincidental memory layout.
21:41Basmattwoodrow: Any idea roughly what ratio of frames would never get any properties at all?
21:41Basjrmuizel: Maybe cache locality. Larger frames mean different frames would be further apart in memory, that could be a reason I suppose?
21:42mattwoodrowBas: No idea sorry. Its obviously easy to create pages at either extreme, but I have no feel for what the average % is, would need telemetry
21:45Basmattwoodrow: Gotya.
22:20milanbas: O
22:20milanbas: O
22:20milanargh
22:20milanI&#39;m pretty sure we measured this
22:20milanbut I can&#39;t recall if it was mstange or somebody else
22:21mstangeI thought it was Bas...
22:51Gankrogw: jrmuizel: I wrote up a bunch of stuff on DisplayList IPC past and future: https://gist.github.com/Gankro/90603728c6b0f9eac708e63802682dfb
22:52jrmuizelGankro: I&#39;ll read it another time
22:53gwGankro: thanks! I&#39;ll take a look today
23:00mstangemattwoodrow: jrmuizel found out that we seem to have introduced the &quot;Fundamental Compositing Bug&quot; into firefox
23:00mstangeor have had it for a long time
23:00mattwoodrowmstange: :(
23:00mattwoodrow*the* fundamental compositing bug?
23:00mstangeor maybe not, we&#39;re still confused
23:01mattwoodrowI thought we tried really hard to not have that
23:01mstangewell, it&#39;s possible our testcases are wrong
23:01jrmuizelmattwoodrow: do you know the bug?
23:01mstangemattwoodrow: adding transform to an element changes how this element is ordered with respect to other elements on the page, do you know why?
23:02mstangeor how exactly it changes ordering?
23:02mstangewe&#39;re trying to make an element with transform go in between another element&#39;s background + foreground
23:02mstangeand failing, because the transform moves it on top of that other element&#39;s foreground
23:04mattwoodrowmstange: Do you have a link?
23:05mstangemattwoodrow: one sec
23:05mstangealso, jrmuizel was deceiving me, and now I&#39;m all confused
23:05gwGankro: I will think more about your doc over the weekend, but in general the proposed new model sounds ideal, at first glance.
23:06mstangemattwoodrow: ok, sorry, we don&#39;t have the bug after all
23:07gwGankro: one question - if we instead make the data Vec<u64> (and pad if required), is that going to make it easier / more likely for llvm to lower that to SIMD instructions for reads/writes? maybe that&#39;s not a big issue - it used to be on some CPUs I&#39;ve worked with previously...
23:08mattwoodrowmstange: ok, wonderful
23:08mattwoodrowwas it to do with transform creating a stacking context/containing block?
23:08gwGankro: also, I&#39;d suggest getting mrobinson to take a look at it - to see if there&#39;s any gotchas with the way the WR code traverses the display list.
23:12mstangemattwoodrow: probably with the stacking context, but it wasn&#39;t the thing that usually comes to mind if you think of a stacking context
23:12mattwoodrowhttps://www.w3.org/TR/CSS2/zindex.html
23:12Gankrogw: grabbing dinner; reply usefully in a bit
23:12mattwoodrowmstange: stacking context descendents get treated as positioned descendents, so step 8
23:13mstangemattwoodrow: ah!
23:13mstangethat&#39;s the one
23:14mstangemattwoodrow: we were being mislead by slides in a presentation by the chrome team, because those slides used a transform in their example
23:15mstangebut that transform defeats the whole purpose of the example
23:15mattwoodrowmstange: ah right, I see
23:17Gankrogw: it&#39;s tricky because bincode strips padding, but honestly my simd -fu is weak
23:19Gankrogw: also I just realized I didn&#39;t fully explain how non-trivial bincode overhead is... was too tired
23:23gwGankro: do you have a sense of what size the DL is in bytes for some typical pages? that&#39;d be an interesting stat - and might make it fairly easy to rule some ideas in or out
23:25Gankrogw: servo/servo is 200k for dl+aux, 300k uncompressed
23:26jrmuizelmattwoodrow: do you know what the fundamental compositing bug is?
23:26Gankrogw: i added stats for it to -wr-stats
23:27gwGankro: ah, yes - thanks
23:29mattwoodrowjrmuizel: iirc it was that they were doing layerization off of the frame tree. So when you promoted some subtree to an active layer, you might need to split existing layers into above/below parts
23:30mattwoodrowand they werent doing that correctly, since its pretty hard to figure out from the frame tree
23:30jrmuizelmattwoodrow: do you know enough to write a test case that would show it?
23:30mattwoodrowjrmuizel: https://docs.google.com/presentation/d/17k62tf1zc5opvIfhCXMiL4UdI9UGvtCJbUEKMPlWZDY/edit#slide=id.g9e3e9073b_2_11
23:30mattwoodrowdoes that work?
23:31mattwoodrowor is that the one you tried
23:31jrmuizelmattwoodrow: I haven&#39;t been able to make it work
23:31jrmuizelmattwoodrow: that&#39;s what I tried
23:31Gankrogw: the timing code is a bit weird because I built it assuming there was only one display list; for normal pages on servo that ends up being true I think?
23:36mstangemattwoodrow: are iframes officially &quot;stacking contexts&quot;?
23:36mstangemattwoodrow: are iframes treated as positioned descendants?
23:36gwGankro: yea - so long as there&#39;s no iframes / browser.html running
23:36mattwoodrowmstange: I think they are
23:37mattwoodrowmstange: Looking at our code, yes and no, in that order
23:38mstangemattwoodrow: great, so we can use iframes to demonstrate the fundamental compositing bug
23:38mstangebecause iframes can get composited layers in chrome
23:38mstangemattwoodrow: thanks!
23:38mattwoodrowmstange: Do other layerised things (like a video?) work?
23:39mstangegood question
23:41jrmuizelmattwoodrow: the problem happens with video too
23:42mattwoodrowjrmuizel: The problem where you cant create the problem?
23:42mattwoodrowvideo elements shouldnt be positioned an