mozilla :: #servo

12 Jul 2017
01:31pcwaltongw: just FYI, Im thinking about using the stencil buffer for Levien style AA so that I can get early Z back
01:31pcwaltonis that OK?
01:32pcwalton(Im not entirely sure the high quality Levien style AA will even be necessary for paths as opposed to fonts, BTW but it might be needed)
01:32pcwaltonthe stencil buffer use will not affect the number of draw calls or increase the load
01:38gwpcwalton: that should be fine - WR doesn't currently allocate or use a stencil buffer
01:38pcwaltongw: cool
01:38pcwaltonthe alternative is to read from a depth buffer in the FS and discard appropriately, but that loses early Z
01:38pcwaltonso I think stencil buffer usage is better
01:40gwyea that doesn't sound ideal
01:41pcwaltongw: btw, Im switching to quadratic Bezier approximations of cubic Beziers for speed, simplicity, and improved numerical stability, per a suggestion from raph,
01:41pcwaltonlooks good so far
01:43xidornbz: ping if you're still around
02:03stshinepcwalton: ping about the mesh idea
02:04pcwaltonstshine: Ill reply on the issue
02:05stshinepcwalton: okay. Forward to see your opinions about images and text in svg
02:44stshineSimonSapin: nox: fix for https://github.com/rust-lang/rust/issues/43102 landed, rustup please :(
02:44crowbotIssue #43102: Since last nightly, winit doesn't create windows any more - https://github.com/rust-lang/rust/issues/43102
03:53pcwaltonlets see if I can do depth peeling without a separate blit...
04:28cpearceDoes servo have a back button?
04:29pcwaltonbackspace
04:29pcwaltonat one point that worked anyway
04:30cpearcehmm, not working here on Windows.
04:30cpearceFirst startup of the servo-latest download was really slow, but subsequent start ups aren't too bad
04:31cpearceMaybe Windows was scanning it or something
04:33est31I think ctrl + left arrow key works
04:33est31not sure though, have no recent servo handy
04:45hirokanru: ping
04:50kanruhiro: pong
04:51hirokanru: hi, I just left a comment in https://bugzilla.mozilla.org/show_bug.cgi?id=1378987#c8
04:51firebotBug 1378987 NEW, nobody@mozilla.org stylo: Slack channel list's channel names move left and become bold text
04:51hirokanru: if you are using linux.
04:51kanruhiro: I'm
04:51hirokanru: great!
04:52kanruhiro: I'll give it a spin
05:01hirokanru: thank you for the quick check!
05:03kanru:)
05:22cpearceCTRL+Left works, though I'm not able to view the page I navigate back to... is this a Windows issue, or is servo like that on other platforms too?
05:24cpearceOr does the blame lie with browser.html? or the interaction between the two?
05:31heycamhiro: I noticed another issue in Fastmail, where an animation keeps running even though it's no longer matching the rule with the animation property. do you know if this is covered by any of the open bugs?
05:32hiroheycam: it does not involve any mouse events?
05:32heycamhiro: the animated thing is a ::before that only shows on :hover
05:32hiroheycam: if not, it should be a new bug.
05:33hiroheycam: oh, hover...
05:33hiroheycam: OK, please file a new bug, it will take some time to investigate it.
05:34heycamhiro: there is a ::before rule with the animation, and there is a :hover ... ::before rule that sets its content
05:34heycamhiro: ok I will try to reduce the issue first, then file
05:34hiroheycam: great thank you! Reduced test case will be really helpful.
05:35heycamlucky Fastmail's code looks pretty clean :-)
06:20xidornheycam: ping
06:53heycamxidorn: pong
06:55xidornheycam: r? https://github.com/upsuper/servo/commit/0ef691561ce9853a1e5098cd63cc334df38c7086
06:57emilioxidorn: are we preserving just for serialization purposes? that size increase is unfortunate, but ok I guess :)
06:57heycamxidorn: seems fine
06:58xidornemilio: would it actually increase size of anything?
06:58heycamemilio: yeah, I did wonder how important re-serializing is. but I'm not sure exactly how people are using the selectors for feature detecting
06:58heycamand doesn't seem like a big deal to preserve it for serialization
06:58emilioxidorn: I don't think so, but we may clone pseudo-elements at some places
06:59emilioxidorn: still, these are probably unfrequent enough, I guess... We may want to split between PseudoElementType and PseudoElement, or something... No big deal I guess
07:00xidornheycam: I guess it's unimportant to keep that in terms of detection
07:01xidornbut we may need that anyway when we come to chrome support
07:01heycamxidorn: that's true
07:01emilioxidorn: just curious, whats the difference between r=foo and rs=foo?
07:02xidornemilio: rs means rubberstamp I believe
07:03xidornemilio: which means the person doesn't really review it, they just trust whatever it would be
07:06emilioxidorn: yeah, makes sense, thanks!
07:28Manishearthbholley: there?
07:30Manishearthemilio: ping?
07:30emilioManishearth: pong
07:31Manishearthemilio: AIUI in the New WOrld this call http://searchfox.org/mozilla-central/source/layout/base/ServoRestyleManager.cpp#502
07:31Manishearthshould be the new context created?
07:32emilioManishearth: no, that's the point of it, right?
07:32Manishearthemilio: ?
07:32Manishearthright, so we should not do the recreateContext stuff and just return that context, yes?
07:32emilioManishearth: the point of the fusing stuff is not having to create a bunch of style contexts wrapping ComputedValues
07:32Manishearthemilio: right
07:33emilioManishearth: well, you still need to do all the SetStyleContext stuff
07:33Manishearthemilio: but in this case there's an explicit recreateCOntext mechanism -- is it trying to mint new contexts on purpose?
07:33emilioManishearth: I'd just call it updateStyle instead of recreateContext
07:33Manishearthokay
07:33emilioManishearth: nope
07:33Manishearththis will be fixed in a different commit
07:33Manishearththanks
07:33emilioManishearth: it's just what it happens to be doing :)
07:33emilioManishearth: np
07:34emilioManishearth: presumably we can stop looking at the display: contents map too
07:34Manishearthwhere do we do that?
07:35emilioManishearth: http://searchfox.org/mozilla-central/source/layout/base/ServoRestyleManager.cpp#495
07:35Manishearthoh up there
07:36emilioManishearth: d'oh, though we don't have the _old_ style context... huh, that may be tricky
07:36Manishearthyeah
07:37emilioManishearth: probably worth keep looking at it for now, we can avoid it, but not worth to do that growing your refactor IMO.
07:37Manishearthyeah
07:38emilioManishearth: we basically need an outparam in Servo_TakeChangeHint that tells whether the style actually changed
07:38emilioManishearth: instead of doing that oldContext && oldContext != new
07:39emilioManishearth: mind filing a bug for this depending on your refactor?
07:43Manishearthemilio: i'm not clear on what you're trying to do
07:43emilioManishearth: avoid needing oldContext in the post-traversal?
07:44emilioManishearth: and using the display: contents map altogether, actually
07:45Manishearthemilio: I mean, can you file the bug -- it seems you have a clearer idea
07:56emilioManishearth: sg, I'm not at my computer r/n, but will do when I'm back
08:02Manishearththx
08:30heycamemilio: should I click autoland for the C++ side of your refactoring?
08:30emilioheycam: I'm on it :)
08:30heycamemilio: oh, it probably needs the servo patches pulled out
08:30heycamok :)
08:32emilioheycam: Should I just push it to autoland? I usually feel quite sad having to drop all the review history from the bug
08:32emilioMaybe it's fine
08:32heycamemilio: I mean, it will fail to merge if you don't take the servo bits out won't it?
08:33emilioheycam: right, I mean, I've got a local branch without the servo bits, already rebased on top of autoland
08:33heycamemilio: oh, sure, if you'd like to push to autoland directly, don't let me stop you ;)
08:33emilioheycam: but the question is whether to |git mozreview push + Automation button| vs just pushing
08:34emilioheycam: usually I'd do the first, but that'd drop all the patches from the bug :(
08:34heycamemilio: I know what you mean about the reviews disappearing. I think sheriffs probably prefer you go through mozreview, but I don't mind...
08:34emilioTomcat|Sheriffduty: Any strong opinion on ^?
08:35Tomcat|Sheriffdutyemilio: yeah mozreview would be cool :)
08:35Tomcat|Sheriffdutyemilio: we use this also for checkin-needed tasks
08:35Tomcat|Sheriffdutyetc
08:35emilioTomcat|Sheriffduty: ok, fair enough, will do. It's just somewhat sad that the reviews disappear from the bug when pushing the non-servo bits, but I guess it's fine
08:47Manishearthemilio: btw, do you think you could poke at the animations thing if you get time?
08:47ManishearthI have a sneaking suspicion it's a NonZero opt biting us somehow
08:48emilioManishearth: I could try, sure. Do you have a branch I can pull when I have the time?
08:49Manishearthemilio: fuse-next on my gecko-dev clone
08:49Manishearther, fork
08:49Manishearthalso im keeping the mozreview stuff up to date
08:50Manishearthemilio: if fuse-next doesn't work (I just rebased it and fixed review comments), try `fuse`
08:54Manishearthemilio: fwiw the testcase for it is a simple transition. i don't think any transitions are working
09:23Tomcat|Sheriffdutyemilio: heycam https://treeherder.mozilla.org/logviewer.html#?job_id=113585737&repo=autoland is fixed by the latest push from emilio or ?
09:24emilioTomcat|Sheriffduty: it should
09:24Tomcat|Sheriffdutyok thanks sir :)
09:24emilionp :)
09:33noxI think we should box the matrix3d() values.
09:54Tomcat|Sheriffdutyemilio: ++ you rock - the error is fixed
09:54noxTomcat|Sheriffduty: At first I parsed that as pros and cons.
09:54nox"Pros: emilio rocks. Cons: the error is fixed."
09:55Tomcat|Sheriffduty:)
09:57xidornahhh...
09:58xidorntest-tidy complains about trailing whitespace on file generated by mako...
10:01xidornand I don't even know where does the whitespace come from...
10:01noxWhat?
10:01noxWhere does it do that?
10:01xidornnox: do what?
10:01noxComplaining.
10:02xidornhttp://build.servo.org/builders/linux-dev/builds/8197/steps/test/logs/stdio
10:03xidornand the template is https://github.com/upsuper/servo/blob/3e35b61beda151a4e64e51335ff893b446f591f2/components/style/gecko/pseudo_element_definition.mako.rs
10:03noxWhy is this in components/ if this is generated would be my first question...
10:04xidornbecause it needs gecko code to generate
10:05noxxidorn: https://github.com/servo/servo/pull/17687/files#diff-a5e7c9672dae1a8704a8af566abcdf8f
10:05noxYou versioned it.
10:06xidornyes
10:06noxAnd indeed it's full of trailing whitespace, so I'm not sure I understand your complaint.
10:07xidornbecause it is generated
10:07noxSo?
10:09noxxidorn: Your pseudo_element_variant mako thing is the culprit.
10:10noxMake it a proper Python function.
10:11xidornok I eventually figured out that I can append a "\" at the end of line to strip the line break
10:12xidornso that it would produce pointless whitespace-only lines
10:12noxxidorn: Still think a Python function would be better, the generated code isn't very readable.
10:14xidornit is readable now
10:15noxCool.
10:17noxSimonSapin: r? https://github.com/servo/servo/pull/17662
10:17crowbotPR #17662: Improve derivation of ToCss again - https://github.com/servo/servo/pull/17662
10:17noxI added a bunch of commits.
10:18xidornok... one issue left...
10:18xidornline is longer than 120 characters...
10:18SimonSapinnox: this is typically where reviewable would help figure out whats already been reviewed :]
10:19noxHeh.
10:21xidornnox: in general, in match block, do we put "|" at the end of lines or the beginning?
10:22noxxidorn: End.
10:22noxBecause this isn't a ML language, where the initial pattern can be preceded by a |so that it looks cool.
10:22xidornk
10:22xidornthat makes sense
10:23xidornalthough if the initial pattern can be preceded by a | as well, that would be great
10:23noxI agree.
10:23xidornis there any rfc to add this?
10:24noxI don't think so.
10:24xidornit would also make generating code much simpler
10:24noxWhy?
10:24noxJust the patterns by " |\n", no?
10:24xidornbecause you can just use template, rather than a hand-written " | ".join
10:24noxJust join*
10:24xidornright, you have to write join
10:25xidornbut if you can precede every line with "|", you don't even need to write that function
10:25* xidorn you can just
10:25xidornwell...
10:26xidornnox: see for example https://github.com/upsuper/servo/blob/3e35b61beda151a4e64e51335ff893b446f591f2/components/style/gecko/pseudo_element_definition.mako.rs#L178-L179
10:27xidornyou would be able to just do
10:27xidornhttps://irccloud.mozilla.com/pastebin/QOccFfGP/
10:27xidorndoesn't it look much better?
10:27noxA proc macro looks even better
10:27noxWhat's a tree pseudo element btw?
10:28xidornsome gecko-specific xul thing...
10:28noxI'm surprised we have that many pseudo elements that just have a slice of strings as arguments.
10:28noxWell there is your ugliness. :)
10:28noxJust unship XUL!
10:28xidornI hate it
10:28xidornbut we cannot unship even this...
10:28xidornbecause this specific thing is widely used as a browser-sniffing hack :/
10:29noxAre builds currently orange?
10:29noxAsking because of https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea3f1c90e7afb195617ae81390631d101a24f806&selectedJob=113604431
10:29noxxidorn: I don't even
10:29noxWhy is XUL exposed to the Web?
10:29xidornnox: xul is not, but this specific pseudo-element is
10:30noxWhich one?
10:30xidorntree pseudo-elements
10:30xidorn:-moz-tree-*
10:30xidornit doesn't match anything
10:30xidornit just need to parse
10:31noxxidorn: And serialise back correctly, I assume?
10:32noxxidorn: Given we know we can't unship it, I assume we already tried in the past?
10:32xidornnox: I actually don't think we need to serialize back it...
10:32xidornnox: we already know it would break sites
10:32xidornand there has been at least one real site issue filed for stylo because of this
10:32xidornbecause of lack of this
10:33noxxidorn: If we don't need to serialise it back, we don't need to store the strings at all, do we?
10:33xidornnox: when we start needing to support chrome document, we need it anyway
10:34noxUgh.
10:35xidornnox: much readable now https://github.com/upsuper/servo/blob/97b2b95f99c97bf6c336b7bb345b35ba0b18a11b/components/style/gecko/generated/pseudo_element_definition.rs
10:35nox" "
10:35noxWE ARE ALL GO DEVS NOW
10:36xidornthat's why I said supporting preceding "|" would make it much better...
10:37noxxidorn: Not really, all the code in the file is overindented. :)
10:37xidornblame mako :)
10:38noxHeh.
10:38noxxidorn: Once I finally implement my ideas for sequential values, we will be able to derive that ToCss implementation.
10:39xidornI have a feeling that we probably want to apply rustfmt on generated files at some point
10:39xidornfor readability
10:39noxI want it to disappear and live where it belongs.
10:39Tomcat|Sheriffdutyerm heycam|away
10:39Tomcat|Sheriffdutybustage again
10:55noxSimonSapin: I think I have found a way to support #[css(identifier)],
10:55noxSimonSapin: which would work on both String, Box<str> and our two Atom types.
10:56noxSimonSapin: We just need to implement From<&Atom> for Cow<str>.
10:56noxSimonSapin: And do serialize_identifier(<::std::borrow::Cow<str> as ::std::convert::From<&_>>::from(&self.field))
10:57noxThis is just enough type annotation to correctly autoderef for fields of types String or Box<str>,
10:57noxbecause &String itself doesn&#39;t implement Cow<str>.
10:58noxActually <&_ as ::std::convert::Into<::std::borrow::Cow<str>>>::into(&foo) is better for full flexibility.
11:01noxI wonder why it works, though.
11:01noxeddyb: Can you explain that to me maybe?
11:02noxeddyb: Given foo: String, <_ as ::std::convert::Into<::std::borrow::Cow<str>>>::into(&foo) doesn&#39;t compile, but <&_ as ::std::convert::Into<::std::borrow::Cow<str>>>::into(&foo) does.
11:02eddybnox: sure?
11:02eddybnox: any type vs a reference to any type
11:02eddybnox: the latter is (far) more constrained
11:03noxeddyb: I expected &_ to only &quot;resolve&quot; to &String, which is the type of &foo here.
11:03eddybnox: why?
11:03noxDidn&#39;t expect autoderef to trigger here, that&#39;s why.
11:03eddybnox: <&_ as ::std::convert::Into<::std::borrow::Cow<str>>> will search impls
11:03noxThat&#39;s neat.
11:03eddybthe other one is too ambiguous
11:04eddybnox: I mean, the only information it has is that.... wait
11:04noxeddyb: https://is.gd/snA8Ml
11:04eddybnox: argh yeah okay, it&#39;s a typeck order annoyance
11:04noxIf you want a playground.
11:04eddyb&foo gives it &String whole
11:04eddybwhereas the other one first gives it &_ which is found to be &str *before* it sees &String
11:05noxeddyb: That&#39;s a bit weird, no?
11:05eddybnox: &quot;a bit&quot;. yes
11:05eddybnox: but expected given the &quot;design&quot; of typeck
11:05noxeddyb: Quoting because that&#39;s more than &quot;a bit&quot;?
11:06eddybnox: sure
11:06noxeddyb: Thanks for the explanation,
11:06noxeddyb: now go back to enum flattening. =)
11:09noxxidorn: https://github.com/rust-lang/rfcs/pull/1925
11:09crowbotPR #1925: Allow an optional vert at the beginning of a match branch - https://github.com/rust-lang/rfcs/pull/1925
11:09xidornnox: great :)
11:10noxxidorn: Time to land the RFC and switch Stylo to nightly when it gets implemented!!1!
11:10xidornyes!1
11:12noxxidorn: https://github.com/rust-lang/rfcs/pull/1925#issuecomment-293997978
11:12crowbotPR #1925: Allow an optional vert at the beginning of a match branch - https://github.com/rust-lang/rfcs/pull/1925
11:12noxCodegen is mentioned.
11:12xidornoh I see
11:13xidornI guess even in proc macro, you need to handle this kind of thing
11:13xidornto write a separate item before or after the loop
12:01ajais this best channel for discussion of stylo-specific css issues?
12:02noxYes.
12:03ajanox, tks. no real issue, other than
12:03aja::first-line, and there&#39;s already bug in works
12:09larsbergIs there any test for stylo apart from checking about:supports to make sure it&#39;s enabled? Working too perfectly; am suspicious.
12:09* larsberg flipped the css.servo thing and stylo says &quot;true&quot;
12:09noxlarsberg: It actually works well.
12:11mrobinsonThe latest rustup has a bug that causes servo to always immediately panic for me. Should I make a commit reverting the rustup until the fix is available in nightly?
12:11noxmrobinson: What bug?
12:12noxmrobinson: We don&#39;t need to wait for nightly to do rustups, but we do when some dependencies need code changes.
12:15mrobinsonnox: https://github.com/servo/servo/issues/17642
12:15crowbotIssue #17642: Servo fails with stacktrace for every website - https://github.com/servo/servo/issues/17642
12:15noxmrobinson: I mean, do you know if there is an upstream bug?
12:15noxOh there is one.
12:15noxAnd it was fixed.
12:15noxLet just me do a new rustup, I guess.
12:17mrobinsonnox: Yep: https://github.com/rust-lang/rust/issues/43102
12:17crowbotIssue #43102: Since last nightly, winit doesn&#39;t create windows any more - https://github.com/rust-lang/rust/issues/43102
12:17mrobinsonnox: Thanks!
12:23noxSimonSapin: Oh damn.
12:23noxerror: type `font_descriptor::__CTFontDescriptor` is private
12:23noxpls kill me already
12:25noxDo we have a patch already that bumps that in servo?
12:25noxmrobinson: I think we need https://github.com/servo/servo/pull/17649 first.
12:25crowbotPR #17649: [DO NOT MERGE] Upgrade to the latest version of WebRender - https://github.com/servo/servo/pull/17649
12:28mrobinsonnox: Oof. I&#39;m looking at the WebGL failures now...
12:29noxmrobinson: Rebuilding with your commit and the rustup.
12:43mrobinsonnox: I think I know what the issue is with the failing WebGL tests, but it is going to take another PR in WebRender.
12:44noxmrobinson: Well, we can wait a bit I guess.
12:44noxmrobinson: How long will it take to make that PR?
12:48mrobinsonnox: I&#39;m cooking it up now.
12:48noxmrobinson: Don&#39;t put too much oil, nor salt.
13:08noxmrobinson: I would prefer if gw or kvark reviewed it.
13:08mrobinsonnox: Okay. No problem!
13:16cynicaldevildoes an mpsc channel receiver get the messages in the order that the Sender sends them?
13:19cynicaldevilnox: ^
13:30noxcynicaldevil: For a given Sender, yes.
13:30noxcynicaldevil: For concurrent threads sending stuff, I don&#39;t know.
13:31cynicaldevilnox: What do you mean?
13:46cynicaldevilnox: ah, do you mean if more than one Sender is involved?
13:47noxcynicaldevil: Yes.
13:48cynicaldevilnox: Yeah, they don&#39;t necessarily arrive in order then. That&#39;s why my tests failed ;_;
14:02avadacatavrastandups: wrote openssl and rustls benchmarks
14:02standupsOk, submitted #48130 for https://www.standu.ps/user/avadacatavra/
14:09noxhttp://propertesting.com
14:09noxI want all of this but for Rust.
14:29tillat least it&#39;s obvious that this is a stylo crash https://crash-stats.mozilla.com/report/index/59eeae74-4632-4229-b22e-c20850170712
14:31tillnox: you mean like https://github.com/BurntSushi/quickcheck or does PropEr something fundamentally better still?
14:33noxtill: AFAIK PropEr&#39;s generators are significantly better.
14:34noxYou can have custom ones and shrinking will still work without you needing to implement support for it, for example.
14:34noxIt&#39;s also too tied to types, as far as I can see.
14:35noxtill: See ?FORALL in http://propertesting.com/book_custom_generators.html#_limitations_of_default_generators
14:35tillnox: heh, I&#39;m looking at that right now
15:03noxjdm: I was on rustup already.
15:03noxjdm: It requires WR bump.
15:03jdmah, ok
15:03noxjdm: mrobinson is on it.
15:04jdmwait, why does it need a WR bump?
15:04noxjdm: Because it needs a core-text bump.
15:04noxjdm: Because a warning became a hard error.
15:04jdmturtles all the way down
15:04noxjdm: Fun time to be alive, right?
15:05noxjdm: The best part is that mrobinson needed rustup to investigate WR errors,
15:05noxWR errors occurring in WR bump required for rustup.
15:05mrobinsonI was touching all these things without knowing about this weird dependency situation.
15:06noxmrobinson: That happens, sometimes.
15:07mrobinsonThe current status is that the WR fix has been approved and I&#39;m just waiting for it to land in order to send the Servo WR update to the bots again.
15:10larsbergjdm: I prefer this version of the &quot;turtles&quot;: https://twitter.com/griggheo/status/884937997375881216
15:10jdmhahaha
15:10noxAnd you can&#39;t find it because it probably starts with a dot for no good reason.
15:11noxmrobinson: Is Travis being slow to land it?
15:11mrobinsonnox: Yes, I think so.
15:14mrobinsonnox: For some reason travis seems stalled here: https://github.com/servo/webrender/pull/1466
15:14crowbotPR #1466: Only convert scroll id to nested if pipeline id is match. - https://github.com/servo/webrender/pull/1466
15:15noxIt was still ongoing
15:15noxhttps://travis-ci.org/servo/webrender/builds/252801456
15:33noxjdm: Do you have any ideas about whether messages are supposed to be received in the order they are sent in ipc-channel?
15:33jdmnox: they are
15:33noxcynicaldevil: ^
15:34cynicaldevilThe oracle has spoken \o/
15:34cynicaldeviljdm how do you know so much about everything
15:34jdmha
15:34jdmnecessity
15:47mrobinsonnox: I guess the slowness is just general slowness with the Mac bots.
15:49larsbergthere was another network outage around travis mac stuff https://www.traviscistatus.com/
15:49larsbergprocessing backlog
16:18Manishearthjyc: FireBlood canaltinova elucas zhitingz http://githubfieldday.com/
16:18Manishearthno idea what it&#39;s going to be like, but it looks fin
16:18Manishearth*fun
16:18Manishearthand the last time i was at a github unconference it was great
16:45elucasManishearth: submitted! Not sure if I&#39;ll be around though
16:46marti_why servo return Servo exited with return value -4
16:47jdmmarti_: probably https://github.com/servo/servo/issues/17642
16:47crowbotIssue #17642: Servo fails with stacktrace for every website - https://github.com/servo/servo/issues/17642
17:00marti_I see :D
17:00marti_ it wil fix
17:00marti_jdm: https://github.com/servo/servo/issues/17661
17:00crowbotIssue #17661: Make CanvasPaintState::new accept an AntialiasMode enum argument instead of a boolean - https://github.com/servo/servo/issues/17661
17:01marti_how to I can get started
17:02fitzgenemilio: ping
17:21FireBloodManishearth : not sure if I&#39;m free on that day. Will register though :)
17:46jycManishearth: sounds interesting! not sure if I&#39;m a student leader :P
17:47jyccanaltinova: lol thanks again! :P
17:47elucasI used the 100% not academic club that I run as proof that I&#39;m a student leader. Not sure if they&#39;ll accept that haha
17:47canaltinovajyc: np :)
17:47canaltinovajyc: btw, me neither but registered anyway :p
17:48jychahaha nice
17:48jycok I will register anyhow then :P
17:48canaltinova:)
17:48elucasworst they can do is say no :P
17:50SimonSapinjdm: if we return PropertyDeclarationParseError::InvalidValue(&quot;${property.ident}&quot;.into()) for invalid property values, why do we bother with error types other than () in value parsing?
17:51jdmSimonSapin: so I was just looking into returning more meaningful errors for parsing invalid color values
17:51SimonSapinjdm: I mean in servos PropertyDeclaration::parse_into
17:51Manishearthjyc: i think it&#39;s just &quot;students who do open source stuff&quot;
17:51jdmSimonSapin: I know
17:51jdmSimonSapin: the point is that InvalidValue is going to become more complicated
17:52SimonSapinjdm: is the plan to eventually replace that `Err(_) =>` pattern with something that uses `_`?
17:52jdmSimonSapin: that&#39;s what it looks like
17:52SimonSapinok
17:54jycManishearth: haha :P
17:59bholleybz: ping
17:59bzbholley: ack
18:00bholleybz: quick question - trying to decide on the best approach in bug 1380106
18:00firebothttps://bugzil.la/1380106 NEW, bobbyholley@gmail.com stylo: Crash in do_QueryFrame::operator<T> nsIAnonymousContentCreator*
18:00bholleybz: given the choice, should I opt to do more or less teardown work in FragmentOrElement::DestroyContent?
18:00bholleybz: i.e. if I have some things that I can drop (like the servo data) which might reduce cycles, is it preferable to drop them there?
18:01bholleybz: or should I take heycam&#39;s suggestion, and check mIsGoingAway in SetBindingParent instead?
18:01bholleybz: (and let nature take its course on the mServoData eventually)
18:02bholleybz: (it&#39;s also not clear to me if DestroyContent is something we eventually want to get rid of, and whether I should avoid adding more to it)
18:03bzIt&#39;s kinda a hack
18:03bzbut one that&#39;s been hard to remove for various reasons
18:04bzSorry, checking something
18:04bholleynp
18:05bzSo the answer to the question you actually aked is &quot;I&#39;m not sure&quot;
18:05bzer, asked
18:05bzBut...
18:06bzWe should never be in a situation where we destroyed the arena but did not null out the frame pointer
18:06bzFurther, it&#39;s not clear to me how the attached patch helps
18:07bholleybz: why?
18:07bzWill we not still call ClearServoDataFromSubtree ?
18:07bzunder SetXBLInsertionParent
18:08bholleybz: no
18:08bholleybz: http://searchfox.org/mozilla-central/rev/5dadcbe55b4ddd1e448c06c77390ff6483aa009b/dom/base/FragmentOrElement.cpp#1220
18:08bholleybz: because it checks HasServoData
18:08bzAh, because it skips it if we have no servo data...
18:08bholleyright
18:08bzAnd we don&#39;t need to do it on the descendants because...
18:08bholleybz: do what
18:08bzclear servo data
18:09bholleybz: because DestroyContent is going to do it anyway
18:09bzDestroyContent only walks the light DOM
18:09bholleybz: and more to the point, because nobody cares about the style data being stale at this point
18:09bznot anon content
18:09bzOK
18:09bzAh, so this is not about style bits at all
18:09bholleybz: but anon content is gone at this point, right?
18:10bholleybz: or rather, the XBL anon content will go away in the nsBindingManager stuff
18:10bzok
18:10bzmmhm
18:10bzI still feel like this is wallpapering something
18:10bholleybz: and it&#39;s not clear to me whether this happens before or after presshell teardown?
18:10* bz would really like to understand what
18:11bzbholley: That part I can answer; one sec
18:11bzbholley: http://searchfox.org/mozilla-central/source/layout/base/nsDocumentViewer.cpp#1789-1795
18:11bzbholley: So after
18:11bholleybz: right, so the frames should all be gone by that point, right?
18:12bholleybz: in which case you&#39;re saying that the frame pointers on the elements should have been nulled out?
18:12bholleybz: because the crash stack indicates contrary
18:12bzright, I see that. ;)
18:12bzone sec
18:16bzI mean, my most plausible hypothesis is that there is no primary frame but we do have a mSubtreeRoot
18:16bzbut mess up and think we have an mPrimaryFrame
18:17bz(Not that I have a hypothesis for how this could happen)
18:17bzDo we have str?
18:17bzAs in, can we actually reproduce and take a look at what&#39;s going on?
18:44bholleybz: (sorry, missed your message)
18:44bholleybz: we don&#39;t have STR
18:44bholleybz: I think we should land the fix though, one way or another
18:45bholleybz: it may wallpaper an issue, but the issue may not impact us with sufficient wallpaper
18:45bzok
18:45bholleybz: which fix do you prefer?
18:46bzI worry that if, as it seems, we have an invariant violation here
18:46bzthat will just keep popping up to bite us
18:47bholleybz: well, this is an invariant violation during teardown
18:48bzYes, but this invariant is expected to hold during this teardown
18:48bholleybz: and possibly a pre-existing one - it may just be that nobody else but the stylo code tries to access this state after presshell teardown
18:48* bz is really tempted to try accessing it and seeing what happens
18:50bholleybz: you&#39;d think we&#39;d have seen this on try,...
18:53bholleybz: I don&#39;t really have a lot of time to spend chasing this though
18:54bzbholley: ok. I think the fix in the bug is ok, but I&#39;m going to try to reprodue the fail locally...
18:54* bz is really worried about this one
18:54bholleybz: ok, if you&#39;re investigating, I&#39;ll hold off
18:55* bholley NIs bz in the bug
19:08SimonSapinbholley: Im now looking at with bloaty the VM size of target/geckolib/release/libgeckoservo.a (to eliminate debug info). My first patch shaves 10 KB
19:08SimonSapinbholley: Though that patch is mostly to enable further changes, hopefully those will have more impact
19:09bholleySimonSapin: sounds great - are you looking at the things that dmajor pointed us to?
19:11SimonSapinbholley: do you mean bug 1377262? Yeah, Im several yaks deep to eventually fix that
19:11firebothttps://bugzil.la/1377262 NEW, simon.sapin@exyr.org substitute_variables_slow functions generate huge amounts of code
19:11bholleySimonSapin: sounds great
19:11SimonSapinbholley: and cut code size in other places along the way
19:11SimonSapinlike parsing
19:11bholley\o/
19:12bholleydefinitely glad we have a path forward there and are moving on it
19:15SimonSapinbholley: any idea if [u32; _] or [u8; _] is preferable for bit maps?
19:15bholleySimonSapin: you mean as the backing store?
19:16bholleySimonSapin: I&#39;d think either u8 or usize, depending on whether we want to optimize for space or alignment
19:16SimonSapinstruct LonghandIdSet { storage: [u32; ...] }
19:17* bholley wonders why we&#39;re using u32
19:17SimonSapinbholley: is a word-aligned word-sized load any different from a single-byte load for extracting a single bit?
19:17bholleySimonSapin: I don&#39;t think so
19:18SimonSapinok, so probably doesnt matter
19:18bholleySimonSapin: I think the alignment only matters if you want multiple bytes
19:18bholley(which could fall across alignment boundaries)
19:19SimonSapinLonghandIdSet is only ever manipulating a single bit at a time
19:33jycemilio: rip thanks again!
19:37emiliojyc: np!
19:38emiliobholley: reviewing your patches now, sorry for the lag... what clears the cache after a restyle?
19:38bholleyemilio: the drop impl I added?
19:39emiliobholley: isn&#39;t it dropped when the AtomicRefCell is dropped?
19:39* emilio takes a closer look
19:39* bholley isn&#39;t sure what emilio means
19:40emiliobholley: err, nvm, i misread the atomic refcell, thinking you used AtomicRefCell<TypelessCache> directly
19:40bholleyemilio: k
19:40bholleyemilio: re: SendElement, not sure it&#39;s worth a comment, since the fact that we needed it before was just an artifact of the ScopedTLS weirdness
19:40bholleyemilio: it&#39;s conceptually thread-local
19:41emiliobholley: I guess... it&#39;s still weird having that and all the TypeLess thing... I&#39;d find it quite confusing if I was reading that code for the first time
19:41bholleyemilio: I&#39;ll try to add some more comments around it
19:42bholleyemilio: and in terms of adding comments to get rid of stuff - I&#39;m not sure we&#39;ll ever be able to get rid of the bloom filter thing - it&#39;s still an allocation where we require zeroed memory, regardless of the memmoves
19:42bholleyemilio: I can comment for the sharing cache though
19:43emiliobholley: should be a calloc() not malloc() + memmove()... I think conceptually it doesn&#39;t make a lot of sense to just have a StyleBloom per thread
19:43bholleyemilio: we still have to poison the memory
19:43emiliobholley: it&#39;s quite unintuitive
19:43emiliobholley: and, we&#39;re basically leaking it now, which seems also unfortunate...
19:44bholleyemilio: why is that a problem?
19:44emiliobholley: and for the whole process lifetime... i guess 4k is not a big deal
19:44bholleyemilio: yeah, I mean we style pretty often during the lifetime of the browser
19:44bholleyemilio: I think doing the allocation once and keeping it around is definitely worth it
19:45bholleyemilio: I _do_ with that the ergonomics of the reuse were better type-system-wise
19:45bholleys/with/wish/
19:45emiliobholley: yeah, I see why, but... IDK. At least comments pointing to the issues that made us do this this would be nice
19:45bholleyk
19:45* emilio still would like to get rid of it eventually though... or make it more elegant at least
19:47SimonSapin9.56Mi, 9.55Mi, 9.51Mi. Im starting to like this trend
19:56emilioSimonSapin++
20:21emiliostshine: just took a look at your <circle> PR :)
20:22poesiajdm: found the source of intermittence: https://github.com/servo/servo/issues/13149
20:22crowbotIssue #13149: Frame tree contains iframes that are hidden / not visible. - https://github.com/servo/servo/issues/13149
20:23jdmoh really?
20:23poesiait turns out the second load event triggered a reflow in the window which eventually made the compositor call webrender.update() and made it aware of the new pipeline
20:24poesiaI was suppressing the event too early; to fix it I had to stop it in the last moment, and let this reflow happen. not sure if it&#39;s the best solution
20:24jdmpoesia: huh, even though the iframe in layerization_layer_size.html should only ever load about:blank once?
20:24poesiajdm: oh yes, it depended on the second (duplicated) load event triggering this reflow :P
20:25jdmnice
20:26poesiapushing it now, but I wish there&#39;s a nicer way than what I did (following the whole path the load event makes through the ping pong between event loops)
20:29emiliojyc: if you&#39;re blocked on PR reviews, please let me know, I&#39;ll happily review or find a reviewer :)
20:29SimonSapindownside of working on code size: I need to do full release builds to measure the effect of my changes, rater than the usual &quot;cargo check&quot;
20:30emilioSimonSapin: yeah, and they&#39;re _so_ slow (at least on my machine... maybe you use an external server or something fancy in the office?)
20:31bzExternal server doesn&#39;t help
20:31bzbecause the slowness is all single-threaded. :(
20:31SimonSapinemilio: I have a fast desktop where `mach build-geckolib --release` after touching style takes a bit over 4 minutes
20:32SimonSapinbz: you can still get better single-thread perf than on a laptop
20:32bzsounds about right
20:32bzMaybe
20:32bzI get about 4.5 minutes on an mbp last I measured
20:32bzAdmittedly, worse than 4
20:32SimonSapin4.6 GHz definitely helps, compared to my laptop at least
20:36SimonSapinalso keeping GNOME and Firefox on the laptop responsive no matter how busy the desktops cores are
20:37waffleshello JEM
20:37jemaloha
20:39jycemilio: thanks! Yep, been working on the main intern project but figured I should try to get these landed. Thanks!
20:39KWiersojyc: ping
20:39jycKWierso: hello!
20:40KWiersojyc: it looks like PR 17538 caused some unexpected passes when it merged to autoland https://treeherder.mozilla.org/logviewer.html#?job_id=113763528&repo=autoland
20:40KWiersoare you in a position to land an expectations update or shall I?
20:41jycKWierso: agh sorry, thought I had caught them all. I am not at my desk right now, if it is not too much trouble could you update them? Otherwise maybe you could back it out or I could do it soon
20:48noxmrobinson: Did the WR bump land?
20:52KiChjangwho would know best how webrtc works here?
20:53KiChjangor should i find someone else in another channel?
20:53jycKWierso: back at desk, can make expectations updte
20:53jycupdate*
20:53KWiersojyc: about to push it myself
20:53jycKWierso: oh, thanks! sorry
20:59canaltinovawaffles, Manishearth: well, it appear