Closed Bug 1401653 Opened 7 years ago Closed 7 years ago

wr-text: implement decoration fusion(?)

Categories

(Core :: Graphics: WebRender, defect, P1)

Product:
Core
Component:
Graphics: WebRender
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla58
Tracking Flags:
Tracking Status
firefox57 --- unaffected
firefox58 --- unaffected

People

(Reporter: Gankra, Assigned: Gankra)

References

(Blocks 1 open bug)

Details

(Whiteboard: [wr-reserve] [gfx-noted])

Attachments

(2 files)

decorations.png
(deleted), image/png
Details
Bug 1401653 - fixup webrender text-decoration bindings.
(deleted), text/x-review-board-request
jrmuizel
: review+
Details
These two reftests fail with webrendest layers-free. They appear to be testing some sort of "decoration fusion" behaviour, that I don't really understand yet. layout/reftests/text-decoration/complex-decoration-style-quirks.html layout/reftests/text-decoration/complex-decoration-style-standards.html
Hey jonathan, any idea what these tests are about?
Flags: needinfo?(jfkthame)
It looks like they're testing how decorations (particularly "complex" ones -- dotted, dashed or wavy lines) are drawn across element boundaries, to check that we don't get discontinuities in what should be a continuous line, and that the decorations are offset appropriately in response to position:relative and shadow offsets. What do the failures you're seeing look like? Do there seem to be glitches at the boundaries between various <span>s in the testcases?
Flags: needinfo?(jfkthame)
Attached image decorations.png (deleted) —
The decorations seem to have overlapping ranges, and get all messed up. It's possible we need to be drawing the decorations so that this overlap is invisible? That or my code to grab the bounds of the lines is wrong (very possible).
The overlaps are one issue, but in addition the various parts seem to be out of phase with each other.
Well each decoration starts at the same position, which seemed to be what gecko did. Is the period of the decoration required to satisfy some specific relationship with its height (thickness) to make this Just Work?
Priority: -- → P3
Whiteboard: [gfx-noted]
Priority: P3 → P2
Whiteboard: [gfx-noted] → [wr-mvp] [gfx-noted]
Blocks: 1407627
Summary: text-layers: implement decoration fusion(?) → wr-text: implement decoration fusion(?)
Finally figured out what's going on here. Gecko is being Very Smart in its line drawing code, and we're breaking the assumptions there. Basically gecko tries to align periodic patterns to a period-multiple offset from some parent element. Then applies clips to keep lines from smashing into eachother. This creates the illusion of a single coherent line across elements and style changes. This not working with WR is a mix of three issues: * Upstream wr doesn't currently match up its wavy/dashed line period with gecko's * We don't push/pop clips for TextDrawTarget (before this was hard, now it's easy) * I didn't actually pass the right dimensions for wavy lines from gecko to wr (so if we did apply the clip, we get bad results)
See Also: → https://github.com/servo/webrender/issues/1913
Priority: P2 → P3
Whiteboard: [wr-mvp] [gfx-noted] → [wr-reserve] [gfx-noted]
I was already working on this before it was marked as P3.
Assignee: nobody → a.beingessner
See Also: → https://github.com/servo/webrender/issues/1928
Comment on attachment 8921639 [details] Bug 1401653 - fixup webrender text-decoration bindings. https://reviewboard.mozilla.org/r/192638/#review197830
Attachment #8921639 - Flags: review?(jmuizelaar) → review+
Try build to make sure there's no collateral damage (I don't expect any): https://treeherder.mozilla.org/#/jobs?repo=try&revision=22551950c875ce73e9137ab8fefbdb49e3c94184
Status: NEW → ASSIGNED
Priority: P3 → P1
Whiteboard: [wr-reserve] [gfx-noted] → [wr-mvp] [gfx-noted]
Try looks good.
Keywords: checkin-needed
You should upgrade your cbindgen to .26 or newer; I think a bunch of the generated changes in this patch are artifacts of your using the older cbindgen.
Blocks: 1403261
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/5055d2703f2f fixup webrender text-decoration bindings. r=jrmuizel
Keywords: checkin-needed
API change landed before me, fixed (added two Nothings to the call)
Flags: needinfo?(a.beingessner)
Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/27ac538d3d8b fixup webrender text-decoration bindings. r=jrmuizel
https://hg.mozilla.org/mozilla-central/rev/27ac538d3d8b
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Whoops, forgot to reopen this bug -- all the work in gecko is done, but upstream webrender work needs to get done to fix one of the cases in these tests and make them actually pass. Specifically https://github.com/servo/webrender/issues/1928 needs to get fixed. Fixing the linked issue will now also fix text-shadow/decorations-multiple-zorder.html since Bug 1399310 just landed, making us use clipped decorations in more places.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → ASSIGNED
Deprioritized to P3 during triage - moving to Reserve Backlog.
Assignee: a.beingessner → nobody
Status: ASSIGNED → NEW
Priority: P1 → P3
Whiteboard: [wr-mvp] [gfx-noted] → [wr-reserve] [gfx-noted]
Gankro, what's the status of this bug?
Flags: needinfo?(a.beingessner)
bugs fixed; tests just need minor fuzzing, looks like
Flags: needinfo?(a.beingessner)
opening new bug for test expectations
Status: NEW → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
Blocks: 1455365
Assignee: nobody → a.beingessner
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: