Closed
Bug 719627
Opened 12 years ago
Closed 12 years ago
Remove ScaledFontCairo in favour of merging its functionality into ScaledFontBase
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla12
People
(Reporter: joe, Assigned: joe)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
16.52 KB,
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
It's easiest to pass cairo_scaled_font_t from the gfxFont down into mozilla::gfx::Factory, so I've created a way to do that, supported only on backends/platforms that use ScaledFontBase. I'm very interested in people's feedback on this.
Attachment #590014 -
Flags: review?(jmuizelaar)
Attachment #590014 -
Flags: feedback?(bas.schouten)
Comment 1•12 years ago
|
||
Comment on attachment 590014 [details] [diff] [review] Merge ScaledFontCairo into ScaledFontBase, and support it on OS X Review of attachment 590014 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/2D.h @@ +780,5 @@ > + * must be used when using the Cairo backend. > + */ > + static TemporaryRef<ScaledFont> > + CreateScaledFont(const NativeFont &aNativeFont, Float aSize, cairo_scaled_font_t* aScaledFont); > + I'm not thrilled with this name. It doesn't reflect the fact that the scaled_font needs to be the same as native font. Personally, I'd prefer this to pull the native font out of the cairo_scaled_font so you might want to add your justification for not doing so. ::: gfx/2d/Factory.cpp @@ +166,2 @@ > } > +#endif This needs to go. You'd prefer to do it in another bug. ::: gfx/2d/ScaledFontBase.cpp @@ +37,5 @@ > > #include "ScaledFontBase.h" > + > +#include "gfxFont.h" > + You can't include 'gfxFont.h' @@ +83,5 @@ > +#endif > +#ifdef USE_CAIRO > + mScaledFont = aFont->GetCairoScaledFont(); > + cairo_scaled_font_reference(mScaledFont); > +#endif This is more of the stuff we shouldn't have. i.e. We should have a ScaledFontFreeType that we use for linux here, instead of the skia lookup craziness. @@ +163,5 @@ > +#ifdef USE_CAIRO > +void > +ScaledFontBase::SetCairoScaledFont(cairo_scaled_font_t* font) > +{ > + cairo_scaled_font_destroy(mScaledFont); I don't know if we want the ability to change this. I'd rather it be set once and that's all you get. You can add an assert! ::: gfx/2d/ScaledFontBase.h @@ +58,5 @@ > ScaledFontBase(Float aSize); > virtual ~ScaledFontBase(); > > virtual TemporaryRef<Path> GetPathForGlyphs(const GlyphBuffer &aBuffer, const DrawTarget *aTarget); > + ScaledFontBase(gfxFont* aFont, Float aSize); This is more of the gfxFont stuff sneaking out, please leave it skia only. @@ +74,5 @@ > friend class DrawTargetSkia; > #ifdef USE_SKIA > SkTypeface* mTypeface; > #endif > +#ifdef USE_SKIA USE_CAIRO?
Attachment #590014 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Comment 2•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/64c5a333c0f6
Assignee: nobody → joe
Target Milestone: --- → mozilla12
Assignee | ||
Comment 3•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/64c5a333c0f6
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Attachment #590014 -
Flags: feedback?(bas)
You need to log in
before you can comment on or make changes to this bug.
Description
•