mozilla :: #gfx

17 Jul 2017
05:46pulsebotCheck-in: - Matt Woodrow - Restore more builder state between paints when we reuse it
08:24pulsebotCheck-in: - Matt Woodrow - Fix build error
09:13pulsebotCheck-in: - Miko Mynttinen - Prune old items correctly when reusing the previous display list
18:29Gankrokvark: quick r? -- fixes test suite
18:56rhuntmstange: ping
18:56mstangerhunt: pong
18:56rhuntmstange: does perf-html use 3D css transforms?
18:57rhuntmstange: bug 1378495
18:57firebot NEW, Move to new window causes graphics corruption
18:57mstangerhunt: no
18:57mstangerhunt: the corruption in the first screenshot seems to be on a canvas
18:58mstangerhunt: yeah it seems to be using the wrong texture for canvases somehow
18:59rhuntmstange: interesting. I can't reproduce it (iMac), can you?
19:03mstangerhunt: I can! not with those exact steps to reproduce, but I'm seeing the issue now
19:09mstangerhunt: the nightly I reproduced in was from July 5th
19:09mstangerhunt: I wasn't able to reproduce it in the July 5th nightly
19:09mstangemaybe it was fixed in the meantime
19:11rhuntmstange: you weren't able to reproduce it since the july 5th nightly?
19:11mstangerhunt: I only tested two nightlies, 5th and 15th, and I was only able to reproduce in the former
19:11mstangerhunt: I'm trying to do a full mozregression --find-fix run now
19:12rhuntmstange: okay, I'll needinfo to see if he can still reproduce
21:00Gankrogw: do you have an implementation of your text-decoration API for servo ready to go / should I handle it / what's the situation ?
21:06gwGankro: I don't have an implementation, yet - I was planning to start building it today on top of your line display item work - does that work for you?
21:17Gankrogw: less work for me sounds great! :)
21:17Gankrogw: I was poking at the implementation but I was doubting whether the abstraction is right
21:18Gankroas-is LineDisplayItem kinda expects a hybrid between logical and physical dimensions, it's a bit odd
21:18gwGankro: ok! If you do want to get stuck into working on some of the WR backend stuff, let me know - happy to help out with either this task or finding something else involving backend work :)
21:18gwGankro: I'll take a look at that LineDisplayItem PR shortly and comment in it.
21:19Gankrogw: ah sorry I was talking about the Servo half of the impl
21:19jrmuizelrhunt: ping
21:19gwGankro: ah right, I don't mind doing that or leaving it to you - either is fine with me.
21:19rhuntjrmuizel: pong
21:20jrmuizelrhunt: so I was exposing new functions from rust
21:20jrmuizelrhunt: first thing, it seems like if the types in the signature are not convertable to C we omit the function declaration correct?
21:21rhuntjrmuizel: yes, that's how I remember it
21:21jrmuizelrhunt: would it be practical to emit a warning in that case?
21:22jrmuizelrhunt: it took me a bit to figure out why my functions weren't coming out
21:22gwGankro: Do you mean the abstraction of a LineDisplayItem doesn't seem right?
21:23Gankrogw: yes -- basically at the point where servo produces its display items for text, it does a conversion from logical to physical units, but the API I have designed is a hybrid -- it assumes you've resolved left-vs-right, but not horizontal vs vertical
21:23rhuntjrmuizel: yes, that would be nice
21:24Gankrogw: it's not hard to get the right result, I'm just wondering if this is a sign the API should change
21:24jrmuizelrhunt: second: say I want to expose a function that takes a Vec by reference
21:24rhuntjrmuizel: we do log it, but it's under 'info', but in this case it should be a warning
21:24Gankrogw: what format does the backend really want?
21:24jrmuizelrhunt: what's the best way to make the Vec opaque?
21:25gwGankro: it really just wants a local-space rectangle to draw, and a local-space direction / axis to draw the dots / waves along that rectangle. local-space meaning the same local space as other display items (e.g. for text, images etc).
21:26rhuntjrmuizel: Vec<T> should be opaque, but the current version of cbindgen can&#39;t handle generics unless they&#39;re behind a type alias
21:26rhuntjrmuizel: so you can do type FooVec = Vec<Foo>
21:26jrmuizelrhunt: my vec is concrete
21:26Gankrogw: I expect wavy-ness will be a special case here?
21:26jrmuizelrhunt: ok, I&#39;ll try that
21:26gwGankro: how so?
21:27Gankrogw: the thickness of the line isn&#39;t the full height of its rect
21:27jrmuizelrhunt: how do I see the log messages?
21:27GankroAlthough I think there&#39;s a linear relationship
21:27rhuntjrmuizel: add &#39;-vv&#39; or &#39;-v&#39;
21:28rhuntjrmuizel: it&#39;s very verbose, it could be improved a lot
21:28nicalgw, kvark: when would be a good time to vidyo about APZ ?
21:28gwGankro: right, I figured in the style enum it might be Wavy(f32) where that is the thickness, or something like that
21:28gwnical: you are Paris time zone, right?
21:29nicalI am, it&#39;s okay if I have a call at midnight every once in a while
21:29Gankrogw: right now the &quot;width&quot; is intended to be that thickness param -- the client has no idea what the bounding rect of the wavy decoration is
21:29gwGankro: ok. I should probably actually go read the PR first :)
21:29gwnical: kvark:
21:30Gankrogw: sure, I&#39;m gonna head home then. Will be online all night if you have questions.
21:30gwGankro: sounds good, thanks!
21:30kvarknical: wow, that&#39;s going to be fun
21:31kvarkI&#39;m pretty much booked for tomorrow, and have an important call on Wed at 5pm. Any other time on Wed is fine by me. Maybe we can even do something that is early morning for you?..
21:31nicaltomorrow at 5pm toronto time ?
21:31gwkvark: nical: bne 5am toronto 3pm paris 9pm - does something like that work?
21:31nicalI won&#39;t be available thursday and wednesday
21:31nicalor we could improvise it now if you are not busy
21:33kvarkif you can&#39;t do Wednesday/Thursday, now would be best
21:33gwI can&#39;t do it now :/
21:33kvarkgw: are you available now?
21:34gwHow about Sat BNE time, which is friday toronto / paris time?
21:35nicalcan&#39;t do friday
21:36nicalactually I can this thursday
21:37nicalnext week monday to thursday works too
21:37kvarkawesome, I only have 1pm to 2pm booked on thursday
21:39nicalgw: would this thursday 5pm toronto time work for you?
21:40gwnical: That&#39;s 5am brisbane time on Wed, right? if so, yep that is good for me
21:40gwerr, 7am that is
21:41gwnical: kvark: any time +/- 2 hrs of then is fine for me
21:42nicalI think that it would be friday 7am in Brisbane
21:43gwnical: err, yes, of course - that&#39;s fine with me
21:43nicalI have until then to figure out how to get vidyo to function on linux
21:43* gw can work out vidyo on linux but not timezones
21:44jrmuizelrhunt: with type VecU8 = Vec<u8>. I&#39;ll get &#39;WARN: specializing VecU8 failed - (couldn&#39;t find aliased type&#39;
21:44jrmuizelrhunt: and I don&#39;t get an opaque type output
21:46rhuntjrmuizel: hm, that&#39;s because Vec is in the stdlib and we don&#39;t actually parse that
21:47jrmuizelrhunt: is there some way I can just say &#39;please make it opaque&#39;?
21:48rhuntjrmuizel: unfortunately not, but I think struct VecU8 { data: Vec<u8> } would work
21:48jrmuizelrhunt: I&#39;ll give that a try
21:52rhuntjrmuizel: I have some cbindgen commits that allow using generics anywhere, I can see if they can improve this situation. I can also add some builtin definitions for things like Vec
21:52jrmuizelrhunt: that would be great
23:17mattwoodrowbirtles: ping
23:17birtlesmattwoodrow: hi
23:18mattwoodrowbirtles: Im trying to debug a failure in web-animations/stacking-context-opacity-changing-target-in-delay.html with retained display lists
23:18mattwoodrowIt looks like we only issue the nsChangeHint_UpdateOpacityLayer for the new element, and not for the old element when we change the target
23:19mattwoodrowFor the old element, nsDisplayOpacity::ShouldFlattenAway returned true, so RestyleManager::AddLayerChangesForAnimation/GetDedicatedLayer cant find the old layer (since there wasnt one)
23:20mattwoodrowand we dont have anything to compare animation generations on
23:21birtlesmattwoodrow: ah right -- RestyleManager::AddLayerChangesForAnimation was only added to deal with certain cases of transform animations
23:22birtlesmattwoodrow: hiro wrote that test by the way so he might have some ideas
23:22birtlesbut I don&#39;t get what you mean by there wasn&#39;t a layer for the old element?
23:23mattwoodrowUltimately, for retained-dl, I need a SchedulePaint call for both the old and new, since they are both changing their display list, and as it is, we only get it for the new element
23:24mattwoodrowbirtles: I mean FrameLayerBuilder::GetDedicatedLayer returns nullptr
23:25hiroso, we need nsChangeHint_UpdateOpacityLaye for the old element, right?
23:25birtlesmattwoodrow: oh ok -- until now we&#39;ve only worried about making sure animations get updated on layers
23:25mattwoodrowEven though we did build nsDisplayOpacity previously (and now wont, since the animation moved to a different target)
23:25mattwoodrowhiro: right
23:25hiroas far as I can tell we don&#39;t do such things at all for now.
23:25birtlesright, we&#39;ll need to add something for that
23:25hiroespecially the opacity animation is in delay phase.
23:26birtleswe have EffectCompositor::RequestRestyle which takes different restyle types like RestyleType::Layer
23:26hiroespecially *when*
23:26birtlesperhaps we need to add another type there to trigger a call to SchedulePaint -- or perhaps it makes sense to do that as part of the RestyleType::Layer handling
23:27mattwoodrowI would have expected the first condition in AddLayerChangesForAnimation to have caught this
23:27mattwoodrowbut nsDisplayOpacity::ShouldFlattenAway breaks GetDedicatedLayer
23:28birtlesYeah AddLayerChangesForAnimation was only added because we had some optimizations for transforms where if the transform didn&#39;t change, we wouldn&#39;t updated layers
23:28hirooh wait, we do it.
23:29birtleshiro: I think Matt&#39;s saying that even though we update the animation generation there, it doesn&#39;t get picked up in AddLayerChangesForAnimation because there&#39;s no layer
23:30mattwoodrowAny time we change whether we build an nsDisplayOpacity, we need to have UpdateOpacityLayer (or just RepaintFrame etc)
23:30mattwoodrowand checking the old state using Layers isnt quite the same condition
23:31hirobirtles: ah, ok. but I don&#39;t quite understand why we have no layer for the old element at the moment.
23:32mattwoodrowhiro: nsDisplayOpacity::ShouldFlattenAway returned true
23:33mattwoodrowI think the difference is that ShouldFlattenAway checks HasAnimationsForCompositor, but nsIFrame::HasOpacityInternal uses nsLayoutUtils::HasAnimationOfProperty
23:33hiromattwoodrow: ah, ok. I think I understand.
23:34mattwoodrowseems like IsTooSmallForActiveLayer could also cause those two checks to diverge
23:38hiromattwoodrow: Ok, the reason I guess is that, once we change the target, we remove the old target from EffectSet.
23:39hiromattwoodrow: And after that, various checks that use EffectSet does not work, I think.
23:41mattwoodrowhiro: What would call SchedulePaint if they did work?
23:43hiromattwoodrow: I think it should work, but I am not sure it the right thing to do. it makes all compositor animations on the compositor pull on the main thread.
23:44birtlesanimation code never directly calls SchedulePaint at the moment -- all it does it update layer generations and trigger restyles (which may, in turn, call SchedulePaint)
23:46birtlesperhaps in this test case the computed style of the opacity hasn&#39;t changed by the time we change target so the restyle doesn&#39;t trigger a call to SchedulePaint
23:46mattwoodrowRight. It seems to me we either need nsStyleDisplay::CalcDifferences to take animations into account, not just mOpacity, or have a way to check the generation change on the animation without going via Layers
23:47mattwoodrowThe computed style hasnt, mOpacity is always 1. But the stacking context/nsDisplayOpacity is conditioned on mOpacity+animation state
23:47mattwoodrowand we need to do something if the latter changes
23:49birtlesyeah -- we store the animation generation on the pres context -- I wonder if we can detect changes there and trigger the call to SchedulePaint
23:50birtlesmaking nsStyleDisplay::CalcDifferences incorporate animation state seems like a bit of a layering violation
23:51mattwoodrowIdeally wed call SchedulePaint on the affected frames, so detecting on the pres shell might be a bit broad
23:55mattwoodrowbirtles: It seems like we need to know what animation generation was used for a given nsIFrame, without looking it up on a Layer
23:56birtlesmattwoodrow: ok, makes sense
23:56mattwoodrowNot sure how to do that though
18 Jul 2017
No messages
Last message: 67 days and 4 hours ago