Closed
Bug 174055
Opened 22 years ago
Closed 14 years ago
deCOM nsIFontMetrics
Categories
(Core :: Graphics, defect)
Core
Graphics
Tracking
()
RESOLVED
FIXED
People
(Reporter: blizzard, Assigned: zwol)
References
(Blocks 1 open bug)
Details
Attachments
(8 files)
30.78 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
31.33 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
68.03 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
68.62 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
32.87 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
81.62 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
7.54 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
3.26 KB,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
After the checkin for bug 126919 none of these apis are used anywhere. See that bug for more details.
Updated•16 years ago
|
Assignee: jag → nobody
Assignee | ||
Comment 1•14 years ago
|
||
nsIWidget::SetFont seems to be gone, and I have a patch series deCOMming nsIFontMetrics that nukes GetFontHandle as a side effect, so I'm stealing this bug to carry that patch series. Hope you don't mind.
Assignee | ||
Comment 2•14 years ago
|
||
Mostly-mechanical patch to remove all unnecessary #includes of ns*FontMetrics.h.
Attachment #525578 -
Flags: review?(roc)
Assignee | ||
Comment 3•14 years ago
|
||
What it says. Note I did not bump the nsIFontMetrics IID, because the very next patch makes the IID go away.
Attachment #525579 -
Flags: review?(roc)
Assignee | ||
Comment 4•14 years ago
|
||
This is the main deCOM patch. gfx/ can be compiled at this point, but the mechanical fixups needed for the rest of the tree are in the next patch.
Attachment #525580 -
Flags: review?(roc)
Assignee | ||
Comment 5•14 years ago
|
||
perl -pi -e 's/\bns(?:I|Thebes)(?=FontMetrics\b)/ns/g; s/\bnsCOMPtr(?=<nsFontMetrics>)/nsRefPtr/g' [every file in the tree]
Assignee | ||
Updated•14 years ago
|
Attachment #525581 -
Flags: review?(roc)
Assignee | ||
Comment 6•14 years ago
|
||
One interesting thing about this patch is that I deleted GetHeight, which was used but documented as "will be removed once callers have been fixed". It did the same thing as GetMaxHeight, so I changed all remaining callers to use that. Also there are a couple of technically-unrelated removals of unused variables in here (I was tired of the warnings).
Attachment #525582 -
Flags: review?(roc)
Assignee | ||
Comment 7•14 years ago
|
||
Also rearranges code in nsFontMetrics.cpp a little so we can make use of the anonymous namespace for private classes. The accessors that now return one nscoord also have the "Get" removed from their names.
Attachment #525583 -
Flags: review?(roc)
Assignee | ||
Comment 8•14 years ago
|
||
All other places that call CreateFontGroup put the gfxFontStyle argument on the stack, and nothing else in nsFontMetrics uses the gfxFontStyle, so we don't need to heap-allocate that and keep it around for the lifetime of the font metrics. mFontGroup is a refptr so it doesn't need to be nulled in the constructor. And there were *two* booleans for whether text runs were right-to-left, one of which was never set to true (but that was the one that was actually getting *used*; I assumed this was just a mistake). This is as far as this patch series goes. I have work past this point, trying to disentangle nsFontMetrics from nsThebesDeviceContext, but it's still got bugs in it, and this is enough for one bug. This set + the set in bug 266236 has passed the try server.
Attachment #525584 -
Flags: review?(roc)
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → zackw
Attachment #525578 -
Flags: review?(roc) → review+
Attachment #525579 -
Flags: review?(roc) → review+
Attachment #525580 -
Flags: review?(roc) → review+
Attachment #525581 -
Flags: review?(roc) → review+
Attachment #525582 -
Flags: review?(roc) → review+
GetLanguage could return a raw nsIAtom*.
Attachment #525583 -
Flags: review?(roc) → review+
Attachment #525584 -
Flags: review?(roc) → review+
Assignee | ||
Comment 10•14 years ago
|
||
(In reply to comment #9) > GetLanguage could return a raw nsIAtom*. Good point; I'll tack on another patch to do that tomorrow morning.
Assignee | ||
Comment 11•14 years ago
|
||
Now I remember why I didn't do this in the first place: the only user is nsFontCache::GetMetricsFor, and I was originally going to have part 8 of this bug delete nsFontCache, so it seemed like unnecessary churn. But that change turns out to expose a bunch of subtle bugs, so this is a better stopping place in case we wind up not being able to delete nsFontCache. This series is waiting for one more review in bug 648197 before it can land, by the way.
Attachment #526019 -
Flags: review?(roc)
Attachment #526019 -
Flags: review?(roc) → review+
Assignee | ||
Comment 12•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/34e0e1992231 http://hg.mozilla.org/mozilla-central/rev/84c4a1cc3655 http://hg.mozilla.org/mozilla-central/rev/2397bc9933ea http://hg.mozilla.org/mozilla-central/rev/d06f1282cc6d http://hg.mozilla.org/mozilla-central/rev/b339ef823c29 http://hg.mozilla.org/mozilla-central/rev/90a2c72237e2 http://hg.mozilla.org/mozilla-central/rev/ea77c7167870 http://hg.mozilla.org/mozilla-central/rev/ee377a1a5e31
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•