Closed Bug 45737 Opened 24 years ago Closed 23 years ago

nsFontMetricsWin.cpp is in a dire straights, and should be restructured

Categories

(Core :: Layout, defect, P3)

x86
Windows 98
defect

Tracking

()

RESOLVED FIXED
Future

People

(Reporter: rbs, Assigned: ftang)

Details

(Keywords: arch)

nsFontMetricsWin.cpp has been changing a lot since it was first created, with additions and work-around for various problems that have surfaced. Unfortunately, due to the fear of breaking anything, most of the changes were done in a "local" manner (i.e, developers took great care to keep their changes in a confined section of the file). This has led to duplicate data/functionalities (e.g., multiple caches for font objects, various structs that could be combined, etc.). Now that most of the functionalities needed for a first release are in the code, this could be a motivation to completely restructure the code from a global perspective as you successfully did in nsFontMetricsGTK.cpp. If viewed as a perf work, it could well be done in the forthcoming perf stage. With all the practical aspects in the code known (and nearly frozen), a better integration of all these aspects could lead to a smaller/faster code in which all the memory leaks (e.g., bug 29358) are addressed from the outset.
erik is on vacation.
Keywords: arch
Hi Roger! Thanks for the suggestion. The Unix side was not only restructured but also fixed to address a number of serious issues. On the Windows side, we still have a few issues such as memory-related ones, and the code could certainly be made more maintainable by restructuring it, but my first impression after this long vacation is to leave it more or less as it is until we ship the first release, which is quite imminent. Please let me think about it some more, while I go through my other bugs, some of which may require more immediate attention.
Status: NEW → ASSIGNED
Target Milestone: --- → Future
I agree with Erik. It would be great to restructure it, but there isn't enough time before we ship.
[mid-air collision. re-submitting] Welcome back Erik! When I saw how you reworked the Unix code, and how the new code was looking rock solid, and so clean and tidy, I couldn't resist filing this bug, asking for the same care on Windows... But with this tight schedule, since a major overhaul is a demanding job, it can wait if there are other urgent fishes to fry on your plate. So just fixing the other reported memory leaks would be okay for the first release.
erik resign. reassign all his bug to ftang for now.
Assignee: erik → ftang
Status: ASSIGNED → NEW
mark all future new as assigned after move from erik to ftang
Status: NEW → ASSIGNED
There have been several gradual improvements ever since I filed this bug, and I did include some clean ups and mlk fixes in my font changes (bug 99010). Something that remains and could be investigated could be to see if the handling of the fontweight hashtable could somehow be combined with the other tables. But I can live with things the way they are. So resolving this bug as FIXED to get if off the radars.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.