Skip to content

Commit

Permalink
Function mapper performance check requires null check for some props
Browse files Browse the repository at this point in the history
Some properties, like `control-point-distances`, do not have non-null default values to fall back on.  These properties are typically override properties:  The property overrides the behaviour of another one and the `null` value indicates that the behaviour override is inactive.  Because most properties do have a default value, it's very subtle.  The performance enhancement in #2239 assumes that all properties have a default value, missing the null check.  This causes the issue.

Ref : When applying a function mapper to control-point-distances, eleProp is null in applyContextStyle() #2423

Ref : Optimizations for mapped style properties #2239
  • Loading branch information
maxkfranz committed Jun 6, 2019
1 parent ba33fcc commit d1343b0
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 8 deletions.
1 change: 1 addition & 0 deletions src/style/apply.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ styfn.applyContextStyle = function( cxtMeta, cxtStyle, ele ){
// save cycles when a mapped context prop doesn't need to be applied
if(
cxtProp.mapped === types.fn // context prop is function mapper
&& eleProp != null // some props can be null even by default (e.g. a prop that overrides another one)
&& eleProp.mapping != null // ele prop is a concrete value from from a mapper
&& eleProp.mapping.value === cxtProp.value // the current prop on the ele is a flat prop value for the function mapper
){ // NB don't write to cxtProp, as it's shared among eles (stored in stylesheet)
Expand Down
27 changes: 19 additions & 8 deletions test/collection-style.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,6 @@ describe('Collection style', function(){

var cy;

var useFn = function( fn ){
return function( arg ){
return fn( arg );
};
};

// test setup
beforeEach(function(){
cy = cytoscape({
Expand All @@ -33,7 +27,7 @@ describe('Collection style', function(){
{
selector: '#n1',
style: {
label: useFn(function(){ return 'n1'; }),
label: function(){ return 'n1'; },
width: 20,
'background-image': ['/test/image.png', '/test/image2.png'],
opacity: 0.5
Expand All @@ -43,7 +37,7 @@ describe('Collection style', function(){
{
selector: '#n2',
style: {
label: useFn(function(){ return 'n2'; })
label: function(){ return 'n2'; }
}
},

Expand Down Expand Up @@ -585,6 +579,23 @@ describe('Collection style', function(){
expect(edge.isBundledBezier(), edge.id()).to.be.false;
});
});

it('ele.style() reads OK for mapped override prop', function(){
cy.style().fromJson([
{
selector: '#n1n2',
style: {
'curve-style': 'unbundled-bezier',
'control-point-distances': function(ele){ return [32, 128]; },
'control-point-weights': [0.5, 0.75]
}
}
]).update();

var d = cy.$('#n1n2').numericStyle('control-point-distances');

expect(d, 'control-point-distances').to.deep.equal([32, 128]);
});
});

describe('eles.addClass() etc', function(){
Expand Down

0 comments on commit d1343b0

Please sign in to comment.