I've been working on a new record/replay system for Azure. This bug will contain the patches needed to make recording work on mozilla-central builds.

Right now we recreate the ScaledFont each time we create a gfxFont, this is wasteful and causes recording to record truetype data every time a gfxFont is used. This patch fixes that.

This allows the Recording system to track scaled font lifetime.

Comment on attachment 662312 [details] [diff] [review] Part 2: Allow storing and creating from TrueType data Review of attachment 662312 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/ScaledFontDWrite.cpp @@ +18,5 @@ > + uint8_t *mData; > + uint32_t mSize; > +}; > + > +class DWriteFontFileLoader : public IDWriteFontFileLoader What do we need to do to use this code to replace gfxDWriteFontFileLoader? @@ +76,5 @@ > + } > + > +private: > + static IDWriteFontFileLoader* mInstance; > +}; trailing whitespace @@ +136,5 @@ > + virtual HRESULT STDMETHODCALLTYPE GetLastWriteTime(OUT UINT64* lastWriteTime); > + > +private: > + std::vector<uint8_t> mData; > + uint32_t mRefCnt; the type should probably be ULONG @@ +137,5 @@ > + > +private: > + std::vector<uint8_t> mData; > + uint32_t mRefCnt; > +}; trailing whitespace @@ +192,5 @@ > + UINT64 fragmentSize, > + void **fragmentContext) > +{ > + // We are required to do bounds checking. > + if (fileOffset + fragmentSize > (UINT64)mData.size()) { c++ cast @@ +197,5 @@ > + return E_FAIL; > + } > + // We should be alive for the duration of this. > + *fragmentStart = &mData[fileOffset]; > + *fragmentContext = NULL; nullptr @@ +303,5 @@ > + file->GetReferenceKey(&referenceKey, &refKeySize); > + > + RefPtr<IDWriteFontFileLoader> loader; > + file->GetLoader(byRef(loader)); > + trailing whitespace @@ +315,5 @@ > + void *context; > + stream->ReadFileFragment(&fragmentStart, 0, fileSize, &context); > + > + aDataCallback((uint8_t*)fragmentStart, fileSize, mFontFace->GetIndex(), mSize, aBaton); > + c++cast

Comment on attachment 662294 [details] [diff] [review] Part 1: Retain ScaledFont objects for gfxFonts Review of attachment 662294 [details] [diff] [review]: ----------------------------------------------------------------- This should remove the caching that is already in gfxMacFont.cpp ::: gfx/thebes/gfxFont.cpp @@ +1894,5 @@ > // draw any remaining glyphs > glyphs.Flush(cr, aDrawMode, isRTL, aObjectPaint, globalMatrix, true); > > } else { > + RefPtr<ScaledFont> scaledFont = GetScaledFont(dt); Does this break other platforms? ::: gfx/thebes/gfxFont.h @@ +1695,5 @@ > nsAutoPtr<gfxFontShaper> mHarfBuzzShaper; > #ifdef MOZ_GRAPHITE > nsAutoPtr<gfxFontShaper> mGraphiteShaper; > #endif > + mozilla::RefPtr<mozilla::gfx::ScaledFont> mMozScaledFont; Why Moz?

There was indeed some other problems here. This should fix them although the superclass GetScaledFont implementation should perhaps do caching itself, but we should look at that on a per-platform basis. Perhaps the cairo font creation portion could also be cross-platform.

Comment on attachment 662429 [details] [diff] [review] Part 4: Add DrawTargetRecording and DrawEventRecorder code. Review of attachment 662429 [details] [diff] [review]: ----------------------------------------------------------------- Missing DrawEventRecorder ::: gfx/2d/DrawTargetRecording.h @@ +12,5 @@ > +#ifdef _MSC_VER > +#include <hash_set> > +#else > +#include <unordered_set> > +#endif I don't see where you use these. ::: gfx/2d/Makefile.in @@ +41,5 @@ > PathCairo.cpp \ > + DrawTargetRecording.cpp \ > + PathRecording.cpp \ > + RecordedEvent.cpp \ > + DrawEventRecorder.cpp \ Whitespace is wrong here ::: gfx/2d/PathRecording.h @@ +14,5 @@ > + > +struct PathOp > +{ > + enum OpType { > + OP_MOVETO = 0, How about OP_MOVE_TO etc.

Comment on attachment 663458 [details] [diff] [review] Part 4: Add DrawTargetRecording and DrawEventRecorder code. v2 Review of attachment 663458 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/RecordedEvent.h @@ +23,5 @@ > +// A change in minor revision means additions of new events. New streams will > +// not play in older players. > +const uint16_t kMinorRevision = 0; > + > +struct ReferencePtr I'd call it SerializablePtr @@ +150,5 @@ > + STROKE, > + DRAWSURFACE, > + DRAWSURFACEWITHSHADOW, > + PATHCREATION, > + PATHDESTRUCTION, Please separate with underscores.

https://hg.mozilla.org/mozilla-central/rev/e307925e4f29 https://hg.mozilla.org/mozilla-central/rev/30bde31be9d4 https://hg.mozilla.org/mozilla-central/rev/68876e7d17ea https://hg.mozilla.org/mozilla-central/rev/ed2e2da7d098 https://hg.mozilla.org/mozilla-central/rev/ee2e60a30934 https://hg.mozilla.org/mozilla-central/rev/5ad72b579f87 Should this have tests of some sort?

(In reply to Ryan VanderMeulen from comment #13) > https://hg.mozilla.org/mozilla-central/rev/ed2e2da7d098 I don't guess this was meant to include a .rej file?

(In reply to :Ms2ger from comment #14) > (In reply to Ryan VanderMeulen from comment #13) > > https://hg.mozilla.org/mozilla-central/rev/ed2e2da7d098 > > I don't guess this was meant to include a .rej file? It was not, I must've screwed something up, I'll clean it up.