Closed Bug 332335 Opened 17 years ago Closed 5 years ago

replace nsRuleNode::Compute##struct##Data with table-driven computation


(Core :: CSS Parsing and Computation, defect)

Not set





(Reporter: dbaron, Assigned: dbaron)


(Depends on 1 open bug)


We should replace nsRuleNode::Compute##struct##Data with table-driven computation.  This will avoid some if-else chains that cause performance problems and make it easier to implement various things in css3-values and my cycle() proposal.  It will also make 'initial' and initial values easier to implement consistently, and avoid the need for copy-constructors on nsStyleStructs.
An additional dependency, like bug 332334, is backing out bug 339358.
Blocks: 363249
Blocks: 363250
QA Contact: ian → style-system
This should also avoid the duplication of initial values between nsStyleStruct constructors and nsRuleNode -moz-initial implementations; if it doesn't, a separate bug should be filed.
can you elaborate on what you had in mind? i've been poking around nsRuleNode but i'm not sure exactly what needs to be table-driven.
All the repetitive code in Compute*Data that computes the value of each property.  And Get*Data as well, although that might be filed as a separate bug already.
Blocks: 435426
Blocks: 435441
Blocks: 435442
So, the basic idea here is the following:

There are tons of if-else chains in the Compute*Data methods in nsRuleNode.cpp, like this:

  // user-input: auto, none, enum, inherit
  if (eCSSUnit_Enumerated == uiData.mUserInput.GetUnit()) {
    ui->mUserInput = uiData.mUserInput.GetIntValue();
  else if (eCSSUnit_Auto == uiData.mUserInput.GetUnit() ||
           eCSSUnit_Initial == uiData.mUserInput.GetUnit()) {
    ui->mUserInput = NS_STYLE_USER_INPUT_AUTO;
  else if (eCSSUnit_None == uiData.mUserInput.GetUnit()) {
    ui->mUserInput = NS_STYLE_USER_INPUT_NONE;
  else if (eCSSUnit_Inherit == uiData.mUserInput.GetUnit()) {
    inherited = PR_TRUE;
    ui->mUserInput = parentUI->mUserInput;

some things, however, use the SetCoord function.  The first step here is to gradually migrate all of these if-else chains over to something like the SetCoord function (perhaps to coexist alongside SetCoord, since SetCoord is for setting nsStyleCoord).

A good first step would be to just take a handful of these if-else chains and write a function that's capable of replacing each one with a single function call.

Having this function is really what's needed for implementing calc(), attr(), etc., since we don't want to add yet another case (and a complicated one) to all of these if-else chains to handle calc(), attr(), etc.

Eventually, once *everything* was replaced by calls to this new function or to SetCoord (which would probably be desired to implement calc() and attr() for all properties, which may not be necessary to do in the first pass of implementing those features), we could replace all the nsRuleNode::Compute*Data functions with a data structure that contains the parameters passed to those function calls, which would let us then replace the Compute*Data functions themselves with just one or two functions.  But doing that isn't necessarily even a blocker for calc(), attr(), etc.

(I think replacing all the Get*Data functions is already filed as a separate bug, and is mostly separate from this.)
Depends on: 441367
nsRuleNode is gone.
Closed: 5 years ago
Resolution: --- → WONTFIX
Could you go through the open bugs that depended on this, and hopefully remove the dependency (or say something about what we can/should do if it cannot be removed)?

Having open bugs depend on a WONTFIX bug isn't great…
No longer blocks: 363250, 435426
You need to log in before you can comment on or make changes to this bug.