running out of frame state bits

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

running out of frame state bits

L. David Baron
So, we're running out of frame state bits, and I think I need another
one for the work I'm doing on the reflow branch.  So what I'm thinking
of doing is the following:

 * create a virtual |GetClassBits| on nsIFrame for bits that are
   constant for a given frame type (if only C++ had |virtual PRUint32
   classBits|!)

 * start by moving NS_FRAME_REPLACED_ELEMENT to it to get the one bit
   that I need

(I think other things that could be part of this are the existing bits
NS_FRAME_EXCLUDE_IGNORABLE_WHITESPACE, NS_FRAME_IS_BOX, and potentially
NS_FRAME_REFLOW_ROOT if we changed which frame had that bit set for text
inputs.  A bunch of existing virtual functions, like IsContainingBlock,
NeedsView, IsFloatContainingBlock, IsLeaf, and SupportsVisibilityHidden,
could probably also be made part of this if we want to shrink vtables.)

Does it seem OK to do this work on the reflow branch, or are other
people likely to need bits before the reflow branch lands, which means I
should do it to the trunk and then re-merge the reflow branch to a new
branch off the trunk?

-David

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

_______________________________________________
dev-tech-layout mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-layout

attachment0 (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: running out of frame state bits

Boris Zbarsky
L. David Baron wrote:
>  * create a virtual |GetClassBits| on nsIFrame for bits that are
>    constant for a given frame type (if only C++ had |virtual PRUint32
>    classBits|!)
>
>  * start by moving NS_FRAME_REPLACED_ELEMENT to it to get the one bit
>    that I need

Isn't this basically what IsFrameOfType() is about?  That is, could we just move
NS_FRAME_REPLACED_ELEMENT into an IsFrameOfType() bit?

> Does it seem OK to do this work on the reflow branch, or are other
> people likely to need bits before the reflow branch lands, which means I
> should do it to the trunk and then re-merge the reflow branch to a new
> branch off the trunk?

I think it's pretty reasonable to add IsFrameOfType() bits on the reflow branch
(but maybe that's because I've done just that for form controls.  ;) ).

-Boris
_______________________________________________
dev-tech-layout mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-layout
Reply | Threaded
Open this post in threaded view
|

Re: running out of frame state bits

Robert O'Callahan-3
Boris Zbarsky wrote:
> Isn't this basically what IsFrameOfType() is about?  That is, could we
> just move NS_FRAME_REPLACED_ELEMENT into an IsFrameOfType() bit?

That sounds good to me.

>> Does it seem OK to do this work on the reflow branch, or are other
>> people likely to need bits before the reflow branch lands, which means I
>> should do it to the trunk and then re-merge the reflow branch to a new
>> branch off the trunk?

It's OK with me.

I'll free up two bits when I remove views, but that'll be too late for you.

Rob
_______________________________________________
dev-tech-layout mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-layout
Reply | Threaded
Open this post in threaded view
|

Re: running out of frame state bits

L. David Baron
In reply to this post by Boris Zbarsky
On Monday 2006-06-05 13:55 -0500, Boris Zbarsky wrote:

> L. David Baron wrote:
> > * create a virtual |GetClassBits| on nsIFrame for bits that are
> >   constant for a given frame type (if only C++ had |virtual PRUint32
> >   classBits|!)
> >
> > * start by moving NS_FRAME_REPLACED_ELEMENT to it to get the one bit
> >   that I need
>
> Isn't this basically what IsFrameOfType() is about?  That is, could we just
> move NS_FRAME_REPLACED_ELEMENT into an IsFrameOfType() bit?
I took this approach.

Unfortunately it looks like all box-related frames other than menubar
should have had this bit set in the past; however, judging by what
happens with my IsFrameOfType implementation, they didn't.  I need to
figure out which of them, if any, actually had the bit set, and why the
code in ConstructXULFrame didn't do what it looks like it did.

-David

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

_______________________________________________
dev-tech-layout mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-layout

attachment0 (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: running out of frame state bits

L. David Baron
On Monday 2006-06-05 16:21 -0700, L. David Baron wrote:
> Unfortunately it looks like all box-related frames other than menubar
> should have had this bit set in the past; however, judging by what
> happens with my IsFrameOfType implementation, they didn't.  I need to
> figure out which of them, if any, actually had the bit set, and why the
> code in ConstructXULFrame didn't do what it looks like it did.

I still haven't figured out what was going on (although I have
eliminated the possibility that it's something wrong with the code I
wrote to change the bit tests to IsFrameOfType tests).


However, I was thinking that we might want to change the way we
implement IsFrameOfType slightly:

 * use virtual *inline* functions
 * make implementations call their base class

So nsIFrame.h would have:

virtual PRBool IsFrameOfType(PRUint32 aFlags) const {
  return !aFlags;
}

a frame that wanted to be SVG would then have:

virtual PRBool IsFrameOfType(PRUint32 aFlags) const {
  return baseclass::IsFrameOfType(aFlags & ~(eSVG));
}

a frame that wanted to undo being a type its base class was would do:

virtual PRBool IsFrameOfType(PRUint32 aFlags) const {
  return !(aFlags & (eReplaced)) && baseclass::IsFrameOfType(aFlags);
}

We might even want macros to do this.

I'd hope that compilers could optimize this to the equivalent of what we
do now (rewriting the whole thing in condensed form in derived classes).


I'm also thinking that I want to make our use of typedefs like
nsBlockFrameSuper more consistent (although many of them are macros).
Though I may want to make them typedefs within the class, e.g.,
nsBlockFrame::super_type.

-David

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

_______________________________________________
dev-tech-layout mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-layout

attachment0 (196 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: running out of frame state bits

Robert O'Callahan-3
In reply to this post by L. David Baron
L. David Baron wrote:

> However, I was thinking that we might want to change the way we
> implement IsFrameOfType slightly:
>
>  * use virtual *inline* functions
>  * make implementations call their base class
>
> So nsIFrame.h would have:
>
> virtual PRBool IsFrameOfType(PRUint32 aFlags) const {
>   return !aFlags;
> }
>
> a frame that wanted to be SVG would then have:
>
> virtual PRBool IsFrameOfType(PRUint32 aFlags) const {
>   return baseclass::IsFrameOfType(aFlags & ~(eSVG));
> }
>
> a frame that wanted to undo being a type its base class was would do:
>
> virtual PRBool IsFrameOfType(PRUint32 aFlags) const {
>   return !(aFlags & (eReplaced)) && baseclass::IsFrameOfType(aFlags);
> }
>
> We might even want macros to do this.
>
> I'd hope that compilers could optimize this to the equivalent of what we
> do now (rewriting the whole thing in condensed form in derived classes).

Sounds good to me.

What was the reasoning behind IsFrameOfType/IsContentOfType having the
signature they have, instead of using methods that return a flags word?
The latter is a bit more expressive and may avoid multiple method calls
in some (hypothetical) cases.

> I'm also thinking that I want to make our use of typedefs like
> nsBlockFrameSuper more consistent (although many of them are macros).
> Though I may want to make them typedefs within the class, e.g.,
> nsBlockFrame::super_type.

Typedef within the class sounds good to me, but I might just call it Super.

Rob
_______________________________________________
dev-tech-layout mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-layout
Reply | Threaded
Open this post in threaded view
|

Re: running out of frame state bits

Jonas Sicking
>> I'd hope that compilers could optimize this to the equivalent of what we
>> do now (rewriting the whole thing in condensed form in derived classes).
>
> Sounds good to me.

Agreed, as long as we check that compilers actually optimize that well.
(it may even be that if we do the below change that optimizers would
have an even easier time optimizing it).

> What was the reasoning behind IsFrameOfType/IsContentOfType having the
> signature they have, instead of using methods that return a flags word?
> The latter is a bit more expressive and may avoid multiple method calls
> in some (hypothetical) cases.

I was, reluctantly, following the precedent of IsContentOfType (now
IsNodeOfType) when I created IsFrameOfType. Having virtual NodeType()
and FrameType() and then inline IsNodeOfType/IsFrameOfType wrapping them
is definitely a better solution performance-wise.

There are already places where we call IsNodeOfType multiple times.

/ Jonas
_______________________________________________
dev-tech-layout mailing list
[hidden email]
https://lists.mozilla.org/listinfo/dev-tech-layout