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)
Core
Graphics: WebRender
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.
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(emilio)
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → emilio
Comment 1•6 years ago
|
||
Blocks: stage-wr-trains
Priority: -- → P2
Assignee | ||
Comment 2•6 years ago
|
||
Probably? I'm finishing up a patch, are you working on that one?
Flags: needinfo?(a.beingessner)
Assignee | ||
Comment 5•6 years ago
|
||
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).
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(emilio)
Comment 6•6 years ago
|
||
Comment on attachment 9005709 [details]
Properly support beveling bc borders in WR.
Alexis Beingessner [:Gankro] has approved the revision.
Attachment #9005709 -
Flags: review+
Assignee | ||
Comment 7•6 years ago
|
||
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.
Assignee | ||
Comment 8•6 years ago
|
||
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...
Assignee | ||
Comment 9•6 years ago
|
||
Finally got time to look into this again. I need non-antialising corners to implement this, but I got a patch that's green.
Updated•6 years ago
|
Updated•6 years ago
|
Depends on: 1493890
See Also: → https://github.com/servo/webrender/pull/3107
Comment 10•6 years ago
|
||
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/mozilla-inbound/rev/198999ea1b79
Properly support beveling bc borders in WR. r=Gankro
Comment 11•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox64:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in
before you can comment on or make changes to this bug.
Description
•