Closed Bug 1379404 Opened 4 years ago Closed 4 years ago

Fine tune text painting

Categories

(Core :: Web Painting, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: u459114, Assigned: u459114)

References

Details

Attachments

(4 files)

1. Pevent unnecessary gfxContext::Save/Load
2. Prevent AzureState copy in GlyphBufferAzure::Flush
Attachment #8884560 - Flags: review?(matt.woodrow)
Attachment #8884561 - Flags: review?(matt.woodrow)
Attachment #8884562 - Flags: review?(matt.woodrow)
Attachment #8884563 - Flags: review?(matt.woodrow)
Comment on attachment 8884560 [details]
Bug 1379404 - Part 1. Prevent using gfxContext::Save/Restore in nsTextFrame.

https://reviewboard.mozilla.org/r/155446/#review160522
Attachment #8884560 - Flags: review?(matt.woodrow) → review+
Comment on attachment 8884561 [details]
Bug 1379404 - Part 2. Prevent using gfxContext::Save/Restore in gfxFont.

https://reviewboard.mozilla.org/r/155448/#review160578
Attachment #8884561 - Flags: review?(matt.woodrow) → review+
Comment on attachment 8884562 [details]
Bug 1379404 - Part 3. Prevent using gfxContext::Save/Restore in gfxTextRun.

https://reviewboard.mozilla.org/r/155450/#review160580
Attachment #8884562 - Flags: review?(matt.woodrow) → review+
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 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 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 on attachment 8884563 [details]
Bug 1379404 - Part 4. Prevent unnecessary AzureState copy in GlyphBufferAzure::Flush.

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

\o/
Attachment #8884563 - Flags: review?(matt.woodrow) → review+
Pushed by cku@mozilla.com:
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
You need to log in before you can comment on or make changes to this bug.