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

RESOLVED WONTFIX

Status

()

Core
CSS Parsing and Computation
RESOLVED WONTFIX
12 years ago
a month ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

(Depends on: 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

12 years ago
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.
(Assignee)

Comment 1

12 years ago
An additional dependency, like bug 332334, is backing out bug 339358.
(Assignee)

Updated

12 years ago
Blocks: 363249
(Assignee)

Updated

12 years ago
Blocks: 363250
(Assignee)

Updated

11 years ago
QA Contact: ian → style-system
(Assignee)

Comment 2

11 years ago
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.

Comment 3

11 years ago
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.
(Assignee)

Comment 4

11 years ago
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.

Updated

10 years ago
Blocks: 435426

Updated

10 years ago
Blocks: 435441

Updated

10 years ago
Blocks: 435442
(Assignee)

Comment 5

10 years ago
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.)

Updated

10 years ago
Depends on: 441367
nsRuleNode is gone.
Status: NEW → RESOLVED
Last Resolved: a month ago
Resolution: --- → WONTFIX

Comment 7

a month ago
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…
(Assignee)

Updated

a month ago
No longer blocks: 363250, 435426
You need to log in before you can comment on or make changes to this bug.