Closed Bug 1407767 Opened 2 years ago Closed 2 years ago

wr-text: (re)implement text bullets

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox57 --- unaffected
firefox58 --- unaffected

People

(Reporter: Gankra, Assigned: Gankra)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [wr-mvp])

Attachments

(2 files)

They're currently implemented using the old text-layers style (before I started on the path of TextDrawTarget). They also never(?) seem to succeed, so we always fallback.

We should just port this to TextDrawTarget, and maybe also remove non-webrender bullet layers.
Whiteboard: [wr-mvp] [triage]
Whiteboard: [wr-mvp] [triage] → [wr-reserve]
Status: NEW → ASSIGNED
Priority: P3 → P1
Whiteboard: [wr-reserve] → [wr-mvp]
Comment on attachment 8919476 [details]
Bug 1407767 - reimplement text bullets to use TextDrawTarget.

https://reviewboard.mozilla.org/r/190326/#review195604

::: layout/generic/nsBulletFrame.cpp:679
(Diff revision 2)
>                                           wr::IpcResourceUpdateQueue& aResources,
>                                           const StackingContextHelper& aSc,
>                                           mozilla::layers::WebRenderLayerManager* aManager,
>                                           nsDisplayListBuilder* aDisplayListBuilder)
>  {
> -  ContainerLayerParameters parameter;
> +  if (!ShouldUseAdvancedLayer(aManager, gfxPrefs::LayersAllowBulletLayers)) {

Let's just drop this check completely. I don't think we have a reason to disable bullet layers.
Attachment #8919476 - Flags: review?(jmuizelaar) → review+
Opt results were all unexpected passes, debug results were busted due to debug assertions.

New push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ad5236e345504f0ecc09a8eb66e212b4d6c2ab74

Removed check as requested.

Also removed two assertions which are no longer valid (checking for stateful things we no longer use in the webrender path).
Ok looks stable and good. Added new passes in second commit.
Comment on attachment 8919541 [details]
Bug 1407767 - mark bullet text tests as passing again.

https://reviewboard.mozilla.org/r/190384/#review195844

Yay!
Attachment #8919541 - Flags: review?(bugmail) → review+
Keywords: checkin-needed
rebase needed
Keywords: checkin-needed
rebased (no changes, modifications were to deleted code)
Keywords: checkin-needed
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2d3b2d4927ef
reimplement text bullets to use TextDrawTarget. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/8edd193955eb
mark bullet text tests as passing again. r=kats
Keywords: checkin-needed
Backed out for unexpectedly passing reftest layout/reftests/writing-mode/1111944-1-list-marker.html on Linux x64 QuantumRender:

https://hg.mozilla.org/integration/autoland/rev/cca075bb35a2075996743530365e6f92f19db741
https://hg.mozilla.org/integration/autoland/rev/a976cfead2c1896eb70b9845f163f57505be9dad

Log with unexpected pass: https://treeherder.mozilla.org/logviewer.html#?job_id=138261972&repo=autoland
REFTEST TEST-UNEXPECTED-PASS | file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/writing-mode/1111944-1-list-marker.html == file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/writing-mode/1111944-1-list-marker-ref.html | image comparison, max difference: 0, number of differing pixels: 0
Flags: needinfo?(a.beingessner)
Marked test as passing.
Flags: needinfo?(a.beingessner)
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/56eeab3db2d7
reimplement text bullets to use TextDrawTarget. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/a347e00ec6ad
mark bullet text tests as passing again. r=kats
https://hg.mozilla.org/mozilla-central/rev/56eeab3db2d7
https://hg.mozilla.org/mozilla-central/rev/a347e00ec6ad
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.