Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

refactoring some of nsHTMLReflowState

0 views
Skip to first unread message

L. David Baron

unread,
Sep 27, 2006, 6:18:12 PM9/27/06
to dev-tec...@lists.mozilla.org
I'm considering refactoring a good bit of nsHTMLReflowState
(ComputeBlockBoxData, InitConstraints) plus things like
nsImageFrame::GetDesiredSize that use NS_INTRINSICSIZE (which has to go
away on the reflow branch) into the following method on nsIFrame:

/**
* Compute the size that a frame will occupy. Called while
* constructing the nsHTMLReflowState to be used to Reflow the frame,
* in order to fill its mComputedWidth and mComputedHeight member
* variables.
*
* The |height| member of the return value may be
* NS_UNCONSTRAINEDSIZE, but the |width| member must not be.
*
* @param aAvailWidth The available width into which the element is
* being placed (i.e., the width of its containing
* block).
* @param aMargin The sum of the left and right margins of the
* frame, including actual values resulting from
* percentages, but not including actual values
* resulting from 'auto'.
* @param aBorder The sum of the left and right border widths of the
* frame.
* @param aPadding The sum of the left and right margins of the
* frame, including actual values resulting from
* percentages.
* @param aShrinkWrap Whether the frame is in a context where
* non-replaced blocks should shrink-wrap (e.g.,
* it's floating, absolutely positioned, or
* inline-block).
*/
virtual nsSize ComputeSize(nscoord aAvailWidth, nscoord aMargin,
nscoord aBorder, nscoord aPadding,
PRBool aShrinkWrap) = 0;


Does this seem reasonable?

One thing I was considering was whether I should pass the reflow state
as a parameter or not. The advantage of doing so would be fewer
parameters (and perhaps ease of depending on additional things in the
reflow state) and perhaps the performance advantage of not re-retrieving
the style structs. However, I consider ease of adding extra
dependencies to be a significant disadvantage (it makes it harder to
enforce invariants) -- especially dependencies on nsHTMLReflowState,
which I'd like to get significantly smaller.

-David

--
L. David Baron <URL: http://dbaron.org/ >
Technical Lead, Layout & CSS, Mozilla Corporation

Boris Zbarsky

unread,
Sep 27, 2006, 9:27:30 PM9/27/06
to
L. David Baron wrote:
> I'm considering refactoring a good bit of nsHTMLReflowState
> (ComputeBlockBoxData, InitConstraints) plus things like
> nsImageFrame::GetDesiredSize that use NS_INTRINSICSIZE

Seconded!

> * @param aAvailWidth The available width into which the element is
> * being placed (i.e., the width of its containing
> * block).

So this is not the same as the availWidth of a reflow state, correct? This is
the value that should be used as a base for percentage widths, basically?

> * @param aPadding The sum of the left and right margins of the

s/margin/padding/

I think this looks pretty reasonable...

> One thing I was considering was whether I should pass the reflow state
> as a parameter or not. The advantage of doing so would be fewer
> parameters (and perhaps ease of depending on additional things in the
> reflow state) and perhaps the performance advantage of not re-retrieving
> the style structs. However, I consider ease of adding extra
> dependencies to be a significant disadvantage (it makes it harder to
> enforce invariants) -- especially dependencies on nsHTMLReflowState,
> which I'd like to get significantly smaller.

Would it make sense to split up the reflow state into two structs (one
inheriting from the other), put the stuff we'd be OK with passing to this method
in the ancestor struct, and make the arg to this method of the ancestor type?
That way we could benefit from the style struct caching if needed, etc. If that
would not be worth it, we might still want to consider passing in a struct
holding those values instead of a bunch of different args.

I assume the actual implementations will make some use of static methods on
nsLayoutUtils or nsHTMLReflowState or something?

-Boris

L. David Baron

unread,
Sep 27, 2006, 10:06:57 PM9/27/06
to dev-tec...@lists.mozilla.org
On Wednesday 2006-09-27 20:27 -0500, Boris Zbarsky wrote:
> > * @param aAvailWidth The available width into which the element is
> > * being placed (i.e., the width of its containing
> > * block).
>
> So this is not the same as the availWidth of a reflow state, correct? This
> is the value that should be used as a base for percentage widths, basically?

Hrm. I suppose I need both, to handle the absolute positioning cases.
(Are there any other cases where they differ?)

L. David Baron

unread,
Sep 27, 2006, 10:08:18 PM9/27/06
to dev-tec...@lists.mozilla.org
On Wednesday 2006-09-27 19:06 -0700, L. David Baron wrote:
> On Wednesday 2006-09-27 20:27 -0500, Boris Zbarsky wrote:
> > > * @param aAvailWidth The available width into which the element is
> > > * being placed (i.e., the width of its containing
> > > * block).
> >
> > So this is not the same as the availWidth of a reflow state, correct? This
> > is the value that should be used as a base for percentage widths, basically?
>
> Hrm. I suppose I need both, to handle the absolute positioning cases.
> (Are there any other cases where they differ?)

Er, never mind that.

I think the two are the same.

When do you think they're different?

Boris Zbarsky

unread,
Sep 27, 2006, 10:38:06 PM9/27/06
to
L. David Baron wrote:
>>> So this is not the same as the availWidth of a reflow state, correct? This
>>> is the value that should be used as a base for percentage widths, basically?
...

> I think the two are the same.
>
> When do you think they're different?

For example, for a replaced inline. The availableWidth in the reflow state is
the width of the parent block's content area minus the horizontal extent of the
boxes already placed in the current line. The base for percentage widths should
just be the parent block's content area.

I do think that ComputeSize just needs the base for percentage widths. It
doesn't care about our availableWidth thing, imo.

-Boris

0 new messages