Closed Bug 1385861 Opened 6 years ago Closed 6 years ago

Fine tune nsDisplaySVGText::Paint

Tracking

()

Status:

RESOLVED FIXED

Milestone:

mozilla57

Project Flags:

Performance Impact

high

Tracking Flags:

Tracking

Status

firefox57

---

fixed

People

(Reporter: u459114, Assigned: u459114)

References

Details

Attachments

(4 files, 1 obsolete file)

Bug 1385861 - Part 1. Remove redundant gfxContext::Save/Restore in nsDisplaySVGText::Paint. 6 years ago u459114 59 bytes, text/x-review-board-request	mattwoodrow : review+	Details
Bug 1385861 - Part 2. Save the state of input context in SVGTextFrame::PaintSVG only when need. 6 years ago u459114 59 bytes, text/x-review-board-request	mattwoodrow : review+	Details
Bug 1385861 - Part 3. Rename nsSVGUtils::SetupCairoStrokeGeometry as nsSVGUtils::SetupStrokeGeometry. 6 years ago u459114 59 bytes, text/x-review-board-request	mattwoodrow : review+	Details
Bug 1385861 - Part 4. Define contextPaint as a local variable. 6 years ago u459114 59 bytes, text/x-review-board-request		Details
Bug 1385861 - Part 4. Reduce usage of SetMatrix in SVGTextFrame::PaintSVG. 6 years ago u459114 59 bytes, text/x-review-board-request	mattwoodrow : review+	Details

u459114

Assignee

Description

•

6 years ago

      No description provided.

Comment hidden (mozreview-request)

nsDisplaySVGText::Paint calls SVGTextFrame::PaintSVG, and we already save/restore
context state in SVGTextFrame::PaintSVG.

Review commit: https://reviewboard.mozilla.org/r/163004/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163004/

Comment hidden (mozreview-request)

Review commit: https://reviewboard.mozilla.org/r/163006/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163006/

Comment hidden (mozreview-request)

Review commit: https://reviewboard.mozilla.org/r/163008/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163008/

u459114

Assignee

Comment 4

•

6 years ago

mozilla::TextRenderedRunIterator::First take time which out of my expectation
Duplicate gfxSave/Restore - I sould find a way to catch all this kind of problem

Astley Chen (inactive)

Updated

•

6 years ago

Status: NEW → ASSIGNED

Priority: -- → P2

u459114

Assignee

Updated

•

6 years ago

Whiteboard: [qf]

Comment hidden (mozreview-request)

To prevent heap allocation in painting.

Review commit: https://reviewboard.mozilla.org/r/163042/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163042/

Comment hidden (mozreview-request)

In while-loop [1],
1. The first SetMatrix calls is totally useless, we can just pass initialMatrix
to contextPaint.
2. This function, SVGTextFrame::PaintSVG, is the only caller of
SetupStrokeGeometry, and it does not need SetupStrokeGeometry to setup matrix at
all, since it will be overwritten at several lines below[2].

[1]
https://hg.mozilla.org/mozilla-central/file/5115e2dea29a/layout/svg/SVGTextFrame.cpp#l3658
[2]
https://hg.mozilla.org/mozilla-central/file/5115e2dea29a/layout/svg/SVGTextFrame.cpp#l3685

Review commit: https://reviewboard.mozilla.org/r/163044/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163044/

Comment hidden (mozreview-request)

Comment on attachment 8891991 [details]
Bug 1385861 - Part 1. Remove redundant gfxContext::Save/Restore in nsDisplaySVGText::Paint.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163004/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8891992 [details]
Bug 1385861 - Part 2. Save the state of input context in SVGTextFrame::PaintSVG only when need.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163006/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8891993 [details]
Bug 1385861 - Part 3. Rename nsSVGUtils::SetupCairoStrokeGeometry as nsSVGUtils::SetupStrokeGeometry.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163008/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8891991 [details]
Bug 1385861 - Part 1. Remove redundant gfxContext::Save/Restore in nsDisplaySVGText::Paint.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163004/diff/2-3/

Comment hidden (mozreview-request)

Comment on attachment 8891992 [details]
Bug 1385861 - Part 2. Save the state of input context in SVGTextFrame::PaintSVG only when need.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163006/diff/2-3/

Comment hidden (mozreview-request)

Comment on attachment 8891993 [details]
Bug 1385861 - Part 3. Rename nsSVGUtils::SetupCairoStrokeGeometry as nsSVGUtils::SetupStrokeGeometry.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163008/diff/2-3/

Comment hidden (mozreview-request)

Comment on attachment 8892039 [details]
Bug 1385861 - Part 4. Define contextPaint as a local variable.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163042/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8892040 [details]
Bug 1385861 - Part 4. Reduce usage of SetMatrix in SVGTextFrame::PaintSVG.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163044/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8891993 [details]
Bug 1385861 - Part 3. Rename nsSVGUtils::SetupCairoStrokeGeometry as nsSVGUtils::SetupStrokeGeometry.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163008/diff/3-4/

Comment hidden (mozreview-request)

Comment on attachment 8892039 [details]
Bug 1385861 - Part 4. Define contextPaint as a local variable.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163042/diff/2-3/

Comment hidden (mozreview-request)

Comment on attachment 8892040 [details]
Bug 1385861 - Part 4. Reduce usage of SetMatrix in SVGTextFrame::PaintSVG.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163044/diff/2-3/

Jean Gong :jgong

Updated

•

6 years ago

Whiteboard: [qf] → [qf:p1]

Comment hidden (mozreview-request)

Comment on attachment 8891992 [details]
Bug 1385861 - Part 2. Save the state of input context in SVGTextFrame::PaintSVG only when need.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163006/diff/3-4/

Comment hidden (mozreview-request)

Comment on attachment 8891993 [details]
Bug 1385861 - Part 3. Rename nsSVGUtils::SetupCairoStrokeGeometry as nsSVGUtils::SetupStrokeGeometry.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163008/diff/4-5/

Comment hidden (mozreview-request)

Comment on attachment 8892040 [details]
Bug 1385861 - Part 4. Reduce usage of SetMatrix in SVGTextFrame::PaintSVG.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163044/diff/3-4/

u459114

Assignee

Updated

•

6 years ago

Attachment #8892039 - Attachment is obsolete: true

u459114

Assignee

Updated

•

6 years ago

Attachment #8891991 - Flags: review?(matt.woodrow)

Attachment #8891992 - Flags: review?(matt.woodrow)

Attachment #8892040 - Flags: review?(matt.woodrow)

u459114

Assignee

Updated

•

6 years ago

Attachment #8891993 - Flags: review?(matt.woodrow)

Comment hidden (mozreview-request)

Comment on attachment 8891991 [details]
Bug 1385861 - Part 1. Remove redundant gfxContext::Save/Restore in nsDisplaySVGText::Paint.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163004/diff/3-4/

Comment hidden (mozreview-request)

Comment on attachment 8891992 [details]
Bug 1385861 - Part 2. Save the state of input context in SVGTextFrame::PaintSVG only when need.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163006/diff/4-5/

Comment hidden (mozreview-request)

Comment on attachment 8891993 [details]
Bug 1385861 - Part 3. Rename nsSVGUtils::SetupCairoStrokeGeometry as nsSVGUtils::SetupStrokeGeometry.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163008/diff/5-6/

Comment hidden (mozreview-request)

Comment on attachment 8892040 [details]
Bug 1385861 - Part 4. Reduce usage of SetMatrix in SVGTextFrame::PaintSVG.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163044/diff/4-5/

Matt Woodrow (:mattwoodrow)

Comment 25

•

6 years ago

mozreview-review

Comment on attachment 8891991 [details]
Bug 1385861 - Part 1. Remove redundant gfxContext::Save/Restore in nsDisplaySVGText::Paint.

https://reviewboard.mozilla.org/r/163004/#review169050

Attachment #8891991 - Flags: review?(matt.woodrow) → review+

Matt Woodrow (:mattwoodrow)

Comment 26

•

6 years ago

mozreview-review

Comment on attachment 8891992 [details]
Bug 1385861 - Part 2. Save the state of input context in SVGTextFrame::PaintSVG only when need.

https://reviewboard.mozilla.org/r/163006/#review169052

Attachment #8891992 - Flags: review?(matt.woodrow) → review+

Matt Woodrow (:mattwoodrow)

Comment 27

•

6 years ago

mozreview-review

Comment on attachment 8891993 [details]
Bug 1385861 - Part 3. Rename nsSVGUtils::SetupCairoStrokeGeometry as nsSVGUtils::SetupStrokeGeometry.

https://reviewboard.mozilla.org/r/163008/#review169058

Attachment #8891993 - Flags: review?(matt.woodrow) → review+

Matt Woodrow (:mattwoodrow)

Comment 28

•

6 years ago

mozreview-review

Comment on attachment 8892040 [details]
Bug 1385861 - Part 4. Reduce usage of SetMatrix in SVGTextFrame::PaintSVG.

https://reviewboard.mozilla.org/r/163044/#review169062

Attachment #8892040 - Flags: review?(matt.woodrow) → review+

Pulsebot

Comment 29

•

6 years ago

Pushed by cku@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/615e94f1555b
Part 1. Remove redundant gfxContext::Save/Restore in nsDisplaySVGText::Paint. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/e8c4ad3acfd0
Part 2. Save the state of input context in SVGTextFrame::PaintSVG only when need. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/61072ea18a88
Part 3. Rename nsSVGUtils::SetupCairoStrokeGeometry as nsSVGUtils::SetupStrokeGeometry. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/5d10630d73e2
Part 4. Reduce usage of SetMatrix in SVGTextFrame::PaintSVG. r=mattwoodrow

Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)

Comment 30

•

6 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/615e94f1555b
https://hg.mozilla.org/mozilla-central/rev/e8c4ad3acfd0
https://hg.mozilla.org/mozilla-central/rev/61072ea18a88
https://hg.mozilla.org/mozilla-central/rev/5d10630d73e2

Status: ASSIGNED → RESOLVED

Closed: 6 years ago

status-firefox57: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla57

Dave Hunt [:davehunt] [he/him] ⌚GMT

Updated

•

1 year ago

Performance Impact: --- → P1

Whiteboard: [qf:p1]

You need to log in before you can comment on or make changes to this bug.