See bug #588. I will provide a patch that gets rid of the need for extra fields in PerFrameData. Hopefully it will also eliminate the need for extra fields in nsLineLayout too.

I have completed the optimization work. I have also cleaned up the code in a few places. I got rid of all the space overhead in nsLineLayout and PerFrameData, at the cost of an extra method in nsIFrame ... which is not really "extra" because I got rid of nsIFrame::AdjustFrameSize at the same time :-). Before this is checked in, please check to see if I broke http://www.trustedpc.org ... it's not working in my build, but I think this is an unrelated regression because when I pull the "align=justify" tags out, the tables are still horribly broken.

Uhoh, there is a bug ... nsTextFrame::ComputeJustificationMetrics should return 0's if the frame unilaterally decides not to justify itself (i.e. PRE text). I will fix.

Sigh... Spin this patch one more time. This fixes a bug where pfd->mCombinedArea was not being updated correctly for justified frames, leading to insufficient repainting during incremental reflows. Now Composer edits justified text with no apparent problems. Woohoo! I also found that in TrimTrailingWhitespaceIn, mCombinedArea.width is not updated along with mBounds.width in the span case ... looks like a bug since they are updated together in the leaf case. I took the liberty of patching it.

If/when nsUInt32Array moves to xpcom/ds (bug #38173), I'll rework the patch to use it.

I will update my patch to use nsUInt32Array when (or if) my patches to bug #38173 land.

welcome to the club, Robert!

Uh oh. :-) What are the implications of this? I don't have CVS access yet.

Robert: I thought I'd look at this, but I'm having a hard time reading the patch. Can you send me the files involved directly, and together with the patch I'll figure out what's going on here. Thanks! Did the data structure you need ever get moved into xpcom/ds?

I'll attach a new patch. I think that this should not all be checked in yet. It does depend on nsUInt32Array being moved into xpcom/ds, which has not happened yet. It also adds a text-related method to nsIFrame, which is poor form; it should be moved to a new nsITextFrame interface. There will also be more changes when I change the justification-breaking algorithm to support CJK; they probably won't change the interface but why check in twice? I don't regard this fix as urgent. In case you still want to play with this, here's an explanation of the patch. Let me know and I'll email you my copies of the patched files. nsBlockFrame.cpp @@ -4591,39 +4592,43 @@ This is just code cleanup, should not change behaviour. nsIFrame.h @@ -894,9 +894,9 @@ Get rid of useless nsIFrame method AdjustFrameSize, replace with new ComputeJustificationMetrics. nsFrame.cpp @@ -1692,9 +1692,10 @@, nsFrame.h @@ -278,7 +278,7 @@ Change default frame impl. to support new nsIFrame method. nsLineLayout.cpp @@ -2383,6 +2380,8 @@ minor fix: pfd->mCombinedArea can be reduced in size along with pfd->mBounds when whitespace is trimmed. This is just a tiny optimization. nsLineLayout.cpp @@ -44,6 +44,8 @@, @@ -894,8 +896,6 @@, @@ -968,9 +968,6 @@, @@ -2432,10 +2431,6 @@, @@ -2483,36 +2478,43 @@, @@ -2618,15 +2618,18 @@, nsLineLayout.h @@ -39,6 +39,7 @@, @@ -299,8 +294,6 @@, @@ -356,11 +349,6 @@ The guts of the patch: pull the justification data out of the pfds and nsLineLayout, request it explicitly during justification computations. Also rework justification computations to handle this and to cache the data in an nsUInt32Array. nsLineLayout.h @@ -467,7 +455,7 @@, @@ -516,7 +505,7 @@, @@ -475,34 +463,35 @@ The last few lines are part of the above. The rest of the changes are just cosmetic cleanup to have some existing functions (not mine) follow naming conventions. nsTextFrame.cpp @@ -702,6 +702,7 @@, @@ -2282,7 +2280,6 @@, @@ -3427,7 +3424,6 @@, @@ -3775,6 +3770,7 @@, @@ -3555,7 +3551,6 @@ Cosmetic, fiddling with blank lines :-) nsTextFrame.cpp @@ -431,11 +431,11 @@, @@ -1426,11 +1427,8 @@, @@ -2473,7 +2470,7 @@, @@ -4206,23 +4184,6 @@, @@ -4296,9 +4257,34 @@ Support the new nsIFrame method ComputeJustificationMetrics, and remove the old nsLineLayout<->nsTextFrame communication path. Among other things, this extends PrepareUnicodeText to handle a null aTextBuffer, since we don't want to actually collect the text while computing space and letter counts. (This is one thing that might have to change.) nsTextFrame @@ -3788,32 +3784,14 @@ Mostly just deleting the commented-out fix I had that didn't work. However, at the end is a tiny real fix: deleting a couple of useless lines of code.

I thought about this a bit. It seems to me that if we have nsTextFrame implementing an nsITextFrame interface, and also inheriting from nsFrame, then every nsTextFrame will have to include an extra vtable pointer. That sounds bad.

Upon managerial request, adding the "testcase" keyword to 84 open layout bugs that do not have the "testcase" keyword and yet have an attachement with the word "test" in the description field. Apologies for any mistakes.

Moving to m1.0

oops, didn't mean to set milestone for non-netscapers

Not a priority at all.