Closed Bug 1487407 Opened 6 years ago Closed 6 years ago

reftests/writing-mode/tables/vertical-border-collapse-2.html still fails with webrender after bug 1393907

Categories

(Core :: Graphics: WebRender, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

Details

Attachments

(2 files)

Looks like we don't render a bunch of the collapsed borders, I'll take a look.
Flags: needinfo?(emilio)
Assignee: nobody → emilio
Probably? I'm finishing up a patch, are you working on that one?
Flags: needinfo?(a.beingessner)
nope, go ahead
Flags: needinfo?(a.beingessner)
The previous border-collapse beveling implementation assumed that there would only be one beveled border per side in the whole table, which is... not true at all. So a bunch of borders ended up clobbering other values in mBevelBorders and never getting painted. I'm actually somewhat scarily surprised that only this reftest seems to fail without this patch... Here we reuse most of the existing one-off beveling / border rendering support in nsCSSRendering, and convert the Gecko bevels into a WebRender display list using rects and borders. This is only remotely possible thanks to Gecko not supporting dotted / dashed beveled borders :) This would slightly easier and presumably also more efficient with a triangle display item in WR instead of (ab)using the border display item to render the bevel, but this is probably relatively edge-casey so maybe not worth it... In any case I've left a TODO comment there, that can be a nice followup if we deem it worth it. Anyway, I'm _so_ sorry for the border trick, I was this (||) close to go and rewrite our border collapsing code, but after a few tries I realized it'd take me a whole lot of time (instead of the day that this has taken me).
Flags: needinfo?(emilio)
Comment on attachment 9005709 [details] Properly support beveling bc borders in WR. Alexis Beingessner [:Gankro] has approved the revision.
Attachment #9005709 - Flags: review+
Looks like I need to fix the snapping if I want to land this... Oh well, will look at that on Monday. Also, this actually fixes a fair amount of tests which used to render black borders and now actually render the borders they're supposed to render.
So I talked with Nical and turns out that it's going to be hard / impossible to draw the borders antialiased without the issues that can be seen in this try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a14771f8365756b55733c7407457fc11386ebaa Looks like Gecko doesn't anti-alias the bevel to avoid that, so it seems reasonable to avoid anti-aliasing it rather than rewriting the border-collapsing code, maybe...
Blocks: 1488294
Attached file GitHub Pull Request (deleted) —
Finally got time to look into this again. I need non-antialising corners to implement this, but I got a patch that's green.
Pushed by emilio@crisal.io: https://hg.mozilla.org/integration/mozilla-inbound/rev/198999ea1b79 Properly support beveling bc borders in WR. r=Gankro
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Depends on: 1521066
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: