Closed
Bug 719288
Opened 12 years ago
Closed 12 years ago
Draw filled and stroked SVG text in a single Draw call
Categories
(Core :: Layout: Text and Fonts, enhancement)
Core
Layout: Text and Fonts
Tracking
()
RESOLVED
FIXED
mozilla13
People
(Reporter: eflores, Assigned: eflores)
References
Details
Attachments
(1 file, 2 obsolete files)
35.74 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
Makes more sense in the context of bug 719286
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #589710 -
Flags: review?(roc)
Comment on attachment 589710 [details] [diff] [review] Patch Review of attachment 589710 [details] [diff] [review]: ----------------------------------------------------------------- This looks pretty good. Just fix those nits. ::: layout/svg/base/src/nsSVGGlyphFrame.cpp @@ +878,5 @@ > return baselineAppUnits * aMetricsScale; > } > > +gfxFont::DrawMode > +nsSVGGlyphFrame::SetupCairoState(gfxContext* context, nsRefPtr<gfxPattern> &strokePattern) { make this nsRefPtr<gfxPattern>* to make it clearer in the caller that this is an out-parameter. @@ +886,5 @@ > + if (HasStroke()) { > + gfxContextMatrixAutoSaveRestore matrixRestore(context); > + context->IdentityMatrix(); > + > + toDraw = (gfxFont::DrawMode)(toDraw | gfxFont::GLYPH_STROKE); Use a constructor style cast here and elsewhere. It's a little more readable. DrawMode(toDraw | gfxFont::GLYPH_STROKE). Introduce "typedef gfxFont::DrawMode DrawMode;" on nsSVGGlyphFrame to avoid having to prefix stuff. @@ +889,5 @@ > + > + toDraw = (gfxFont::DrawMode)(toDraw | gfxFont::GLYPH_STROKE); > + > + SetupCairoStrokeHitGeometry(context); > + float opacity = MaybeOptimizeOpacity(style->mStrokeOpacity); nsSVGUtils::CanOptimizeOpacity always returns false for glyph frames, so you can just use mStrokeOpacity without calling MaybeOptimizeOpacity. ::: layout/svg/base/src/nsSVGUtils.cpp @@ +1525,5 @@ > + > +/* static */ void > +nsSVGUtils::GetFallbackOrPaintColor(gfxContext *aContext, nsStyleContext *aStyleContext, > + nsStyleSVGPaint nsStyleSVG::*aFillOrStroke, > + float &aOpacity, nscolor &color) Make these out-parameters pointers so we can see in the caller that they are.
Assignee | ||
Comment 3•12 years ago
|
||
Attachment #589710 -
Attachment is obsolete: true
Attachment #589710 -
Flags: review?(roc)
Attachment #592866 -
Flags: review?(roc)
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #592867 -
Flags: review?(roc)
Which patch should I review? :-)
Assignee | ||
Updated•12 years ago
|
Attachment #592866 -
Attachment is obsolete: true
Attachment #592866 -
Flags: review?(roc)
Attachment #592867 -
Flags: review?(roc) → review+
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Updated•12 years ago
|
Whiteboard: [autoland-try]
Updated•12 years ago
|
Whiteboard: [autoland-try] → [autoland-in-queue]
Comment 6•12 years ago
|
||
Autoland Patchset: Patches: 592867 Branch: mozilla-central => try Destination: http://hg.mozilla.org/try/rev/319f852447b5 Try run started, revision 319f852447b5. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=319f852447b5
Comment 7•12 years ago
|
||
Try run for 319f852447b5 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=319f852447b5 Results (out of 207 total builds): exception: 1 success: 183 warnings: 23 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-319f852447b5 Timed out after 06 hours without completing.
Comment 9•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8dec46c6439c
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 10•12 years ago
|
||
Are you planning to use GetFallbackOrPaintColor outside of nsSVGGeometryFrame or its subclasses in some other bug? Is that why you moved the code into nsSVGUtils?
I suppose we could have added it as a static method to nsSVGGeometryFrame, but it doesn't matter much, does it?
Comment 12•12 years ago
|
||
Not really, no. It's just that nsSVGUtils is rather a dumping ground.
All the Utils classes are dumping grounds. That's what they're for :-). (IMHO a "dumping ground" is better than the alternatives of a) duplicating code or b) putting helper functions into some place that happens to be accessible to the N current users, but probably won't make sense for the N+1'th user.)
You need to log in
before you can comment on or make changes to this bug.
Description
•