mozilla :: #gfx

13 Jul 2017
02:48gwGankro: around?
02:49Gankrogw: pong
02:49gwGankro: just looking at your PR for the servo side of text shadow - - it looks like it doesn't add the actual normal text display item in the case of a shadow being present?
02:50gwGankro: I assume the intention was that the DL builder would add a normal text item, right?
02:50gwGankro: or did you intend for items in a push/pop text-shadow to also draw a normal text run?
02:50Gankrogw: ?
02:51Gankrogw: and
02:51gwGankro: right - but if there are shadows in that loop, there's no "normal" text run added
02:52gwGankro: shouldn't there be another call to add the text run without shadows?
02:52gwGankro: unless I'm misunderstanding the intended semantics of push/pop text-shadow, perhaps
02:52Gankrogw: oh I see -- yeah I assumed anything inside a shadow was itself rendered
02:52gwGankro: ok, that seems fair. I'll update WR-side to do that, then it should all be happy :)
02:53gwGankro: that will be a simple change, thanks
02:53Gankrogw: the "only shadow" semantic is easily constructed with transparent content
02:53gwGankro: yep, that seems reasonable, simplifies the common case
06:08pulsebotCheck-in: - Miko Mynttinen - Invalidate nsMathMLmactionFrame when attribute changes
15:10Gankrokvark: if you run ./ png reftests/text/shadow-complex.yaml the result isn't right -- it sort of looks like the glyphs are being placed at the same place, but the clipping box is being offset? So offsets just clip the text...
15:12Gankrokvark: ^ here I cranked up the offset on the red shadow to 10x10
15:16GankroAdjust the bounds on the shadow seems to do nothing
15:19kvarkGankro: filed a bug on it yet?
15:19Gankrokvark: I just left some comment here so far:
15:19GankroI wasn't sure if it was me or webrender making a mistake
15:19Gankrolooks like webrender now
15:22Gankrokvark: filling now
18:13Gankrokats: adusting some text expectations -- found a skiaContent&&webrender, not sure if the results I'm seeing have skiaContent active, under what contexts is it?
18:15katsGankro: looks like that was added as part of a skia update
18:15katsin bug 1340627
18:15firebot FIXED, Update skia to version 59
18:16katsso presumably the reftest was failing with the skia update and so i made the fuzzy-if condition webrender&&skiaContent to narrow it in scope (and since the case in question was obviously due to skia)
18:17katsGankro: but i guess to answer your question, you can check the logfile to see what's active or not
18:17katsthe reftest sandbox contents should be dumped out in the log file
18:19Gankrokats: ah, it gives the bounds in the full log, thanks!
18:19Gankrokats: should I keep this one "fully fuzzy" or make it strict?
18:19katsGankro: i prefer making strict wherever possible
18:19Gankrokats: ok, will do!
18:23Gankrokats: also, do you know how to kick off an "only quantum render" try? The syntax chooser doesn't seem to have that
18:24Gankrokats: hah, perfect thanks!
18:24pcwaltonnical: for what its worth, I thought about doing Loop Blinn yesterday, but I couldnt figure out how to do it without regressing antialiasing quality over the Raph Levien algorithm
18:24pcwaltonthere may yet be a solution but itll either involve some weird hacks or Greens Theorem
18:54Gankrokats: rebasing stuff one of the lines is haunted and it's impossible for me to properly move it to the right commit -- fine with you if one "fails-if" ends up in my fuzzing commit?
18:55katsGankro: that sounds like a challenge
18:55katsGankro: are you using git or mercurial?
18:57Gankrokats: git -- but a gui -- basically it's in the middle of a bunch of changes, and if I try to pull the line out my tool ends up rearranging the nearby lines from 1,2,3,4 to 3,1,2,4. The second commit then puts it back as 1,2,3,4
18:57GankroI suppose I might be able to get it work with a proper rewrite
18:58katsGankro: that's bizarre. can you use rebase -i?
18:58Gankrokats: yeah that's what I'm gonna try
19:01Gankrokats: ok yeah that worked. stupid tools
19:12Gankrokats: ok rebased and pushed to mozreview -- haven't finished try build, might be a syntax error/typo:
19:14katsGankro: put the link to the try push in the bug please. it's good to have there, makes life easier for sheriffs if failures pop up later too
19:14Gankrokats: is there a fast way to do a full syntax check locally?
19:15katsGankro: for reftest.list files? not that i know of
19:15katsyou could run a reftest which should do some syntax checking but not enough
19:16jgilbertWhy do we require that rgbx has x=255? jrmuizel?
19:16jgilbertwhat's even the point in rgbx then :(
19:16jrmuizelavoiding blending to composite it
19:17lsalzmanrgbx is an optimization for skia
19:17lsalzmani've argued with skia team before about having a true rgbx format, but they were not in favor
19:18jgilbertthat's rgba+opaque flag, not rgbx :\
19:18jgilbert*sigh*, ok
19:19lsalzmanthe idea has been floated of just somehow replacing skia, but that is pie-in-the-sky
19:27katsGankro: so in part 3 you add a fails-if in layout/reftests/w3c-css/failures.list, and then in part 5 you remove it (along with some other nearby fails-ifs). seems like you could just not add it in part 3. also the ones in part 4 - are those actually random with borders enabled? because i already went through and marked a bunch of stuff random-if and i'm not sure any other ones also need to be marked random-if
19:28Gankrokats: at least one of them needs to be (it started passing) -- the others used to be marked skip-if in my old PR
19:29katsGankro: ok. do you happen to have a try push with just the border pref changed (and none of the reftest.list fixups)?
19:30Gankrokats: no I hadn't tried that because there's enough changes that it's a ton of work to dig through them all (took me days to do this the first time)
19:31katshmm ok
19:33Gankrokats: just checking I'm not missing anything -- just the one line was messed up in failures.list?
19:33katsGankro: that's the only one that i saw. let's wait until your try push is done
19:33katsif it's green then i don't really care about the one line, we can just land it
19:34katsif there are still failures then we can fix those along with the one line
19:34Gankrokats: oh awesome
19:39katsGankro: also note that i have a review request pending for you
19:40katsi wonder if the WR update will trigger more/different reftest failures with border layers
19:55Gankrokats: shouldn't, the border snap code has been stable for a while...
19:56katsGankro: your try push isn't looking so hot
19:56Gankrokats: yeah I forgot those lines are for code gen, need to edit the resulting file
19:58katsalso where are these crash dumps coming from
19:58Gankro.... yeah those are weird
19:58katslooks like this will still need some work
19:58katshopefully not too much
19:59katsi'll go ahead and land the WR update then. i'd rather have landed your stuff first but i don't want to wait too long
20:00katsnot that the trees are open anyway
20:01Gankrokats: yeah no worries, I can handle the fallout
20:01Gankrokats: hmm! bugs/379349-1a.xhtml seems to be genuinely fuzzy! 1-1,5-5 on some modes, perfect on others
20:02katsGankro: i did kick off a try push with the WR update + your patches, at but i guess it won't be too useful at this point, since it will show at least the same failures as your try push did
20:02Gankrosorry :(
20:02katsno worries
20:03jrmuizelrhunt: can we do the euclid cbindgen stuff now?
20:03katsupdating reftest.list files is not easy, it bitrots very easily
20:04rhuntjrmuizel: we should be able to, I started writing a patch yesterday
20:04jrmuizelrhunt: glorious
20:06jrmuizelkats: is it convenient for you to come into the office sometime and look at webrender apz performance? e.g bug 1376044?
20:06firebot NEW, APZ takes an appreciable amount of time with WebRender
20:06katsjrmuizel: i can come in on monday
20:06jrmuizelkats: thats sounds great
20:07katsjrmuizel: although i'm wondering how relevant that is with layers-free mode
20:07katsanyway i'll come in and we can take a look
20:07jrmuizelkats: yeah, at minimum it may help guide some design decisions with layers-free mode
20:08pcwaltonnical: Im going to try Greens Theorem, damn the torpedoes
20:08pcwaltoncan always fall back to tessellation if it doesnt work out
20:10Gankrokats: does random-if handle "got an exception while checking " (layer test thing)
20:11katsGankro: good question, not sure. let me check
20:14katsGankro: it should, yeah. at failedExtraCheck will be false, which makes test_passed false. expected will be EXPECTED_RANDOM, so output will be assigned and since FAIL == FAIL it should be good
21:21gwGankro: looking into those text shadow issues today
21:21Gankrogw: cool -- btw, what other than glyphs does your impl support appearing in a shadow?
21:21gwGankro: semi-related, I have been thinking about the other text decorations like wavy underline, and I think I have come up with a good way to integrate these into WR - will work on those next week
21:22gwGankro: nothing, right now - all this impl gives you is multiple text runs / glyphs per single shadow
21:22gwGankro: but it adds the framework for adding blob images etc quite easily
21:22gw(which I'll also be working on today / next week)
21:23Gankrogw: ah ok -- the way gecko does it is it just sets the color to black, and renders everything in the shadow as a mask it can then apply to solid colors
21:23gwGankro: ok
21:24Gankrodunno if that makes sense for y'all, but that's the model I was picturing. Might be easier to just pick ~4 things that you support
21:25gwGankro: Could you expand what you mean by ~4 things?
21:26Gankrogw: like, servo currently only puts glyphs + solid color boxes in there
21:27gwGankro: what I was basically planning was that other items (e.g. wavy underlines) are rendered by GPU jobs into the texture cache, and then can be treated as normal glyphs by the glyph shader. I think this should give us a lot of flexibility and be relatively simple to implement.
21:28Gankroah interesting
21:28Gankrothat should work
21:29Gankrogw: would the full line be a glyph, or would a line be a series of them tiled?
21:29gwGankro: I expect they would be tiled (at least if they are a long line)
21:30gwGankro: probably have a threshold, of say 64px, and tile decorations longer than that
21:30GankroI'll assume you're confident in the seems looking ok
21:30gwGankro: yep, we should be able to make it seamless
21:36Gankrogw: ok so you'd want decorations to be first-class, and not just like boxes/borders?
21:37gwGankro: I ... *think* so. I need to prototype it first I guess, but I was thinking they would be part of the text display item, yea. Does that fit in with what you were thinking / how gecko works?
21:38Gankrogw: putting them on TDI will probably be more bloaty than we want -- I'd rather have a LineDisplayItem or something
21:38Gankro(because most Text has no or only one decorations, but in theory can have 3)
21:39gwGankro: fair - making them a top level display item is also fine
21:41Gankrogw: struct LineDisplayItem { x_start: f32, x_end: f32, y: f32, type: enum Style { Solid, Dotted, Dashed, Wavy } }
21:41GankroDouble lines could just be pushing two of those
21:42gwGankro: something like that sounds good to me. we'll probably want to change the terminology from x/y to something more suitable for vertical text, but that can be done later (and obviously involves changes to lots of other interfaces too).
21:43Gankrogw: hmm, yeah we want to support either axis aligned, but not any kind of diagonal, I think
21:43GankroNot a nice way to encode that, sadly
21:44gwGankro: yep - in servo (and in gecko, I assume?) we refer to inline direction and block direction, where inline means text layout direction axis
21:44GankroI guess (baseline: f32, start: f32, end: f32, orientation: bool)
21:44gwGankro: right, sounds good
21:46gwGankro: I'll work on those artifacts and offset bug first though - will work on the other decorations next week :)
21:47Gankrogw: yeah I'll do the frontend/servo/gecko work for it ahead of ya
21:47gwGankro: sounds good, thanks!
23:05Gankrooh gw, just verifying: if I pass you the exact same local_rect for a shadow and its text, that's the correct thing you want? All offset/expanding work will be handled by the backend?
23:06gwGankro: yep, that's right.
23:07gwGankro: ah, I see the issue with offsetting - simple fix. I'll see if I can repro those other artifacts on my mac mini though, not seeing them on my linux machine.
23:07Gankrogw: might be hidpi-specific; I've been working from home so I don't have a normal monitor to test
23:08gwGankro: sounds plausible
23:14Gankrogw: just tried your latest shadow stuff on servo -- looks like you might be drawing the shadows over the text?
23:14GankroWe should probably add a reftest with non-transparent text >.>,
23:17gwGankro: hmmm, I wouldn't expect that, but maybe... I'll take a look today
23:18Gankrogw: should I file a bug?
23:18gwGankro: sure, that'd be great, thanks
23:38gwGankro: ah, I see why they are drawing in the wrong order - I'll fix that at the same time as I fix this offset issue
23:39GankroYeah figured as much
23:39Gankrogw: excited for the next version to feature upside down text :)
14 Jul 2017
No messages
Last message: 72 days and 18 hours ago