From d1343b08ba91b3dbeff33016b3f0e4afb6344b11 Mon Sep 17 00:00:00 2001 From: Max Franz Date: Thu, 6 Jun 2019 12:49:43 -0400 Subject: [PATCH] Function mapper performance check requires null check for some props 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 --- src/style/apply.js | 1 + test/collection-style.js | 27 +++++++++++++++++++-------- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/style/apply.js b/src/style/apply.js index 2704debc3d..f6f7647203 100644 --- a/src/style/apply.js +++ b/src/style/apply.js @@ -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) diff --git a/test/collection-style.js b/test/collection-style.js index dd3308e1ca..36d6c76ebd 100644 --- a/test/collection-style.js +++ b/test/collection-style.js @@ -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({ @@ -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 @@ -43,7 +37,7 @@ describe('Collection style', function(){ { selector: '#n2', style: { - label: useFn(function(){ return 'n2'; }) + label: function(){ return 'n2'; } } }, @@ -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(){