1379404 - Fine tune text painting

Status: RESOLVED FIXED

(Core :: Web Painting, enhancement)

Description

[u459114]

1. Pevent unnecessary gfxContext::Save/Load
2. Prevent AzureState copy in GlyphBufferAzure::Flush

Comment 1

[u459114]

Attachment: Bug 1379404 - Part 1. Prevent using gfxContext::Save/Restore in nsTextFrame.

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

Comment 2

[u459114]

Attachment: Bug 1379404 - Part 2. Prevent using gfxContext::Save/Restore in gfxFont.

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

Comment 3

[u459114]

Attachment: Bug 1379404 - Part 3. Prevent using gfxContext::Save/Restore in gfxTextRun.

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

Comment 4

[u459114]

Attachment: Bug 1379404 - Part 4. Prevent unnecessary AzureState copy in GlyphBufferAzure::Flush.

~AzureState is expansive, especially in GlyphBufferAzure::Flush, which is a high
fan-in function.

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

Comment 5

[u459114]

Comment on attachment 8884561 [details]
Bug 1379404 - Part 2. Prevent using gfxContext::Save/Restore in gfxFont.

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

Comment 6

[u459114]

Comment on attachment 8884562 [details]
Bug 1379404 - Part 3. Prevent using gfxContext::Save/Restore in gfxTextRun.

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

Comment 7

[u459114]

Comment on attachment 8884563 [details]
Bug 1379404 - Part 4. Prevent unnecessary AzureState copy in GlyphBufferAzure::Flush.

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

Comment 8

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8884560 [details]
Bug 1379404 - Part 1. Prevent using gfxContext::Save/Restore in nsTextFrame.

https://reviewboard.mozilla.org/r/155446/#review160522

Comment 9

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8884561 [details]
Bug 1379404 - Part 2. Prevent using gfxContext::Save/Restore in gfxFont.

https://reviewboard.mozilla.org/r/155448/#review160578

Comment 10

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8884562 [details]
Bug 1379404 - Part 3. Prevent using gfxContext::Save/Restore in gfxTextRun.

https://reviewboard.mozilla.org/r/155450/#review160580

Comment 11

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8884563 [details]
Bug 1379404 - Part 4. Prevent unnecessary AzureState copy in GlyphBufferAzure::Flush.

https://reviewboard.mozilla.org/r/155452/#review160582

::: gfx/thebes/gfxFont.cpp:1648
(Diff revision 2)
>  
>          gfx::GlyphBuffer buf;
>          buf.mGlyphs = mGlyphBuffer;
>          buf.mNumGlyphs = mNumGlyphs;
>  
> -        gfxContext::AzureState state = mRunParams.context->CurrentState();
> +        gfxContext::AzureState &state = mRunParams.context->CurrentState();

Can we make this const?

Seems like we don't really want to be mutating the state of the context directly.

Comment 12

[u459114]

Comment on attachment 8884563 [details]
Bug 1379404 - Part 4. Prevent unnecessary AzureState copy in GlyphBufferAzure::Flush.

https://reviewboard.mozilla.org/r/155452/#review160582

> Can we make this const?
> 
> Seems like we don't really want to be mutating the state of the context directly.

Actually, we can't(Unless we change gfxPattern::GetPattern to a const member funciton.)

We will call a mutatable member function of  gfxContext::AzureState::pattern at
http://searchfox.org/mozilla-central/source/gfx/thebes/gfxFont.cpp#1664

Comment 13

[u459114]

Comment on attachment 8884563 [details]
Bug 1379404 - Part 4. Prevent unnecessary AzureState copy in GlyphBufferAzure::Flush.

https://reviewboard.mozilla.org/r/155452/#review160582

> Actually, we can't(Unless we change gfxPattern::GetPattern to a const member funciton.)
> 
> We will call a mutatable member function of  gfxContext::AzureState::pattern at
> http://searchfox.org/mozilla-central/source/gfx/thebes/gfxFont.cpp#1664

And we will change the context of gfxContext::AzureState::pattern of mRunParams.context->CurrentState(), before or after this change.

Comment 14

[u459114]

Comment on attachment 8884563 [details]
Bug 1379404 - Part 4. Prevent unnecessary AzureState copy in GlyphBufferAzure::Flush.

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

Comment 15

[u459114]

Comment on attachment 8884560 [details]
Bug 1379404 - Part 1. Prevent using gfxContext::Save/Restore in nsTextFrame.

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

Comment 16

[u459114]

Comment on attachment 8884561 [details]
Bug 1379404 - Part 2. Prevent using gfxContext::Save/Restore in gfxFont.

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

Comment 17

[u459114]

Comment on attachment 8884562 [details]
Bug 1379404 - Part 3. Prevent using gfxContext::Save/Restore in gfxTextRun.

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

Comment 18

[u459114]

Comment on attachment 8884563 [details]
Bug 1379404 - Part 4. Prevent unnecessary AzureState copy in GlyphBufferAzure::Flush.

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

Comment 19

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8884563 [details]
Bug 1379404 - Part 4. Prevent unnecessary AzureState copy in GlyphBufferAzure::Flush.

https://reviewboard.mozilla.org/r/155452/#review161046

\o/

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d34a24ce5014
Part 1. Prevent using gfxContext::Save/Restore in nsTextFrame. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/24294f4c838e
Part 2. Prevent using gfxContext::Save/Restore in gfxFont. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/a7e790563964
Part 3. Prevent using gfxContext::Save/Restore in gfxTextRun. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/aa7da1e0163a
Part 4. Prevent unnecessary AzureState copy in GlyphBufferAzure::Flush. r=mattwoodrow

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d34a24ce5014
https://hg.mozilla.org/mozilla-central/rev/24294f4c838e
https://hg.mozilla.org/mozilla-central/rev/a7e790563964
https://hg.mozilla.org/mozilla-central/rev/aa7da1e0163a